subtree: more consistent error propagation

Ensure that every $(subshell) that calls a function (as opposed to an external executable) is followed by `|| exit $?`. Similarly, ensure that every `cmd | while read; do ... done` loop is followed by `|| exit $?`. Both of those constructs mean that it can miss `die` calls, and keep running when it shouldn't. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Luke Shumaker committed Apr 27, 2021 at 15:17 UTC d2f0f819547de35ffc923fc963f806f1656eb2ca
1 file changed +14 -14
contrib/subtree/git-subtree.sh
+14 -14
@@ -243,7 +243,7 @@ cache_miss () {
243 }
244
245 check_parents () {
246 - missed=$(cache_miss "$1")
246 + missed=$(cache_miss "$1") || exit $?
247 local indent=$(($2 + 1))
248 for miss in $missed
249 do
@@ -345,7 +345,7 @@ find_latest_squash () {
345 sub=
346 ;;
347 esac
348 - done
348 + done || exit $?
349 }
350
351 find_existing_splits () {
@@ -394,7 +394,7 @@ find_existing_splits () {
394 sub=
395 ;;
396 esac
397 - done
397 + done || exit $?
398 }
399
400 copy_commit () {
@@ -508,7 +508,7 @@ subtree_for_commit () {
508 test "$type" = "commit" && continue # ignore submodules
509 echo $tree
510 break
511 - done
511 + done || exit $?
512 }
513
514 tree_changed () {
@@ -518,7 +518,7 @@ tree_changed () {
518 then
519 return 0 # weird parents, consider it changed
520 else
521 - ptree=$(toptree_for_commit $1)
521 + ptree=$(toptree_for_commit $1) || exit $?
522 if test "$ptree" != "$tree"
523 then
524 return 0 # changed
@@ -652,7 +652,7 @@ process_split_commit () {
652 progress "$revcount/$revmax ($createcount) [$extracount]"
653
654 debug "Processing commit: $rev"
655 - exists=$(cache_get "$rev")
655 + exists=$(cache_get "$rev") || exit $?
656 if test -n "$exists"
657 then
658 debug " prior: $exists"
@@ -661,10 +661,10 @@ process_split_commit () {
661 createcount=$(($createcount + 1))
662 debug " parents: $parents"
663 check_parents "$parents" "$indent"
664 - newparents=$(cache_get $parents)
664 + newparents=$(cache_get $parents) || exit $?
665 debug " newparents: $newparents"
666
667 - tree=$(subtree_for_commit "$rev" "$dir")
667 + tree=$(subtree_for_commit "$rev" "$dir") || exit $?
668 debug " tree is: $tree"
669
670 # ugly. is there no better way to tell if this is a subtree
@@ -750,7 +750,7 @@ cmd_add_commit () {
750 commit=$(add_squashed_msg "$rev" "$dir" |
751 git commit-tree "$tree" $headp -p "$rev") || exit $?
752 else
753 - revp=$(peel_committish "$rev") &&
753 + revp=$(peel_committish "$rev") || exit $?
754 commit=$(add_msg "$dir" $headrev "$rev" |
755 git commit-tree "$tree" $headp -p "$revp") || exit $?
756 fi
@@ -773,10 +773,10 @@ cmd_split () {
773 # any parent we find there can be used verbatim
774 debug " cache: $rev"
775 cache_set "$rev" "$rev"
776 - done
776 + done || exit $?
777 fi
778
779 - unrevs="$(find_existing_splits "$dir" "$revs")"
779 + unrevs="$(find_existing_splits "$dir" "$revs")" || exit $?
780
781 # We can't restrict rev-list to only $dir here, because some of our
782 # parents have the $dir contents the root, and those won't match.
@@ -792,7 +792,7 @@ cmd_split () {
792 process_split_commit "$rev" "$parents" 0
793 done || exit $?
794
795 - latest_new=$(cache_get latest_new)
795 + latest_new=$(cache_get latest_new) || exit $?
796 if test -z "$latest_new"
797 then
798 die "No new revisions were found"
@@ -801,7 +801,7 @@ cmd_split () {
801 if test -n "$rejoin"
802 then
803 debug "Merging split branch into HEAD..."
804 - latest_old=$(cache_get latest_old)
804 + latest_old=$(cache_get latest_old) || exit $?
805 git merge -s ours \
806 --allow-unrelated-histories \
807 -m "$(rejoin_msg "$dir" "$latest_old" "$latest_new")" \
@@ -834,7 +834,7 @@ cmd_merge () {
834
835 if test -n "$squash"
836 then
837 - first_split="$(find_latest_squash "$dir")"
837 + first_split="$(find_latest_squash "$dir")" || exit $?
838 if test -z "$first_split"
839 then
840 die "Can't squash-merge: '$dir' was never added."