Re: [PATCH] rebase -i: handle fixup of root commit correctly
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 24, 2012, 19:22 UTC
- Message-ID
- <7vlii9ezgl.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120724121703.GG26014@arachsys.com>
Chris Webb <chris@arachsys.com> writes:
Show 16 quoted lines
> 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.
OK, so instead of "reset --soft HEAD^ && pick -n && commit -F msg" to back up one step and then build on top of it, the new sequence "pick -n && commit --amend -F msg" modifies and then amends, whose end result should be the same but the important difference is that the latter would work even if the current commit is a root one.
Makes sense. Thanks for catching and fixing it.
Show 87 quoted lines
>
> 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