merge-ort: ensure we consult df_conflict and path_conflicts

newren · gitster · commit a492d5331cb9 · 2021-06-30T14:40:10.000-07:00
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 &lt;andersk@mit.edu&gt;
Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/merge-ort.c b/merge-ort.c
@@ -3002,7 +3002,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;
@@ -3014,6 +3014,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);
@@ -3186,6 +3188,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)) {
 			/*
@@ -3212,6 +3215,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
@@ -5000,7 +5000,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 &&
@@ -5058,7 +5058,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 &&
@@ -5116,7 +5116,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 &&

Original file line number	Diff line number	Diff line change
`@@ -5000,7 +5000,7 @@ test_setup_12i () {`
`5000`	`5000`	`)`
`5001`	`5001`	`}`
`5002`	`5002`
`5003`		`-test_expect_merge_algorithm failure failure '12i: Directory rename causes rename-to-self' '`
	`5003`	`+test_expect_merge_algorithm failure success '12i: Directory rename causes rename-to-self' '`
`5004`	`5004`	`test_setup_12i &&`
`5005`	`5005`	`(`
`5006`	`5006`	`cd 12i &&`
`@@ -5058,7 +5058,7 @@ test_setup_12j () {`
`5058`	`5058`	`)`
`5059`	`5059`	`}`
`5060`	`5060`
`5061`		`-test_expect_merge_algorithm failure failure '12j: Directory rename to root causes rename-to-self' '`
	`5061`	`+test_expect_merge_algorithm failure success '12j: Directory rename to root causes rename-to-self' '`
`5062`	`5062`	`test_setup_12j &&`
`5063`	`5063`	`(`
`5064`	`5064`	`cd 12j &&`
`@@ -5116,7 +5116,7 @@ test_setup_12k () {`
`5116`	`5116`	`)`
`5117`	`5117`	`}`
`5118`	`5118`
`5119`		`-test_expect_merge_algorithm failure failure '12k: Directory rename with sibling causes rename-to-self' '`
	`5119`	`+test_expect_merge_algorithm failure success '12k: Directory rename with sibling causes rename-to-self' '`
`5120`	`5120`	`test_setup_12k &&`
`5121`	`5121`	`(`
`5122`	`5122`	`cd 12k &&`