All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Webb <chris@arachsys.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] rebase -i: handle fixup of root commit correctly
Date: Tue, 24 Jul 2012 13:17:03 +0100	[thread overview]
Message-ID: <20120724121703.GG26014@arachsys.com> (raw)

There is a bug with git rebase -i --root when a fixup or squash line is
applied to the new root. We attempt to amend the commit onto which they
apply with git reset --soft HEAD^ followed by a normal commit. Unlike a
real commit --amend, this sequence will fail against a root commit as it
has no parent.

Fix rebase -i to use commit --amend for fixup and squash instead, and
add a test for the case of a fixup of the root commit.

Signed-off-by: Chris Webb <chris@arachsys.com>
---

Sorry, I should have spotted this issue when I did the original root-rebase
series. I've checked that this patch doesn't break any of the existing
tests, as well as satisfying the newly introduced check for the root-fixup
case.

 git-rebase--interactive.sh    | 25 +++++++++++++------------
 t/t3404-rebase-interactive.sh |  8 ++++++++
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..0d2056f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,25 +493,28 @@ do_next () {
 		author_script_content=$(get_author_ident_from_commit HEAD)
 		echo "$author_script_content" > "$author_script"
 		eval "$author_script_content"
-		output git reset --soft HEAD^
-		pick_one -n $sha1 || die_failed_squash $sha1 "$rest"
+		if ! pick_one -n $sha1
+		then
+			git rev-parse --verify HEAD >"$amend"
+			die_failed_squash $sha1 "$rest"
+		fi
 		case "$(peek_next_command)" in
 		squash|s|fixup|f)
 			# This is an intermediate commit; its message will only be
 			# used in case of trouble.  So use the long version:
-			do_with_author output git commit --no-verify -F "$squash_msg" ||
+			do_with_author output git commit --amend --no-verify -F "$squash_msg" ||
 				die_failed_squash $sha1 "$rest"
 			;;
 		*)
 			# This is the final command of this squash/fixup group
 			if test -f "$fixup_msg"
 			then
-				do_with_author git commit --no-verify -F "$fixup_msg" ||
+				do_with_author git commit --amend --no-verify -F "$fixup_msg" ||
 					die_failed_squash $sha1 "$rest"
 			else
 				cp "$squash_msg" "$GIT_DIR"/SQUASH_MSG || exit
 				rm -f "$GIT_DIR"/MERGE_MSG
-				do_with_author git commit --no-verify -e ||
+				do_with_author git commit --amend --no-verify -F "$GIT_DIR"/SQUASH_MSG -e ||
 					die_failed_squash $sha1 "$rest"
 			fi
 			rm -f "$squash_msg" "$fixup_msg"
@@ -748,7 +751,6 @@ In both case, once you're done, continue with:
 		fi
 		. "$author_script" ||
 			die "Error trying to find the author identity to amend commit"
-		current_head=
 		if test -f "$amend"
 		then
 			current_head=$(git rev-parse --verify HEAD)
@@ -756,13 +758,12 @@ In both case, once you're done, continue with:
 			die "\
 You have uncommitted changes in your working tree. Please, commit them
 first and then run 'git rebase --continue' again."
-			git reset --soft HEAD^ ||
-			die "Cannot rewind the HEAD"
+			do_with_author git commit --amend --no-verify -F "$msg" -e ||
+				die "Could not commit staged changes."
+		else
+			do_with_author git commit --no-verify -F "$msg" -e ||
+				die "Could not commit staged changes."
 		fi
-		do_with_author git commit --no-verify -F "$msg" -e || {
-			test -n "$current_head" && git reset --soft $current_head
-			die "Could not commit staged changes."
-		}
 	fi
 
 	record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078db6..3f75d32 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '
 	git rebase --abort
 '
 
+test_expect_success 'rebase -i --root fixup root commit' '
+	git checkout B &&
+	FAKE_LINES="1 fixup 2" git rebase -i --root &&
+	test A = $(git cat-file commit HEAD | sed -ne \$p) &&
+	test B = $(git show HEAD:file1) &&
+	test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
+'
+
 test_done
-- 
1.7.11.2.251.g6a928a6

             reply	other threads:[~2012-07-24 12:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-24 12:17 Chris Webb [this message]
2012-07-24 19:22 ` [PATCH] rebase -i: handle fixup of root commit correctly Junio C Hamano
2012-07-31  9:14 ` Johannes Sixt
2012-07-31 11:19   ` Chris Webb
2012-07-31 12:48     ` Chris Webb
2012-07-31 20:04       ` Johannes Sixt
2012-07-31 22:47         ` Chris Webb

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=20120724121703.GG26014@arachsys.com \
    --to=chris@arachsys.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.