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

[PATCH v4 32/34] directory rename detection: new testcases showcasing a pair of bugs

From
Elijah Newren <newren@gmail.com>
Date
Nov 29, 2017, 01:42 UTC
Message-ID
<20171129014237.32570-33-newren@gmail.com>
In-Reply-To
<20171129014237.32570-1-newren@gmail.com>

Add a testcase showing spurious rename/rename(1to2) conflicts occurring due to directory rename detection.

Also add a pair of testcases dealing with moving directory hierarchies around that were suggested by Stefan Beller as "food for thought" during his review of an earlier patch series, but which actually uncovered a bug. Round things out with a test that is a cross between the two testcases that showed existing bugs in order to make sure we aren't merely addressing problems in isolation but in general.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6043-merge-rename-directories.sh | 284 ++++++++++++++++++++++++++++++++++++
 1 file changed, 284 insertions(+)
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index f89cce4377..5a750d5511 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -153,6 +153,7 @@ test_expect_success '1b-check: Merge a directory with another' '
 # Testcase 1c, Transitive renaming
 #   (Related to testcases 3a and 6d -- when should a transitive rename apply?)
 #   (Related to testcases 9c and 9d -- can transitivity repeat?)
+#   (Related to testcase 12b -- joint-transitivity?)
 #   Commit O: z/{b,c},   x/d
 #   Commit A: y/{b,c},   x/d
 #   Commit B: z/{b,c,d}
@@ -2760,6 +2761,67 @@ test_expect_failure '9g-check: Renamed directory that only contained immediate s
 	)
 '
 
+# Testcase 9h, Avoid implicit rename if involved as source on other side
+#   (Extremely closely related to testcase 3a)
+#   Commit O: z/{b,c,d_1}
+#   Commit A: z/{b,c,d_2}
+#   Commit B: y/{b,c}, x/d_1
+#   Expected: y/{b,c}, x/d_2
+#   NOTE: If we applied the z/ -> y/ rename to z/d, then we'd end up with
+#         a rename/rename(1to2) conflict (z/d -> y/d vs. x/d)
+test_expect_success '9h-setup: Avoid dir rename on merely modified path' '
+	test_create_repo 9h &&
+	(
+		cd 9h &&
+
+		mkdir z &&
+		echo b >z/b &&
+		echo c >z/c &&
+		printf "1\n2\n3\n4\n5\n6\n7\n8\nd\n" >z/d &&
+		git add z &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		test_tick &&
+		echo more >>z/d &&
+		git add z/d &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		mkdir y &&
+		mkdir x &&
+		git mv z/b y/ &&
+		git mv z/c y/ &&
+		git mv z/d x/ &&
+		rmdir z &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '9h-check: Avoid dir rename on merely modified path' '
+	(
+		cd 9h &&
+
+		git checkout A^0 &&
+
+		git merge -s recursive B^0 &&
+
+		test 3 -eq $(git ls-files -s | wc -l) &&
+
+		git rev-parse >actual \
+			HEAD:y/b HEAD:y/c HEAD:x/d &&
+		git rev-parse >expect \
+			O:z/b O:z/c A:z/d &&
+		test_cmp expect actual
+	)
+'
+
 ###########################################################################
 # Rules suggested by section 9:
 #
@@ -3541,4 +3603,226 @@ test_expect_success '11f-check: Avoid deleting not-uptodate with dir rename/rena
 	)
 '
 
+###########################################################################
+# SECTION 12: Everything else
+#
+# Tests suggested by others.  Tests added after implementation completed
+# and submitted.  Grab bag.
+###########################################################################
+
+# Testcase 12a, Moving one directory hierarchy into another
+#   (Related to testcase 9a)
+#   Commit O: node1/{leaf1,leaf2}, node2/{leaf3,leaf4}
+#   Commit A: node1/{leaf1,leaf2,node2/{leaf3,leaf4}}
+#   Commit B: node1/{leaf1,leaf2,leaf5}, node2/{leaf3,leaf4,leaf6}
+#   Expected: node1/{leaf1,leaf2,leaf5,node2/{leaf3,leaf4,leaf6}}
+
+test_expect_success '12a-setup: Moving one directory hierarchy into another' '
+	test_create_repo 12a &&
+	(
+		cd 12a &&
+
+		mkdir -p node1 node2 &&
+		echo leaf1 >node1/leaf1 &&
+		echo leaf2 >node1/leaf2 &&
+		echo leaf3 >node2/leaf3 &&
+		echo leaf4 >node2/leaf4 &&
+		git add node1 node2 &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv node2/ node1/ &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		echo leaf5 >node1/leaf5 &&
+		echo leaf6 >node2/leaf6 &&
+		git add node1 node2 &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_success '12a-check: Moving one directory hierarchy into another' '
+	(
+		cd 12a &&
+
+		git checkout A^0 &&
+
+		git merge -s recursive B^0 &&
+
+		test 6 -eq $(git ls-files -s | wc -l) &&
+
+		git rev-parse >actual \
+			HEAD:node1/leaf1 HEAD:node1/leaf2 HEAD:node1/leaf5 HEAD:node1/node2/leaf3 HEAD:node1/node2/leaf4 HEAD:node1/node2/leaf6 &&
+		git rev-parse >expect \
+			O:node1/leaf1 O:node1/leaf2 B:node1/leaf5 O:node2/leaf3 O:node2/leaf4 B:node2/leaf6 &&
+		test_cmp expect actual
+	)
+'
+
+# Testcase 12b, Moving two directory hierarchies into each other
+#   (Related to testcases 1c and 12c)
+#   Commit O: node1/{leaf1, leaf2}, node2/{leaf3, leaf4}
+#   Commit A: node1/{leaf1, leaf2, node2/{leaf3, leaf4}}
+#   Commit B: node2/{leaf3, leaf4, node1/{leaf1, leaf2}}
+#   Expected: node1/node2/node1/{leaf1, leaf2},
+#             node2/node1/node2/{leaf3, leaf4}
+#   NOTE: Without directory renames, we would expect
+#                   node2/node1/{leaf1, leaf2},
+#                   node1/node2/{leaf3, leaf4}
+#         with directory rename detection, we note that
+#             commit A renames node2/ -> node1/node2/
+#             commit B renames node1/ -> node2/node1/
+#         therefore, applying those directory renames to the initial result
+#         (making all four paths experience a transitive renaming), yields
+#         the expected result.
+#
+#         You may ask, is it weird to have two directories rename each other?
+#         To which, I can do no more than shrug my shoulders and say that
+#         even simple rules give weird results when given weird inputs.
+
+test_expect_success '12b-setup: Moving one directory hierarchy into another' '
+	test_create_repo 12b &&
+	(
+		cd 12b &&
+
+		mkdir -p node1 node2 &&
+		echo leaf1 >node1/leaf1 &&
+		echo leaf2 >node1/leaf2 &&
+		echo leaf3 >node2/leaf3 &&
+		echo leaf4 >node2/leaf4 &&
+		git add node1 node2 &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv node2/ node1/ &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		git mv node1/ node2/ &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '12b-check: Moving one directory hierarchy into another' '
+	(
+		cd 12b &&
+
+		git checkout A^0 &&
+
+		git merge -s recursive B^0 &&
+
+		test 4 -eq $(git ls-files -s | wc -l) &&
+
+		git rev-parse >actual \
+			HEAD:node1/node2/node1/leaf1 \
+			HEAD:node1/node2/node1/leaf2 \
+			HEAD:node2/node1/node2/leaf3 \
+			HEAD:node2/node1/node2/leaf4 &&
+		git rev-parse >expect \
+			O:node1/leaf1 O:node1/leaf2 \
+			O:node2/leaf3 O:node2/leaf4 &&
+		test_cmp expect actual
+	)
+'
+
+# Testcase 12c, Moving two directory hierarchies into each other w/ content merge
+#   (Related to testcase 12b)
+#   Commit O: node1/{       leaf1_1, leaf2_1}, node2/{leaf3_1, leaf4_1}
+#   Commit A: node1/{       leaf1_2, leaf2_2,  node2/{leaf3_2, leaf4_2}}
+#   Commit B: node2/{node1/{leaf1_3, leaf2_3},        leaf3_3, leaf4_3}
+#   Expected: Content merge conflicts for each of:
+#               node1/node2/node1/{leaf1, leaf2},
+#               node2/node1/node2/{leaf3, leaf4}
+#   NOTE: This is *exactly* like 12c, except that every path is modified on
+#         each side of the merge.
+
+test_expect_success '12c-setup: Moving one directory hierarchy into another w/ content merge' '
+	test_create_repo 12c &&
+	(
+		cd 12c &&
+
+		mkdir -p node1 node2 &&
+		printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf1\n" >node1/leaf1 &&
+		printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf2\n" >node1/leaf2 &&
+		printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf3\n" >node2/leaf3 &&
+		printf "1\n2\n3\n4\n5\n6\n7\n8\nleaf4\n" >node2/leaf4 &&
+		git add node1 node2 &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		git mv node2/ node1/ &&
+		for i in `git ls-files`; do echo side A >>$i; done &&
+		git add -u &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		git mv node1/ node2/ &&
+		for i in `git ls-files`; do echo side B >>$i; done &&
+		git add -u &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_failure '12c-check: Moving one directory hierarchy into another w/ content merge' '
+	(
+		cd 12c &&
+
+		git checkout A^0 &&
+
+		test_must_fail git merge -s recursive B^0 &&
+
+		test 12 -eq $(git ls-files -u | wc -l) &&
+
+		git rev-parse >actual \
+			:1:node1/node2/node1/leaf1 \
+			:1:node1/node2/node1/leaf2 \
+			:1:node2/node1/node2/leaf3 \
+			:1:node2/node1/node2/leaf4 \
+			:2:node1/node2/node1/leaf1 \
+			:2:node1/node2/node1/leaf2 \
+			:2:node2/node1/node2/leaf3 \
+			:2:node2/node1/node2/leaf4 \
+			:3:node1/node2/node1/leaf1 \
+			:3:node1/node2/node1/leaf2 \
+			:3:node2/node1/node2/leaf3 \
+			:3:node2/node1/node2/leaf4 &&
+		git rev-parse >expect \
+			O:node1/leaf1 \
+			O:node1/leaf2 \
+			O:node2/leaf3 \
+			O:node2/leaf4 \
+			A:node1/leaf1 \
+			A:node1/leaf2 \
+			A:node1/node2/leaf3 \
+			A:node1/node2/leaf4 \
+			B:node2/node1/leaf1 \
+			B:node2/node1/leaf2 \
+			B:node2/leaf3 \
+			B:node2/leaf4 &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
2.15.0.408.g850bc54b15
Previous: Elijah NewrenNext: Elijah Newren
Message 25 of 40 in “Add directory rename detection to git”
  1. 00/34 Add directory rename detection to gitElijah Newren, Nov 29, 2017
  2. 15/34 merge-recursive: move the get_renames() functionElijah Newren, Nov 29, 2017
  3. 24/34 merge-recursive: add a new hashmap for storing file collisionsElijah Newren, Nov 29, 2017
  4. 13/34 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Nov 29, 2017
  5. 05/34 directory rename detection: directory splitting testcasesElijah Newren, Nov 29, 2017
  6. 12/34 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Nov 29, 2017
  7. 19/34 merge-recursive: split out code for determining diff_filepairsElijah Newren, Nov 29, 2017
  8. 22/34 merge-recursive: add get_directory_renames()Elijah Newren, Nov 29, 2017
  9. 02/34 merge-recursive: fix logic ordering issueElijah Newren, Nov 29, 2017
  10. 18/34 merge-recursive: make !o->detect_rename codepath more obviousElijah Newren, Nov 29, 2017
  11. 17/34 merge-recursive: fix leaks of allocated renames and diff_filepairsElijah Newren, Nov 29, 2017
  12. 31/34 merge-recursive: fix remaining directory rename + dirty overwrite casesElijah Newren, Nov 29, 2017
  13. 04/34 directory rename detection: basic testcasesElijah Newren, Nov 29, 2017
  14. 26/34 merge-recursive: check for file level conflicts then get new nameElijah Newren, Nov 29, 2017
  15. 16/34 merge-recursive: introduce new functions to handle rename logicElijah Newren, Nov 29, 2017
  16. 28/34 merge-recursive: apply necessary modifications for directory renamesElijah Newren, Nov 29, 2017
  17. 23/34 merge-recursive: check for directory level conflictsElijah Newren, Nov 29, 2017
  18. 06/34 directory rename detection: testcases to avoid taking detection too farElijah Newren, Nov 29, 2017
  19. 11/34 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Nov 29, 2017
  20. 03/34 merge-recursive: add explanation for src_entry and dst_entryElijah Newren, Nov 29, 2017
  21. 14/34 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Nov 29, 2017
  22. 25/34 merge-recursive: add computation of collisions due to dir rename & mergingElijah Newren, Nov 29, 2017
  23. 33/34 merge-recursive: avoid spurious rename/rename conflict from dir renamesElijah Newren, Nov 29, 2017
  24. 27/34 merge-recursive: when comparing files, don't include treesElijah Newren, Nov 29, 2017
  25. 32/34 directory rename detection: new testcases showcasing a pair of bugsElijah Newren, Nov 29, 2017
  26. 21/34 merge-recursive: make a helper function for cleanup for handle_renamesElijah Newren, Nov 29, 2017
  27. 10/34 directory rename detection: more involved edge/corner testcasesElijah Newren, Nov 29, 2017
  28. 20/34 merge-recursive: add a new hashmap for storing directory renamesElijah Newren, Nov 29, 2017
  29. 09/34 directory rename detection: testcases checking which side did the renameElijah Newren, Nov 29, 2017
  30. 30/34 merge-recursive: fix overwriting dirty files involved in renamesElijah Newren, Nov 29, 2017
  31. 29/34 merge-recursive: avoid clobbering untracked files with directory renamesElijah Newren, Nov 29, 2017
  32. 08/34 directory rename detection: files/directories in the way of some renamesElijah Newren, Nov 29, 2017
  33. 34/34 merge-recursive: ensure we write updates for directory-renamed fileElijah Newren, Nov 29, 2017
  34. 07/34 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Nov 29, 2017
  35. 01/34 Tighten and correct a few testcases for merging and cherry-pickingElijah Newren, Nov 29, 2017
  36. Junio C HamanoDec 13, 2017
  37. Junio C HamanoDec 13, 2017
  38. Elijah NewrenDec 13, 2017
  39. Ramsay JonesDec 13, 2017
  40. Junio C HamanoDec 13, 2017

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.