git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Elijah Newren
Message 81 of 81 in “Add directory rename detection to git”
  1. 00/30 Add directory rename detection to gitElijah Newren, Nov 10, 2017
  2. 01/30 Tighten and correct a few testcases for merging and cherry-pickingElijah Newren, Nov 10, 2017
  3. Stefan BellerNov 13, 2017
  4. 24/30 merge-recursive: Add computation of collisions due to dir rename & mergingElijah Newren, Nov 10, 2017
  5. René ScharfeJun 10, 2018
  6. René ScharfeJun 10, 2018
  7. Jeff KingJun 10, 2018
  8. Elijah NewrenJun 11, 2018
  9. Junio C HamanoJun 14, 2018
  10. 23/30 merge-recursive: Add a new hashmap for storing file collisionsElijah Newren, Nov 10, 2017
  11. 11/30 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Nov 10, 2017
  12. Stefan BellerNov 14, 2017
  13. Elijah NewrenNov 14, 2017
  14. 16/30 merge-recursive: Introduce new functions to handle rename logicElijah Newren, Nov 10, 2017
  15. Junio C HamanoNov 14, 2017
  16. Junio C HamanoNov 14, 2017
  17. Elijah NewrenNov 14, 2017
  18. 09/30 directory rename detection: testcases checking which side did the renameElijah Newren, Nov 10, 2017
  19. Stefan BellerNov 14, 2017
  20. Elijah NewrenNov 14, 2017
  21. 06/30 directory rename detection: testcases to avoid taking detection too farElijah Newren, Nov 10, 2017
  22. Stefan BellerNov 13, 2017
  23. Elijah NewrenNov 14, 2017
  24. 08/30 directory rename detection: files/directories in the way of some renamesElijah Newren, Nov 10, 2017
  25. Stefan BellerNov 14, 2017
  26. Elijah NewrenNov 14, 2017
  27. 05/30 directory rename detection: directory splitting testcasesElijah Newren, Nov 10, 2017
  28. Stefan BellerNov 13, 2017
  29. 07/30 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Nov 10, 2017
  30. Stefan BellerNov 14, 2017
  31. 26/30 merge-recursive: When comparing files, don't include treesElijah Newren, Nov 10, 2017
  32. 29/30 merge-recursive: Fix overwriting dirty files involved in renamesElijah Newren, Nov 10, 2017
  33. 12/30 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Nov 10, 2017
  34. Stefan BellerNov 15, 2017
  35. Elijah NewrenNov 16, 2017
  36. 10/30 directory rename detection: more involved edge/corner testcasesElijah Newren, Nov 10, 2017
  37. Stefan BellerNov 14, 2017
  38. Elijah NewrenNov 14, 2017
  39. Stefan BellerNov 14, 2017
  40. 14/30 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Nov 10, 2017
  41. 15/30 merge-recursive: Move the get_renames() functionElijah Newren, Nov 10, 2017
  42. Junio C HamanoNov 14, 2017
  43. Elijah NewrenNov 14, 2017
  44. Junio C HamanoNov 15, 2017
  45. 22/30 merge-recursive: Check for directory level conflictsElijah Newren, Nov 10, 2017
  46. 19/30 merge-recursive: Split out code for determining diff_filepairsElijah Newren, Nov 10, 2017
  47. Junio C HamanoNov 14, 2017
  48. 13/30 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Nov 10, 2017
  49. 27/30 merge-recursive: Apply necessary modifications for directory renamesElijah Newren, Nov 10, 2017
  50. Stefan BellerNov 15, 2017
  51. Elijah NewrenNov 16, 2017
  52. 18/30 merge-recursive: Make !o->detect_rename codepath more obviousElijah Newren, Nov 10, 2017
  53. 17/30 merge-recursive: Fix leaks of allocated renames and diff_filepairsElijah Newren, Nov 10, 2017
  54. Junio C HamanoNov 14, 2017
  55. 28/30 merge-recursive: Avoid clobbering untracked files with directory renamesElijah Newren, Nov 10, 2017
  56. 30/30 merge-recursive: Fix remaining directory rename + dirty overwrite casesElijah Newren, Nov 10, 2017
  57. 21/30 merge-recursive: Add get_directory_renames()Elijah Newren, Nov 10, 2017
  58. Junio C HamanoNov 14, 2017
  59. Elijah NewrenNov 14, 2017
  60. 25/30 merge-recursive: Check for file level conflicts then get new nameElijah Newren, Nov 10, 2017
  61. 20/30 merge-recursive: Add a new hashmap for storing directory renamesElijah Newren, Nov 10, 2017
  62. 02/30 merge-recursive: Fix logic ordering issueElijah Newren, Nov 10, 2017
  63. Stefan BellerNov 13, 2017
  64. Elijah NewrenNov 13, 2017
  65. Stefan BellerNov 13, 2017
  66. Elijah NewrenNov 13, 2017
  67. Stefan BellerNov 13, 2017
  68. 04/30 directory rename detection: basic testcasesElijah Newren, Nov 10, 2017
  69. Stefan BellerNov 13, 2017
  70. Elijah NewrenNov 14, 2017
  71. Stefan BellerNov 14, 2017
  72. Elijah NewrenNov 14, 2017
  73. Junio C HamanoNov 14, 2017
  74. 03/30 merge-recursive: Add explanation for src_entry and dst_entryElijah Newren, Nov 10, 2017
  75. Stefan BellerNov 13, 2017
  76. Elijah NewrenNov 13, 2017
  77. Stefan BellerNov 13, 2017
  78. Junio C HamanoNov 14, 2017
  79. Philip OakleyNov 10, 2017
  80. Elijah NewrenNov 10, 2017
  81. Philip OakleyNov 13, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.