Re: [RFC PATCH 0/5] Improve path collision conflict resolutions

From: Elijah Newren <newren@gmail.com>
Date: Mar 12, 2018, 18:19 UTC
Message-ID: <CABPp-BHh9byDtdr=HcUTY17dqnjd4V=My_xTH0jzLUv4bfWf5w@mail.gmail.com>
In-Reply-To: <20180305171125.22331-1-newren@gmail.com>

On Mon, Mar 5, 2018 at 9:11 AM, Elijah Newren <newren@gmail.com> wrote:

Show 25 quoted lines

>   2) Doing content merges for a rename before looking at the path collision
>      between a rename and some other file.  In particular (in what I most
>      suspect others might have an objection to from this series), record
>      that content-merged file -- which may have conflict markers -- in the
>      index at the appropriate higher stage.
>
> 2)
>
> Previously, rename/rename(2to1) conflict resolution for the colliding path
> would just accept the index changes made by unpack_trees(), meaning that
> each of the higher order stages in the index for the path collision would
> implicitly ignore any changes to each renamed file from the other side of
> history.  Since, as noted above, all traces of the rename-source path were
> removed from both the index and the working tree, this meant that the index
> was missing information about changes to such files.  If the user tried to
> resolve the conflict using the index rather than the working copy, they
> would end up with a silent loss of changes.
>
> I "fixed" this by doing the three-way content merge for each renamed-file,
> and then recorded THAT in the index at either stage 2 or 3 as appropriate.
> Since that merge might have conflict markers, that could mean recording in
> the index a file with conflict markers as though it were a given side.
> (See patch 2 for a more detailed explanation.)  I figure this might be the
> most controversial change I made.  I can think of a few alternatives, but I
> liked all of them less.  Opinions?

Actually, I realized this last weekend that this wasn't unprecedented after all, and thus likely not at all controversial. In particular, there is already a case where git stores the sha1 of a file with conflict markers from a "provisional content merge" at a non-zero stage in the index: If a recursive merge is needed and there is a file with content conflicts when creating the virtual merge base, then the file with all the conflict markers will be accepted as part of the virtual merge base and, depending on how the outer merge goes, the sha1 of this file with conflict markers can appear in the index at stage 1.

So, that really only leaves the changes to the working tree files. And based on a few factors including things mentioned in the cover letter, I suspect most would only have an opinion about how this patch series affects add/add conflicts. I'll send a separate email to ask about that, to make it clear I want folks opinions on that issue even if they don't have enough time to review the patch series or even read my really long cover letter.