Re: [PATCH 00/30] Add directory rename detection to git
- From
Philip Oakley <philipoakley@iee.org>
- Date
- Nov 13, 2017, 15:04 UTC
- Message-ID
- <0CA45D65AEDF405C987769D865EE0836@PhilipOakley>
- In-Reply-To
- <CABPp-BE1+32fQQdYuwP-7m+GXqyhoUpHKgMM6SCcW4e9geutvw@mail.gmail.com>
From: "Elijah Newren" <newren@gmail.com> : Friday, November 10, 2017 11:26 PM
Show 35 quoted lines
> 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.
Show 5 quoted lines
> 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.
Show 9 quoted lines
> > 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.
Show 6 quoted lines
> 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.
Show 11 quoted lines
> 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