Re: [PATCH v3 04/15] merge-tree: implement real merges

From: Johannes Altmanninger <aclopte@gmail.com>
Date: Feb 27, 2022, 17:35 UTC
Message-ID: <20220227173522.wfshhu2rsyss576e@gmail.com>
In-Reply-To: <xmqqh78nj0q0.fsf@gitster.g>

On Thu, Feb 24, 2022 at 03:36:55PM -0800, Junio C Hamano wrote:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > It is more like
> > ...
> 
> Actually, I misspoke.  It is a bit different.
> 
> In my mind, the building block hierarchy would have been
> 
>  (1) Take three tree objects A, B, and O, and do the three-way
>      merge.  No history relation is assumed among them.
> 
>  (2) Take two tree objects A and B, with one or more commit objects
>      Os; use (2) recursively to reduce Os into a single O and then
>      apply (1) on A, B and O.

Accepting multiple bases is nice (because it frees users of having to recursively merge their merge-bases),

Let's say we take this series in its current form:

	git merge-tree [--write-tree] [<options>] <branch1> <branch2>
	git merge-tree [--trivial-merge] <base-tree> <branch1> <branch2> (deprecated)

and later discover we want to add (2), one possible syntax would be

	git merge-tree --write-tree [<options>] <branch1> <branch2> <base>...

(or put the bases in the middle like merge-file). Though the mandatory --write-tree leaves a bad taste.

A separate option is a better alternative:

	git merge-tree [--write-tree] [<options>] --base=<base1>,<base2>,... <branch1> <branch2>

Anyway, no need to worry about that now, especially since the root cause of the ugliness is the legacy --trivial-merge, and there is no way avoid that, even if we add this now.

Show 31 quoted lines

> 
>  (3) Take two commit objects A and B.  Compute Os out of A and B and
>      use (2) once to merge A and B.
> 
> I think the basic primitive that should be exposed to an external
> world (read: plumbing) this year, after all years of experience with
> merge-recursive, should be (2), not (1).  
> 
> If you have (2), then (3) is trivially possible (it is just a single
> call to get_merge_base()).  "git merge-tree A B" without having to
> spell out bases is so convenent and you do not have to write
> "git merge-tree A B $(git merge-base --all A B)", so I am OK for it
> to exist, but it is not essential.
> 
> If you have (2) and exposed (2) as the primitive plumbing,
> cherry-pick and revert would be a narrow special case of passing
> only one O to the machinery.
> 
> And coming from the above point of view, exposing (3) as the
> primitive plumbing to scripters and system builders, and later
> having to _add_ support to allow (2), felt backwards.  It should be
> trivial for us to make (2) available before we can even offer (3),
> but what is happening to this new plumbing command goes in the
> opposite order.
> 
> It may be, as you said, the problem the underlying ort API has that
> somehow makes it harder to expose (2), in which case, yes, I think
> that is what bugs me.
> 
> 
>