git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Philippe Blain <levraiphilippeblain@gmail.com>
Subject: [PATCH 1/3] rebase -r: do create merge commit after empty resolution
Date: Fri, 28 Mar 2025 17:03:19 +0000	[thread overview]
Message-ID: <6c8f77cb71c7e0c820704b1725331f4601d8876e.1743181401.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1897.git.1743181401.gitgitgadget@gmail.com>

From: Philippe Blain <levraiphilippeblain@gmail.com>

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 <levraiphilippeblain@gmail.com>
---
 sequencer.c                |  3 ++-
 t/t3418-rebase-continue.sh | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index ad0ab75c8d4..2baaf716a3c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4349,6 +4349,7 @@ static int do_merge(struct repository *r,
 		error(_("could not even attempt to merge '%.*s'"),
 		      merge_arg_len, arg);
 		unlink(git_path_merge_msg(r));
+		unlink(git_path_merge_head(r));
 		goto leave_merge;
 	}
 	/*
@@ -5364,7 +5365,7 @@ static int commit_staged_changes(struct repository *r,
 		flags |= AMEND_MSG;
 	}
 
-	if (is_clean) {
+	if (is_clean && !file_exists(git_path_merge_head(r))) {
 		if (refs_ref_exists(get_main_ref_store(r),
 				    "CHERRY_PICK_HEAD") &&
 		    refs_delete_ref(get_main_ref_store(r), "",
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f7225..f4b459fea16 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -111,6 +111,30 @@ test_expect_success 'rebase -r passes merge strategy options correctly' '
 	git rebase --continue
 '
 
+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 &&
+	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 &&
+	echo side1 >main.txt &&
+	git add main.txt &&
+	git rebase --continue &&
+	git log --merges >out &&
+	test_grep "Merge branch '\''rebase_i_merge_side'\''" out
+'
+
 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 &&
-- 
gitgitgadget


  reply	other threads:[~2025-03-28 17:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-28 17:03 [PATCH 0/3] rebase -r: a bugfix and two status-related improvements Philippe Blain via GitGitGadget
2025-03-28 17:03 ` Philippe Blain via GitGitGadget [this message]
2025-03-28 17:14   ` [PATCH 1/3] rebase -r: do create merge commit after empty resolution Eric Sunshine
2025-03-28 17:23     ` Eric Sunshine
2025-04-01 16:17       ` Johannes Schindelin
2025-03-31 15:37   ` Phillip Wood
2025-03-28 17:03 ` [PATCH 2/3] wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase Philippe Blain via GitGitGadget
2025-03-31 15:37   ` Phillip Wood
2025-03-28 17:03 ` [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction Philippe Blain via GitGitGadget
2025-03-31 15:38   ` Phillip Wood
2025-04-01 16:22   ` Johannes Schindelin
2025-04-02 13:09     ` phillip.wood123
2025-04-03 12:17       ` Johannes Schindelin
2025-04-03 15:08         ` phillip.wood123
2025-04-04 11:41           ` Johannes Schindelin
2025-04-04 14:13             ` Phillip Wood
2025-03-31 15:38 ` [PATCH 0/3] rebase -r: a bugfix and two status-related improvements Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6c8f77cb71c7e0c820704b1725331f4601d8876e.1743181401.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).