Re: [PATCH 00/30] Add directory rename detection to git

["Philip Oakley" <philipoakley@iee.org>]

From: "Elijah Newren" <newren@gmail.com>
: Friday, November 10, 2017 11:26 PM
> On Fri, Nov 10, 2017 at 2:27 PM, Philip Oakley <philipoakley@iee.org> 
> wrote:
>> From: "Elijah Newren" <newren@gmail.com>
>>>
>>> In this patchset, I introduce directory rename detection to
>>> merge-recursive,
>>> predominantly so that when files are added to directories on one side of
>>> history and those directories are renamed on the other side of history,
>>> the
>>> files will end up in the proper location after a merge or cherry-pick.
>>>
>>> However, this isn't limited to that simplistic case.  More interesting
>>> possibilities exist, such as:
>>>
>>>  * a file being renamed into a directory which is renamed on the other
>>>    side of history, causing the need for a transitive rename.
>>>
>>
>> How does this cope with the case insensitive case preserving file systems 
>> on
>> Mac and Windows, esp when core.ignorecase is true. If it's a bigger 
>> problem
>> that the series already covers, would the likely changes be reasonably
>> localised?
>>
>> This came up recently on GfW for `git checkout` of a branch where the 
>> case
>> changed ("Test" <-> "test"), but git didn't notice that it needed to 
>> rename
>> the directories on such an file system.
>> https://github.com/git-for-windows/git/issues/1333
>
> I wasn't aware there were problems with git on case insensitive case
> preserving filesystems; fixing them wasn't something I had in mind
> when writing this series.

I was mainly ensuring awareness of the potential issue, as it's not easy to 
solve.

> However, the particular bug you mention is
> actually completely orthogonal to this series; it talks about
> git-checkout without the -m/--merge option, which doesn't touch any
> code path I modified in my series, so my series can't really fix or
> worsen that particular issue.

That's good.
>
> But, if there are further issues with such filesystems that also
> affect merges/cherry-picks/rebases, then I don't think my series will
> either help or hurt there either.  The recursive merge machinery
> already has remove_file() and update_file() wrappers that it uses
> whenever it needs to remove/add/update a file in the working directory
> and/or index, and I have simply continued using those, so the number
> of places you'd need to modify to fix issues would remain just as
> localized as before.

It's when the working directory path/filename has a case change that goes 
undetected (one way or another) that can cause issues. I think that part of 
the problem (after awareness) is not having a cannonical expectation of 
which way is 'right', and what options there may be. E,g. if a project is 
wholly on a case insensitive system then the filenames in the worktree never 
matter, but aligning the path/filenames in the repository would still be a 
problem.

>  Also, I continue to depend on the reading of the
> index & trees that unpack_trees() does, which I haven't modified, so
> again it'd be the same number of places that someone would need to
> fix.  (However, the whole design to have unpack_trees() do the initial
> work and then have recursive merge try to "fix it up" is really
> starting to strain.

Interesting point.

>  I'm starting to think, again, that merge
> recursive needs a redesign, and have some arguments I wanted to float
> out there...but I've dumped enough on the list for a day.)
>
> It's possible that this series fixes one particular issue -- namely
> when merging, if the merge-base contained a "Test" directory, one side
> added a file to that directory, and the other side renamed "Test" to
> "test", and if the presence of both "Test" and "test" directories in
> the merge result is problematic, then at least with my fixes you
> wouldn't end up with both directories and could thus avoid that
> problem in a narrow set of cases.

I'll think on that. It may provide extra clues as to what the right 
solutions could be!
>
> Sorry that I don't have any better news than that for you.
>
> Elijah

Thanks
--
Philip