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

Re: [BUG] All files in folder are moved when cherry-picking commit that moves fewer files

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 7, 2019, 05:45 UTC
Message-ID
<xmqqy35rme4q.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<CABPp-BF=GFgFRfgdG2rFK89x1tYrJO8TpECyv6BoTgz9q3Rb+g@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> I'm not sure I'm following well enough to answer any your question(s),
> so I'll instead ask a few of my own:
>   * Are you referring to stage #2 and stage #3 of the old path or the
> new path or one of each or both of each?

If they moved the sole neighbouring file X/A to Y/A, while we added a new path to X/B, I would expect we'd have two blobs with the same content we added, one at X/B at stage #2 (because that is what we wanted to have) and the other at Y/B at stage #3 (that's their wish the heuristic is guessing). If it were the other way, the stages would be different.

>   * Would this be consistent with how normal renames behave?
> and, as a partial answer to your question:

The use of the three-way merge logic at the tree structure level, unlike the "because neighbours moved" heuristics, gives a more definite answer. We see that the contents in the original at path A (in the common ancestor version) is now at path B (with or without slight modification) in their tree, while we kept it at path A (again, with or without modification). Or vice versa. Either way, at the tree structure level, that is "one side changes, while the other side does not modify" which is resolved cleanly to "take the new path the side that modifies wants the file to have". So we do not need to use the stages to represent "these sides wants to place it at different paths, which one is correct?" Instead, we'd have stages #1 (common ancestor), #2 (ours) and #3 (theirs) all at path B (i.e. the cleanly resolved structural change conflict) to help the user reconcile content level conflicts (iff needed).

Show 8 quoted lines
> I would prefer to tell the user they can resolve the conflict with two
> other commands.  I'm currently thinking of something like these two:
>     git add $directory_renamed_path
>     git mv $directory_renamed_path $original_path [&& git add $original_path ?]
> This pair assumes we write the working tree and index entries for
> $directory_renamed_path and leave $original_path empty in both the
> working tree and index.  We could alternatively reverse that, with an
> equivalent but swapped set of commands for resolution.

Yeah, the final version may have difference in detail, but I think it is sensible to leave a state, from which the user can go in either direction, with a simple command or two, like the above one.

Show 15 quoted lines
>> ...  With that
>> attitude, we may be able to get rid of choice (a) altogether, which
>> may make things a tad simpler.  IOW, we assume the heuristics would
>> suggest the right solution most of the time, just like we assume
>> rerere gives a reasonable resolution most of the time, with a knob
>> to accept the result blindly, together with hints to recover when
>> heuristics makes mistakes.
>
> I like that analogy.  I think that aligns well with my possible
> conflict-resolution commands, and further answers the question about
> whether to put the new path or the old path in the working tree and
> index.
>
> However, we do need some form of choice (a) for commands which can't
> provide accurate tree information, such as git-am.

Yeah, if we are not even running the heuristics for that case (a), hence it is not sensible to expect to offer "if we were doing directory renames, this path might have gone to that other place", then I think I am fine with not giving any hint to change their mind (after all, they can "git reset --hard" and retry the merge, this time not using the non-default (a) mode). And keeping around the simplest "we do not even attempt to" (a) mode as another option is fine by me.

Thanks.
Previous: Junio C HamanoNext: Elijah Newren
Message 14 of 49 in “[BUG] All files in folder are moved when cherry-picking commit that moves fewer files”
  1. Linus NilssonFeb 27, 2019
  2. Phillip WoodFeb 27, 2019
  3. Elijah NewrenFeb 27, 2019
  4. Jeff KingFeb 27, 2019
  5. Elijah NewrenFeb 27, 2019
  6. Linus NilssonFeb 28, 2019
  7. Junio C HamanoMar 1, 2019
  8. Elijah NewrenMar 2, 2019
  9. Junio C HamanoMar 3, 2019
  10. Elijah NewrenMar 6, 2019
  11. Junio C HamanoMar 6, 2019
  12. Elijah NewrenMar 7, 2019
  13. Junio C HamanoMar 7, 2019
  14. Junio C HamanoMar 7, 2019
  15. 00/15 Switch directory rename detection defaultElijah Newren, Mar 30, 2019
  16. 08/15 merge-recursive: remove ren[12]_other fields from rename_conflict_infoElijah Newren, Mar 30, 2019
  17. 10/15 merge-recursive: cleanup handle_rename_* function signaturesElijah Newren, Mar 30, 2019
  18. 01/15 Use 'unsigned short' for mode, like diff_filespec doesElijah Newren, Mar 30, 2019
  19. 04/15 merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf'Elijah Newren, Mar 30, 2019
  20. 14/15 merge-recursive: give callers of handle_content_merge() access to contentsElijah Newren, Mar 30, 2019
  21. 11/15 merge-recursive: switch from (oid,mode) pairs to a diff_filespecElijah Newren, Mar 30, 2019
  22. 07/15 merge-recursive: shrink rename_conflict_infoElijah Newren, Mar 30, 2019
  23. 12/15 t6043: fix copied test description to match its purposeElijah Newren, Mar 30, 2019
  24. 09/15 merge-recursive: track branch where rename occurred in rename structElijah Newren, Mar 30, 2019
  25. 13/15 merge-recursive: track information associated with directory renamesElijah Newren, Mar 30, 2019
  26. 15/15 merge-recursive: switch directory rename detection defaultElijah Newren, Mar 30, 2019
  27. Ævar Arnfjörð BjarmasonMar 30, 2019
  28. Elijah NewrenApr 1, 2019
  29. 02/15 merge-recursive: rename merge_options argument from 'o' to 'opt'Elijah Newren, Mar 30, 2019
  30. 05/15 merge-recursive: use 'ci' for rename_conflict_info variable nameElijah Newren, Mar 30, 2019
  31. 03/15 merge-recursive: rename diff_filespec 'one' to 'o'Elijah Newren, Mar 30, 2019
  32. 06/15 merge-recursive: move some struct declarations togetherElijah Newren, Mar 30, 2019
  33. 00/15 Switch directory rename detection defaultElijah Newren, Apr 5, 2019
  34. 03/15 merge-recursive: rename diff_filespec 'one' to 'o'Elijah Newren, Apr 5, 2019
  35. 05/15 merge-recursive: use 'ci' for rename_conflict_info variable nameElijah Newren, Apr 5, 2019
  36. 09/15 merge-recursive: track branch where rename occurred in rename structElijah Newren, Apr 5, 2019
  37. 06/15 merge-recursive: move some struct declarations togetherElijah Newren, Apr 5, 2019
  38. 07/15 merge-recursive: shrink rename_conflict_infoElijah Newren, Apr 5, 2019
  39. 08/15 merge-recursive: remove ren[12]_other fields from rename_conflict_infoElijah Newren, Apr 5, 2019
  40. 04/15 merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf'Elijah Newren, Apr 5, 2019
  41. 15/15 merge-recursive: switch directory rename detection defaultElijah Newren, Apr 5, 2019
  42. 14/15 merge-recursive: give callers of handle_content_merge() access to contentsElijah Newren, Apr 5, 2019
  43. 12/15 t6043: fix copied test description to match its purposeElijah Newren, Apr 5, 2019
  44. 13/15 merge-recursive: track information associated with directory renamesElijah Newren, Apr 5, 2019
  45. 02/15 merge-recursive: rename merge_options argument from 'o' to 'opt'Elijah Newren, Apr 5, 2019
  46. 10/15 merge-recursive: cleanup handle_rename_* function signaturesElijah Newren, Apr 5, 2019
  47. 01/15 Use 'unsigned short' for mode, like diff_filespec doesElijah Newren, Apr 5, 2019
  48. 11/15 merge-recursive: switch from (oid,mode) pairs to a diff_filespecElijah Newren, Apr 5, 2019
  49. Jacob KellerApr 5, 2019

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.