Re: [PATCH 1/3] merge-tree -z: always show the original file name first

From: Elijah Newren <newren@gmail.com>
Date: Aug 21, 2022, 02:00 UTC
Message-ID: <CABPp-BG0NGEmGCcF36HhGCNNGL6csJXqAa5s6cXvXWFJfDor7w@mail.gmail.com>
In-Reply-To: <CABPp-BGPioxH2-2Pde8vAqFOOoW7WvCSf_fLzRWZB3uy=9Xc=g@mail.gmail.com>

On Sat, Aug 20, 2022 at 4:17 PM Elijah Newren <newren@gmail.com> wrote:

> On Thu, Aug 18, 2022 at 11:57 PM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:

[...]

Show 46 quoted lines

> > @@ -4022,7 +4022,7 @@ static void process_entry(struct merge_options *opt,
> >                         if (S_ISGITLINK(merged_file.mode))
> >                                 reason = _("submodule");
> >                         path_msg(opt, CONFLICT_CONTENTS, 0,
> > -                                path, NULL, NULL, NULL,
> > +                                orig_path, NULL, NULL, NULL,
> >                                  _("CONFLICT (%s): Merge conflict in %s"),
> >                                  reason, path);
> >                 }
>
> Here's another case where path == orig_path, so you haven't made an
> effective change.  But this one highlights something interesting...
>
> In addition to the fact that path/orig_path may be a location that
> didn't exist on either side of history, there might actually be two
> relevant paths from the two different sides of history which are
> involved in the content merge, with neither of them being path or
> orig_path.  Let me break it down, starting with a simpler two path
> case:
>
> If we have a standard rename, e.g. foo -> bar, and both sides modified
> the file but did so in a conflicting manner, then we will end up in
> this chunk of code.  The conflict info emitted by merge-tree -z which
> is always of the form
>   <number-of-paths>NUL<path1>NUL<path2>NUL<pathN>NUL<stable-short-type-description>NUL<message>NUL
> will in this particular case be
>    1<NUL>bar<NUL>Auto-merging<NUL>Auto-merging bar<newline><NUL>
>    1<NUL>bar<NUL>CONFLICT (contents)<NUL>CONFLICT (content): Merge
> conflict in bar<newline><NUL>
> Neither of these messages has any mention of "foo", despite the fact
> that "foo" was the name of the file in both the merge base and one
> side, and "bar" was only the name of the file on one side.
>
> Now, let's make it more interesting.  side A modifies foo, and renames
> olddir/->newdir/.  side B modifies foo in a conflicting manner, and
> renames foo->olddir/bar.  Our `merge-tree -z` conflict information
> emitted will be of the form
>    1<NUL>newdir/bar<NUL>Auto-merging<NUL>Auto-merging newdir/bar<newline><NUL>
>    1<NUL>newdir/bar<NUL>CONFLICT (contents)<NUL>CONFLICT (content):
> Merge conflict in newdir/bar<newline><NUL>
> For this more interesting case, the only files that existed were "foo"
> and "olddir/bar", neither of which are mentioned in the conflict info.
> The conflict info only reports on "newdir/bar".
>
> And for both of these examples, your patch doesn't change the existing
> behavior at all since path == orig_path for this hunk of the patch.

But, actually, trying to create some more testcases for the testsuite, maybe this isn't so bad. For example, with this interesting testcase:

   Commit O: foo, olddir/{a,b,c}
   Commit A: delete foo, rename olddir/ -> newdir/, add newdir/bar/file
   Commit B: modify foo & rename foo -> olddir/bar

Which involves four different types of conflicts:

  * a directory rename (which further modifies foo->olddir/bar to end
up at newdir/bar)
  * a rename/delete (delete foo vs. rename to olddir/bar)
  * a modify/delete (since foo was modified as well on one side)
  * a directory/file conflict (newdir/bar vs newdir/bar/file, forcing
newdir/bar to be further renamed to newdir/bar~B^0)

The <Conflicted file info> section will look like
    100644 <oldhash> 1 newdir/bar~B^0
    100644 <newhash> 3 newdir/bar~B^0

While that only includes the name "newdir/bar~B^0" and not "newdir/bar", or "olddir/bar", or "foo", the <Informational messages> has all necessary information to tie it together. Replacing null characters with newlines, the <Informational messages> section is:

    2
    newdir/bar
    olddir/bar
    CONFLICT (directory rename suggested)
    CONFLICT (file location): foo renamed to olddir/bar in B^0, inside
a directory that was renamed in A^0, suggesting it should perhaps be
moved to newdir/bar.

    2
    newdir/bar
    foo
    CONFLICT (rename/delete)
    CONFLICT (rename/delete): foo renamed to newdir/bar in B^0, but
deleted in A^0.

    2
    newdir/bar~B^0
    newdir/bar
    CONFLICT (file/directory)
    CONFLICT (file/directory): directory in the way of newdir/bar from
B^0; moving it to newdir/bar~B^0 instead.

    1
    newdir/bar~B^0
    CONFLICT (modify/delete)
    CONFLICT (modify/delete): newdir/bar~B^0 deleted in A^0 and
modified in B^0.  Version B^0 of newdir/bar~B^0 left in tree.

This provides all the mappings to tie together foo, olddir/bar, newdir/bar, and newdir/bar~B^0, and shows the four conflict types present. So, all the information you need is present, it just can't be parsed out of a single line like you want. (But adding the "newdir/bar" name to the modify/delete conflict at least does seem like it'd be a little nicer.)

And trying to parse it out of a single line is probably tantamount to trying to enforce a 1-to-1 mapping between paths and conflicts, which will paint us into a corner, as I've pointed out before[1,2]

[1] https://lore.kernel.org/git/CABPp-BGCL0onSmpgKuO1k2spYCkx=v27ed9TSSxFib=OdDcLbw@mail.gmail.com/ [2] https://lore.kernel.org/git/CABPp-BGnqXdFBNAyKRXgvCHv+aUZTMg-CgcQf95dKAR-e1zSjQ@mail.gmail.com/