Thread (36 messages) 36 messages, 5 authors, 6h ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help