Re: [PATCH 2/7] merge-ort: add ability to record conflict messages in a file

[Elijah Newren <newren@gmail.com>]

On Thu, Sep 30, 2021 at 12:58 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Sep 28, 2021 at 11:25:20PM -0700, Elijah Newren wrote:
>
> > > Just brainstorming some alternatives:
> > >
> > >  - we have diff.orderFile, etc. Could we stuff this data into a less
> > >    confusing name (even just "$filename.conflict_msg"), and then provide
> > >    a custom ordering to the diff code? I think it could be done by
> > >    generating a static ordering ahead of time, but it might even just be
> > >    possible to tell diffcore_order() to take the ".conflict_msg"
> > >    extension into account in its comparison function.
> >
> > I can't just go on the ".conflict_msg" extension.  As you noted above,
> > this scheme is not sufficient for avoiding collisions.  So I need to
> > append extra "cruft" to the name in the case of collisions -- meaning
> > we can't special case on just that extension.
>
> Sure, but we can call it filename.conflict_msg.1, etc, and the sort code
> can pattern-match. It can never be fully robust (if you really did have
> a foo.conflict_msg, we'd sort it differently), but I think it's OK if
> the worst-case is that pathological trees get ordered slightly
> sub-optimally).
>
> That said, I think it could also make sense to annotate the conflict
> files by giving them an unusual set of mode bits. That would be easier
> to recognize (and while real trees _could_ have silly modes, we do
> complain about them in fsck, so it shouldn't happen in practice).

I tried giving things unusual mode bits in early versions of merge-ort
for other reasons.  It doesn't work: canon_mode() will "fix" the
unusualness so there's no way for the reader to know that they had
unusual bits.

But, as you said later, avoiding these pseudo-files is going to be
cleaner anyway, so let's just do that.



> > I also don't like how diff.orderFile provides a global ordering of the
> > files listed, rather than providing some scheme for relative
> > orderings.  That'd either force me to precompute the diff to determine
> > all the files that were different so I can list _all_ of them, or put
> > up with the fact that the files with non-content conflicts will be
> > listed very first in the output, even if their name is
> > 'zee-last-file.c' -- surprising users at the output ordering.
> >
> > This also means that if the user had their own ordering defined, then
> > I'm overriding it and messing up their ordering, which might be
> > problematic.
>
> Agreed. I do think it may be too horrible unless you teach
> diffcore_order() to actually understand your annotations in code.  But
> that wouldn't be too hard if it's done in the mode bits.
>
> But I do think anything that avoids these pseudo-files is going to be a
> lot cleaner overall.
>
> > >  - there can be other non-diff data between the individual segments. For
> > >    example, "patch" will skip over non-diff lines. And certainly in Git
> > >    we have our own custom headers. I'm wondering if we could attach
> > >    these annotations to the diff-pair somehow, and then show something
> > >    like:
> > >
> > >      diff --git a/foo.c b/foo.c
> > >      index 1234abcd..5678cdef 100644
> > >      conflict modify/delete foo.c
> >
> > A couple things here...
>
> I think Junio already indicated that we can pretty much make this look
> like whatever we want. As soon as any reader sees "conflict", it should
> happily ignore the rest as something it doesn't know about. And my short
> example here was just meant to be illustrative. I agree it probably
> needs more details (and the whole CONFLICT line that usually goes to
> stderr is probably the best thing).
>
> -Peff