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

Re: [PATCH 07/11] merge-ort: add implementation of both sides renaming differently

From
Derrick Stolee <stolee@gmail.com>
Date
Dec 11, 2020, 03:39 UTC
Message-ID
<5b9f5db0-b4d5-f60d-b5c5-2f8376d4bf12@gmail.com>
In-Reply-To
<d4595397052568ea6ea0cf46190374c74140fed7.1607542887.git.gitgitgadget@gmail.com>
On 12/9/2020 2:41 PM, Elijah Newren via GitGitGadget wrote:
Show 19 quoted lines
> From: Elijah Newren <newren@gmail.com>
> 
> Implement rename/rename(1to2) handling, i.e. both sides of history
> renaming a file and rename it differently.  This code replaces the
> following from merge-recurisve.c:
> 
>   * all the 1to2 code in process_renames()
>   * the RENAME_ONE_FILE_TO_TWO case of process_entry()
>   * handle_rename_rename_1to2()
> 
> Also, there is some shared code from merge-recursive.c for multiple
> different rename cases which we will no longer need for this case (or
> other rename cases):
> 
>   * handle_file_collision()
>   * setup_rename_conflict_info()
> 
> The consolidation of five separate codepaths into one is made possible
> by a change in design:
Excellent!
Show 10 quoted lines
>  			/* This is a rename/rename(1to2) */
> -			die("Not yet implemented");
> +			clean_merge = handle_content_merge(opt,
> +							   pair->one->path,
> +							   &base->stages[0],
> +							   &side1->stages[1],
> +							   &side2->stages[2],
> +							   pathnames,
> +							   1 + 2 * opt->priv->call_depth,
> +							   &merged);
(this method currently die()s. ok)
Show 5 quoted lines
> +			if (!clean_merge &&
> +			    merged.mode == side1->stages[1].mode &&
> +			    oideq(&merged.oid, &side1->stages[1].oid)) {
> +				was_binary_blob = 1;
> +			}
nit: Extraneous braces?
Show 34 quoted lines
> +			memcpy(&side1->stages[1], &merged, sizeof(merged));
> +			if (was_binary_blob) {
> +				/*
> +				 * Getting here means we were attempting to
> +				 * merge a binary blob.
> +				 *
> +				 * Since we can't merge binaries,
> +				 * handle_content_merge() just takes one
> +				 * side.  But we don't want to copy the
> +				 * contents of one side to both paths.  We
> +				 * used the contents of side1 above for
> +				 * side1->stages, let's use the contents of
> +				 * side2 for side2->stages below.
> +				 */
> +				oidcpy(&merged.oid, &side2->stages[2].oid);
> +				merged.mode = side2->stages[2].mode;
> +			}
> +			memcpy(&side2->stages[2], &merged, sizeof(merged));
> +
> +			side1->path_conflict = 1;
> +			side2->path_conflict = 1;
> +			/*
> +			 * TODO: For renames we normally remove the path at the
> +			 * old name.  It would thus seem consistent to do the
> +			 * same for rename/rename(1to2) cases, but we haven't
> +			 * done so traditionally and a number of the regression
> +			 * tests now encode an expectation that the file is
> +			 * left there at stage 1.  If we ever decide to change
> +			 * this, add the following two lines here:
> +			 *    base->merged.is_null = 1;
> +			 *    base->merged.clean = 1;
> +			 * and remove the setting of base->path_conflict to 1.
> +			 */
> +			base->path_conflict = 1;

I'm getting the point of the review/evening where I'm starting to gloss over these important details. Time to take a break (after this patch).

Show 6 quoted lines
> +			path_msg(opt, oldpath, 0,
> +				 _("CONFLICT (rename/rename): %s renamed to "
> +				   "%s in %s and to %s in %s."),
> +				 pathnames[0],
> +				 pathnames[1], opt->branch1,
> +				 pathnames[2], opt->branch2);

This output differs a bit from handle_rename_rename_1to2() in merge-recursive.c:

	output(opt, 1, _("CONFLICT (rename/rename): "
	       "Rename \"%s\"->\"%s\" in branch \"%s\" "
	       "rename \"%s\"->\"%s\" in \"%s\"%s"),
	       o->path, a->path, ci->ren1->branch,
	       o->path, b->path, ci->ren2->branch,
	       opt->priv->call_depth ? _(" (left unresolved)") : "");

How much do we want to have _exact_ output matches between the two strategies, at least in the short term?

Show 13 quoted lines
> @@ -1257,13 +1309,13 @@ static void process_entry(struct merge_options *opt,
>  		int side = (ci->filemask == 4) ? 2 : 1;
>  		ci->merged.result.mode = ci->stages[side].mode;
>  		oidcpy(&ci->merged.result.oid, &ci->stages[side].oid);
> -		ci->merged.clean = !ci->df_conflict;
> +		ci->merged.clean = !ci->df_conflict && !ci->path_conflict;
>  	} else if (ci->filemask == 1) {
>  		/* Deleted on both sides */
>  		ci->merged.is_null = 1;
>  		ci->merged.result.mode = 0;
>  		oidcpy(&ci->merged.result.oid, &null_oid);
> -		ci->merged.clean = 1;
> +		ci->merged.clean = !ci->path_conflict;

These exist because this is the first time we assign path_conflict. Sure.

Thanks, -Stolee

Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 30 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.