Re: [RFC PATCH 0/2] Introduce new merge-tree-ort command

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 7, 2022, 19:54 UTC
Message-ID: <nycvar.QRO.7.76.6.2201072044120.339@tvgsbejvaqbjf.bet>
In-Reply-To: <CAP8UFD0wKnAg5oyMWchXysPTg3K9Vb4M1tRcPzPE81QM903pYg@mail.gmail.com>

Hi Christian,

On Fri, 7 Jan 2022, Christian Couder wrote:

Show 11 quoted lines

> On Wed, Jan 5, 2022 at 5:54 PM Elijah Newren <newren@gmail.com> wrote:
> >
> > On Wed, Jan 5, 2022 at 8:33 AM Christian Couder
> > <christian.couder@gmail.com> wrote:
>
> > > To move forward on this, this small RFC patch series introduces a
> > > new `git merge-tree-ort` command with the following design:
> >
> > Slightly dislike the command name.
>
> I am ok with changing the command name.

Or just going with `merge-tree`?

That command name has the distinct advantage of _already_ being used by Git. So there is no chance for users to have a `git-merge-tree` script lying around that all of a sudden would stop working because a superseding built-in using the same name is introduced.

That is a distinct advantage of using the existing command for the new code flow, even if the command name could be interpreted as misleading. Of course, the name could also be construed to be on point: it merges, and it produces a tree, hence "merge-tree".

Show 10 quoted lines

> > >   - the diff API, actually diff_tree_oid() is called 3 times: once for
> > >     the diff versus branch1 ("ours"), once for the diff versus branch2
> > >     ("theirs"), and once for the diff versus the base.
> >
> > Why?  That seems to be a performance penalty for anyone that doesn't
> > want/need the diffs, and since we return a tree, a caller can go and
> > get whatever diffs they like.
>
> I say somewhere else that I am planning to add options to disable this
> or parts of this diff output.

Well, now you got me really curious. Since you are listed as part of GitLab (https://about.gitlab.com/company/team/#chriscool), I assume that you are pretty familiar with how merges are done on GitLab's server side, and plan on experimenting with your own work to use `merge-ort` on GitLab's servers.

Which makes me wonder where that idea comes from to use the diff API?

What operations does GitLab need from that Git command, which code flows need to be supported (and what inputs/outputs are there)?

Show 14 quoted lines

> I think it's still interesting for the command to be able to output
> diffs, especially diffs of conflicting parts. In [2] Ævar said:
>
> => I.e. I'm not the first or last to have (not for anything serious)
> => implement a dry-run bare-repo merge with something like:
> =>
> =>     git merge-tree origin/master git-for-windows/main origin/seen >diff
> =>     # Better regex needed, but basically this
> =>     grep "^\+<<<<<<< \.our$" diff && conflict=t
>
> Also `git merge-tree` currently outputs diffs, so I thought it would
> be sad if the new command couldn't do it.
>
> [2] https://lore.kernel.org/git/211109.861r3qdpt8.gmgdl@evledraar.gmail.com/

But `git merge-tree` was only meant as a proof-of-concept, intended to entice other developers to turn it into something useful. As it took _this_ long, we can safely assume that the user interface of `merge-tree` left a bit of a room for improvement. Wouldn't you agree?

> Also I thought that you might not be interested in this anymore as you
> didn't reply to [1] and [2] where Ævar and I both said that we were
> interested in your opinion on a git merge-tree on steroids.

Elijah stated at the time that he was scarce on time for Git.

But back to the topic: have you played with using `merge-ort` in GitLab already? What were the needs you saw that would be required of the Git command, what flows would it have to support?

Ciao, Johannes