On Tue, Sep 7, 2010 at 10:23 AM, Schalk, Ken <ken.schalk@xxxxxxxxx> wrote: >>Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are >>identical 2010-09-01), a separate if-block was added to provide a special >>case for the rename/add conflict case that can be resolved (namely when >>the contents on the destination side are identical). However, as a >>separate if block, it's not immediately obvious that its code is related to >>the subsequent code checking for a rename/add conflict. We can combine and >>simplify the check slightly. > > Originally I tried the fix the way you've re-structured it, just adding a test to the if around the rename/add conflict handling. Unfortunately that didn't completely solve the problem in the case that originally motivated the fix (rename vs. rename+symlink, as in my initial post and my first attempt at adding a test to t/t3030-merge-recursive.sh). That's why I changed it to a separate if block. > > The problem comes down in the code inside the "if(try_merge)" block below. It merges the source of the rename on the other side with the renamed file, rather than the destination. In the case with the symlink on the other side, this code merged a symlink with a regular file which resulted in a conflict. I was trying to eliminate both conflicts in this case by avoiding the final else that sets try_merge=1. > > Your re-structuring will therefore only solve half the problem I was trying to solve. > > I suppose an alternative solution would have been to change the "if(try_merge)" code to merge with the destination of the rename on the other side, if it exists and is the same type. However that clearly would have had a much more significant impact on other merge cases, so it didn't seem like a good choice to me. Interesting...that means we probably should have stuck with the original testcase you suggested (though marking it with the SYMLINK dependence), since the new one doesn't fail with my modifications but the old one would. The typechange is critical. So I'll drop that portion of my patch. Perhaps you could submit another patch changing your testcase back to using a symlink to make sure someone like me doesn't break your original testcase in the future? Thanks, Elijah -- 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