rebase -r: a bugfix and two status-related improvements

When a user runs 'git rebase --continue' to conclude a conflicted merge during a 'git rebase -r' invocation, we do not create a merge commit if the resolution was empty (i.e. if the index and HEAD are identical). We simply continue the rebase as if no 'merge' instruction had been given. This is confusing since all commits from the side branch are absent from the rebased history. What's more, if that 'merge' is the last instruction in the todo list, we fail to remove the merge state, such that running 'git status' shows we are still merging after the rebase has concluded.
[...]
Make sure to also remove MERGE_HEAD when a merge command fails to start. We already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list for failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this situation, upon 'git rebase --continue' we still exit early in 'commit_staged_changes', without calling 'run_git_commit'. This is already covered by t5407.11, which fails without this change because we enter 'run_git_commit' and then fail to find 'rebase_path_message'.

Signed-off-by Philippe Blain <[email protected]>

---

t/t3418-rebase-continue.sh +7 -0

+test_expect_success '--continue creates merge commit after empty resolution' '
+       git reset --hard main &&
+       git checkout -b rebase_i_merge &&
+       test_commit unrelated &&
+       git checkout -b rebase_i_merge_side &&
+       test_commit side2 main.txt &&
+       git checkout rebase_i_merge &&

t/t3418-rebase-continue.sh test_expect_success 'rebase -r passes merge strategy options correctly' ' · line 130

Quoted 7 lines · Philippe Blain via GitGitGadget

t/t3418-rebase-continue.sh +6 -0

+test_expect_success '--continue creates merge commit after empty resolution' '
+       [...]
+       git commit --no-edit &&
+       FAKE_LINES="1 2 3 5 6 7 8 9 10 11" &&
+       export FAKE_LINES &&
+       test_must_fail git rebase -ir main &&

I don't think you want to be setting FAKE_LINES like this since doing so will pollute the environment for all tests following this one. You can find existing precedent in this script which demonstrates the correct way to handle this case. Specifically, you'd want:

    test_must_fail env FAKE_LINES="1 2 3 5 6 7 8 9 10 11" \
        git rebase -ir main &&

Quoted 15 lines · Eric Sunshine

t/t3418-rebase-continue.sh test_expect_success 'rebase -r passes merge strategy options correctly' ' · line 130

Quoted 7 lines · Philippe Blain via GitGitGadget

t/t3418-rebase-continue.sh +6 -0

+test_expect_success '--continue creates merge commit after empty resolution' '
+       [...]
+       git commit --no-edit &&
+       FAKE_LINES="1 2 3 5 6 7 8 9 10 11" &&
+       export FAKE_LINES &&
+       test_must_fail git rebase -ir main &&

I don't think you want to be setting FAKE_LINES like this since doing so will pollute the environment for all tests following this one. You can find existing precedent in this script which demonstrates the correct way to handle this case. Specifically, you'd want:

Quoted 2 lines

    test_must_fail env FAKE_LINES="1 2 3 5 6 7 8 9 10 11" \
        git rebase -ir main &&

To clarify, by "pollute", I mean that it can impact subsequent tests which don't take care to override FAKE_LINES as necessary. There certainly are test scripts which use the:

    FAKE_LINES=... &&
    export FAKE_LINES &&

form successfully, but such scripts are careful to override/set FAKE_LINES in every test. This particular script (t3418), on the other hand, does not otherwise employ the form in which the variable is exported, so introducing it in a test which is inserted into the middle of the script feels dangerous.

From: Philippe Blain <[email protected]>

When a user runs 'git rebase --continue' to conclude a conflicted merge during a 'git rebase -r' invocation, we do not create a merge commit if the resolution was empty (i.e. if the index and HEAD are identical). We simply continue the rebase as if no 'merge' instruction had been given. This is confusing since all commits from the side branch are absent from the rebased history. What's more, if that 'merge' is the last instruction in the todo list, we fail to remove the merge state, such that running 'git status' shows we are still merging after the rebase has concluded.

This happens because in 'sequencer.c::commit_staged_changes', we exit early before calling 'run_git_commit' if 'is_clean' is true, i.e. if nothing is staged. Fix this by also checking for the presence of MERGE_HEAD before exiting early, such that we do call 'run_git_commit' when MERGE_HEAD is present. This also ensures that we unlink git_path_merge_head later in 'commit_staged_changes' to clear the merge state.

Make sure to also remove MERGE_HEAD when a merge command fails to start. We already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list for failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this situation, upon 'git rebase --continue' we still exit early in 'commit_staged_changes', without calling 'run_git_commit'. This is already covered by t5407.11, which fails without this change because we enter 'run_git_commit' and then fail to find 'rebase_path_message'.

Signed-off-by Philippe Blain <[email protected]>

---
  sequencer.c                |  3 ++-
  t/t3418-rebase-continue.sh | 24 ++++++++++++++++++++++++
  2 files changed, 26 insertions(+), 1 deletion(-)

sequencer.c +0 -0

index ad0ab75c8d4..2baaf716a3c 100644

sequencer.c +13 -0

+test_expect_success '--continue creates merge commit after empty resolution' '
+	git reset --hard main &&
+	git checkout -b rebase_i_merge &&
+	test_commit unrelated &&
+	git checkout -b rebase_i_merge_side &&
+	test_commit side2 main.txt &&
+	git checkout rebase_i_merge &&
+	test_commit side1 main.txt &&
+	PICK=$(git rev-parse --short rebase_i_merge) &&
+	test_must_fail git merge rebase_i_merge_side &&
+	echo side1 >main.txt &&
+	git add main.txt &&
+	test_tick &&

test_expect_success '--skip after failed fixup cleans commit message' '
test_when_finished "test_might_fail git rebase --abort" && git checkout -b with-conflicting-fixup &&

From: Philippe Blain <[email protected]>

When "git status" is invoked during a rebase, we print the last commands done and the next commands to do, and abbreviate commit hashes found in those lines. However, we only abbreviate hashes in 'pick', 'squash' and plain 'fixup' lines, not those in 'merge -C' and 'fixup -C' lines, as the parsing done in wt-status.c::abbrev_oid_in_line is not prepared for such lines.

Improve the parsing done by this function by special casing 'fixup' and 'merge' such that the hash to abbreviate is the string found in the third field of 'split', instead of the second one for other commands. Introduce a 'hash' strbuf pointer to point to the correct field in all cases.

Signed-off-by Philippe Blain <[email protected]>

---
  wt-status.c | 31 ++++++++++++++++++++++---------
  1 file changed, 22 insertions(+), 9 deletions(-)

wt-status.c +22 -9

index 1da5732f57b..d11d9f9f142 100644
@@ -1342,9 +1342,11 @@ static int split_commit_in_progress(struct wt_status *s)
 
  /*
   * Turn
- * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message"
+ * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message" and
+ * "merge -C d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some-branch"
   * into
- * "pick d6a2f03 some message"
+ * "pick d6a2f03 some message" and
+ * "merge -C d6a2f03 some-branch"
   *
   * The function assumes that the line does not contain useless spaces
   * before or after the command.

@@ -1360,20 +1362,31 @@ static void abbrev_oid_in_line(struct strbuf *line)
  	    starts_with(line->buf, "l "))
  		return;
 
-	split = strbuf_split_max(line, ' ', 3);
+	split = strbuf_split_max(line, ' ', 4);
  	if (split[0] && split[1]) {
  		struct object_id oid;
-
+		struct strbuf *hash;
+
+		if ((!strcmp(split[0]->buf, "merge ") ||
+		     !strcmp(split[0]->buf, "m "    ) ||
+		     !strcmp(split[0]->buf, "fixup ") ||
+		     !strcmp(split[0]->buf, "f "    )) &&
+		    (!strcmp(split[1]->buf, "-C ") ||
+		     !strcmp(split[1]->buf, "-c "))) {
+			hash = split[2];
+		} else {
+			hash = split[1];
+		}
  		/*
  		 * strbuf_split_max left a space. Trim it and re-add
  		 * it after abbreviation.
  		 */
-		strbuf_trim(split[1]);
-		if (!repo_get_oid(the_repository, split[1]->buf, &oid)) {
-			strbuf_reset(split[1]);
-			strbuf_add_unique_abbrev(split[1], &oid,
+		strbuf_trim(hash);
+		if (!repo_get_oid(the_repository, hash->buf, &oid)) {
+			strbuf_reset(hash);
+			strbuf_add_unique_abbrev(hash, &oid,
  						 DEFAULT_ABBREV);
-			strbuf_addch(split[1], ' ');
+			strbuf_addch(hash, ' ');
  			strbuf_reset(line);
  			for (i = 0; split[i]; i++)
  				strbuf_addbuf(line, split[i]);

From: Philippe Blain <[email protected]>

Since 982288e9bd (status: rebase and merge can be in progress at the same time, 2018-11-12), when a merge is in progress as part of a 'git rebase -r' operation, 'wt_longstatus_print_state' shows information about the in-progress rebase (via show_rebase_information), and then calls 'show_merge_in_progress' to help the user conclude the merge. This function suggests using 'git commit' to do so, but this throws away the authorship information from the original merge, which is not ideal. Using 'git rebase --continue' instead preserves the authorship information, since we enter 'sequencer.c:run_git_commit' which calls read_env_script to read the author-script file.

Note however that this only works when a merge was scheduled using a 'merge' instruction in the rebase todo list. Indeed, when using 'exec git merge', the state files necessary for 'git rebase --continue' are not present, and one must use 'git commit' (or 'git merge --continue') in that case.

Be more helpful to the user by suggesting either 'git rebase --continue', when the merge was scheduled using a 'merge' instruction, and 'git commit' otherwise. As such, add a 'merge_during_rebase_in_progress' field to 'struct wt_status_state', and detect this situation in wt_status_check_rebase by looking at the last command done. Adjust wt_longstatus_print_state to check this field and suggest 'git rebase --continue' if a merge came from a 'merge' instruction, by calling show_rebase_in_progress directly.

Add two tests for the new behaviour, using 'merge' and 'exec git merge' instructions.

t/t7512-status-help.sh +10 -0

+test_expect_success 'status during rebase -ir after conflicted merge (exec git merge)' '
+	git reset --hard main &&
+	git checkout -b rebase_i_merge &&
+	test_commit unrelated &&
+	git checkout -b rebase_i_merge_side &&
+	test_commit side2 main.txt &&
+	git checkout rebase_i_merge &&
+	test_commit side1 main.txt &&
+	PICK=$(git rev-parse --short rebase_i_merge) &&
+	test_must_fail git merge rebase_i_merge_side &&

wt-status.c +1 -1

  		state->branch = get_branch(wt, "rebase-merge/head-name");
  		state->onto = get_branch(wt, "rebase-merge/onto");
+		string_list_clear(&have_done, 0);
  	} else
  		return 0;
  	return 1;

@@ -1855,10 +1860,15 @@ static void wt_longstatus_print_state(struct wt_status *s)
 
  	if (state->merge_in_progress) {
  		if (state->rebase_interactive_in_progress) {
-			show_rebase_information(s, state_color);

wt-status.c +2 -0

+		} else
+			show_merge_in_progress(s, state_color);
  	} else if (state->am_in_progress)
  		show_am_in_progress(s, state_color);
  	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)

wt-status.h +1 -0

index 4e377ce62b8..84bedfcd48f 100644
@@ -87,6 +87,7 @@ struct wt_status_state {
  	int am_empty_patch;
  	int rebase_in_progress;
  	int rebase_interactive_in_progress;
+	int merge_during_rebase_in_progress;
  	int cherry_pick_in_progress;
  	int bisect_in_progress;
  	int revert_in_progress;

From: Philippe Blain <[email protected]>

Since 982288e9bd (status: rebase and merge can be in progress at the same time, 2018-11-12), when a merge is in progress as part of a 'git rebase -r' operation, 'wt_longstatus_print_state' shows information about the in-progress rebase (via show_rebase_information), and then calls 'show_merge_in_progress' to help the user conclude the merge. This function suggests using 'git commit' to do so, but this throws away the authorship information from the original merge, which is not ideal.

Hi Philippe,

Quoted 9 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

Since 982288e9bd (status: rebase and merge can be in progress at the same time, 2018-11-12), when a merge is in progress as part of a 'git rebase -r' operation, 'wt_longstatus_print_state' shows information about the in-progress rebase (via show_rebase_information), and then calls 'show_merge_in_progress' to help the user conclude the merge. This function suggests using 'git commit' to do so, but this throws away the authorship information from the original merge, which is not ideal.

It is unfortunate that we cannot fix this, as git commit with an interrupted pick _would_ retain authorship, right?

we really not use the same trick with merges?)

Ciao,
Johannes

Quoted 13 lines · Johannes Schindelin

Quoted 10 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

Since 982288e9bd (status: rebase and merge can be in progress at the same time, 2018-11-12), when a merge is in progress as part of a 'git rebase -r' operation, 'wt_longstatus_print_state' shows information about the in-progress rebase (via show_rebase_information), and then calls 'show_merge_in_progress' to help the user conclude the merge. This function suggests using 'git commit' to do so, but this throws away the authorship information from the original merge, which is not ideal.

Quoted 2 lines

It is unfortunate that we cannot fix this, as git commit with an interrupted pick _would_ retain authorship, right?

Unfortunately not. Running "git commit" rather than "git rebase --continue" to commit a conflict resolution when rebasing always loses the authorship.

Quoted 1 line

(Why is that so? Can we really not use the same trick with merges?)

Hi Phillip,

Quoted 9 lines · On Wed, 2 Apr 2025, [email protected] wrote:

Quoted 2 lines · Johannes Schindelin

It is unfortunate that we cannot fix this, as git commit with an interrupted pick _would_ retain authorship, right?

Unfortunately not. Running "git commit" rather than "git rebase --continue" to commit a conflict resolution when rebasing always loses the authorship.

Quoted 1 line

Quoted 1 line

(Why is that so? Can we really not use the same trick with merges?)

Authorship is retained when a git cherry-pick (what an unwieldy command name for _such_ a common operation!) failed with merge conflicts and those conflicts were resolved and the user then calls git commit, though.

Why can this technique not be used in interrupted pick/merge commands of git rebase?git cherry-pick retains authorship by writing CHERRY_PICK_HEAD which

Quoted 10 lines · Johannes Schindelin

Quoted 9 lines · On Wed, 2 Apr 2025, [email protected] wrote:

Quoted 2 lines · Johannes Schindelin

It is unfortunate that we cannot fix this, as git commit with an interrupted pick _would_ retain authorship, right?

Unfortunately not. Running "git commit" rather than "git rebase --continue" to commit a conflict resolution when rebasing always loses the authorship.

Quoted 8 lines

Quoted 1 line

Quoted 1 line

(Why is that so? Can we really not use the same trick with merges?)

Authorship is retained when a git cherry-pick (what an unwieldy command name for _such_ a common operation!) failed with merge conflicts and those conflicts were resolved and the user then calls git commit, though.

Why can this technique not be used in interrupted pick/merge commands of git rebase?

git cherry-pick retains authorship by writing CHERRY_PICK_HEAD which git commit uses to look up the commit message and authorship.

When we're rebasing the sequencer removes CHERRY_PICK_HEAD and instead writes the commit message to MERGE_MSG and the authorship to .git/rebase-merge/author-script. I think the reason for the different behavior is to avoid confusing things like git status.

CHERRY_PICK_HEAD has been removed when rebasing since it was introduced in d7e5c0cbfb0 (Introduce CHERRY_PICK_HEAD, 2011-02-19). These days rebase supports --reset-author-date which means it cannot use the same mechanism as cherry-pick.

Personally I'd much rather we tell people to use "git rebase --continue" to commit their conflict resolutions as using "git commit" has never worked if one wanted to preserve authorship and I think making it work would be a pain and probably fragile as I'm not sure how we'd ensure "git commit" knew it was committing a conflict resolution created by "git rebase" rather than one created by some other commit run while the rebase was stopped or by an exec command.

Quoted 17 lines · On Thu, 3 Apr 2025, [email protected] wrote:

Quoted 16 lines · Johannes Schindelin

Quoted 10 lines · On Wed, 2 Apr 2025, [email protected] wrote:

Quoted 2 lines · Johannes Schindelin

It is unfortunate that we cannot fix this, as git commit with an interrupted pick _would_ retain authorship, right?

Unfortunately not. Running "git commit" rather than "git rebase --continue" to commit a conflict resolution when rebasing always loses the authorship.

Quoted 1 line

(Why is that so? Can we really not use the same trick with merges?)

Authorship is retained when a git cherry-pick (what an unwieldy command name for _such_ a common operation!) failed with merge conflicts and those conflicts were resolved and the user then calls git commit, though.

Quoted 2 lines

Quoted 2 lines

Why can this technique not be used in interrupted pick/merge commands of git rebase?

[Fixed totally garbled formatting that pretended that the first half of this sentence was written by me, the second half by you:]

Quoted 2 lines

git cherry-pick retains authorship by writing CHERRY_PICK_HEAD which git commit uses to look up the commit message and authorship.

And why can we not teach git commit to use the author information recorded in .git/rebase-merge/author-script, too, and teach git reset --hard to delete it?

Quoted 4 lines

When we're rebasing the sequencer removes CHERRY_PICK_HEAD and instead writes the commit message to MERGE_MSG and the authorship to .git/rebase-merge/author-script. I think the reason for the different behavior is to avoid confusing things like git status.

The reason is probably more that you can mix git rebase and git cherry-pick (why does this common operation have such a long name, again?). I actually do this quite often, I frequently even have something like this in my rebase scripts:

exec git cherry-pick ..upstream/seen^{xx/something-something}^2

Quoted 4 lines

CHERRY_PICK_HEAD has been removed when rebasing since it was introduced in d7e5c0cbfb0 (Introduce CHERRY_PICK_HEAD, 2011-02-19). These days rebase supports --reset-author-date which means it cannot use the same mechanism as cherry-pick.

Right. But it can recapitulate cherry-pick's strategy in spirit. After all, git commit had to be taught about an interrupted git cherry-pick so that it _could_ pick up the necessary information and use that. Likewise, git commit could be taught about an interrupted git rebase and similarly pick up the author information from what git rebase recorded.

Quoted 7 lines

Personally I'd much rather we tell people to use "git rebase --continue" to commit their conflict resolutions as using "git commit" has never worked if one wanted to preserve authorship and I think making it work would be a pain and probably fragile as I'm not sure how we'd ensure "git commit" knew it was committing a conflict resolution created by "git rebase" rather than one created by some other commit run while the rebase was stopped or by an exec command.

Even I, the inventor of git rebase -i, have run afoul of this authorship resetting on more than a dozen occasions.

This is proof enough for me that Git is unnecessarily confusing (no big revelation there, right? Git earned that reputation very effortlessly, not only in this particular scenario).

I'd rather like this usability problem to be fixed, even if it is a pain. If the pain stems from the way the source code is organized, well, then maybe this hints at the need to clean up a little?

Hi,

this series started as only 3/3, which I wrote when I noticed that 'git status' suggested 'git commit' instead of 'git rebase --continue' to conclude a merge, and doing that I lost the original authorship of the merge commit.

2/3 is a small improvement I noticed along the way, and while testing these I discovered the bug which I fix in 1/3. I guess 1/3 could go in a different series, if we prefer, but for simplicity I'm submitting them together.

Philippe Blain (3):
rebase -r: do create merge commit after empty resolution
wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase
wt-status: suggest 'git rebase --continue' to conclude 'merge'
instruction

  sequencer.c                |  3 +-
  t/t3418-rebase-continue.sh | 24 ++++++++++++
  t/t7512-status-help.sh     | 75 ++++++++++++++++++++++++++++++++++++++
  wt-status.c                | 49 ++++++++++++++++++-------
  wt-status.h                |  1 +
  5 files changed, 138 insertions(+), 14 deletions(-)

base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1897%2Fphil-blain%2Fstatus-abbreviate-merge-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1897/phil-blain/status-abbreviate-merge-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1897