Re: [PATCH v2 3/3] replay: offer an option to linearize the commit topology
- From
Elijah Newren <newren@gmail.com>
- Date
- Jun 14, 2026, 06:56 UTC
- Message-ID
- <CABPp-BGRi2obnqRGEY9pSMyvRbNGs8AdVUpZmr0C6vZSgHb=cg@mail.gmail.com>
- In-Reply-To
- <20260610-toon-git-replay-drop-merges-v2-3-5714a71c6d83@iotcl.com>
Hi,
On Wed, Jun 10, 2026 at 7:51 AM Toon Claes <toon@iotcl.com> wrote:
Show 12 quoted lines
> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de> > > One of the stated goals of git-replay(1) is to allow implementing the > git-rebase(1) functionality on the server side. > > The default mode of git-rebase(1) is to act as if `--no-rebase-merges` > was given. This mode drops merge commits instead of replaying them, and > linearizes the commit history into a sequence of the > regular (single-parent) commits. > > Add option `--linearize` to git-replay(1) to do the same.
I think this version is nicer overall than the one from my replay-upstream branch; sorry for repeatedly getting distracted from that, but this does look nice.
A few small comments:
Show 23 quoted lines
> Co-authored-by: Toon Claes <toon@iotcl.com> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > Signed-off-by: Toon Claes <toon@iotcl.com> > --- > Documentation/git-replay.adoc | 5 +++++ > builtin/replay.c | 4 ++++ > replay.c | 30 +++++++++++++++++++++++------- > replay.h | 5 +++++ > t/t3650-replay-basics.sh | 26 ++++++++++++++++++++++++++ > 5 files changed, 63 insertions(+), 7 deletions(-) > > diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc > index a32f72aead..41c96c7061 100644 > --- a/Documentation/git-replay.adoc > +++ b/Documentation/git-replay.adoc > @@ -88,6 +88,11 @@ incompatible with `--contained` (which is a modifier for `--onto` only). > + > The default mode can be configured via the `replay.refAction` configuration variable. > > +--linearize:: > + In this mode, `git replay` imitates `git rebase --no-rebase-merges`, > + i.e. it cherry-picks only non-merge commits, each one on top of the > + previous one.
The SYNOPSIS block at the top of the file is missing this new flag.
The replay_usage[] variable in cmd_replay is also missing this new flag.
Show 13 quoted lines
> <revision-range>::
> Range of commits to replay; see "Specifying Ranges" in
> linkgit:git-rev-parse[1]. In `--advance=<branch>` or
> diff --git a/builtin/replay.c b/builtin/replay.c
> index 39e3a86f6c..fedfe46dc6 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -111,6 +111,8 @@ int cmd_replay(int argc,
> N_("mode"),
> N_("control ref update behavior (update|print)"),
> PARSE_OPT_NONEG),
> + OPT_BOOL(0, "linearize", &opts.linearize,
> + N_("ignore merge commits instead of replaying them")),"ignore" feels a bit ambiguous to me. Can we use "drop" instead, matching your commit message?
Show 9 quoted lines
> OPT_END() > }; > > @@ -132,6 +134,8 @@ int cmd_replay(int argc, > opts.contained, "--contained"); > die_for_incompatible_opt2(!!opts.ref, "--ref", > !!opts.contained, "--contained"); > + die_for_incompatible_opt2(!!opts.revert, "--revert", > + opts.linearize, "--linearize");
Sensible; should the docs mention this incompatibility? (I'm not sure myself; just throwing it out as food for thought.)
Show 22 quoted lines
>
> /* Parse ref action mode from command line or config */
> ref_mode = get_ref_action_mode(repo, ref_action);
> diff --git a/replay.c b/replay.c
> index 7921d7dba3..81033fb889 100644
> --- a/replay.c
> +++ b/replay.c
> @@ -277,12 +277,16 @@ static struct commit *pick_regular_commit(struct repository *repo,
> struct commit *onto,
> struct merge_options *merge_opt,
> struct merge_result *result,
> + struct commit *replayed_base,
> bool reverse,
> enum replay_empty_commit_action empty)
> {
> - struct commit *base, *replayed_base;
> + struct commit *base;
> struct tree *pickme_tree, *base_tree, *replayed_base_tree;
>
> + if (replayed_base && reverse)
> + BUG("Linearizing commits is not supported when replaying in reverse");
> +This is dead code given the die_for_incompatible_opt2 check above, right? Just extra defense in depth?
Show 26 quoted lines
> if (pickme->parents) {
> base = pickme->parents->item;
> base_tree = repo_get_commit_tree(repo, base);
> @@ -291,7 +295,8 @@ static struct commit *pick_regular_commit(struct repository *repo,
> base_tree = lookup_tree(repo, repo->hash_algo->empty_tree);
> }
>
> - replayed_base = get_mapped_commit(replayed_commits, base, onto);
> + if (!replayed_base)
> + replayed_base = get_mapped_commit(replayed_commits, base, onto);
> replayed_base_tree = repo_get_commit_tree(repo, replayed_base);
> pickme_tree = repo_get_commit_tree(repo, pickme);
>
> @@ -430,12 +435,23 @@ int replay_revisions(struct rev_info *revs,
> while ((commit = get_revision(revs))) {
> const struct name_decoration *decoration;
>
> - if (commit->parents && commit->parents->next)
> - die(_("replaying merge commits is not supported yet!"));
> + if (commit->parents && commit->parents->next) {
> + if (!opts->linearize)
> + die(_("replaying merge commits is not supported yet!"));
> + /*
> + * When linearizing, a merge commit itself is not picked,
> + * but refs that point to it might need updating.
> + */Is it worth pointing out that last_commit is intentionally not updated by this code path? That is implied by your comment, but it takes a bit of reasoning to get there, and I think it might help future readers to just explicitly state it.
Show 58 quoted lines
> + } else {
> + struct commit *to_pick = reverse ? last_commit : onto;
> + last_commit =
> + pick_regular_commit(revs->repo, commit,
> + replayed_commits, to_pick,
> + &merge_opt, &result,
> + opts->linearize ? last_commit : NULL,
> + reverse, opts->empty);
> + }
>
> - last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
> - reverse ? last_commit : onto,
> - &merge_opt, &result, reverse, opts->empty);
> if (!last_commit)
> break;
>
> diff --git a/replay.h b/replay.h
> index 1851a07705..07e6fdcca3 100644
> --- a/replay.h
> +++ b/replay.h
> @@ -62,6 +62,11 @@ struct replay_revisions_options {
> * Defaults to REPLAY_EMPTY_COMMIT_DROP.
> */
> enum replay_empty_commit_action empty;
> +
> + /*
> + * Whether to linearize the commits (i.e. drop merge commits).
> + */
> + int linearize;
> };
>
> /* This struct is used as an out-parameter by `replay_revisions()`. */
> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> index 3353bc4a4d..64e0731188 100755
> --- a/t/t3650-replay-basics.sh
> +++ b/t/t3650-replay-basics.sh
> @@ -565,4 +565,30 @@ test_expect_success '--onto with --ref rejects multiple revision ranges' '
> test_grep "cannot be used with multiple revision ranges" err
> '
>
> +test_expect_success 'replay merge commit fails' '
> + echo "fatal: replaying merge commits is not supported yet!" >expect &&
> + test_must_fail git replay --ref-action=print --onto main I..P 2>actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'replay to rebase merge commit with --linearize' '
> + git replay --ref-action=print --linearize --onto main I..topic-with-merge >result &&
> +
> + test_line_count = 1 result &&
> +
> + git log --format=%s $(cut -f 3 -d " " result) >actual &&
> + test_write_lines O N J M L B A >expect &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'replay to rebase merge commit with --linearize down to root commit' '
> + git replay --ref-action=print --linearize --onto main A..topic-with-merge >result &&You'd need to drop "A.." to have it go down to the root commit, as Junio mentioned elsewhere.
Show 9 quoted lines
> + > + test_line_count = 1 result && > + > + git log --format=%s $(cut -f 3 -d " " result) >actual && > + test_write_lines O N J I M L B A >expect && > + test_cmp expect actual > +' > + > test_done
Should there also be a testcase combining --linearize and --advance?
Should there be a test with the incompatibility of --revert & --linearize? I think we have a few other tests for incompatible options.
One additional testing idea, borrowed from an older variant of this patch I had sitting in a local branch (dscho's original linearize patch, adapted): in addition to checking specific commit subjects, it's worth verifying that the linearized chain produces the *same patches* as the original. Something along the lines of:
test_expect_success '--linearize preserves patches' '
test_when_finished "git update-ref -d refs/heads/merge_I_L" &&
test_tick &&
git checkout -b merge_I_L I &&
git merge --no-edit L &&git replay --linearize --onto A B..merge_I_L &&
# range-diff ignores merges, so the original
# {I, L, merge} reduces to {I, L} on the LHS,
# and the replayed chain on the RHS should match.
git range-diff B..merge_I_L@{1} B..merge_I_L >out &&
! test_grep -v "=" out && git log --oneline A..merge_I_L >out &&
test_line_count = 2 out
'The range-diff check is nice because it asserts patch equivalence rather than tying the test to a particular replay ordering, which makes the test less brittle if the rev-walk order ever changes. Feel free to take, adapt, or ignore.
Anyway, thanks for working on this; looking good.
Elijah