archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <>
To: Johannes Schindelin via GitGitGadget <>
Cc: Git Mailing List <>,
	Johannes Schindelin <>
Subject: Re: [PATCH 1/3] merge-tree -z: always show the original file name first
Date: Sat, 20 Aug 2022 19:00:22 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Sat, Aug 20, 2022 at 4:17 PM Elijah Newren <> wrote:
> On Thu, Aug 18, 2022 at 11:57 PM Johannes Schindelin via GitGitGadget
> <> wrote:
> > @@ -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:

    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.

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

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

    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]


  reply	other threads:[~2022-08-21  2:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-19  6:57 [PATCH 0/3] Show original filenames in merge tree Johannes Schindelin via GitGitGadget
2022-08-19  6:57 ` [PATCH 1/3] merge-tree -z: always show the original file name first Johannes Schindelin via GitGitGadget
2022-08-19  9:17   ` Johannes Schindelin
2022-08-20 23:22     ` Elijah Newren
2022-08-20 23:17   ` Elijah Newren
2022-08-21  2:00     ` Elijah Newren [this message]
2022-08-22 20:12     ` Johannes Schindelin
2022-08-23  6:24       ` Elijah Newren
2022-08-26 15:35         ` Johannes Schindelin
2022-08-30  2:13           ` Elijah Newren
2022-08-19  6:57 ` [PATCH 2/3] merge-tree: show the original file names in the conflict output Johannes Schindelin via GitGitGadget
2022-08-19  6:57 ` [PATCH 3/3] t4301: add a test case involving a rename, type change & modification Johannes Schindelin via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).