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 <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index d18aba62f557923b93f94948657a98b33fcfaae9..d66856818a00628597115dcc9c4bf16b0aacf4d4 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -1916,7 +1916,9 @@ static int do_pick_commit(struct repository *r,
         * However, if the merge did not even start, then we don't want to
         * write it at all.
         */
-       if (command == TODO_PICK && !opts->no_commit && (res == 0 || res == 1) &&
+       if ((command == TODO_PICK || command == TODO_REWORD ||
+            command == TODO_EDIT) && !opts->no_commit &&
+           (res == 0 || res == 1) &&
            update_ref(NULL, "CHERRY_PICK_HEAD", &commit->object.oid, NULL,
                       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
                res = -1;

diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index ee8a8dba528697569a8a3f469bd28200f555d763..db7e917248ef305d9500eb89bd87df803263cbce 100755 (executable)
--- 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" &&
        (