Re: [PATCH v4 3/3] replay: offer an option to linearize the commit topology
- From
Toon Claes <toon@iotcl.com>
- Date
- Jun 26, 2026, 05:36 UTC
- Message-ID
- <87qzltyiao.fsf@emacs.iotcl.com>
- In-Reply-To
- <ajk-a4a3KSJ2u7Ju@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 14 quoted lines
> git-rebase(1) essentially knows about three different modes: > > - "--no-rebase-merges", which is the default and maps to your > "--linearize". > > - "--rebase-merges", which by default doesn't rebase cousins by using > "--ancestry-path" internally. > > - "--rebase-merges=rebase-cousins", which doesn't pass the above > option. > > So it's not a simple boolean there, which makes me wonder whether we > should mirror the same interface so that all of git-rebase(1)'s modes > can be represented, as well.
That's a valid question, although I don't know a good answer to that.
Basically you're asking for what the command line options will look like? Allow me to think out loud.
In this series I'm adding --linearize to git-replay(1). As mentioned, I don't think it makes sense to add it to git-history(1) as well. Without this option, the process aborts when it encounters a merge.
Dscho sent a patch series to properly replay (2-way) merges. I think this should become the default for both git-replay(1) and git-history(1).
But then, do we want to have an option that brings back the current behavior of aborting at merges? Maybe with --no-merges?
Then there's the option of rebasing cousins left. That's something that isn't covered by Dscho's series yet. Maybe --replay-cousins?
To reiterate what the final design could look like:
* <nothing>: replay merges preserving topology. * "--linearize": flattens merges (only git-replay(1)). * "--no-merges": dies when the process tries to replay a merge. * "--replay-cousins": does what --rebase-merges=rebase-cousins does.
Now, all these options are (I think) mutually exclusive, so we could consider an option "--replay-merges=<mode>", but personally I find "--<option>=<value>" arguments harder to use than specifying separate options.
I think I'm avoiding your question, because the design of the command line parameters doesn't need tot 1-on-1 correlate to the internal datastructure. And I agree the mode isn't a boolean, but does that mean we want to use an enum internally? Well, I don't know. And I also don't think that matters right now. Code is easy to change, I think the command line options should be designed with the future in mind, which I believe we do with "--linearize".
Sorry for this long-winded rambling, but bottom line I think it's fine to add --linearize and in the future add more options and see how the code should evolve to support those.
Show 20 quoted lines
>> diff --git a/replay.c b/replay.c
>> index 7921d7dba3..5539daff00 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");
>
> Nit: Error messages should typically start with a lower-case letter.Thanks.
Show 19 quoted lines
>> @@ -430,12 +435,25 @@ 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!"));
>> + /*
>> + * Drop the merge commit: do not pick it and leave
>> + * last_commit unchanged, so its children (and any ref
>> + * pointing at it) are reparented onto the previous
>> + * non-merge commit, which the ref-update loop below uses.
>> + */
>
> One could add a hint here that tells the user to pass the option. But I
> guess that might be somewhat weird, as we cannot assume that we're
> called by git-replay(1) here.Yeah, true...
-- Cheers, Toon