[PATCH v2 14/23] rebase -i: do not die in do_pick
- From
Fabian Ruch <bafain@gmail.com>
- Date
- Aug 6, 2014, 23:59 UTC
- Message-ID
- <2bd4cfaa20c582c9dd78d2ff917294e8250bf3b6.1407368621.git.bafain@gmail.com>
- In-Reply-To
- <cover.1407368621.git.bafain@gmail.com>
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@gmail.com> --- 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 b8c734e..d812bad 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" @@ -532,8 +537,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 } @@ -548,21 +552,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