git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Dec 6, 2019, 16:06 UTC
Message-ID
<20191206160614.631724-10-phillip.wood123@gmail.com>
In-Reply-To
<20191206160614.631724-1-phillip.wood123@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Since the inception of CHERRY_PICK_HEAD in d7e5c0cbfb ("Introduce CHERRY_PICK_HEAD", 2011-02-19) 'rebase -i' has removed it when there are conflicts. The rationale for this was that the rebase wanted to handle the conflicts itself. However sometimes (e.g. after an edit command) the user wants to commit the conflict resolution before making some other changes or running some tests. Without CHERRY_PICK_HEAD the authorship information is lost when the user makes the commit. Fix this by leaving CHERRY_PICK_HEAD when we're not amending.

Note that this changes the output of `git status`. The advice to run `git reset` is not appropriate for rebase as we do not allow partial commits.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Notes:
    This has semantic conflicts with ra/rebase-i-more-options as it does not
    respect the options passed to rebase when the user commits
    
    I haven't checked how this affects the shell prompt in contrib yet, I
    suspect it may need changing to cope the presence of CHERRY_PICK_HEAD
    during a rebase.
    
    I'd like to change the existing authorship tests to rely on the "Original
    Author" changes here, but they are a web of hidden interdependencies which is
    hard to untangle.
 sequencer.c                   |  12 ++--
 t/t3404-rebase-interactive.sh | 104 +++++++++++++++++++++++++---------
 t/t7512-status-help.sh        |   2 -
 3 files changed, 85 insertions(+), 33 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 64242f4ce7..624e96c930 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -372,11 +372,15 @@ static void print_advice(struct repository *r, int show_hint,
 	if (msg) {
 		fprintf(stderr, "%s\n", msg);
 		/*
-		 * A conflict has occurred but the porcelain
-		 * (typically rebase --interactive) wants to take care
-		 * of the commit itself so remove CHERRY_PICK_HEAD
+		 * A conflict has occurred but the porcelain wants to take care
+		 * of the commit itself so remove CHERRY_PICK_HEAD. Note that we
+		 * do not do this for interactive rebases anymore in order to
+		 * preserve the author identity when the user runs 'git commit'
+		 * to commit the conflict resolution rather than relying on
+		 * 'rebase --continue' to do it for them.
 		 */
-		unlink(git_path_cherry_pick_head(r));
+		if (!is_rebase_i(opts))
+			unlink(git_path_cherry_pick_head(r));
 		return;
 	}
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 5afa6f28cd..5cd7db18f8 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -33,31 +33,35 @@ Initial setup:
 # in the expect2 file for the 'stop on conflicting pick' test.
 
 test_expect_success 'setup' '
-	test_commit A file1 &&
-	test_commit B file1 &&
-	test_commit C file2 &&
-	test_commit D file1 &&
-	test_commit E file3 &&
-	git checkout -b branch1 A &&
-	test_commit F file4 &&
-	test_commit G file1 &&
-	test_commit H file5 &&
-	git checkout -b branch2 F &&
-	test_commit I file6 &&
-	git checkout -b conflict-branch A &&
-	test_commit one conflict &&
-	test_commit two conflict &&
-	test_commit three conflict &&
-	test_commit four conflict &&
-	git checkout -b no-conflict-branch A &&
-	test_commit J fileJ &&
-	test_commit K fileK &&
-	test_commit L fileL &&
-	test_commit M fileM &&
-	git checkout -b no-ff-branch A &&
-	test_commit N fileN &&
-	test_commit O fileO &&
-	test_commit P fileP
+	(
+		GIT_AUTHOR_NAME="Original Author" &&
+		GIT_AUTHOR_EMAIL="original.author@example.com" &&
+		test_commit A file1 &&
+		test_commit B file1 &&
+		test_commit C file2 &&
+		test_commit D file1 &&
+		test_commit E file3 &&
+		git checkout -b branch1 A &&
+		test_commit F file4 &&
+		test_commit G file1 &&
+		test_commit H file5 &&
+		git checkout -b branch2 F &&
+		test_commit I file6 &&
+		git checkout -b conflict-branch A &&
+		test_commit one conflict &&
+		test_commit two conflict &&
+		test_commit three conflict &&
+		test_commit four conflict &&
+		git checkout -b no-conflict-branch A &&
+		test_commit J fileJ &&
+		test_commit K fileK &&
+		test_commit L fileL &&
+		test_commit M fileM &&
+		git checkout -b no-ff-branch A &&
+		test_commit N fileN &&
+		test_commit O fileO &&
+		test_commit P fileP
+	)
 '
 
 # "exec" commands are run with the user shell by default, but this may
@@ -252,12 +256,12 @@ test_expect_success 'stop on conflicting pick' '
 	-A
 	+G
 	EOF
-	cat >expect2 <<-\EOF &&
+	cat >expect2 <<-EOF &&
 	<<<<<<< HEAD
 	D
 	=======
 	G
-	>>>>>>> 5d18e54... G
+	>>>>>>> $(git rev-parse --short HEAD)... G
 	EOF
 	git tag new-branch1 &&
 	test_must_fail git rebase -i master &&
@@ -1628,6 +1632,52 @@ test_expect_success 'correct error message for commit --amend after empty pick'
 	test_i18ngrep "middle of a rebase -- cannot amend." err
 '
 
+test_expect_success 'correct error message for partial commit after confilct' '
+	test_when_finished "git rebase --abort" &&
+	git checkout D &&
+	(
+		set_fake_editor &&
+		FAKE_LINES="2 3" &&
+		export FAKE_LINES &&
+		test_must_fail git rebase -i A
+	) &&
+	echo x >file1 &&
+	echo y >file2 &&
+	git add file1 file2 &&
+	test_must_fail git commit file1 2>err &&
+	test_i18ngrep "cannot do a partial commit during a rebase." err
+'
+
+test_expect_success 'correct error message for commit --amend after conflict' '
+	test_when_finished "git rebase --abort" &&
+	git checkout D &&
+	(
+		set_fake_editor &&
+		FAKE_LINES=3 &&
+		export FAKE_LINES &&
+		test_must_fail git rebase -i A
+	) &&
+	echo x>file1 &&
+	test_must_fail git commit -a --amend 2>err &&
+	test_i18ngrep "middle of a rebase -- cannot amend." err
+'
+
+test_expect_success 'correct authorship and message after conflict' '
+	git checkout D &&
+	(
+		set_fake_editor &&
+		FAKE_LINES=3 &&
+		export FAKE_LINES &&
+		test_must_fail git rebase -i A
+	) &&
+	echo x >file1 &&
+	git commit -a &&
+	git log --pretty=format:"%an <%ae>%n%ad%n%B" -1 D >expect &&
+	git log --pretty=format:"%an <%ae>%n%ad%n%B" -1 HEAD >actual &&
+	test_cmp expect actual &&
+	git rebase --continue
+'
+
 # This must be the last test in this file
 test_expect_success '$EDITOR and friends are unchanged' '
 	test_editor_unchanged
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index c1eb72555d..2adceb35e2 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -148,7 +148,6 @@ You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO
   (use "git rebase --abort" to check out the original branch)
 
 Unmerged paths:
-  (use "git reset HEAD <file>..." to unstage)
   (use "git add <file>..." to mark resolution)
 
 	both modified:   main.txt
@@ -176,7 +175,6 @@ You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO
   (all conflicts fixed: run "git rebase --continue")
 
 Changes to be committed:
-  (use "git reset HEAD <file>..." to unstage)
 
 	modified:   main.txt
 

-- 
2.24.0
Previous: Junio C HamanoNext: Phillip Wood
Message 27 of 31 in “commit: fix advice for empty commits during rebases”
  1. 0/3 commit: fix advice for empty commits during rebasesJohannes Schindelin via GitGitGadget, Oct 22, 2019
  2. 2/3 sequencer: export the function to get the path of `.git/rebase-merge/`Johannes Schindelin via GitGitGadget, Oct 22, 2019
  3. 1/3 cherry-pick: add test for `--skip` advice in `git commit`Johannes Schindelin via GitGitGadget, Oct 22, 2019
  4. 3/3 commit: give correct advice for empty commit during a rebaseJohannes Schindelin via GitGitGadget, Oct 22, 2019
  5. Junio C HamanoOct 23, 2019
  6. Phillip WoodOct 24, 2019
  7. Johannes SchindelinOct 25, 2019
  8. Phillip WoodOct 25, 2019
  9. Junio C HamanoNov 8, 2019
  10. Johannes SchindelinNov 8, 2019
  11. Phillip WoodNov 11, 2019
  12. Junio C HamanoOct 23, 2019
  13. Johannes SchindelinOct 25, 2019
  14. Junio C HamanoOct 29, 2019
  15. Johannes SchindelinOct 29, 2019
  16. 0/9 commit: fix advice for empty commits during rebasesPhillip Wood, Dec 6, 2019
  17. 2/9 cherry-pick: add test for `--skip` advice in `git commit`Phillip Wood, Dec 6, 2019
  18. 4/9 sequencer: write CHERRY_PICK_HEAD for reword and editPhillip Wood, Dec 6, 2019
  19. 3/9 cherry-pick: check commit error messagesPhillip Wood, Dec 6, 2019
  20. 1/9 t3404: use test_cmp_revPhillip Wood, Dec 6, 2019
  21. Junio C HamanoDec 6, 2019
  22. 6/9 commit: encapsulate determine_whence() for sequencerPhillip Wood, Dec 6, 2019
  23. Junio C HamanoDec 6, 2019
  24. Phillip WoodDec 18, 2019
  25. 5/9 commit: use enum value for multiple cherry-picksPhillip Wood, Dec 6, 2019
  26. Junio C HamanoDec 6, 2019
  27. 9/9 [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflictsPhillip Wood, Dec 6, 2019
  28. Phillip WoodDec 18, 2019
  29. 7/9 commit: give correct advice for empty commit during a rebasePhillip Wood, Dec 6, 2019
  30. 8/9 [RFC] rebase: fix advice when a fixup creates an empty commitPhillip Wood, Dec 6, 2019
  31. Elijah NewrenFeb 26, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.