All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fabian Ruch <bafain@gmail.com>
To: git@vger.kernel.org
Subject: [RFC PATCH 5/7] rebase -i: Do not die in do_pick
Date: Thu, 19 Jun 2014 05:28:41 +0200	[thread overview]
Message-ID: <53A258E9.7030909@gmail.com> (raw)
In-Reply-To: <cover.1403146774.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. Moreover, if `do_pick`
is called while a squash or fixup is happening, `die_with_patch` will
discard `$squash_msg` as commit message. Indicate an error in `do_pick`
using return statements and properly kill the script at the call sites.
Remove unused commit message title argument from `do_pick`'s signature.

Signed-off-by: Fabian Ruch <bafain@gmail.com>
---
 git-rebase--interactive.sh | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f903599..e4992dc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,14 +493,10 @@ 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.
 #
-# <title>
-#     The commit message title of <commit>. Used for information
-#     purposes only.
-#
 # <commit>
 #     The commit to cherry-pick.
 #
@@ -508,6 +504,12 @@ record_in_rewritten() {
 #     After picking <commit>, open an editor and let the user edit the
 #     commit message. The editor contents becomes the commit message of
 #     the new head.
+#
+# 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 () {
 	rewrite=
 	rewrite_amend=
@@ -528,7 +530,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
@@ -545,11 +551,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"
@@ -557,8 +561,7 @@ do_pick () {
 		git commit --allow-empty --no-post-rewrite -n -q \
 			   ${rewrite_amend:+--amend} \
 			   ${rewrite_edit:+--edit} \
-			   ${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
 }
 
@@ -573,21 +576,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.0

  parent reply	other threads:[~2014-06-19  3:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1403146774.git.bafain@gmail.com>
2014-06-19  3:28 ` [RFC PATCH 1/7] rebase -i: Make option handling in pick_one more flexible Fabian Ruch
2014-06-20 13:40   ` Michael Haggerty
2014-06-20 19:53     ` Junio C Hamano
2014-06-23  0:04       ` Fabian Ruch
2014-06-21 23:21     ` Fabian Ruch
2014-06-23 16:09       ` Johannes Schindelin
2014-06-19  3:28 ` [RFC PATCH 2/7] rebase -i: Teach do_pick the option --edit Fabian Ruch
2014-06-20 13:41   ` Michael Haggerty
2014-06-22  0:09     ` Fabian Ruch
2014-06-19  3:28 ` [RFC PATCH 3/7] rebase -i: Stop on root commits with empty log messages Fabian Ruch
2014-06-21  0:33   ` Eric Sunshine
2014-06-22  0:32     ` Fabian Ruch
2014-06-19  3:28 ` [RFC PATCH 4/7] rebase -i: Commit only once when rewriting picks Fabian Ruch
2014-06-19  3:28 ` Fabian Ruch [this message]
2014-06-19  3:28 ` [RFC PATCH 6/7] rebase -i: Prepare for squash in terms of do_pick --amend Fabian Ruch
2014-06-19  3:28 ` [RFC PATCH 7/7] rebase -i: Teach do_pick the options --amend and --file Fabian Ruch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A258E9.7030909@gmail.com \
    --to=bafain@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.