Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>, Toon Claes <toon@iotcl.com>
Cc: git@vger.kernel.org, Elijah Newren <newren@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v5 3/3] replay: offer an option to linearize the commit topology
Date: Sat, 27 Jun 2026 14:44:11 +0100	[thread overview]
Message-ID: <b5d70a0b-ef32-49c9-84ba-8a64b7809574@gmail.com> (raw)
In-Reply-To: <xmqq5x358byf.fsf@gitster.g>

On 26/06/2026 18:10, Junio C Hamano wrote:
> Toon Claes <toon@iotcl.com> writes:
> 
>>   Documentation/git-replay.adoc |  8 ++++-
>>   builtin/replay.c              |  6 +++-
>>   replay.c                      | 50 ++++++++++++++++----------
>>   replay.h                      |  5 +++
>>   t/t3650-replay-basics.sh      | 84 ++++++++++++++++++++++++++++++++++++++++++-
>>   5 files changed, 132 insertions(+), 21 deletions(-)
> 
> "replay --linearize" behaves differently from the flattening rebase
> in a case where X and Y that forked from A are merged at Z, and we
> ask to flatten the history leading to Z, doesn't it?

That's a good point. rebase takes the list of commits given by "git 
rev-list --reverse --no-merges" and cherry-picks each on on top of the 
previous one. In contrast replay cherry-picks each commit on top of its 
rewritten parent so it does not flatten the topology.

Thanks

Phillip
>       A----X
>        \    \
>         Y----Z (tip)
> 
> A typical flattening rebase would rewrite X to X', Y to Y', while
> dropping Z, and would leave us a flattened history, like
> 
>       A---X'---Y' (updated tip, the order of X' and Y' may be swapped)
> 
> I may be misreading the logic, but doesn't "replay --linearize"
> instead produce
> 
>       A----X' (dangling)
>        \
>         Y' (tip -- Z is dropped and gets mapped)
> 
> and leave X' dangling (or Y'; the point is that only one of them
> will survive), never incorporating it in the resulting history?
> 
>> +		if (commit->parents && commit->parents->next) {
>> +			if (!opts->linearize)
>> +				die(_("replaying merge commits is not supported yet!"));
>> +			/*
>> +			 * Drop the merge commit: do not pick it, leave
>> +			 * `last_commit` unchanged, and fall through to the
>> +			 * rest of the loop. As a result:
>> +			 * - the merge commit is mapped to `last_commit` in
>> +			 *   `replayed_commits`, this will become the parent for
>> +			 *   the child commits.
>> +			 * - refs previously pointing to the merge commit are
>> +			 *   rewritten to point to the previous non-merge commit.
>> +			 */
>> +		} else {
>> +			/*
>> +			 * pick_regular_commit() looks up the parent of `commit` in
>> +			 * `replayed_commits` to determine the ancestor to replay onto.
>> +			 * The `default_base` parameter is used when no ancestor is found,
>> +			 * which happens for the first commit in the revision range.
>> +			 * When reverting, commits are replayed in reverse order, so the
>> +			 * lookup never succeeds, and we need to pass `last_commit`.
>> +			 */
>> +			struct commit *base = onto;
>> +			if (mode == REPLAY_MODE_REVERT)
>> +				base = last_commit;
>> +
>> +			last_commit = pick_regular_commit(revs->repo, commit, base,
>> +							  replayed_commits,
>> +							  &merge_opt, &result,
>> +							  mode, opts->empty);
>> +		}
>> +
>>   		if (!last_commit)
>>   			break;
> 
> Immediately after this hunk beyond the post-context are these lines.
> 
> 		/* Record commit -> last_commit mapping */
> 		put_mapped_commit(replayed_commits, commit, last_commit);
> 
> Let's imagine X gets processed first. X (and other commits on its
> branch) gets replayed, last_commit is set to X' (which is the
> rewritten X).  replayed_commits mapping holds X->X' mapping.
> 
> Then let's imagine the history leading to Y is replayed next.
> last_commit becomes Y', and Y->Y' mapping is stored in
> replayed_commits.
> 
> Finally, we see Z.  We are going to _drop_ it.  last_commit is left
> unchanged, pointing at Y'.  Then last_commit (i.e., Y') is used as
> the merge commit Z maps to (i.e., correctly dropping Z).
> 
> Any descendants of Z, if any, will be grafted as descendants of Y'.
> If X did not have any descendants other than Z in the rewritten part
> of the history, then X' (and commits leading to it) would be lost,
> no?
> 
> This "loss of the other branch" may be an inherent characteristic of
> this feature (i.e., I do not think it is necessarily a bug, and it
> may even be that the "bug" is in the way I am reading the patch),
> but then I wonder if the user may want to have control over which
> side branch should survive, perhaps?  It would probably need to be
> documented, and a test or two to cast this behaviour in stone.
> 
>> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
>> index 3353bc4a4d..34c038eab9 100755
>> --- a/t/t3650-replay-basics.sh
>> +++ b/t/t3650-replay-basics.sh
>> @@ -52,8 +52,12 @@ test_expect_success 'setup' '
> 
> The pre-context here has
> 
> 	git switch --detach topic4 &&
> 	test_commit N &&
> 	test_commit O &&
> 	git switch -c topic-with-merge topic4 &&
> 
>>   	test_merge P O --no-ff &&
>>   	git switch main &&
> 
> The above does prepare topic-with-merge branch, but ...
> 
>> +test_expect_success 'replay to rebase merge commit with --linearize' '
>> +	git replay --ref-action=print --linearize \
>> +		--onto main I..topic-with-merge >result &&
> 
> ... this does not really exersize linearizing replay in a typical
> mergy history.  P merges O with --no-ff because otherwise there
> won't be a merge, since O is a descendant of the commit "test_merge
> P O" runs on (i.e., topic4 == topic-with-merge).
> 
>      topic4 --- N --- O
>            \           \
>             .-----------P
> 
> So, as long as O is replayed later than the parent of N (which is
> true), O' will be the surviving tip (corresponds to Y' that the
> dropped Z was mapped to in the earlier example), and nothing gets
> orphaned, I think.
> 
> Perhaps a test to try a real merge may look something like this.
> 
> diff --git c/t/t3650-replay-basics.sh w/t/t3650-replay-basics.sh
> index 34c038eab9..bb737f729a 100755
> --- c/t/t3650-replay-basics.sh
> +++ w/t/t3650-replay-basics.sh
> @@ -647,4 +647,37 @@ test_expect_success 'replay with --linearize to rebase multiple divergent branch
>   	test_cmp expect actual
>   '
>   
> +test_expect_success 'replay with --linearize of a divergent merge drops one branch' '
> +	git switch -c topic-divergent-base main &&
> +	test_commit base &&
> +	# Fork 1: base -> X
> +	git switch -c topic-divergent-x &&
> +	test_commit X &&
> +	# Fork 2: base -> Y
> +	git switch topic-divergent-base &&
> +	git switch -c topic-divergent-y &&
> +	test_commit Y &&
> +	# Merge them at Z
> +	git switch topic-divergent-x &&
> +	test_merge Z topic-divergent-y --no-ff &&
> +
> +	# History is now:
> +	#
> +	#       X - Z (topic-divergent-x)
> +	#      /   /
> +	#  base - Y
> +	#
> +
> +	git replay --ref-action=print --linearize \
> +		--onto main topic-divergent-base..topic-divergent-x >result &&
> +	test_line_count = 1 result &&
> +	tip=$(cut -f 3 -d " " result) &&
> +	# Get the commits replayed onto main
> +	git log --format=%s main..$tip >actual &&
> +	# We expect exactly one commit to be replayed (either X or Y)
> +	# because the other one is left dangling due to the merge being dropped.
> +	test_line_count = 1 actual &&
> +	test_grep "^[XY]$" actual
> +'
> +
>   test_done
> 


      reply	other threads:[~2026-06-27 13:44 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-08 18:37 [PATCH 0/3] Teach git-replay(1) to linearize merge commits Toon Claes
2026-06-08 18:37 ` [PATCH 1/3] replay: refactor enum replay_mode into a bool Toon Claes
2026-06-08 18:37 ` [PATCH 2/3] replay: add helper to put entry into mapped_commits Toon Claes
2026-06-08 18:37 ` [PATCH 3/3] replay: offer an option to linearize the commit topology Toon Claes
2026-06-08 19:29   ` Junio C Hamano
2026-06-10 14:26     ` Toon Claes
2026-06-10 14:49 ` [PATCH v2 0/3] Teach git-replay(1) to linearize merge commits Toon Claes
2026-06-10 14:49   ` [PATCH v2 1/3] replay: refactor enum replay_mode into a bool Toon Claes
2026-06-11 15:09     ` Justin Tobler
2026-06-12  8:19       ` Toon Claes
2026-06-10 14:49   ` [PATCH v2 2/3] replay: add helper to put entry into mapped_commits Toon Claes
2026-06-10 14:49   ` [PATCH v2 3/3] replay: offer an option to linearize the commit topology Toon Claes
2026-06-10 17:02     ` Junio C Hamano
2026-06-16  8:38       ` Toon Claes
2026-06-14  6:56     ` Elijah Newren
2026-06-16  7:09       ` Toon Claes
2026-06-16  9:26   ` [PATCH v3 0/3] Teach git-replay(1) to linearize merge commits Toon Claes
2026-06-16  9:26     ` [PATCH v3 1/3] replay: refactor enum replay_mode into a bool Toon Claes
2026-06-16  9:26     ` [PATCH v3 2/3] replay: add helper to put entry into mapped_commits Toon Claes
2026-06-16  9:26     ` [PATCH v3 3/3] replay: offer an option to linearize the commit topology Toon Claes
2026-06-22 12:41     ` [PATCH v4 0/3] Teach git-replay(1) to linearize merge commits Toon Claes
2026-06-22 12:41       ` [PATCH v4 1/3] replay: refactor enum replay_mode into a bool Toon Claes
2026-06-22 13:53         ` Patrick Steinhardt
2026-06-22 15:43           ` Junio C Hamano
2026-06-24 19:15             ` Toon Claes
2026-06-22 12:41       ` [PATCH v4 2/3] replay: add helper to put entry into mapped_commits Toon Claes
2026-06-22 13:53         ` Patrick Steinhardt
2026-06-22 12:41       ` [PATCH v4 3/3] replay: offer an option to linearize the commit topology Toon Claes
2026-06-22 13:53         ` Patrick Steinhardt
2026-06-26  5:36           ` Toon Claes
2026-06-26  5:48       ` [PATCH v5 0/3] Teach git-replay(1) to linearize merge commits Toon Claes
2026-06-26  5:48         ` [PATCH v5 1/3] replay: add helper to put entry into mapped_commits Toon Claes
2026-06-26 16:50           ` Junio C Hamano
2026-06-26  5:48         ` [PATCH v5 2/3] replay: better explain how pick_regular_commit() picks a base Toon Claes
2026-06-26  5:48         ` [PATCH v5 3/3] replay: offer an option to linearize the commit topology Toon Claes
2026-06-26 17:10           ` Junio C Hamano
2026-06-27 13:44             ` Phillip Wood [this message]

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=b5d70a0b-ef32-49c9-84ba-8a64b7809574@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=toon@iotcl.com \
    /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