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

[PATCH v10 07/36] directory rename detection: more involved edge/corner testcases

From
Elijah Newren <newren@gmail.com>
Date
Apr 19, 2018, 17:57 UTC
Message-ID
<20180419175823.7946-8-newren@gmail.com>
In-Reply-To
<20180419175823.7946-1-newren@gmail.com>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6043-merge-rename-directories.sh | 396 ++++++++++++++++++++++++++++
 1 file changed, 396 insertions(+)
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index 9ae245a522..6db1439675 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -1516,4 +1516,400 @@ test_expect_success '6e-check: Add/add from one side' '
 #   side of history is the one doing the renaming.
 ###########################################################################
 
+
+###########################################################################
+# SECTION 7: More involved Edge/Corner cases
+#
+# The ruleset we have generated in the above sections seems to provide
+# well-defined merges.  But can we find edge/corner cases that either (a)
+# are harder for users to understand, or (b) have a resolution that is
+# non-intuitive or suboptimal?
+#
+# The testcases in this section dive into cases that I've tried to craft in
+# a way to find some that might be surprising to users or difficult for
+# them to understand (the next section will look at non-intuitive or
+# suboptimal merge results).  Some of the testcases are similar to ones
+# from past sections, but have been simplified to try to highlight error
+# messages using a "modified" path (due to the directory rename).  Are
+# users okay with these?
+#
+# In my opinion, testcases that are difficult to understand from this
+# section is due to difficulty in the testcase rather than the directory
+# renaming (similar to how t6042 and t6036 have difficult resolutions due
+# to the problem setup itself being complex).  And I don't think the
+# error messages are a problem.
+#
+# On the other hand, the testcases in section 8 worry me slightly more...
+###########################################################################
+
+# Testcase 7a, rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file
+#   Commit O: z/{b,c}
+#   Commit A: y/{b,c}
+#   Commit B: w/b, x/c, z/d
+#   Expected: y/d, CONFLICT(rename/rename for both z/b and z/c)
+#   NOTE: There's a rename of z/ here, y/ has more renames, so z/d -> y/d.
+
+test_expect_success '7a-setup: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '
+	test_create_repo 7a &&
+	(
+		cd 7a &&
+
+		mkdir z &&
+		echo b >z/b &&
+		echo c >z/c &&
+		git add z &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv z y &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		mkdir w &&
+		mkdir x &&
+		git mv z/b w/ &&
+		git mv z/c x/ &&
+		echo d > z/d &&
+		git add z/d &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '7a-check: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '
+	(
+		cd 7a &&
+
+		git checkout A^0 &&
+
+		test_must_fail git merge -s recursive B^0 >out &&
+		test_i18ngrep "CONFLICT (rename/rename).*z/b.*y/b.*w/b" out &&
+		test_i18ngrep "CONFLICT (rename/rename).*z/c.*y/c.*x/c" out &&
+
+		git ls-files -s >out &&
+		test_line_count = 7 out &&
+		git ls-files -u >out &&
+		test_line_count = 6 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
+
+		git rev-parse >actual \
+			:1:z/b :2:y/b :3:w/b :1:z/c :2:y/c :3:x/c :0:y/d &&
+		git rev-parse >expect \
+			 O:z/b  O:z/b  O:z/b  O:z/c  O:z/c  O:z/c  B:z/d &&
+		test_cmp expect actual &&
+
+		git hash-object >actual \
+			y/b   w/b   y/c   x/c &&
+		git rev-parse >expect \
+			O:z/b O:z/b O:z/c O:z/c &&
+		test_cmp expect actual
+	)
+'
+
+# Testcase 7b, rename/rename(2to1), but only due to transitive rename
+#   (Related to testcase 1d)
+#   Commit O: z/{b,c},     x/d_1, w/d_2
+#   Commit A: y/{b,c,d_2}, x/d_1
+#   Commit B: z/{b,c,d_1},        w/d_2
+#   Expected: y/{b,c}, CONFLICT(rename/rename(2to1): x/d_1, w/d_2 -> y_d)
+
+test_expect_success '7b-setup: rename/rename(2to1), but only due to transitive rename' '
+	test_create_repo 7b &&
+	(
+		cd 7b &&
+
+		mkdir z &&
+		mkdir x &&
+		mkdir w &&
+		echo b >z/b &&
+		echo c >z/c &&
+		echo d1 > x/d &&
+		echo d2 > w/d &&
+		git add z x w &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv z y &&
+		git mv w/d y/ &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		git mv x/d z/ &&
+		rmdir x &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '7b-check: rename/rename(2to1), but only due to transitive rename' '
+	(
+		cd 7b &&
+
+		git checkout A^0 &&
+
+		test_must_fail git merge -s recursive B^0 >out &&
+		test_i18ngrep "CONFLICT (rename/rename)" out &&
+
+		git ls-files -s >out &&
+		test_line_count = 4 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 3 out &&
+
+		git rev-parse >actual \
+			:0:y/b :0:y/c :2:y/d :3:y/d &&
+		git rev-parse >expect \
+			 O:z/b  O:z/c  O:w/d  O:x/d &&
+		test_cmp expect actual &&
+
+		test_path_is_missing y/d &&
+		test_path_is_file y/d~HEAD &&
+		test_path_is_file y/d~B^0 &&
+
+		git hash-object >actual \
+			y/d~HEAD y/d~B^0 &&
+		git rev-parse >expect \
+			O:w/d    O:x/d &&
+		test_cmp expect actual
+	)
+'
+
+# Testcase 7c, rename/rename(1to...2or3); transitive rename may add complexity
+#   (Related to testcases 3b and 5c)
+#   Commit O: z/{b,c}, x/d
+#   Commit A: y/{b,c}, w/d
+#   Commit B: z/{b,c,d}
+#   Expected: y/{b,c}, CONFLICT(x/d -> w/d vs. y/d)
+#   NOTE: z/ was renamed to y/ so we do want to report
+#         neither CONFLICT(x/d -> w/d vs. z/d)
+#         nor CONFLiCT x/d -> w/d vs. y/d vs. z/d)
+
+test_expect_success '7c-setup: rename/rename(1to...2or3); transitive rename may add complexity' '
+	test_create_repo 7c &&
+	(
+		cd 7c &&
+
+		mkdir z &&
+		echo b >z/b &&
+		echo c >z/c &&
+		mkdir x &&
+		echo d >x/d &&
+		git add z x &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv z y &&
+		git mv x w &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		git mv x/d z/ &&
+		rmdir x &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '7c-check: rename/rename(1to...2or3); transitive rename may add complexity' '
+	(
+		cd 7c &&
+
+		git checkout A^0 &&
+
+		test_must_fail git merge -s recursive B^0 >out &&
+		test_i18ngrep "CONFLICT (rename/rename).*x/d.*w/d.*y/d" out &&
+
+		git ls-files -s >out &&
+		test_line_count = 5 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
+
+		git rev-parse >actual \
+			:0:y/b :0:y/c :1:x/d :2:w/d :3:y/d &&
+		git rev-parse >expect \
+			 O:z/b  O:z/c  O:x/d  O:x/d  O:x/d &&
+		test_cmp expect actual
+	)
+'
+
+# Testcase 7d, transitive rename involved in rename/delete; how is it reported?
+#   (Related somewhat to testcases 5b and 8d)
+#   Commit O: z/{b,c}, x/d
+#   Commit A: y/{b,c}
+#   Commit B: z/{b,c,d}
+#   Expected: y/{b,c}, CONFLICT(delete x/d vs rename to y/d)
+#   NOTE: z->y so NOT CONFLICT(delete x/d vs rename to z/d)
+
+test_expect_success '7d-setup: transitive rename involved in rename/delete; how is it reported?' '
+	test_create_repo 7d &&
+	(
+		cd 7d &&
+
+		mkdir z &&
+		echo b >z/b &&
+		echo c >z/c &&
+		mkdir x &&
+		echo d >x/d &&
+		git add z x &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv z y &&
+		git rm -rf x &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		git mv x/d z/ &&
+		rmdir x &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '7d-check: transitive rename involved in rename/delete; how is it reported?' '
+	(
+		cd 7d &&
+
+		git checkout A^0 &&
+
+		test_must_fail git merge -s recursive B^0 >out &&
+		test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
+
+		git ls-files -s >out &&
+		test_line_count = 3 out &&
+		git ls-files -u >out &&
+		test_line_count = 1 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
+
+		git rev-parse >actual \
+			:0:y/b :0:y/c :3:y/d &&
+		git rev-parse >expect \
+			 O:z/b  O:z/c  O:x/d &&
+		test_cmp expect actual
+	)
+'
+
+# Testcase 7e, transitive rename in rename/delete AND dirs in the way
+#   (Very similar to 'both rename source and destination involved in D/F conflict' from t6022-merge-rename.sh)
+#   (Also related to testcases 9c and 9d)
+#   Commit O: z/{b,c},     x/d_1
+#   Commit A: y/{b,c,d/g}, x/d/f
+#   Commit B: z/{b,c,d_1}
+#   Expected: rename/delete(x/d_1->y/d_1 vs. None) + D/F conflict on y/d
+#             y/{b,c,d/g}, y/d_1~B^0, x/d/f
+
+#   NOTE: The main path of interest here is d_1 and where it ends up, but
+#         this is actually a case that has two potential directory renames
+#         involved and D/F conflict(s), so it makes sense to walk through
+#         each step.
+#
+#         Commit A renames z/ -> y/.  Thus everything that B adds to z/
+#         should be instead moved to y/.  This gives us the D/F conflict on
+#         y/d because x/d_1 -> z/d_1 -> y/d_1 conflicts with y/d/g.
+#
+#         Further, commit B renames x/ -> z/, thus everything A adds to x/
+#         should instead be moved to z/...BUT we removed z/ and renamed it
+#         to y/, so maybe everything should move not from x/ to z/, but
+#         from x/ to z/ to y/.  Doing so might make sense from the logic so
+#         far, but note that commit A had both an x/ and a y/; it did the
+#         renaming of z/ to y/ and created x/d/f and it clearly made these
+#         things separate, so it doesn't make much sense to push these
+#         together.  Doing so is what I'd call a doubly transitive rename;
+#         see testcases 9c and 9d for further discussion of this issue and
+#         how it's resolved.
+
+test_expect_success '7e-setup: transitive rename in rename/delete AND dirs in the way' '
+	test_create_repo 7e &&
+	(
+		cd 7e &&
+
+		mkdir z &&
+		echo b >z/b &&
+		echo c >z/c &&
+		mkdir x &&
+		echo d1 >x/d &&
+		git add z x &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv z y &&
+		git rm x/d &&
+		mkdir -p x/d &&
+		mkdir -p y/d &&
+		echo f >x/d/f &&
+		echo g >y/d/g &&
+		git add x/d/f y/d/g &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		git mv x/d z/ &&
+		rmdir x &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '7e-check: transitive rename in rename/delete AND dirs in the way' '
+	(
+		cd 7e &&
+
+		git checkout A^0 &&
+
+		test_must_fail git merge -s recursive B^0 >out &&
+		test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
+
+		git ls-files -s >out &&
+		test_line_count = 5 out &&
+		git ls-files -u >out &&
+		test_line_count = 1 out &&
+		git ls-files -o >out &&
+		test_line_count = 2 out &&
+
+		git rev-parse >actual \
+			:0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d &&
+		git rev-parse >expect \
+			 A:x/d/f  A:y/d/g  O:z/b  O:z/c  O:x/d &&
+		test_cmp expect actual &&
+
+		git hash-object y/d~B^0 >actual &&
+		git rev-parse O:x/d >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.17.0.290.ge988e9ce2a
Previous: Elijah NewrenNext: Elijah Newren
Message 5 of 78 in “Add directory rename detection to git”
  1. 00/36 Add directory rename detection to gitElijah Newren, Apr 19, 2018
  2. 36/36 merge-recursive: fix check for skipability of working tree updatesElijah Newren, Apr 19, 2018
  3. 29/36 merge-recursive: improve add_cacheinfo error handlingElijah Newren, Apr 19, 2018
  4. 19/36 merge-recursive: check for directory level conflictsElijah Newren, Apr 19, 2018
  5. 07/36 directory rename detection: more involved edge/corner testcasesElijah Newren, Apr 19, 2018
  6. 26/36 merge-recursive: fix remaining directory rename + dirty overwrite casesElijah Newren, Apr 19, 2018
  7. 14/36 merge-recursive: fix leaks of allocated renames and diff_filepairsElijah Newren, Apr 19, 2018
  8. 33/36 merge-recursive: fix was_tracked() to quit lying with some renamed pathsElijah Newren, Apr 19, 2018
  9. Martin ÅgrenApr 19, 2018
  10. Elijah NewrenApr 19, 2018
  11. SZEDER GáborApr 20, 2018
  12. Elijah NewrenApr 20, 2018
  13. 5/36 unpack_trees: fix memory corruption with split_index when src != dstElijah Newren, Apr 21, 2018
  14. Elijah NewrenApr 21, 2018
  15. Duy NguyenApr 22, 2018
  16. Elijah NewrenApr 23, 2018
  17. Duy NguyenApr 23, 2018
  18. Elijah NewrenApr 23, 2018
  19. unpack_trees: fix breakage when o->src_index != o->dst_indexElijah Newren, Apr 24, 2018
  20. Junio C HamanoApr 24, 2018
  21. Junio C HamanoApr 24, 2018
  22. unpack_trees: fix breakage when o->src_index != o->dst_indexElijah Newren, Apr 24, 2018
  23. Duy NguyenApr 29, 2018
  24. Johannes SchindelinApr 29, 2018
  25. Duy NguyenApr 30, 2018
  26. Duy NguyenApr 30, 2018
  27. Elijah NewrenApr 30, 2018
  28. Duy NguyenApr 30, 2018
  29. 24/36 merge-recursive: avoid clobbering untracked files with directory renamesElijah Newren, Apr 19, 2018
  30. 34/36 merge-recursive: fix remainder of was_dirty() to use original indexElijah Newren, Apr 19, 2018
  31. 31/36 merge-recursive: avoid triggering add_cacheinfo error with dirty modElijah Newren, Apr 19, 2018
  32. 20/36 merge-recursive: add computation of collisions due to dir rename & mergingElijah Newren, Apr 19, 2018
  33. 32/36 t6046: testcases checking whether updates can be skipped in a mergeElijah Newren, Apr 19, 2018
  34. SZEDER GáborApr 19, 2018
  35. Elijah NewrenApr 19, 2018
  36. 35/36 merge-recursive: make "Auto-merging" comment show for other mergesElijah Newren, Apr 19, 2018
  37. 23/36 merge-recursive: apply necessary modifications for directory renamesElijah Newren, Apr 19, 2018
  38. 30/36 merge-recursive: move more is_dirty handling to merge_contentElijah Newren, Apr 19, 2018
  39. 28/36 merge-recursive: avoid spurious rename/rename conflict from dir renamesElijah Newren, Apr 19, 2018
  40. 18/36 merge-recursive: add get_directory_renames()Elijah Newren, Apr 19, 2018
  41. SZEDER GáborMay 6, 2018
  42. fixup! merge-recursive: add get_directory_renames()Elijah Newren, May 7, 2018
  43. Johannes SchindelinOct 9, 2019
  44. Elijah NewrenOct 11, 2019
  45. Johannes SchindelinOct 12, 2019
  46. 25/36 merge-recursive: fix overwriting dirty files involved in renamesElijah Newren, Apr 19, 2018
  47. Martin ÅgrenApr 19, 2018
  48. Martin ÅgrenApr 19, 2018
  49. Elijah NewrenApr 19, 2018
  50. 22/36 merge-recursive: when comparing files, don't include treesElijah Newren, Apr 19, 2018
  51. 11/36 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Apr 19, 2018
  52. 06/36 directory rename detection: testcases checking which side did the renameElijah Newren, Apr 19, 2018
  53. 13/36 merge-recursive: introduce new functions to handle rename logicElijah Newren, Apr 19, 2018
  54. 10/36 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Apr 19, 2018
  55. 21/36 merge-recursive: check for file level conflicts then get new nameElijah Newren, Apr 19, 2018
  56. 17/36 merge-recursive: make a helper function for cleanup for handle_renamesElijah Newren, Apr 19, 2018
  57. 16/36 merge-recursive: split out code for determining diff_filepairsElijah Newren, Apr 19, 2018
  58. 15/36 merge-recursive: make !o->detect_rename codepath more obviousElijah Newren, Apr 19, 2018
  59. 27/36 directory rename detection: new testcases showcasing a pair of bugsElijah Newren, Apr 19, 2018
  60. 01/36 directory rename detection: basic testcasesElijah Newren, Apr 19, 2018
  61. 08/36 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Apr 19, 2018
  62. 04/36 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Apr 19, 2018
  63. 09/36 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Apr 19, 2018
  64. 02/36 directory rename detection: directory splitting testcasesElijah Newren, Apr 19, 2018
  65. 12/36 merge-recursive: move the get_renames() functionElijah Newren, Apr 19, 2018
  66. 03/36 directory rename detection: testcases to avoid taking detection too farElijah Newren, Apr 19, 2018
  67. 05/36 directory rename detection: files/directories in the way of some renamesElijah Newren, Apr 19, 2018
  68. Elijah NewrenApr 19, 2018
  69. Stefan BellerApr 19, 2018
  70. Derrick StoleeApr 19, 2018
  71. Elijah NewrenApr 19, 2018
  72. Junio C HamanoApr 20, 2018
  73. Elijah NewrenApr 23, 2018
  74. 1/2 fixup! merge-recursive: fix was_tracked() to quit lying with some renamed pathsElijah Newren, Apr 24, 2018
  75. 2/2 fixup! t6046: testcases checking whether updates can be skipped in a mergeElijah Newren, Apr 24, 2018
  76. Elijah NewrenApr 23, 2018
  77. Junio C HamanoApr 23, 2018
  78. Elijah NewrenApr 24, 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.