From: Philip Oakley Date: Mon, 13 Nov 2017 15:04:52 GMT Subject: Re: [PATCH 00/30] Add directory rename detection to git Message-ID: <0CA45D65AEDF405C987769D865EE0836@PhilipOakley> In-Reply-To: From: "Elijah Newren" : Friday, November 10, 2017 11:26 PM > On Fri, Nov 10, 2017 at 2:27 PM, Philip Oakley > wrote: >> From: "Elijah Newren" >>> >>> 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