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

From: Elijah Newren <newren@gmail.com>
Date: Jun 29, 2010, 15:55 UTC
Message-ID: <AANLkTilggM9-vBabNvJiYMiQZyZtJMLhfWleYKvuJNMv@mail.gmail.com>
In-Reply-To: <AANLkTil7CdCoP3wLVKX0MEiwp8KaKWFLvRtUWzt2a3Nh@mail.gmail.com>

On Tue, Jun 29, 2010 at 7:36 AM, Alex Riesen <raa.lkml@gmail.com> wrote:

Show 18 quoted lines

> On Tue, Jun 29, 2010 at 14:52, Elijah Newren <newren@gmail.com> wrote:
>> 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.

Well, as far as this particular if-block is concerned, blame suggests that you and Miklos were responsible (I apologize if gmail screws up and inserts line wrapping)::

$ git blame -C -C -L 1020,1038 merge-recursive.c
9047ebbc (Miklos Vajna  2008-08-12 18:45:14 +0200 1020)
                 if (mfi.clean &&
9047ebbc (Miklos Vajna  2008-08-12 18:45:14 +0200 1021)
                     sha_eq(mfi.sha, ren1->pair->two->sha1) &&
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1022)
                     mfi.mode == ren1->pair->two->mode) {
8a359819 (Alex Riesen   2007-04-25 22:07:45 +0200 1023)
                         /*
8a359819 (Alex Riesen   2007-04-25 22:07:45 +0200 1024)
                          * This messaged is part of
8a359819 (Alex Riesen   2007-04-25 22:07:45 +0200 1025)
                          * t6022 test. If you change
8a359819 (Alex Riesen   2007-04-25 22:07:45 +0200 1026)
                          * it update the test too.
8a359819 (Alex Riesen   2007-04-25 22:07:45 +0200 1027)
                          */
8a2fce18 (Miklos Vajna  2008-08-25 16:25:57 +0200 1028)
                         output(o, 3, "Skipped %s (merged same as
existing)", ren1_
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1029)
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1030)
                         /* If this was a rename across a path
involved
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1031)
                          * in a D/F conflict, there may be more work
to
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1032)
                          * do.
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1033)
                          */
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1034)
                         for (i=1; i<=3; ++i) {
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1035)
                                 if (ren1->dst_entry->stages[i].mode)
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1036)
                                         ren1->dst_entry->processed =
0;
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1037)
                         }
de4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1038)
                 } else {

With D/F conflicts, the files would be loaded into higher stages in the index (before it gets to process_renames()), which I detected via a non-zero mode. If there's a different way I should be checking for higher stage entries that still need to be resolved, I'd be happy to use it.