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

[Junio C Hamano <gitster@pobox.com>]

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
> linearizes the commit history into a sequence of the
> regular (single-parent) commits.
>
> Add option `--linearize` to git-replay(1) to do the same.
>
> 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(-)
>
> @@ -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.
> +			 */

In the review response during the previous iteration, I commented
that (1) the original excluded only merges, but (2) your version
excluded both merges and the root commits the same way.  Your
response was:

    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.

which I took as saying "it may be confusing, but it correctly
expresses what we want to do", meaning "yes, roots and merges should
be handled the same way".  But the above no longer treats roots the
same way as merges.  I think that is intended, but just wanted to
double check.

> 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;
>  };

OK.

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

As with other test pieces, this "git replay" command line is overly
long and hides the important bit which is that the range being
replayed is *not* actually down to the root, which is A (it excludes
A).  Intended?

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

Thanks.