Git development
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>, Patrick Steinhardt <ps@pks.im>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH/RFC 5/5] t3454: cover merge-replay scenarios with the historian helper
Date: Wed, 06 May 2026 22:43:24 +0000	[thread overview]
Message-ID: <2dec28b43a8c12e9d0cb309945c06d927833bcf3.1778107405.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2106.git.1778107405.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Add a dedicated test script for `git history reword` (and
`git replay` via the same code path) across 2-parent merges, using
the `test-tool historian` fixture builder so each scenario reads as
a small declarative recipe rather than a sequence of plumbing
commands.

The script exercises the cases that motivated the merge-replay
work:

  * a clean merge where each side touches unrelated files;

  * a non-trivial merge where the same line was changed on both
    sides and the user resolved by hand (textual manual resolution
    must be preserved through the replay);

  * a non-trivial merge where the user also touched a line outside
    any conflict region (a "semantic" edit must also be preserved
    through the replay);

  * an octopus merge in the rewrite path, which is rejected;

  * a function rename across the merge with a brand-new caller
    introduced by the rewritten parents. The pre-existing caller
    that the user manually renamed in the original merge must keep
    its rename, and the brand-new caller must _not_ be rewritten
    (calvin/hobbes naming chosen for legibility). This second part
    is the documented limitation: the replay propagates the textual
    diffs the user actually made, it does not extrapolate
    symbol-level intent. Symbol-aware refactoring is out of scope,
    just as it is for plain rebase.

The fixture builder lets each scenario sit in roughly a dozen lines
of historian directives plus the assertions, which keeps the test
file readable when more scenarios are added later.

Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/meson.build             |   1 +
 t/t3454-history-merges.sh | 308 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 309 insertions(+)
 create mode 100755 t/t3454-history-merges.sh

diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5..25b0119d43 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -397,6 +397,7 @@ integration_tests = [
   't3450-history.sh',
   't3451-history-reword.sh',
   't3452-history-split.sh',
+  't3454-history-merges.sh',
   't3500-cherry.sh',
   't3501-revert-cherry-pick.sh',
   't3502-cherry-pick-merge.sh',
diff --git a/t/t3454-history-merges.sh b/t/t3454-history-merges.sh
new file mode 100755
index 0000000000..2eb3c947eb
--- /dev/null
+++ b/t/t3454-history-merges.sh
@@ -0,0 +1,308 @@
+#!/bin/sh
+
+test_description='git history reword across merge commits
+
+Exercises the merge-replay path in `git history reword` using the
+`test-tool historian` test fixture builder so each scenario is
+described in a small declarative input rather than a sprawling
+sequence of plumbing commands. The interesting cases are:
+
+  * a clean merge with each side touching unrelated files;
+  * a non-trivial merge whose conflicting line was resolved by hand
+    (textually) and whose resolution must be preserved through the
+    replay;
+  * a non-trivial merge with a manual *semantic* edit (an additional
+    change outside the conflict region) that must also be preserved.
+'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+# Replace the commit's message via a fake editor and run reword.
+reword_to () {
+	new_msg="$1"
+	target="$2"
+	write_script fake-editor.sh <<-EOF &&
+	echo "$new_msg" >"\$1"
+	EOF
+	test_set_editor "$(pwd)/fake-editor.sh" &&
+	git history reword "$target" &&
+	rm fake-editor.sh
+}
+
+build_clean_merge () {
+	test-tool historian <<-\EOF
+	# Setup:
+	#       A (a) --- C (a, h) ----+--- M (a, g, h)
+	#        \                    /
+	#         +-- B (a, g) ------+
+	#
+	# Topic touches `g` only; main touches `h` only. The auto-merge
+	# at M is clean.
+	blob a "shared content"
+	blob g guarded
+	blob h host
+	commit A main "A" a=a
+	commit B topic "B (introduces g)" from=A a=a g=g
+	commit C main "C (introduces h)" a=a h=h
+	commit M main "Merge topic" merge=B a=a g=g h=h
+	EOF
+}
+
+test_expect_success 'clean merge: both sides touch unrelated files' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		build_clean_merge &&
+
+		reword_to "AA" A &&
+
+		# The merge is still a 2-parent merge with the same subject
+		# and tree (clean replay leaves content unchanged).
+		test_cmp_rev HEAD^{tree} M^{tree} &&
+
+		echo "Merge topic" >expect-subject &&
+		git log -1 --format=%s HEAD >subject &&
+		test_cmp expect-subject subject &&
+
+		git rev-list --merges HEAD~..HEAD >merges &&
+		test_line_count = 1 merges
+	)
+'
+
+build_textual_resolution () {
+	test-tool historian <<-\EOF
+	# Both sides change the same line of `a`; the user resolved with
+	# their own combined text, recorded directly as the merge tree.
+	blob a_v1 line1 line2 line3
+	blob a_main line1 line2-main line3
+	blob a_topic line1 line2-topic line3
+	blob a_resolution line1 line2-merged-by-hand line3
+	commit A main "A" a=a_v1
+	commit B topic "B (line2 on topic)" from=A a=a_topic
+	commit C main "C (line2 on main)" a=a_main
+	commit M main "Merge topic" merge=B a=a_resolution
+	EOF
+}
+
+test_expect_success 'non-trivial merge: textual manual resolution is preserved' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		build_textual_resolution &&
+
+		reword_to "AA" A &&
+
+		git show HEAD:a >after &&
+		test_write_lines line1 line2-merged-by-hand line3 >expect &&
+		test_cmp expect after
+	)
+'
+
+build_semantic_edit () {
+	test-tool historian <<-\EOF
+	# Topic and main conflict on line2 of `a`. The user's resolution
+	# at M not only picks combined text on line2 but ALSO touches
+	# line5 (a "semantic" edit outside any conflict region) -- this
+	# kind of edit is invisible to a naive pick-one-side strategy and
+	# must be preserved by replay.
+	blob a_v1 line1 line2 line3 line4 line5
+	blob a_main line1 line2-main line3 line4 line5
+	blob a_topic line1 line2-topic line3 line4 line5
+	blob a_resolution line1 line2-merged line3 line4 line5-touched
+	commit A main "A" a=a_v1
+	commit B topic "B (line2 on topic)" from=A a=a_topic
+	commit C main "C (line2 on main)" a=a_main
+	commit M main "Merge topic" merge=B a=a_resolution
+	EOF
+}
+
+test_expect_success 'non-trivial merge: semantic edit outside conflict region is preserved' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		build_semantic_edit &&
+
+		reword_to "AA" A &&
+
+		git show HEAD:a >after &&
+		test_write_lines line1 line2-merged line3 line4 line5-touched \
+			>expect &&
+		test_cmp expect after
+	)
+'
+
+build_octopus () {
+	test-tool historian <<-\EOF
+	blob a "x"
+	commit A main "A" a=a
+	commit B b1 "B" from=A a=a
+	commit C b2 "C" from=A a=a
+	commit D b3 "D" from=A a=a
+	commit O main "octopus" merge=B merge=C merge=D a=a
+	EOF
+}
+
+test_expect_success 'octopus merge in the rewrite path is rejected' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		build_octopus &&
+
+		test_must_fail git -c core.editor=true history reword \
+			--dry-run A 2>err &&
+		test_grep "octopus" err
+	)
+'
+
+build_with_boundary_other_than_onto () {
+	test-tool historian <<-\EOF
+	# Setup an "evil merge" topology where the rewrite range crosses
+	# a 2-parent merge whose first parent sits outside that range:
+	#
+	#   side -- O (a=v0)
+	#            \
+	#             M (parent1=O, parent2=R, a=v0, s=top)
+	#            /
+	#   A (a=v0) -- R (a=v0) -- T (a=v0, s=top)
+	#       |
+	#       reword target
+	#
+	# The walk for `history reword A` excludes A and its ancestors,
+	# so O sits outside the rewrite range and is not the boundary
+	# either. Replaying M correctly requires that first parent to
+	# remain at O (preserve, not replant).
+	blob v0 line1 line2 line3
+	blob top "marker"
+	commit X side "X" v0=v0
+	commit O side "O" v0=v0
+	commit A main "A" from=X v0=v0
+	commit R main "R" v0=v0
+	commit M main "Merge side into main" from=O merge=R v0=v0 s=top
+	commit T main "T" v0=v0 s=top
+	EOF
+}
+
+# A descendant merge whose first parent sits outside the rewrite
+# range is a topology that any reasonable replay of merges has to
+# handle correctly: the first parent must be preserved verbatim,
+# while the in-range second parent is rewritten. Without that, the
+# replayed merge would silently graft itself onto a different
+# ancestry than the author chose, which is far worse than a loud
+# failure.
+test_expect_success 'merge whose first parent sits outside the rewrite range keeps that parent' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		build_with_boundary_other_than_onto &&
+
+		reword_to "AA" A &&
+
+		# The replayed M (now HEAD~) is still a 2-parent merge.
+		# Its first parent is the original O (preserved, outside
+		# the rewrite range), its second parent is the rewritten
+		# R. T was rebased on top of M, so HEAD = T.
+		git rev-list --parents -1 HEAD~ >parents &&
+		new_p1=$(awk "{print \$2}" parents) &&
+		new_p2=$(awk "{print \$3}" parents) &&
+
+		# First parent is preserved verbatim.
+		test_cmp_rev O $new_p1 &&
+
+		# Second parent is the rewritten R: a fresh commit whose
+		# subject is still "R" but whose OID differs from the
+		# original (because its parent A is now reworded).
+		echo R >expect &&
+		git log -1 --format=%s $new_p2 >actual &&
+		test_cmp expect actual &&
+		! test_cmp_rev R $new_p2 &&
+
+		# T was rebased on top of the new M, and its tree still
+		# contains the s=top marker introduced in the original M.
+		echo "marker" >expect &&
+		git show HEAD:s >actual &&
+		test_cmp expect actual
+	)
+'
+
+build_function_rename () {
+	test-tool historian <<-\EOF
+	# Topic renames harry() -> hermione() (defs.h plus caller1). main
+	# adds caller2 calling harry(); the original merge M manually
+	# renames caller2 to hermione(). The "newer" base on a side branch
+	# contains caller2 AND a brand-new caller3 calling harry();
+	# replaying onto `newer` therefore introduces caller3 into the
+	# merged tree.
+	blob defs_harry "void harry(void);"
+	blob defs_hermione "void hermione(void);"
+	blob harry_call "harry();"
+	blob hermione_call "hermione();"
+	commit A main "A" defs.h=defs_harry caller1=harry_call
+	commit B topic "B (rename)" from=A defs.h=defs_hermione caller1=hermione_call
+	commit C main "C (caller2 calls harry)" defs.h=defs_harry caller1=harry_call caller2=harry_call
+	commit M main "Merge topic" merge=B defs.h=defs_hermione caller1=hermione_call caller2=hermione_call
+	commit NEW newer "newer base with caller3" from=A defs.h=defs_harry caller1=harry_call caller2=harry_call caller3=harry_call
+	EOF
+}
+
+# This case checks two things at once. First, the manual semantic
+# edit in M (renaming caller2) must be preserved when we replay onto
+# a different base; that is the case `git history` and `git replay`
+# need to handle correctly, even though nothing in the conflict
+# markers tells us about it. Second, a file that only enters the
+# tree via the rewritten parents (caller3, present on the `newer`
+# base) is _not_ renamed by the replay. The replay propagates the
+# textual diffs the user actually made in M; it does _not_ infer
+# the user's symbol-level intent ("rename every caller of harry").
+# This is a known and intentional limitation. Symbol-aware
+# refactoring is out of scope here, just as it is for plain rebase.
+test_expect_success 'preserves manual rename of pre-existing caller; does not extrapolate to new files' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		build_function_rename &&
+
+		# Replay (C, B, M) onto the newer base. A `main..M` style
+		# range across two unrelated branches is awkward; spin up a
+		# temp branch and use --advance.
+		git branch tmp main &&
+		git replay --ref-action=print --onto NEW A..tmp >result &&
+		new_tip=$(cut -f 3 -d " " result) &&
+
+		# defs.h and caller1 came from B (clean cherry-pick of the
+		# rename commit) and must reflect the rename.
+		echo "void hermione(void);" >expect &&
+		git show $new_tip:defs.h >actual &&
+		test_cmp expect actual &&
+
+		echo "hermione();" >expect &&
+		git show $new_tip:caller1 >actual &&
+		test_cmp expect actual &&
+
+		# caller2 existed in the original M; its manual rename to
+		# hermione() is the semantic edit the replay must preserve.
+		echo "hermione();" >expect &&
+		git show $new_tip:caller2 >actual &&
+		test_cmp expect actual &&
+
+		# caller3 only exists on the newer base, so it was brought
+		# in by N (the auto-merge of the rewritten parents). The
+		# replay has no way to know the user intended to rename
+		# every caller; caller3 keeps harry(). The resulting tree
+		# is therefore _not_ symbol-correct and needs a follow-up
+		# edit. This is the documented limitation.
+		echo "harry();" >expect &&
+		git show $new_tip:caller3 >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
gitgitgadget

  parent reply	other threads:[~2026-05-06 22:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-06 22:43 [PATCH/RFC 0/5] replay: support replaying 2-parent merges Johannes Schindelin via GitGitGadget
2026-05-06 22:43 ` [PATCH/RFC 1/5] " Johannes Schindelin via GitGitGadget
2026-05-08  9:36   ` Phillip Wood
2026-05-08 10:05     ` Phillip Wood
2026-05-06 22:43 ` [PATCH/RFC 2/5] replay: short-circuit merge replay when parent and base trees are unchanged Johannes Schindelin via GitGitGadget
2026-05-06 22:43 ` [PATCH/RFC 3/5] history.adoc: describe merge-replay support and its limits Johannes Schindelin via GitGitGadget
2026-05-06 22:43 ` [PATCH/RFC 4/5] test-tool: add a "historian" subcommand for building merge fixtures Johannes Schindelin via GitGitGadget
2026-05-12 10:54   ` Toon Claes
2026-05-06 22:43 ` Johannes Schindelin via GitGitGadget [this message]
2026-05-07 14:14 ` [PATCH/RFC 0/5] replay: support replaying 2-parent merges D. Ben Knoble
2026-05-07 15:06   ` Johannes Schindelin
2026-05-07 15:39     ` Ben Knoble

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=2dec28b43a8c12e9d0cb309945c06d927833bcf3.1778107405.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=newren@gmail.com \
    --cc=ps@pks.im \
    /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