Re: [PATCH 3/3] replay: offer an option to linearize the commit topology

> Toon Claes <toon@iotcl.com> writes:
>
>> 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
>> linearized the commit history into a sequence of the
>> regular (single-parent) commits.
>
> "linearized" -> "linearizes"?

>> @@ -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 */
>
>> +		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?

> 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.