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

[Elijah Newren <newren@gmail.com>]

On Wed, Feb 23, 2022 at 12:07 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > The objection you are arguing against is not my position.  In fact,
> > I'm not even objecting to having a single-step cherry-pick, I'm
> > objecting to providing it _now_, which I thought would have been clear
> > from the portion of my email you snipped ("...I'm happy to add [a
> > single step cherry-pick primitive] along with the tool I submit
> > later...").
>
> The entry point into the in-core merge machinery of ort already
> knows how to accept externally defined merge-base(s) to bypass the
> "caller didn't give us the merge base, so let's figure them out by
> using the two heads being merged" logic, so it just felt backwards
> *not* to have a vanilla three-way merge that can take three trees
> and be used for merge, cherry-pick or revert as the single primitive
> in the very beginning before we talk about multi-step operations.

"The entry point"?

There are two entry points: merge_incore_recursive(), and
merge_incore_nonrecursive().  The former is analogous to
merge-recursive's merge_recursive(), and the latter is analogous to
merge_trees().

"git merge" always uses merge_incore_recursive()/merge_recursive().
The other big merge-y operations (cherry-pick, rebase, revert), always
use merge_incore_nonrecursive()/merge_trees().

(Technically, merge-recursive has a third entry point used by am &
stash, but let's ignore it for a moment.)

> So, I guess I am still not getting where the "I'm happy to _add_"
> part comes from.  If we start with a primitive (internally callable
> from C) "here are three trees O A B, do the three-way merge", then
> there is nothing to "add" at all to expose a single-step
> cherry-pick.  In fact, to the users of merge-tree, the result does
> not have to have any fixed meaning.  If they pass common ancestors
> as the merge bases as Os and the current HEAD and the other branch
> as A and B, they get a merge.  If they pass the commit to be picked
> as B, current HEAD as A and B's parent as O, they get a cherry-pick.
>
> Perhaps starting from "You are allowed to give me two commits A B,
> and I do not let you specify the commit O to use as a common
> 'ancestor'" is the root cause of making this thing feel backwards.
> I agree with the goal of having an all-incore machinery that can do
> a merge.  I just do not see the reason why you have to build it in a
> way that cannot be reused for other two directions of merge-y
> operations.

So, am I correct to understand that what bugs you is actually
merge-recursive's and merge-ort's API?  That you don't want these two
types of merges to have different entry points, and that there should
in fact only be one?

That might be an interesting line of investigation to try to modify
the API to achieve that, but that feels like a bigger task that is
somewhat tangential to this series.

Without such a thing, handling both merging-of-divergent-branches and
three-way-merge-with-specified-merge-base would be separate codepaths
that call different functions.  I implemented one of those two
codepaths, and if we want the other then it needs to be added.  That's
where the "add" part comes from, in my view.


Does that help, or am I still missing what you're saying?