Re: [PATCH 3/3] merge-recursive: handle rename-to-self case

From: Derrick Stolee <stolee@gmail.com>
Date: Jun 29, 2021, 12:55 UTC
Message-ID: <7854e7b7-f128-e389-cc06-8b599c7b3e32@gmail.com>
In-Reply-To: <xmqqeecl1foi.fsf@gitster.g>

On 6/29/2021 12:02 AM, Junio C Hamano wrote:

Show 18 quoted lines

> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Elijah Newren <newren@gmail.com>
>>
>> Directory rename detection can cause transitive renames, e.g. if the two
>> different sides of history each do one half of:
>>     A/file -> B/file
>>     B/     -> C/
>> then directory rename detection transitively renames to give us
>>     A/file -> C/file
>>
>> However, when C/ == A/, note that this gives us
>>     A/file -> A/file.
> 
> Heh, of course this is recent because the "guess that directory B
> has been moved to C in its entirety" heuristic is quite new.
> 
> Nicely fixed and described.  Will queue.  Thanks.

I agree that the fixes in patches 2 and 3 are rather clean. The tests added in patch 1 provide significant confidence in the result, though I had a minor question about a possible third test case.

Seeing the solutions that make the two tests pass leaves me to consider that the third test case is likely unnecessary, because there is nothing special about how the directories are being renamed that must be handled by the fix.

Thanks, -Stolee