On Tue, Jun 29, 2010 at 7:36 AM, Alex Riesen <raa.lkml@xxxxxxxxx> wrote: > On Tue, Jun 29, 2010 at 14:52, Elijah Newren <newren@xxxxxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html