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

Re: [PATCH v2 11/11] merge-ort: add implementation of type-changed rename handling

From
Derrick Stolee <stolee@gmail.com>
Date
Dec 15, 2020, 14:31 UTC
Message-ID
<fcc6b18e-c5eb-27a1-b6b5-4dcdd97e2b90@gmail.com>
In-Reply-To
<ff09ddb9caf73632c9792c07f1f7499a75a09606.1607962900.git.gitgitgadget@gmail.com>
On 12/14/2020 11:21 AM, Elijah Newren via GitGitGadget wrote:
Show 24 quoted lines
> From: Elijah Newren <newren@gmail.com>
> 
> Implement cases where renames are involved in type changes (i.e. the
> side of history that didn't rename the file changed its type from a
> regular file to a symlink or submodule).  There was some code to handle
> this in merge-recursive but only in the special case when the renamed
> file had no content changes.  The code here works differently -- it
> knows process_entry() can handle mode conflicts, so it does a few
> minimal tweaks to ensure process_entry() can just finish the job as
> needed.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-ort.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/merge-ort.c b/merge-ort.c
> index 9aac33c8e31..11e33f56edf 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -778,7 +778,32 @@ static int process_renames(struct merge_options *opt,
>  			 S_ISREG(newinfo->stages[target_index].mode));
>  		if (type_changed && collision) {
>  			/* special handling so later blocks can handle this */
Perhaps drop this comment, or incorporate it into the lower one?
Show 24 quoted lines
> -			die("Not yet implemented");
> +			/*
> +			 * if type_changed && collision are both true, then this
> +			 * was really a double rename, but one side wasn't
> +			 * detected due to lack of break detection.  I.e.
> +			 * something like
> +			 *    orig: has normal file 'foo'
> +			 *    side1: renames 'foo' to 'bar', adds 'foo' symlink
> +			 *    side2: renames 'foo' to 'bar'
> +			 * In this case, the foo->bar rename on side1 won't be
> +			 * detected because the new symlink named 'foo' is
> +			 * there and we don't do break detection.  But we detect
> +			 * this here because we don't want to merge the content
> +			 * of the foo symlink with the foo->bar file, so we
> +			 * have some logic to handle this special case.  The
> +			 * easiest way to do that is make 'bar' on side1 not
> +			 * be considered a colliding file but the other part
> +			 * of a normal rename.  If the file is very different,
> +			 * well we're going to get content merge conflicts
> +			 * anyway so it doesn't hurt.  And if the colliding
> +			 * file also has a different type, that'll be handled
> +			 * by the content merge logic in process_entry() too.
> +			 *
> +			 * See also t6430, 'rename vs. rename/symlink'
I appreciate the callout to a test that exercises this behavior.
> +			 */
> +			collision = 0;
> +		}

Here, we regain that closing curly brace, fixing the compiler errors from earlier.

Show 13 quoted lines
>  		if (source_deleted) {
>  			if (target_index == 1) {
>  				rename_branch = opt->branch1;
> @@ -858,7 +883,11 @@ static int process_renames(struct merge_options *opt,
>  			newinfo->pathnames[0] = oldpath;
>  			if (type_changed) {
>  				/* rename vs. typechange */
> -				die("Not yet implemented");
> +				/* Mark the original as resolved by removal */
> +				memcpy(&oldinfo->stages[0].oid, &null_oid,
> +				       sizeof(oldinfo->stages[0].oid));
> +				oldinfo->stages[0].mode = 0;
> +				oldinfo->filemask &= 0x06;

This matches your explanation in the comment above. I wonder if 0x06 could be less magical, but we are really deep in the weeds here already.

Thanks, -Stolee

Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 49 of 65 in “merge-ort: add basic rename detection”
  1. 00/11 merge-ort: add basic rename detectionElijah Newren via GitGitGadget, Dec 9, 2020
  2. 04/11 merge-ort: implement compare_pairs() and collect_renames()Elijah Newren via GitGitGadget, Dec 9, 2020
  3. Derrick StoleeDec 11, 2020
  4. Elijah NewrenDec 11, 2020
  5. 03/11 merge-ort: implement detect_regular_renames()Elijah Newren via GitGitGadget, Dec 9, 2020
  6. Derrick StoleeDec 11, 2020
  7. Elijah NewrenDec 11, 2020
  8. 02/11 merge-ort: add initial outline for basic rename detectionElijah Newren via GitGitGadget, Dec 9, 2020
  9. Derrick StoleeDec 11, 2020
  10. Elijah NewrenDec 11, 2020
  11. Elijah NewrenDec 13, 2020
  12. Derrick StoleeDec 14, 2020
  13. Johannes SchindelinDec 14, 2020
  14. Elijah NewrenDec 14, 2020
  15. Johannes SchindelinDec 14, 2020
  16. Elijah NewrenDec 14, 2020
  17. 05/11 merge-ort: add basic outline for process_renames()Elijah Newren via GitGitGadget, Dec 9, 2020
  18. Derrick StoleeDec 11, 2020
  19. Elijah NewrenDec 11, 2020
  20. 06/11 merge-ort: add implementation of both sides renaming identicallyElijah Newren via GitGitGadget, Dec 9, 2020
  21. Derrick StoleeDec 11, 2020
  22. 10/11 merge-ort: add implementation of normal rename handlingElijah Newren via GitGitGadget, Dec 9, 2020
  23. 09/11 merge-ort: add implementation of rename/delete conflictsElijah Newren via GitGitGadget, Dec 9, 2020
  24. 01/11 merge-ort: add basic data structures for handling renamesElijah Newren via GitGitGadget, Dec 9, 2020
  25. Derrick StoleeDec 11, 2020
  26. Elijah NewrenDec 11, 2020
  27. 08/11 merge-ort: add implementation of rename collisionsElijah Newren via GitGitGadget, Dec 9, 2020
  28. 11/11 merge-ort: add implementation of type-changed rename handlingElijah Newren via GitGitGadget, Dec 9, 2020
  29. 07/11 merge-ort: add implementation of both sides renaming differentlyElijah Newren via GitGitGadget, Dec 9, 2020
  30. Derrick StoleeDec 11, 2020
  31. Elijah NewrenDec 11, 2020
  32. 00/11 merge-ort: add basic rename detectionElijah Newren via GitGitGadget, Dec 14, 2020
  33. 01/11 merge-ort: add basic data structures for handling renamesElijah Newren via GitGitGadget, Dec 14, 2020
  34. 03/11 merge-ort: implement detect_regular_renames()Elijah Newren via GitGitGadget, Dec 14, 2020
  35. 02/11 merge-ort: add initial outline for basic rename detectionElijah Newren via GitGitGadget, Dec 14, 2020
  36. 04/11 merge-ort: implement compare_pairs() and collect_renames()Elijah Newren via GitGitGadget, Dec 14, 2020
  37. 08/11 merge-ort: add implementation of rename collisionsElijah Newren via GitGitGadget, Dec 14, 2020
  38. Derrick StoleeDec 15, 2020
  39. Elijah NewrenDec 15, 2020
  40. 10/11 merge-ort: add implementation of normal rename handlingElijah Newren via GitGitGadget, Dec 14, 2020
  41. Derrick StoleeDec 15, 2020
  42. 06/11 merge-ort: add implementation of both sides renaming identicallyElijah Newren via GitGitGadget, Dec 14, 2020
  43. 05/11 merge-ort: add basic outline for process_renames()Elijah Newren via GitGitGadget, Dec 14, 2020
  44. 09/11 merge-ort: add implementation of rename/delete conflictsElijah Newren via GitGitGadget, Dec 14, 2020
  45. Derrick StoleeDec 15, 2020
  46. Elijah NewrenDec 15, 2020
  47. Derrick StoleeDec 15, 2020
  48. 11/11 merge-ort: add implementation of type-changed rename handlingElijah Newren via GitGitGadget, Dec 14, 2020
  49. Derrick StoleeDec 15, 2020
  50. Elijah NewrenDec 15, 2020
  51. 07/11 merge-ort: add implementation of both sides renaming differentlyElijah Newren via GitGitGadget, Dec 14, 2020
  52. Derrick StoleeDec 15, 2020
  53. Junio C HamanoDec 15, 2020
  54. 00/11 merge-ort: add basic rename detectionElijah Newren via GitGitGadget, Dec 15, 2020
  55. 01/11 merge-ort: add basic data structures for handling renamesElijah Newren via GitGitGadget, Dec 15, 2020
  56. 02/11 merge-ort: add initial outline for basic rename detectionElijah Newren via GitGitGadget, Dec 15, 2020
  57. 03/11 merge-ort: implement detect_regular_renames()Elijah Newren via GitGitGadget, Dec 15, 2020
  58. 05/11 merge-ort: add basic outline for process_renames()Elijah Newren via GitGitGadget, Dec 15, 2020
  59. 04/11 merge-ort: implement compare_pairs() and collect_renames()Elijah Newren via GitGitGadget, Dec 15, 2020
  60. 11/11 merge-ort: add implementation of type-changed rename handlingElijah Newren via GitGitGadget, Dec 15, 2020
  61. 10/11 merge-ort: add implementation of normal rename handlingElijah Newren via GitGitGadget, Dec 15, 2020
  62. 09/11 merge-ort: add implementation of rename collisionsElijah Newren via GitGitGadget, Dec 15, 2020
  63. 07/11 merge-ort: add implementation of both sides renaming differentlyElijah Newren via GitGitGadget, Dec 15, 2020
  64. 08/11 merge-ort: add implementation of rename/delete conflictsElijah Newren via GitGitGadget, Dec 15, 2020
  65. 06/11 merge-ort: add implementation of both sides renaming identicallyElijah Newren via GitGitGadget, Dec 15, 2020

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.