Skip to content

Commit 9f50d32

Browse files
mackylegitster
authored andcommitted
rebase: avoid non-function use of "return" on FreeBSD
Since a1549e1, 15d4bf2 and 01a1e64 (first appearing in v1.8.4) the git-rebase--*.sh scripts have used a "return" to stop execution of the dot-sourced file and return to the "dot" command that dot-sourced it. The /bin/sh utility on FreeBSD however behaves poorly under some circumstances when such a "return" is executed. In particular, if the "dot" command is contained within a function, then when a "return" is executed by the script it runs (that is not itself inside a function), control will return from the function that contains the "dot" command skipping any statements that might follow the dot command inside that function. Commit 99855dd (first appearing in v1.8.4.1) addresses this by making the "dot" command the last line in the function. Unfortunately the FreeBSD /bin/sh may also execute some statements in the script run by the "dot" command that appear after the troublesome "return". The fix in 99855dd does not address this problem. For example, if you have script1.sh with these contents: run_script2() { . "$(dirname -- "$0")/script2.sh" _e=$? echo only this line should show [ $_e -eq 5 ] || echo expected status 5 got $_e return 3 } run_script2 e=$? [ $e -eq 3 ] || { echo expected status 3 got $e; exit 1; } And script2.sh with these contents: if [ 5 -gt 3 ]; then return 5 fi case bad in *) echo always shows esac echo should not get here ! : When running script1.sh (e.g. '/bin/sh script1.sh' or './script1.sh' after making it executable), the expected output from a POSIX shell is simply the single line: only this line should show However, when run using FreeBSD's /bin/sh, the following output appears instead: should not get here expected status 3 got 1 Not only did the lines following the "dot" command in the run_script2 function in script1.sh get skipped, but additional lines in script2.sh following the "return" got executed -- but not all of them (e.g. the "echo always shows" line did not run). These issues can be avoided by not using a top-level "return" in script2.sh. If script2.sh is changed to this: main() { if [ 5 -gt 3 ]; then return 5 fi case bad in *) echo always shows esac echo should not get here ! : } main Then it behaves the same when using FreeBSD's /bin/sh as when using other more POSIX compliant /bin/sh implementations. We fix the git-rebase--*.sh scripts in a similar fashion by moving the top-level code that contains "return" statements into its own function and then calling that as the last line in the script. Signed-off-by: Kyle J. McKay <[email protected]> Acked-by: Matthieu Moy <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0bc85ab commit 9f50d32

File tree

3 files changed

+45
-0
lines changed

3 files changed

+45
-0
lines changed

git-rebase--am.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,17 @@
44
# Copyright (c) 2010 Junio C Hamano.
55
#
66

7+
# The whole contents of this file is run by dot-sourcing it from
8+
# inside a shell function. It used to be that "return"s we see
9+
# below were not inside any function, and expected to return
10+
# to the function that dot-sourced us.
11+
#
12+
# However, FreeBSD /bin/sh misbehaves on such a construct and
13+
# continues to run the statements that follow such a "return".
14+
# As a work-around, we introduce an extra layer of a function
15+
# here, and immediately call it after defining it.
16+
git_rebase__am () {
17+
718
case "$action" in
819
continue)
920
git am --resolved --resolvemsg="$resolvemsg" &&
@@ -73,3 +84,7 @@ then
7384
fi
7485

7586
move_to_original_branch
87+
88+
}
89+
# ... and then we call the whole thing.
90+
git_rebase__am

git-rebase--interactive.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,17 @@ add_exec_commands () {
810810
mv "$1.new" "$1"
811811
}
812812

813+
# The whole contents of this file is run by dot-sourcing it from
814+
# inside a shell function. It used to be that "return"s we see
815+
# below were not inside any function, and expected to return
816+
# to the function that dot-sourced us.
817+
#
818+
# However, FreeBSD /bin/sh misbehaves on such a construct and
819+
# continues to run the statements that follow such a "return".
820+
# As a work-around, we introduce an extra layer of a function
821+
# here, and immediately call it after defining it.
822+
git_rebase__interactive () {
823+
813824
case "$action" in
814825
continue)
815826
# do we have anything to commit?
@@ -1042,3 +1053,7 @@ GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
10421053
output git checkout $onto || die_abort "could not detach HEAD"
10431054
git update-ref ORIG_HEAD $orig_head
10441055
do_rest
1056+
1057+
}
1058+
# ... and then we call the whole thing.
1059+
git_rebase__interactive

git-rebase--merge.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,17 @@ finish_rb_merge () {
101101
say All done.
102102
}
103103

104+
# The whole contents of this file is run by dot-sourcing it from
105+
# inside a shell function. It used to be that "return"s we see
106+
# below were not inside any function, and expected to return
107+
# to the function that dot-sourced us.
108+
#
109+
# However, FreeBSD /bin/sh misbehaves on such a construct and
110+
# continues to run the statements that follow such a "return".
111+
# As a work-around, we introduce an extra layer of a function
112+
# here, and immediately call it after defining it.
113+
git_rebase__merge () {
114+
104115
case "$action" in
105116
continue)
106117
read_state
@@ -151,3 +162,7 @@ do
151162
done
152163

153164
finish_rb_merge
165+
166+
}
167+
# ... and then we call the whole thing.
168+
git_rebase__merge

0 commit comments

Comments
 (0)