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

[PATCH v3 5/5] merge-ort: fix issue with dual rename and add/add conflict

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Jul 1, 2022, 05:23 UTC
Message-ID
<121761e26e2468d77910a7bee2e1f18776995c68.1656653000.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1268.v3.git.1656653000.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>

There is code in both merge-recursive and merge-ort for avoiding doubly transitive renames (i.e. one side renames directory A/ -> B/, and the other side renames directory B/ -> C/), because this combination would otherwise make a mess for new files added to A/ on the first side and wondering which directory they end up in -- especially if there were even more renames such as the first side renaming C/ -> D/. In such cases, it just turns "off" directory rename detection for the higher order transitive cases.

The testcases added in t6423 a couple commits ago are slightly different but similar in principle. They involve a similar case of paired renaming but instead of A/ -> B/ and B/ -> C/, the second side renames a leading directory of B/ to C/. And both sides add a new file somewhere under the directory that the other side will rename. While the new files added start within different directories and thus could logically end up within different directories, it is weird for a file on one side to end up where the other one started and not move along with it. So, let's just turn off directory rename detection in this case as well.

Another way to look at this is that if the source name involved in a directory rename on one side is the target name of a directory rename operation for a file from the other side, then we avoid the doubly transitive rename. (More concretely, if a directory rename on side D wants to rename a file on side E from OLD_NAME -> NEW_NAME, and side D already had a file named NEW_NAME, and a directory rename on side E wants to rename side D's NEW_NAME -> NEWER_NAME, then we turn off the directory rename detection for NEW_NAME to prevent the NEW_NAME -> NEWER_NAME rename, and instead end up with an add/add conflict on NEW_NAME.)

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c                         | 8 +++++++-
 t/t6423-merge-rename-directories.sh | 4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index b496f0e3803..daa0e4496f8 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2292,10 +2292,16 @@ static char *check_for_directory_rename(struct merge_options *opt,
 	struct strmap_entry *rename_info;
 	struct strmap_entry *otherinfo;
 	const char *new_dir;
+	int other_side = 3 - side_index;
 
-	/* Cases where we don't have a directory rename for this path */
+	/*
+	 * Cases where we don't have or don't want a directory rename for
+	 * this path.
+	 */
 	if (strmap_empty(dir_renames))
 		return NULL;
+	if (strmap_get(&collisions[other_side], path))
+		return NULL;
 	rename_info = check_dir_renamed(path, dir_renames);
 	if (!rename_info)
 		return NULL;
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index ed5586de28c..99baf77cbfd 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -5250,7 +5250,7 @@ test_setup_12l () {
 	)
 }
 
-test_expect_merge_algorithm failure failure '12l (B into A): Rename into each other + add/add conflict' '
+test_expect_merge_algorithm failure success '12l (B into A): Rename into each other + add/add conflict' '
 	test_setup_12l BintoA &&
 	(
 		cd 12l_BintoA &&
@@ -5277,7 +5277,7 @@ test_expect_merge_algorithm failure failure '12l (B into A): Rename into each ot
 	)
 '
 
-test_expect_merge_algorithm failure failure '12l (A into B): Rename into each other + add/add conflict' '
+test_expect_merge_algorithm failure success '12l (A into B): Rename into each other + add/add conflict' '
 	test_setup_12l AintoB &&
 	(
 		cd 12l_AintoB &&
-- 
gitgitgadget
Previous: Junio C HamanoNext: Elijah Newren via GitGitGadget
Message 38 of 45 in “Fix dual rename into each other plus conflicting adds”
  1. 0/3 Fix dual rename into each other plus conflicting addsElijah Newren via GitGitGadget, Jun 22, 2022
  2. 1/3 t6423: add tests of dual directory rename plus add/add conflictElijah Newren via GitGitGadget, Jun 22, 2022
  3. Jonathan TanJun 27, 2022
  4. Elijah NewrenJun 30, 2022
  5. Jonathan TanJun 30, 2022
  6. Elijah NewrenJul 1, 2022
  7. Calvin WanJun 27, 2022
  8. Elijah NewrenJun 30, 2022
  9. 2/3 merge-ort: shuffle the computation and cleanup of potential collisionsElijah Newren via GitGitGadget, Jun 22, 2022
  10. Jonathan TanJun 27, 2022
  11. Calvin WanJun 27, 2022
  12. Elijah NewrenJun 30, 2022
  13. 3/3 merge-ort: fix issue with dual rename and add/add conflictElijah Newren via GitGitGadget, Jun 22, 2022
  14. Jonathan TanJun 27, 2022
  15. Elijah NewrenJun 30, 2022
  16. Jonathan TanJul 6, 2022
  17. Elijah NewrenJun 22, 2022
  18. 0/3 Fix dual rename into each other plus conflicting addsElijah Newren via GitGitGadget, Jun 30, 2022
  19. 1/3 t6423: add tests of dual directory rename plus add/add conflictElijah Newren via GitGitGadget, Jun 30, 2022
  20. Ævar Arnfjörð BjarmasonJun 30, 2022
  21. Elijah NewrenJul 1, 2022
  22. Ævar Arnfjörð BjarmasonJul 1, 2022
  23. 2/3 merge-ort: shuffle the computation and cleanup of potential collisionsElijah Newren via GitGitGadget, Jun 30, 2022
  24. Ævar Arnfjörð BjarmasonJun 30, 2022
  25. Elijah NewrenJul 1, 2022
  26. 3/3 merge-ort: fix issue with dual rename and add/add conflictElijah Newren via GitGitGadget, Jun 30, 2022
  27. Ævar Arnfjörð BjarmasonJun 30, 2022
  28. Elijah NewrenJul 1, 2022
  29. 0/5 Fix dual rename into each other plus conflicting addsElijah Newren via GitGitGadget, Jul 1, 2022
  30. 1/5 t6423: add tests of dual directory rename plus add/add conflictElijah Newren via GitGitGadget, Jul 1, 2022
  31. 2/5 merge-ort: small cleanups of check_for_directory_renameElijah Newren via GitGitGadget, Jul 1, 2022
  32. 3/5 merge-ort: make a separate function for freeing struct collisionsElijah Newren via GitGitGadget, Jul 1, 2022
  33. 4/5 merge-ort: shuffle the computation and cleanup of potential collisionsElijah Newren via GitGitGadget, Jul 1, 2022
  34. Ævar Arnfjörð BjarmasonJul 1, 2022
  35. C99 "for (int ..." form on "master" (was: [PATCH v3 4/5] merge-ort: shuffle the computation and cleanup of potential collisions)Ævar Arnfjörð Bjarmason, Jul 25, 2022
  36. Elijah NewrenJul 26, 2022
  37. Junio C HamanoJul 26, 2022
  38. 5/5 merge-ort: fix issue with dual rename and add/add conflictElijah Newren via GitGitGadget, Jul 1, 2022
  39. 0/5 Fix dual rename into each other plus conflicting addsElijah Newren via GitGitGadget, Jul 5, 2022
  40. 1/5 t6423: add tests of dual directory rename plus add/add conflictElijah Newren via GitGitGadget, Jul 5, 2022
  41. 2/5 merge-ort: small cleanups of check_for_directory_renameElijah Newren via GitGitGadget, Jul 5, 2022
  42. 3/5 merge-ort: make a separate function for freeing struct collisionsElijah Newren via GitGitGadget, Jul 5, 2022
  43. 4/5 merge-ort: shuffle the computation and cleanup of potential collisionsElijah Newren via GitGitGadget, Jul 5, 2022
  44. 5/5 merge-ort: fix issue with dual rename and add/add conflictElijah Newren via GitGitGadget, Jul 5, 2022
  45. Junio C HamanoJul 6, 2022

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.