Re: [PATCH 3/3] replay: offer an option to linearize the commit topology
From: Toon Claes <hidden>
Date: 2026-06-10 14:26:27
Junio C Hamano [off-list ref] writes:
Toon Claes [off-list ref] writes:quoted
From: Johannes Schindelin <redacted> 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 linearized the commit history into a sequence of the regular (single-parent) commits."linearized" -> "linearizes"?
Thanks.
quoted
Add option `--linearize` to git-replay(1) do the same."do the same" -> "to do the same"?
Ack.
quoted
Co-authored-by: Toon Claes [off-list ref]There is no sign-off by any of the authors?
My bad. I'll add mine. @Johannes, can I re-add yours? I've removed it because I've made some changes on top of the patch you wrote, but if you agree, I'll add your Sign-off back.
quoted
@@ -430,12 +435,20 @@ int replay_revisions(struct rev_info *revs, while ((commit = get_revision(revs))) { const struct name_decoration *decoration; - if (commit->parents && commit->parents->next) + if (opts->linearize && (!commit->parents || commit->parents->next)) + ; /* map current commit to the same as the previous commit */This uses the same treatment on either root commits or merge commits? If this were a mistake and this wants to handle merges but not roots, shouldn't it be more like if (opts->linearize && (commit->parents && commit->parents->next)) ; /* map the merge to the previous */quoted
+ else if (commit->parents && commit->parents->next) die(_("replaying merge commits is not supported yet!"));And because the next one is also about merges, perhaps the early part of this if/else if cascade can be written if (commit->parents && commit->parents->next) { /* We have a merge */ if (!opts->linearize) die(_("can't replay a merge (yet)")); ; /* map current to the previous */ } else { ... wouldn't it?
The way it was written in v1 was maybe a bit too smart and hard to follow. I agree with your suggestion and will adopt this (with some tweaks) in the next version.
If the "map current to prev" is applicable to root, any root are mapped to the last_commit in the above, and if we saw a root as the first thing in the loop, last_commit is NULL, we do not do anything here, and after the if/else if/else cascade, we see last_commit is NULL and break out of the loop.
Yes, good observation. I did not test this.
Perhaps we would want to have a test that replays all the way down to the root commit?
I'll add it. -- Cheers, Toon