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

[PATCH v4 03/10] merge-recursive: increase marker length with depth of recursion

From
Elijah Newren <newren@gmail.com>
Date
Nov 2, 2018, 18:53 UTC
Message-ID
<20181102185317.31015-4-newren@gmail.com>
In-Reply-To
<20181102185317.31015-1-newren@gmail.com>

Later patches in this series will modify file collision conflict handling (e.g. from rename/add and rename/rename(2to1) conflicts) so that multiply nested conflict markers can arise even before considering conflicts in the virtual merge base. Including the virtual merge base will provide a way to get triply (or higher) nested conflict markers. This new way to get nested conflict markers will force the need for a more general mechanism to extend the length of conflict markers in order to differentiate between different nestings.

Along with this change to conflict marker length handling, we want to make sure that we don't regress handling for other types of conflicts with nested conflict markers. Add a more involved testcase using merge.conflictstyle=diff3, where not only does the virtual merge base contain conflicts, but its virtual merge base does as well (i.e. a case with triply nested conflict markers). While there are multiple reasonable ways to handle nested conflict markers in the virtual merge base for this type of situation, the easiest approach that dovetails well with the new needs for the file collision conflict handling is to require that the length of the conflict markers increase with each subsequent nesting.

Subsequent patches which change the rename/add and rename/rename(2to1) conflict handling will modify the extra_marker_size flag appropriately for their new needs.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 ll-merge.c                        |   4 +-
 ll-merge.h                        |   1 +
 merge-recursive.c                 |  25 +++--
 t/t6036-recursive-corner-cases.sh | 151 ++++++++++++++++++++++++++++++
 4 files changed, 172 insertions(+), 9 deletions(-)
diff --git a/ll-merge.c b/ll-merge.c
index 0e2800f7bb..aabc1b5c2e 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -384,7 +384,9 @@ int ll_merge(mmbuffer_t *result_buf,
 	if (opts->virtual_ancestor) {
 		if (driver->recursive)
 			driver = find_ll_merge_driver(driver->recursive);
-		marker_size += 2;
+	}
+	if (opts->extra_marker_size) {
+		marker_size += opts->extra_marker_size;
 	}
 	return driver->fn(driver, result_buf, path, ancestor, ancestor_label,
 			  ours, our_label, theirs, their_label,
diff --git a/ll-merge.h b/ll-merge.h
index b72b19921e..5b4e158502 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -11,6 +11,7 @@ struct ll_merge_options {
 	unsigned virtual_ancestor : 1;
 	unsigned variant : 2;	/* favor ours, favor theirs, or union merge */
 	unsigned renormalize : 1;
+	unsigned extra_marker_size;
 	long xdl_opts;
 };
 
diff --git a/merge-recursive.c b/merge-recursive.c
index 73b5710386..f795c92a69 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1058,7 +1058,8 @@ static int merge_3way(struct merge_options *o,
 		      const struct diff_filespec *a,
 		      const struct diff_filespec *b,
 		      const char *branch1,
-		      const char *branch2)
+		      const char *branch2,
+		      const int extra_marker_size)
 {
 	mmfile_t orig, src1, src2;
 	struct ll_merge_options ll_opts = {0};
@@ -1066,6 +1067,7 @@ static int merge_3way(struct merge_options *o,
 	int merge_status;
 
 	ll_opts.renormalize = o->renormalize;
+	ll_opts.extra_marker_size = extra_marker_size;
 	ll_opts.xdl_opts = o->xdl_opts;
 
 	if (o->call_depth) {
@@ -1300,6 +1302,7 @@ static int merge_mode_and_contents(struct merge_options *o,
 				   const char *filename,
 				   const char *branch1,
 				   const char *branch2,
+				   const int extra_marker_size,
 				   struct merge_file_info *result)
 {
 	if (o->branch1 != branch1) {
@@ -1310,7 +1313,8 @@ static int merge_mode_and_contents(struct merge_options *o,
 		 */
 		return merge_mode_and_contents(o, one, b, a,
 					       filename,
-					       branch2, branch1, result);
+					       branch2, branch1,
+					       extra_marker_size, result);
 	}
 
 	result->merge = 0;
@@ -1351,7 +1355,8 @@ static int merge_mode_and_contents(struct merge_options *o,
 			int ret = 0, merge_status;
 
 			merge_status = merge_3way(o, &result_buf, one, a, b,
-						  branch1, branch2);
+						  branch1, branch2,
+						  extra_marker_size);
 
 			if ((merge_status < 0) || !result_buf.ptr)
 				ret = err(o, _("Failed to execute internal merge"));
@@ -1640,7 +1645,8 @@ static int handle_rename_rename_1to2(struct merge_options *o,
 		struct diff_filespec other;
 		struct diff_filespec *add;
 		if (merge_mode_and_contents(o, one, a, b, one->path,
-					    ci->branch1, ci->branch2, &mfi))
+					    ci->branch1, ci->branch2,
+					    o->call_depth * 2, &mfi))
 			return -1;
 
 		/*
@@ -1707,9 +1713,11 @@ static int handle_rename_rename_2to1(struct merge_options *o,
 	path_side_1_desc = xstrfmt("version of %s from %s", path, a->path);
 	path_side_2_desc = xstrfmt("version of %s from %s", path, b->path);
 	if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc,
-				    o->branch1, o->branch2, &mfi_c1) ||
+				    o->branch1, o->branch2,
+				    o->call_depth * 2, &mfi_c1) ||
 	    merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc,
-				    o->branch1, o->branch2, &mfi_c2))
+				    o->branch1, o->branch2,
+				    o->call_depth * 2, &mfi_c2))
 		return -1;
 	free(path_side_1_desc);
 	free(path_side_2_desc);
@@ -2755,7 +2763,7 @@ static int process_renames(struct merge_options *o,
 
 					if (merge_mode_and_contents(o, &one, &a, &b, ren1_dst,
 								    branch1, branch2,
-								    &mfi)) {
+								    o->call_depth * 2, &mfi)) {
 						clean_merge = -1;
 						goto cleanup_and_return;
 					}
@@ -3052,7 +3060,8 @@ static int handle_content_merge(struct merge_options *o,
 			df_conflict_remains = 1;
 	}
 	if (merge_mode_and_contents(o, &one, &a, &b, path,
-				    o->branch1, o->branch2, &mfi))
+				    o->branch1, o->branch2,
+				    o->call_depth * 2, &mfi))
 		return -1;
 
 	/*
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index f229d7e47b..3091cbc06a 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -1596,4 +1596,155 @@ test_expect_failure 'check nested conflicts' '
 	)
 '
 
+# Setup:
+#          L1---L2---L3
+#         /  \ /  \ /  \
+#   master    X1   X2   ?
+#         \  / \  / \  /
+#          R1---R2---R3
+#
+# Where:
+#   master has one file named 'content'
+#   branches L1 and R1 both modify each of the two files in conflicting ways
+#
+#   L<n> (n>1) is a merge of R<n-1> into L<n-1>
+#   R<n> (n>1) is a merge of L<n-1> into R<n-1>
+#   L<n> and R<n> resolve the conflicts differently.
+#
+#   X<n> is an auto-generated merge-base used when merging L<n+1> and R<n+1>.
+#   By construction, X1 has conflict markers due to conflicting versions.
+#   X2, due to using merge.conflictstyle=3, has nested conflict markers.
+#
+#   So, merging R3 into L3 using merge.conflictstyle=3 should show the
+#   nested conflict markers from X2 in the base version -- that means we
+#   have three levels of conflict markers.  Can we distinguish all three?
+
+test_expect_success 'setup virtual merge base with nested conflicts' '
+	test_create_repo virtual_merge_base_has_nested_conflicts &&
+	(
+		cd virtual_merge_base_has_nested_conflicts &&
+
+		# Create some related files now
+		for i in $(test_seq 1 10)
+		do
+			echo Random base content line $i
+		done >content &&
+
+		# Setup original commit
+		git add content &&
+		test_tick && git commit -m initial &&
+
+		git branch L &&
+		git branch R &&
+
+		# Create L1
+		git checkout L &&
+		echo left >>content &&
+		git add content &&
+		test_tick && git commit -m "version L1 of content" &&
+		git tag L1 &&
+
+		# Create R1
+		git checkout R &&
+		echo right >>content &&
+		git add content &&
+		test_tick && git commit -m "verson R1 of content" &&
+		git tag R1 &&
+
+		# Create L2
+		git checkout L &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge R1 &&
+		git checkout L1 content &&
+		test_tick && git commit -m "version L2 of content" &&
+		git tag L2 &&
+
+		# Create R2
+		git checkout R &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge L1 &&
+		git checkout R1 content &&
+		test_tick && git commit -m "version R2 of content" &&
+		git tag R2 &&
+
+		# Create L3
+		git checkout L &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge R2 &&
+		git checkout L1 content &&
+		test_tick && git commit -m "version L3 of content" &&
+		git tag L3 &&
+
+		# Create R3
+		git checkout R &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge L2 &&
+		git checkout R1 content &&
+		test_tick && git commit -m "version R3 of content" &&
+		git tag R3
+	)
+'
+
+test_expect_success 'check virtual merge base with nested conflicts' '
+	(
+		cd virtual_merge_base_has_nested_conflicts &&
+
+		git checkout L3^0 &&
+
+		# Merge must fail; there is a conflict
+		test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R3^0 &&
+
+		# Make sure the index has the right number of entries
+		git ls-files -s >out &&
+		test_line_count = 3 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		# Ensure we have the correct number of untracked files
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
+
+		# Compare :[23]:content to expected values
+		git rev-parse L1:content R1:content >expect &&
+		git rev-parse :2:content :3:content >actual &&
+		test_cmp expect actual &&
+
+		# Imitate X1 merge base, except without long enough conflict
+		# markers because a subsequent sed will modify them.  Put
+		# result into vmb.
+		git cat-file -p master:content >base &&
+		git cat-file -p L:content >left &&
+		git cat-file -p R:content >right &&
+		cp left merged-once &&
+		test_must_fail git merge-file --diff3 \
+			-L "Temporary merge branch 1" \
+			-L "merged common ancestors"  \
+			-L "Temporary merge branch 2" \
+			merged-once \
+			base        \
+			right       &&
+		sed -e "s/^\([<|=>]\)/\1\1\1/" merged-once >vmb &&
+
+		# Imitate X2 merge base, overwriting vmb.  Note that we
+		# extend both sets of conflict markers to make them longer
+		# with the sed command.
+		cp left merged-twice &&
+		test_must_fail git merge-file --diff3 \
+			-L "Temporary merge branch 1" \
+			-L "merged common ancestors"  \
+			-L "Temporary merge branch 2" \
+			merged-twice \
+			vmb          \
+			right        &&
+		sed -e "s/^\([<|=>]\)/\1\1\1/" merged-twice >vmb &&
+
+		# Compare :1:content to expected value
+		git cat-file -p :1:content >actual &&
+		test_cmp vmb actual &&
+
+		# Determine expected content in final outer merge, compare to
+		# what the merge generated.
+		cp -f left expect &&
+		test_must_fail git merge-file --diff3                      \
+			-L "HEAD"  -L "merged common ancestors"  -L "R3^0" \
+			expect     vmb                           right     &&
+		test_cmp expect content
+	)
+'
+
 test_done

-- 
2.19.0.232.gd14c2061fc
Previous: Elijah NewrenNext: Elijah Newren
Message 36 of 52 in “Improve path collision conflict resolutions”
  1. 0/7 Improve path collision conflict resolutionsElijah Newren, Oct 14, 2018
  2. 1/7 Add testcases for consistency in file collision conflict handlingElijah Newren, Oct 14, 2018
  3. 4/7 merge-recursive: fix rename/add conflict handlingElijah Newren, Oct 14, 2018
  4. 3/7 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Oct 14, 2018
  5. 2/7 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Oct 14, 2018
  6. 6/7 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Oct 14, 2018
  7. 5/7 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Oct 14, 2018
  8. 7/7 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Oct 14, 2018
  9. 0/8 Improve path collision conflict resolutionsElijah Newren, Oct 19, 2018
  10. 3/8 merge-recursive: increase marker length with depth of recursionElijah Newren, Oct 19, 2018
  11. 2/8 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Oct 19, 2018
  12. Derrick StoleeOct 31, 2018
  13. Elijah NewrenNov 1, 2018
  14. 4/8 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Oct 19, 2018
  15. Derrick StoleeOct 31, 2018
  16. Derrick StoleeOct 31, 2018
  17. Elijah NewrenNov 1, 2018
  18. 1/8 Add testcases for consistency in file collision conflict handlingElijah Newren, Oct 19, 2018
  19. 6/8 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Oct 19, 2018
  20. 5/8 merge-recursive: fix rename/add conflict handlingElijah Newren, Oct 19, 2018
  21. 7/8 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Oct 19, 2018
  22. 8/8 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Oct 19, 2018
  23. Derrick StoleeOct 31, 2018
  24. Elijah NewrenNov 1, 2018
  25. Elijah NewrenNov 2, 2018
  26. Derrick StoleeNov 2, 2018
  27. 00/10 Improve path collision conflict resolutionsElijah Newren, Nov 2, 2018
  28. 07/10 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Nov 2, 2018
  29. 02/10 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Nov 2, 2018
  30. 06/10 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Nov 2, 2018
  31. 08/10 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Nov 2, 2018
  32. merge-recursive: combine error handlingDerrick Stolee, Nov 2, 2018
  33. 05/10 merge-recursive: fix rename/add conflict handlingElijah Newren, Nov 2, 2018
  34. 09/10 fixup! merge-recursive: fix rename/add conflict handlingElijah Newren, Nov 2, 2018
  35. 04/10 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Nov 2, 2018
  36. 03/10 merge-recursive: increase marker length with depth of recursionElijah Newren, Nov 2, 2018
  37. 01/10 Add testcases for consistency in file collision conflict handlingElijah Newren, Nov 2, 2018
  38. 10/10 fixup! merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Nov 2, 2018
  39. Derrick StoleeNov 2, 2018
  40. Elijah NewrenNov 2, 2018
  41. 00/10 Improve path collision conflict resolutionsElijah Newren, Nov 8, 2018
  42. 01/10 Add testcases for consistency in file collision conflict handlingElijah Newren, Nov 8, 2018
  43. 05/10 merge-recursive: fix rename/add conflict handlingElijah Newren, Nov 8, 2018
  44. 04/10 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Nov 8, 2018
  45. 07/10 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Nov 8, 2018
  46. 09/10 t6036, t6043: increase code coverage for file collision handlingElijah Newren, Nov 8, 2018
  47. 10/10 merge-recursive: combine error handlingElijah Newren, Nov 8, 2018
  48. Junio C HamanoNov 8, 2018
  49. 06/10 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Nov 8, 2018
  50. 02/10 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Nov 8, 2018
  51. 08/10 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Nov 8, 2018
  52. 03/10 merge-recursive: increase marker length with depth of recursionElijah Newren, Nov 8, 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.