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

Re: [PATCH v3 2/3] merge: Add merge.renames config setting

From
Eckhard Maaß <eckhard.s.maass@googlemail.com>
Date
Apr 30, 2018, 08:03 UTC
Message-ID
<20180430080341.GA28348@esm>
In-Reply-To
<CABPp-BHwM1jx2+VTxt7hga7v-E6gvHuxVNPqm-MPRXYe5CDVtA@mail.gmail.com>
On Fri, Apr 27, 2018 at 01:23:20PM -0700, Elijah Newren wrote:
> I doubt it has ever been discussed before this thread.  But, if you're
> curious, I'll try to dump a few thoughts.

Thank you, I try to dump some of mine, too. Maybe let me first stress that for me copy detection without --find-copies-harder is much more a "find content extracted" (like methods being factored out). In a way this is nearer to a rename than to a real copy.

Show 8 quoted lines
> [...] Let's say we have branches
> A and B, and:
>    A: modifies file z
>    B: copies z to y
> 
> Should the modifications to z done in A propagate to both z and y?  If
> not, what good is copy detection?  If so, then there are several
> ramifications...

If one just assumes the most likely outcome is that something from z wad factored out to y, it might just be sufficient to see whether the modifications of the two branches apply cleanly - if A touched the parts of B that have been factored out there would be a normal merge conflict (where one could be nice and give a hint that some content was copied to y on the B branch), if A did not touched the parts touched (or moved) by B, then there is no problem. If A exactly deleted the content moved by B, there will be no conflict - but this is seems to be strange anyway.

I admit that a "real" copy would get unnoticed that way. But the semantics of such a copy isn't too clear for me either - did I copy the other part to make it independent of the other or did I just employ a copy and paste tactic? The former does not want the changes, the later does. But I am happy catering to the former here.

To sum up:
- fail as before for conflicting merges, but give a hint that one has
  copied to quicken up resolution.
> - If B not only copied z but also first modified it, then do we have
>   potential conflicts with both z and y -- possibly the exact same
>   conflicts, making the user resolve them repeatedly?

With the above suggestion, if there are conflicts, you fail and give a hint.

> - What if A copied z to x?  Do changes to z propagate to all three of
>   z and x and y?  Do changes to either x or y affect z?  Do they
>   affect each other?

A copy on branch to x and one another to y seems strange even if z merges cleanly. Did both sides try to factor the same thing out to different files? Or did they try to make something independent, but managed to make it to different files? For this I would be inclined to just suggest fail with a copy/copy(somewhere else). But this is a real corner case after all. Has anyone seen just thing in practice?

> - If A deleted z, does that give us a copy/delete conflict for y?  Do
>   we also have to worry about copy/add conflicts?  copy/add/delete?
>   rename/copy (multiple variants)?  copy/copy?

We do have the modified/deleted conflict where we could hint that content also has been copied and then not try to do more.

Show 5 quoted lines
> - Extra degrees of freedom may mean new conflict types:
> 
>   - The extra degrees of freedom from renames introduced multiple new
>     conflict types (e.g. rename/add, rename/rename(1to2),
>     rename/rename(2to1)).

For renaming one side and coping the other, I would think doing the same as above is sensible enough: if there are conflicts one can give an additional hint of the one part having been copied, but not change the kind of conflicts much.

> The more I think about it, the more I think that attempting to detect
> copies in a merge algorithm just doesn't make sense.  Anything I can
> think of that someone might attempt to use detected copies for would
> just surprise users in a bad way...

Hm, it didn't sound like that. Would you think that users would be surprised by my suggestions? Or are they all too corner casey to be worth implementing anyway?

Greetings, Eckhard

Previous: Elijah NewrenNext: Elijah Newren
Message 50 of 65 in “add additional config settings for merge”
  1. 0/2 add additional config settings for mergeBen Peart, Apr 20, 2018
  2. 1/2 merge: Add merge.renames config settingBen Peart, Apr 20, 2018
  3. Elijah NewrenApr 20, 2018
  4. Elijah NewrenApr 20, 2018
  5. Ben PeartApr 23, 2018
  6. Ben PeartApr 20, 2018
  7. Elijah NewrenApr 20, 2018
  8. Junio C HamanoApr 21, 2018
  9. Ben PeartApr 23, 2018
  10. Junio C HamanoApr 23, 2018
  11. Johannes SchindelinApr 24, 2018
  12. Elijah NewrenApr 24, 2018
  13. Johannes SchindelinApr 25, 2018
  14. Eckhard MaaßApr 22, 2018
  15. Ben PeartApr 23, 2018
  16. Eckhard MaaßApr 23, 2018
  17. Ben PeartApr 24, 2018
  18. Ben PeartApr 23, 2018
  19. 2/2 merge: Add merge.aggressive config settingBen Peart, Apr 20, 2018
  20. Elijah NewrenApr 20, 2018
  21. Ben PeartApr 24, 2018
  22. Elijah NewrenApr 24, 2018
  23. Junio C HamanoApr 24, 2018
  24. Ben PeartApr 25, 2018
  25. Elijah NewrenApr 20, 2018
  26. Ben PeartApr 20, 2018
  27. 0/2 add additional config settings for mergeBen Peart, Apr 24, 2018
  28. 1/2 merge: Add merge.renames config settingBen Peart, Apr 24, 2018
  29. Elijah NewrenApr 24, 2018
  30. Elijah NewrenApr 24, 2018
  31. Ben PeartApr 24, 2018
  32. Elijah NewrenApr 25, 2018
  33. 2/2 merge: Add merge.aggressive config settingBen Peart, Apr 24, 2018
  34. Junio C HamanoApr 25, 2018
  35. Ben PeartApr 25, 2018
  36. Junio C HamanoApr 26, 2018
  37. 0/3 add merge.renames config settingBen Peart, Apr 26, 2018
  38. 1/3 merge: update documentation for {merge,diff}.renameLimitBen Peart, Apr 26, 2018
  39. Elijah NewrenApr 26, 2018
  40. Jonathan TanApr 26, 2018
  41. 2/3 merge: Add merge.renames config settingBen Peart, Apr 26, 2018
  42. Elijah NewrenApr 26, 2018
  43. Ben PeartApr 27, 2018
  44. Junio C HamanoApr 27, 2018
  45. Elijah NewrenApr 27, 2018
  46. Johannes SchindelinApr 27, 2018
  47. Elijah NewrenApr 27, 2018
  48. Eckhard MaaßApr 27, 2018
  49. Elijah NewrenApr 27, 2018
  50. Eckhard MaaßApr 30, 2018
  51. Elijah NewrenApr 30, 2018
  52. Elijah NewrenApr 27, 2018
  53. Elijah Newren, Apr 27, 2018
  54. Ben PeartApr 30, 2018
  55. Elijah NewrenApr 30, 2018
  56. Ben PeartMay 2, 2018
  57. 3/3 merge: pass aggressive when rename detection is turned offBen Peart, Apr 26, 2018
  58. Elijah NewrenApr 26, 2018
  59. Elijah NewrenApr 26, 2018
  60. 0/3 add additional config settings for mergeBen Peart, May 2, 2018
  61. 1/3 merge: update documentation for {merge,diff}.renameLimitBen Peart, May 2, 2018
  62. 2/3 merge: Add merge.renames config settingBen Peart, May 2, 2018
  63. Junio C HamanoMay 4, 2018
  64. 3/3 merge: pass aggressive when rename detection is turned offBen Peart, May 2, 2018
  65. Elijah NewrenMay 2, 2018

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.