Since `do_pick` might be executed in a sub-shell (a modified author
environment for instance), calling `die` in `do_pick` has no effect
but exiting the sub-shell with a failure exit status. The
git-rebase--interactive script is not terminated. Moreover, if
`do_pick` is called while a squash or fixup is in effect,
`die_with_patch` will discard `$squash_msg` as commit message.
Lastly, after a `die` in `do_pick` `do_next` has no chance to
reschedule tasks that failed before changes could be applied.
Indicate an error in `do_pick` using return statements and properly
kill the script at the call sites. Although possible in principle,
the issued error messages are no more indicating whether `do_pick`
failed while applying or while committing the changes. This reduces
code complexity at the call site and does not matter from a user's
point of view because a glance at the index reveals whether there are
conflicts or not and in-depth troubleshooting is still possible using
the `--verbose` option.
Remove the commit message title argument from `do_pick`'s interface,
which has become unused.
Signed-off-by: Fabian Ruch <bafain@xxxxxxxxx>
---
git-rebase--interactive.sh | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 181e06a..a5a8aa3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,7 +464,7 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--edit] <commit> <title>
+# do_pick [--edit] <commit>
#
# Wrapper around git-cherry-pick.
#
@@ -476,9 +476,11 @@ record_in_rewritten() {
# <commit>
# The commit to cherry-pick.
#
-# <title>
-# The commit message title of <commit>. Used for information
-# purposes only.
+# The return value is 1 if applying the changes resulted in a conflict
+# and 2 if the specified arguments were incorrect. If the changes could
+# be applied successfully but creating the commit failed, a value
+# greater than 2 is returned. No commit is created in either case and
+# the index is left with the (conflicting) changes in place.
do_pick () {
allow_empty_message=y
rewrite=
@@ -493,7 +495,8 @@ do_pick () {
allow_empty_message=
;;
-*)
- die "do_pick: unrecognized option -- $1"
+ warn "do_pick: unrecognized option -- $1"
+ return 2
;;
*)
break
@@ -501,7 +504,11 @@ do_pick () {
esac
shift
done
- test $# -ne 2 && die "do_pick: wrong number of arguments"
+ if test $# -ne 1
+ then
+ warn "do_pick: wrong number of arguments"
+ return 2
+ fi
if test "$(git rev-parse HEAD)" = "$squash_onto"
then
@@ -519,11 +526,9 @@ do_pick () {
# rebase --continue.
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1 &&
- pick_one -n $1 ||
- die_with_patch $1 "Could not apply $1... $2"
+ pick_one -n $1 || return 1
else
- pick_one ${rewrite:+-n} $1 ||
- die_with_patch $1 "Could not apply $1... $2"
+ pick_one ${rewrite:+-n} $1 || return 1
fi
if test -n "$rewrite"
@@ -534,8 +539,7 @@ do_pick () {
${allow_empty_message:+--allow-empty-message} \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit --commit-msg} \
- ${gpg_sign_opt:+"$gpg_sign_opt"} ||
- die_with_patch $1 "Could not rewrite commit after successfully picking $1... $2"
+ ${gpg_sign_opt:+"$gpg_sign_opt"} || return 3
fi
}
@@ -550,21 +554,21 @@ do_next () {
comment_for_reflog pick
mark_action_done
- do_pick $sha1 "$rest"
+ do_pick $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest"
record_in_rewritten $sha1
;;
reword|r)
comment_for_reflog reword
mark_action_done
- do_pick --edit $sha1 "$rest"
+ do_pick --edit $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest"
record_in_rewritten $sha1
;;
edit|e)
comment_for_reflog edit
mark_action_done
- do_pick $sha1 "$rest"
+ do_pick $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest"
warn "Stopped at $sha1... $rest"
exit_with_patch $sha1 0
;;
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html