From: Elijah Newren <newren@xxxxxxxxx>
At GitHub, we've got a real-world repository that has been triggering
failures of the form:
git: merge-ort.c:3007: process_renames: Assertion `newinfo && !newinfo->merged.clean' failed.
which comes from the line:
VERIFY_CI(newinfo);
Unfortunately, this one has been quite complex to unravel, and is a
bit complex to explain. So, I'm going to carefully try to explain each
relevant piece needed to understand the fix, then carefully build up
from a simple testcase to some of the relevant testcases.
== New special case we need to consider ==
Rename pairs in the diffcore machinery connect the source path of a
rename with the destination path of a rename. Since we have rename
pairs to consider on both sides of history since the merge base,
merging has to consider a few special cases of possible overlap:
A) two rename pairs having the same target path
B) two rename pairs having the same source path
C) the source path of one rename pair being the target path of a
different rename pair
Some of these came up often enough that we gave them names:
A) a rename/rename(2to1) conflict (looks similar to an add/add conflict)
B) a rename/rename(1to2) conflict, which represents the same path being
renamed differently on the two sides of history
C) not yet named
merge-ort is well-prepared to handle cases (A) and (B), as was
merge-recursive (which was merge-ort's predecessor). Case (C) was
briefly considered during the years of merge-recursive maintenance,
but the full extent of support it got was a few FIXME/TODO comments
littered around the code highlighting some of the places that would
probably need to be fixed to support it. When I wrote merge-ort I
ignored case (C) entirely, since I believed that case (C) was only
possible if we were to support break detection during merges. Not
only had break detection never been supported by any merge algorithm,
I thought break detection wasn't worth the effort to support in a
merge algorithm. However, it turns out that case (C) can be triggered
without break detection, if there's enough moving pieces.
Before I dive into how to trigger case (C) with directory renames plus
other renames, it might be helpful to use a simpler example with break
detection first. And before we get to that it may help to explain
some more basics of handling renames in the merge algorithm. So, let
me first backup and provide a quick refresher on each of
* handling renames
* what break detection would mean, if supported in merging
* handling directory renames
