diff mbox series

[v2,4/9] sequencer: write CHERRY_PICK_HEAD for reword and edit

Message ID 20191206160614.631724-5-phillip.wood123@gmail.com (mailing list archive)
State New, archived
Headers show
Series commit: fix advice for empty commits during rebases | expand

Commit Message

Phillip Wood Dec. 6, 2019, 4:06 p.m. UTC 
From: Phillip Wood <phillip.wood@dunelm.org.uk>

`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>
---
 sequencer.c            |  4 +++-
 t/t3403-rebase-skip.sh | 53 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/sequencer.c b/sequencer.c
index 408b7885c7..4e0370277b 100644
--- 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 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" &&
 	(