Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)

[Elijah Newren <newren@gmail.com>]

On Fri, Jan 7, 2022 at 10:46 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 6:27 PM Elijah Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>
> > This series attempts to guess what kind of output would be wanted, basically
> > choosing:
> >
> >  * clean merge or conflict signalled via exit status
>
> (Maybe s/signalled/signaled/)

I can't determine the difference after a few Google searches, and both
seem to be in dictionaries with the same meaning so I'm having
difficulty figuring out which is preferred.  Usually my searches will
either suggest that one is a misspelling or at least bring up whether
one is a regional variance, but I'm not seeing anything of the sort.

It can't hurt to switch, though, so I'm happy to switch.

> Not sure that's the best way by default. I think it's very likely that
> many users will be interested in parsing the command ouput, and they
> might prefer that merge related errors be signaled in a different way
> than other errors.

That's fair.

I was thinking in terms of various plumbing commands: hash-object,
mktree, commit-tree, read-tree, write-tree and update-ref.  Output
from commands in that last can be fed as input to other commands and
be chained together to do various interesting and useful things.  I
have done that at various times in the past.  I thought merge-tree
might augment that category of commands (particularly since Peff
suggested to make the command be low-level at the summit), and thus
outputting just a tree (at least by default) would make the command be
a useful building block within that context.  That was part of my
reason for including the code snippet

   NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)
   test $? -eq 0 || die "There were conflicts..."
   NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)
   git update-ref $BRANCH1 $NEWCOMMIT

in the cover letter.

But merge-tree is much more likely to run into problems (i.e. into
merge conflicts), so maybe it doesn't belong in the same set, and the
NEWTREE definition perhaps deserves to have additional special case
command-line parsing that the user needs to do.

I'm curious about others' thoughts on this matter too.

> >  * stdout consists solely of printing the hash of the resulting tree (though
> >    that tree may include files that have conflict markers)
>
> Maybe users will want diffs, the conflicted list and other things on
> stdout, as they might want to parse it anyway, and it would be a
> burden to have to perform diffs, or get other interesting info in a
> different way or using a different process or call.

You mention the stdout thing both above and below, so I'll concentrate
here on the diffs part.

Do you have a specific usecase you have in mind where diffs are
wanted, separate from the two examples you gave in the other thread
(namely Ævar's misguided hack for looking for whether there were
conflicts, and a desire to just follow merge-tree's convoluted
precedent)?  I'd rather not add diffs pre-emptively on the basis that
users "might" want them, especially if they come with the huge gamut
of options Ævar was spitballing in [1] (some of which appeared to have
misguided assumptions relative to the possibility of renames and might
introduce edge and corner case bugs that'd be with us forever).  If we
don't have concrete usecases yet, I'd rather avoid adding such options
until we do have concrete usecases so we don't paint ourselves into a
corner.

[1] https://lore.kernel.org/git/211109.861r3qdpt8.gmgdl@evledraar.gmail.com/

> >  * new optional --messages flag for specifying a file where informational
> >    messages (e.g. conflict notices and files involved in three-way-content
> >    merges) can be written; by default, this output is simply discarded
> >  * new optional --conflicted-list flag for specifying a file where the names
> >    of conflicted-files can be written in a NUL-character-separated list
>
> It would be nice if output was printed on stdout when the above flags
> are used without argument.

Oh, that's an interesting idea.  The --conflicted-list flag, though,
separates filenames by NUL characters, for simplicity of parsing.  If
I'm printing them to stdout, would that be problematic? (If so, should
it instead print them in e.g. ls-tree format, where it escapes
filenames only when necessary)?

> Thanks for working on this!