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

[PATCH v2 2/3] merge-ort: ensure we consult df_conflict and path_conflicts

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Jun 30, 2021, 17:29 UTC
Message-ID
<6e7ef18bf53a579e360e4970a6fbcc0a68a821f6.1625074200.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1039.v2.git.git.1625074200.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>

Path conflicts (typically rename path conflicts, e.g. rename/rename(1to2) or rename/add/delete), and directory/file conflicts should obviously result in files not being marked as clean in the merge. We had a codepath where we missed consulting the path_conflict and df_conflict flags, based on match_mask. Granted, it requires an unusual setup to trigger this codepath (directory rename causing rename-to-self is the only case I can think of), but we still need to handle it. To make it clear that we have audited the other codepaths that do not explicitly mention these flags, add some assertions that the flags are not set.

Reported-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c                         | 6 +++++-
 t/t6423-merge-rename-directories.sh | 6 +++---
 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index b954f7184a5..373dbac5079 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -3237,7 +3237,7 @@ static void process_entry(struct merge_options *opt,
 	 *       above.
 	 */
 	if (ci->match_mask) {
-		ci->merged.clean = 1;
+		ci->merged.clean = !ci->df_conflict && !ci->path_conflict;
 		if (ci->match_mask == 6) {
 			/* stages[1] == stages[2] */
 			ci->merged.result.mode = ci->stages[1].mode;
@@ -3249,6 +3249,8 @@ static void process_entry(struct merge_options *opt,
 
 			ci->merged.result.mode = ci->stages[side].mode;
 			ci->merged.is_null = !ci->merged.result.mode;
+			if (ci->merged.is_null)
+				ci->merged.clean = 1;
 			oidcpy(&ci->merged.result.oid, &ci->stages[side].oid);
 
 			assert(othermask == 2 || othermask == 4);
@@ -3421,6 +3423,7 @@ static void process_entry(struct merge_options *opt,
 				   path)) {
 			ci->merged.is_null = 1;
 			ci->merged.clean = 1;
+			assert(!ci->df_conflict && !ci->path_conflict);
 		} else if (ci->path_conflict &&
 			   oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {
 			/*
@@ -3447,6 +3450,7 @@ static void process_entry(struct merge_options *opt,
 		ci->merged.is_null = 1;
 		ci->merged.result.mode = 0;
 		oidcpy(&ci->merged.result.oid, null_oid());
+		assert(!ci->df_conflict);
 		ci->merged.clean = !ci->path_conflict;
 	}
 
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 89fe6778b94..c3968dc3642 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -5058,7 +5058,7 @@ test_setup_12i () {
 	)
 }
 
-test_expect_merge_algorithm failure failure '12i: Directory rename causes rename-to-self' '
+test_expect_merge_algorithm failure success '12i: Directory rename causes rename-to-self' '
 	test_setup_12i &&
 	(
 		cd 12i &&
@@ -5116,7 +5116,7 @@ test_setup_12j () {
 	)
 }
 
-test_expect_merge_algorithm failure failure '12j: Directory rename to root causes rename-to-self' '
+test_expect_merge_algorithm failure success '12j: Directory rename to root causes rename-to-self' '
 	test_setup_12j &&
 	(
 		cd 12j &&
@@ -5174,7 +5174,7 @@ test_setup_12k () {
 	)
 }
 
-test_expect_merge_algorithm failure failure '12k: Directory rename with sibling causes rename-to-self' '
+test_expect_merge_algorithm failure success '12k: Directory rename with sibling causes rename-to-self' '
 	test_setup_12k &&
 	(
 		cd 12k &&
-- 
gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 11 of 12 in “Fix bugs from interesting renaming pairs: one side renames A/file -> B/file, the other B/ -> A/”
  1. 0/3 Fix bugs from interesting renaming pairs: one side renames A/file -> B/file, the other B/ -> A/Elijah Newren via GitGitGadget, Jun 26, 2021
  2. 1/3 t6423: test directory renames causing rename-to-selfElijah Newren via GitGitGadget, Jun 26, 2021
  3. Derrick StoleeJun 29, 2021
  4. Elijah NewrenJun 30, 2021
  5. 2/3 merge-ort: ensure we consult df_conflict and path_conflictsElijah Newren via GitGitGadget, Jun 26, 2021
  6. 3/3 merge-recursive: handle rename-to-self caseElijah Newren via GitGitGadget, Jun 26, 2021
  7. Junio C HamanoJun 29, 2021
  8. Derrick StoleeJun 29, 2021
  9. 0/3 Fix bugs from interesting renaming pairs: one side renames A/file -> B/file, the other B/ -> A/Elijah Newren via GitGitGadget, Jun 30, 2021
  10. 1/3 t6423: test directory renames causing rename-to-selfElijah Newren via GitGitGadget, Jun 30, 2021
  11. 2/3 merge-ort: ensure we consult df_conflict and path_conflictsElijah Newren via GitGitGadget, Jun 30, 2021
  12. 3/3 merge-recursive: handle rename-to-self caseElijah Newren via GitGitGadget, Jun 30, 2021

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.