From 21b11c6d1de3840dcaf3fa82dae14ca3924f689f Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:09 +0000 Subject: sequencer: write CHERRY_PICK_HEAD for reword and edit `git commit` relies on the presence of CHERRY_PICK_HEAD to show the correct error message in the case of an empty pick. This fixes a regression introduced by the conversion from shell to C. In the shell version everything was a cherry-pick as far as the sequencer code was concerned so it always wrote CHERRY_PICK_HEAD. The conversion to C forgot to update the code that creates CHERRY_PICK_HEAD. We do not want to create CHERRY_PICK_HEAD for fixup and squash commands as that would prevent `git commit --amend` from running. Note that the error message shown by `git commit` for an empty pick during a rebase is currently wrong as it talks about running `git cherry-pick --skip` rather than `git rebase --skip`. This will be fixed in a future commit which is why the tests are in t3403-rebase-skip.sh. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- t/t3403-rebase-skip.sh | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) (limited to 't/t3403-rebase-skip.sh') diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh index ee8a8dba52..db7e917248 100755 --- a/t/t3403-rebase-skip.sh +++ b/t/t3403-rebase-skip.sh @@ -29,6 +29,13 @@ test_expect_success setup ' test_tick && git commit -m reverted-goodbye && git tag reverted-goodbye && + git checkout goodbye && + test_tick && + GIT_AUTHOR_NAME="Another Author" \ + GIT_AUTHOR_EMAIL="another.author@example.com" \ + git commit --amend --no-edit -m amended-goodbye && + test_tick && + git tag amended-goodbye && git checkout -f skip-reference && echo moo > hello && @@ -85,6 +92,52 @@ test_expect_success 'moved back to branch correctly' ' test_debug 'gitk --all & sleep 1' +test_expect_success 'correct advice upon picking empty commit' ' + test_when_finished "git rebase --abort" && + test_must_fail git rebase -i --onto goodbye \ + amended-goodbye^ amended-goodbye 2>err && + test_i18ngrep "previous cherry-pick is now empty" err && + test_i18ngrep "git cherry-pick --skip" err && + test_must_fail git commit && + test_i18ngrep "git cherry-pick --skip" err +' + +test_expect_success 'correct authorship when committing empty pick' ' + test_when_finished "git rebase --abort" && + test_must_fail git rebase -i --onto goodbye \ + amended-goodbye^ amended-goodbye && + git commit --allow-empty && + git log --pretty=format:"%an <%ae>%n%ad%B" -1 amended-goodbye >expect && + git log --pretty=format:"%an <%ae>%n%ad%B" -1 HEAD >actual && + test_cmp expect actual +' + +test_expect_success 'correct advice upon rewording empty commit' ' + test_when_finished "git rebase --abort" && + ( + set_fake_editor && + test_must_fail env FAKE_LINES="reword 1" git rebase -i \ + --onto goodbye amended-goodbye^ amended-goodbye 2>err + ) && + test_i18ngrep "previous cherry-pick is now empty" err && + test_i18ngrep "git cherry-pick --skip" err && + test_must_fail git commit && + test_i18ngrep "git cherry-pick --skip" err +' + +test_expect_success 'correct advice upon editing empty commit' ' + test_when_finished "git rebase --abort" && + ( + set_fake_editor && + test_must_fail env FAKE_LINES="edit 1" git rebase -i \ + --onto goodbye amended-goodbye^ amended-goodbye 2>err + ) && + test_i18ngrep "previous cherry-pick is now empty" err && + test_i18ngrep "git cherry-pick --skip" err && + test_must_fail git commit && + test_i18ngrep "git cherry-pick --skip" err +' + test_expect_success 'fixup that empties commit fails' ' test_when_finished "git rebase --abort" && ( -- cgit v1.2.3 From 430b75f7209c554d88e3554eb54cebf4ba1e4608 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:12 +0000 Subject: commit: give correct advice for empty commit during a rebase In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02), `git commit` learned to suggest to run `git cherry-pick --skip` when trying to cherry-pick an empty patch. However, it was overlooked that there are more conditions than just a `git cherry-pick` when this advice is printed (which originally suggested the neutral `git reset`): the same can happen during a rebase. Let's suggest the correct command, even during a rebase. While at it, we adjust more places in `builtin/commit.c` that incorrectly assumed that the presence of a `CHERRY_PICK_HEAD` meant that surely this must be a `cherry-pick` in progress. Note: we take pains to handle the situation when a user runs a `git cherry-pick` _during_ a rebase. This is quite valid (e.g. in an `exec` line in an interactive rebase). On the other hand, it is not possible to run a rebase during a cherry-pick, meaning: if both `rebase-merge/` and `sequencer/` exist or CHERRY_PICK_HEAD and REBASE_HEAD point to the same commit , we still want to advise to use `git cherry-pick --skip`. Original-patch-by: Johannes Schindelin Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- t/t3403-rebase-skip.sh | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) (limited to 't/t3403-rebase-skip.sh') diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh index db7e917248..a927774910 100755 --- a/t/t3403-rebase-skip.sh +++ b/t/t3403-rebase-skip.sh @@ -97,9 +97,9 @@ test_expect_success 'correct advice upon picking empty commit' ' test_must_fail git rebase -i --onto goodbye \ amended-goodbye^ amended-goodbye 2>err && test_i18ngrep "previous cherry-pick is now empty" err && - test_i18ngrep "git cherry-pick --skip" err && + test_i18ngrep "git rebase --skip" err && test_must_fail git commit && - test_i18ngrep "git cherry-pick --skip" err + test_i18ngrep "git rebase --skip" err ' test_expect_success 'correct authorship when committing empty pick' ' @@ -120,9 +120,9 @@ test_expect_success 'correct advice upon rewording empty commit' ' --onto goodbye amended-goodbye^ amended-goodbye 2>err ) && test_i18ngrep "previous cherry-pick is now empty" err && - test_i18ngrep "git cherry-pick --skip" err && + test_i18ngrep "git rebase --skip" err && test_must_fail git commit && - test_i18ngrep "git cherry-pick --skip" err + test_i18ngrep "git rebase --skip" err ' test_expect_success 'correct advice upon editing empty commit' ' @@ -133,8 +133,34 @@ test_expect_success 'correct advice upon editing empty commit' ' --onto goodbye amended-goodbye^ amended-goodbye 2>err ) && test_i18ngrep "previous cherry-pick is now empty" err && - test_i18ngrep "git cherry-pick --skip" err && + test_i18ngrep "git rebase --skip" err && test_must_fail git commit && + test_i18ngrep "git rebase --skip" err +' + +test_expect_success 'correct advice upon cherry-picking an empty commit during a rebase' ' + test_when_finished "git rebase --abort" && + ( + set_fake_editor && + test_must_fail env FAKE_LINES="1 exec_git_cherry-pick_amended-goodbye" \ + git rebase -i goodbye^ goodbye 2>err + ) && + test_i18ngrep "previous cherry-pick is now empty" err && + test_i18ngrep "git cherry-pick --skip" err && + test_must_fail git commit 2>err && + test_i18ngrep "git cherry-pick --skip" err +' + +test_expect_success 'correct advice upon multi cherry-pick picking an empty commit during a rebase' ' + test_when_finished "git rebase --abort" && + ( + set_fake_editor && + test_must_fail env FAKE_LINES="1 exec_git_cherry-pick_goodbye_amended-goodbye" \ + git rebase -i goodbye^^ goodbye 2>err + ) && + test_i18ngrep "previous cherry-pick is now empty" err && + test_i18ngrep "git cherry-pick --skip" err && + test_must_fail git commit 2>err && test_i18ngrep "git cherry-pick --skip" err ' -- cgit v1.2.3