Web lists-archives.com

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




From: "Elijah Newren" <newren@xxxxxxxxx>
: Friday, November 10, 2017 11:26 PM
On Fri, Nov 10, 2017 at 2:27 PM, Philip Oakley <philipoakley@xxxxxxx> wrote:
From: "Elijah Newren" <newren@xxxxxxxxx>

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