Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts

On Tue, Jun 29, 2010 at 14:52, Elijah Newren [off-list ref] wrote:

On Tue, Jun 29, 2010 at 1:54 AM, Miklos Vajna [off-list ref] wrote:

quoted

On Mon, Jun 28, 2010 at 07:12:15PM -0600, newren@gmail.com wrote:

quoted

I'm a little uneasy with this change, mainly because I don't fully
understand the rename processing logic (I was actually kind of surprised
when I made these changes and it worked).  Although I verified that
these changes (and my others in this patch series) introduce no new
breakages in the testsuite and even fix a known issue, I'm still not
quite sure I follow the logic well enough to feel fully confident in
this change.  I'm particularly worried I may have neglected some closely
related cases that I should have fixed but which may still be broken.

Alex: I think the basic idea is just that the rename logic isn't aware
that there may be higher stage entries in the index due to D/F
conflicts; by checking for such cases and marking the entry as not
processed, it allows process_entry() later to look at it and handle
those higher stages.  But I'm not sure if that's the right way to
handle it, or if just having process_renames() should take care of
clearing out the higher stage entries, or if something else entirely
should be done.

Nor am I. You may be still off by some commits in detecting the authorship :)
This code was seldom touched since it was written (by Johannes). It has
survived in this sorry state all (at least my) attempts to fix it. OTOH I never
tried really hard. Maybe because the D/F conflicts are rare and are relatively
simple to work around.

I cannot say much about your change... Are you sure about D/F conflict
detection, though? You just test if target mode not 0.