From 2f3eb68f10be8541b6ffdcbb16d996fd3c7a9e82 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 31 Aug 2018 16:45:02 -0700 Subject: [PATCH 1/2] rebase -i --autosquash: demonstrate a problem skipping the last squash The `git commit --squash` command can be used not only to amend commit messages and changes, but also to record notes for an upcoming rebase. For example, when the author information of a given commit is incorrect, a user might call `git commit --allow-empty -m "Fix author" --squash `, to remind them to fix that during the rebase. When the editor would pop up, the user would simply delete the commit message to abort the rebase at this stage, fix the author information, and continue with `git rebase --skip`. (This is a real-world example from the rebase of Git for Windows onto v2.19.0-rc1.) However, there is a bug in `git rebase` that will cause the squash message *not* to be forgotten in this case. It will therefore be reused in the next fixup/squash chain (if any). This patch adds a test case to demonstrate this breakage. Signed-off-by: Johannes Schindelin --- t/t3415-rebase-autosquash.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index e364c12622f..7d5ea340b32 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -330,4 +330,23 @@ test_expect_success 'wrapped original subject' ' test $base = $parent ' +test_expect_failure 'abort last squash' ' + test_when_finished "test_might_fail git rebase --abort" && + test_when_finished "git checkout master" && + + git checkout -b some-squashes && + git commit --allow-empty -m first && + git commit --allow-empty --squash HEAD && + git commit --allow-empty -m second && + git commit --allow-empty --squash HEAD && + + test_must_fail git -c core.editor="grep -q ^pick" \ + rebase -ki --autosquash HEAD~4 && + : do not finish the squash, but resolve it manually && + git commit --allow-empty --amend -m edited-first && + git rebase --skip && + git show >actual && + ! grep first actual +' + test_done From 10d2f35436fb350c42bdb8396aee424a9bce44b5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 31 Aug 2018 16:45:04 -0700 Subject: [PATCH 2/2] rebase -i: be careful to wrap up fixup/squash chains When an interactive rebase was stopped at the end of a fixup/squash chain, the user might have edited the commit manually before continuing (with either `git rebase --skip` or `git rebase --continue`, it does not really matter which). We need to be very careful to wrap up the fixup/squash chain also in this scenario: otherwise the next fixup/squash chain would try to pick up where the previous one was left. Signed-off-by: Johannes Schindelin --- sequencer.c | 17 ++++++++++++++--- t/t3415-rebase-autosquash.sh | 2 +- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index 4034c0461b5..cccec6b70cc 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3438,9 +3438,20 @@ static int commit_staged_changes(struct replay_opts *opts, * the commit message and if there was a squash, let the user * edit it. */ - if (is_clean && !oidcmp(&head, &to_amend) && - opts->current_fixup_count > 0 && - file_exists(rebase_path_stopped_sha())) { + if (!is_clean || !opts->current_fixup_count) + ; /* this is not the final fixup */ + else if (oidcmp(&head, &to_amend) || + !file_exists(rebase_path_stopped_sha())) { + /* was a final fixup or squash done manually? */ + if (!is_fixup(peek_command(todo_list, 0))) { + unlink(rebase_path_fixup_msg()); + unlink(rebase_path_squash_msg()); + unlink(rebase_path_current_fixups()); + strbuf_reset(&opts->current_fixups); + opts->current_fixup_count = 0; + } + } else { + /* we are in a fixup/squash chain */ const char *p = opts->current_fixups.buf; int len = opts->current_fixups.len; diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index 7d5ea340b32..13f5688135d 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -330,7 +330,7 @@ test_expect_success 'wrapped original subject' ' test $base = $parent ' -test_expect_failure 'abort last squash' ' +test_expect_success 'abort last squash' ' test_when_finished "test_might_fail git rebase --abort" && test_when_finished "git checkout master" &&