Merge branch 'jc/read-tree-df'

* jc/read-tree-df:
  t3030: merge-recursive backend test.
  merge-recursive: handle D/F conflict case more carefully.
  merge-recursive: do not barf on "to be removed" entries.
  Treat D/F conflict entry more carefully in unpack-trees.c::threeway_merge()
  t1000: fix case table.

4 files changed, 587 insertions, 24 deletions

diff --git a/merge-recursive.c b/merge-recursive.c
index 3096594b3..595b0226a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -596,9 +596,31 @@ static void update_file_flags(const unsigned char *sha,
 
 		if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
 			int fd;
-			if (mkdir_p(path, 0777))
-				die("failed to create path %s: %s", path, strerror(errno));
-			unlink(path);
+			int status;
+			const char *msg = "failed to create path '%s'%s";
+
+			status = mkdir_p(path, 0777);
+			if (status) {
+				if (status == -3) {
+					/* something else exists */
+					error(msg, path, ": perhaps a D/F conflict?");
+					update_wd = 0;
+					goto update_index;
+				}
+				die(msg, path, "");
+			}
+			if (unlink(path)) {
+				if (errno == EISDIR) {
+					/* something else exists */
+					error(msg, path, ": perhaps a D/F conflict?");
+					update_wd = 0;
+					goto update_index;
+				}
+				if (errno != ENOENT)
+					die("failed to unlink %s "
+					    "in preparation to update: %s",
+					    path, strerror(errno));
+			}
 			if (mode & 0100)
 				mode = 0777;
 			else
@@ -620,6 +642,7 @@ static void update_file_flags(const unsigned char *sha,
 			die("do not know what to do with %06o %s '%s'",
 			    mode, sha1_to_hex(sha), path);
 	}
+ update_index:
 	if (update_cache)
 		add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
 }
@@ -1018,9 +1041,9 @@ static int process_renames(struct path_list *a_renames,
 	return clean_merge;
 }
 
-static unsigned char *has_sha(const unsigned char *sha)
+static unsigned char *stage_sha(const unsigned char *sha, unsigned mode)
 {
-	return is_null_sha1(sha) ? NULL: (unsigned char *)sha;
+	return (is_null_sha1(sha) || mode == 0) ? NULL: (unsigned char *)sha;
 }
 
 /* Per entry merge function */
@@ -1033,12 +1056,12 @@ static int process_entry(const char *path, struct stage_data *entry,
 	print_index_entry("\tpath: ", entry);
 	*/
 	int clean_merge = 1;
-	unsigned char *o_sha = has_sha(entry->stages[1].sha);
-	unsigned char *a_sha = has_sha(entry->stages[2].sha);
-	unsigned char *b_sha = has_sha(entry->stages[3].sha);
 	unsigned o_mode = entry->stages[1].mode;
 	unsigned a_mode = entry->stages[2].mode;
 	unsigned b_mode = entry->stages[3].mode;
+	unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
+	unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
+	unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
 
 	if (o_sha && (!a_sha || !b_sha)) {
 		/* Case A: Deleted in one */
@@ -1139,6 +1162,12 @@ static int process_entry(const char *path, struct stage_data *entry,
 				update_file_flags(mfi.sha, mfi.mode, path,
 					      0 /* update_cache */, 1 /* update_working_directory */);
 		}
+	} else if (!o_sha && !a_sha && !b_sha) {
+		/*
+		 * this entry was deleted altogether. a_mode == 0 means
+		 * we had that path and want to actively remove it.
+		 */
+		remove_file(1, path, !a_mode);
 	} else
 		die("Fatal merge failure, shouldn't happen.");
 
diff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh
index e26a36cf0..de4e5eb61 100755
--- a/t/t1000-read-tree-m-3way.sh
+++ b/t/t1000-read-tree-m-3way.sh
@@ -184,7 +184,7 @@ checked.
   9  exists  O!=A    missing   no merge    must match A and be
                                            up-to-date, if exists.
  ------------------------------------------------------------------
- 10  exists  O==A    missing   remove      ditto
+ 10  exists  O==A    missing   no merge    must match A
  ------------------------------------------------------------------
  11  exists  O!=A    O!=B      no merge    must match A and be
                      A!=B                  up-to-date, if exists.
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
new file mode 100755
index 000000000..aef92b9b9
--- /dev/null
+++ b/t/t3030-merge-recursive.sh
@@ -0,0 +1,528 @@
+#!/bin/sh
+
+test_description='merge-recursive backend test'
+
+. ./test-lib.sh
+
+test_expect_success 'setup 1' '
+
+	echo hello >a &&
+	o0=$(git hash-object a) &&
+	cp a b &&
+	cp a A &&
+	mkdir d &&
+	cp a d/e &&
+
+	test_tick &&
+	git add a b A d/e &&
+	git commit -m initial &&
+	c0=$(git rev-parse --verify HEAD) &&
+	git branch side &&
+	git branch df-1 &&
+	git branch df-2 &&
+	git branch df-3 &&
+	git branch remove &&
+
+	echo hello >>a &&
+	cp a d/e &&
+	o1=$(git hash-object a) &&
+
+	git add a d/e &&
+
+	test_tick &&
+	git commit -m "master modifies a and d/e" &&
+	c1=$(git rev-parse --verify HEAD) &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o1	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o1	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o1 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+'
+
+test_expect_success 'setup 2' '
+
+	rm -rf [Aabd] &&
+	git checkout side &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual &&
+
+	echo goodbye >>a &&
+	o2=$(git hash-object a) &&
+
+	git add a &&
+
+	test_tick &&
+	git commit -m "side modifies a" &&
+	c2=$(git rev-parse --verify HEAD) &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o2	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o2 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+'
+
+test_expect_success 'setup 3' '
+
+	rm -rf [Aabd] &&
+	git checkout df-1 &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual &&
+
+	rm -f b && mkdir b && echo df-1 >b/c && git add b/c &&
+	o3=$(git hash-object b/c) &&
+
+	test_tick &&
+	git commit -m "df-1 makes b/c" &&
+	c3=$(git rev-parse --verify HEAD) &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o3	b/c"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o3 0	b/c"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+'
+
+test_expect_success 'setup 4' '
+
+	rm -rf [Aabd] &&
+	git checkout df-2 &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual &&
+
+	rm -f a && mkdir a && echo df-2 >a/c && git add a/c &&
+	o4=$(git hash-object a/c) &&
+
+	test_tick &&
+	git commit -m "df-2 makes a/c" &&
+	c4=$(git rev-parse --verify HEAD) &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o4	a/c"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o4 0	a/c"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+'
+
+test_expect_success 'setup 5' '
+
+	rm -rf [Aabd] &&
+	git checkout remove &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual &&
+
+	rm -f b &&
+	echo remove-conflict >a &&
+
+	git add a &&
+	git rm b &&
+	o5=$(git hash-object a) &&
+
+	test_tick &&
+	git commit -m "remove removes b and modifies a" &&
+	c5=$(git rev-parse --verify HEAD) &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o5	a"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o5 0	a"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'setup 6' '
+
+	rm -rf [Aabd] &&
+	git checkout df-3 &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o0	d/e"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o0 0	d/e"
+	) >expected &&
+	git diff -u expected actual &&
+
+	rm -fr d && echo df-3 >d && git add d &&
+	o6=$(git hash-object d) &&
+
+	test_tick &&
+	git commit -m "df-3 makes d" &&
+	c6=$(git rev-parse --verify HEAD) &&
+	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	(
+		echo "100644 blob $o0	A"
+		echo "100644 blob $o0	a"
+		echo "100644 blob $o0	b"
+		echo "100644 blob $o6	d"
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o6 0	d"
+	) >expected &&
+	git diff -u expected actual
+'
+
+test_expect_success 'merge-recursive simple' '
+
+	rm -fr [Aabd] &&
+	git checkout -f "$c2" &&
+
+	git-merge-recursive "$c0" -- "$c2" "$c1"
+	status=$?
+	case "$status" in
+	1)
+		: happy
+		;;
+	*)
+		echo >&2 "why status $status!!!"
+		false
+		;;
+	esac
+'
+
+test_expect_success 'merge-recursive result' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 1	a"
+		echo "100644 $o2 2	a"
+		echo "100644 $o1 3	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'merge-recursive remove conflict' '
+
+	rm -fr [Aabd] &&
+	git checkout -f "$c1" &&
+
+	git-merge-recursive "$c0" -- "$c1" "$c5"
+	status=$?
+	case "$status" in
+	1)
+		: happy
+		;;
+	*)
+		echo >&2 "why status $status!!!"
+		false
+		;;
+	esac
+'
+
+test_expect_success 'merge-recursive remove conflict' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 1	a"
+		echo "100644 $o1 2	a"
+		echo "100644 $o5 3	a"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'merge-recursive d/f simple' '
+	rm -fr [Aabd] &&
+	git reset --hard &&
+	git checkout -f "$c1" &&
+
+	git-merge-recursive "$c0" -- "$c1" "$c3"
+'
+
+test_expect_success 'merge-recursive result' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o1 0	a"
+		echo "100644 $o3 0	b/c"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'merge-recursive d/f conflict' '
+
+	rm -fr [Aabd] &&
+	git reset --hard &&
+	git checkout -f "$c1" &&
+
+	git-merge-recursive "$c0" -- "$c1" "$c4"
+	status=$?
+	case "$status" in
+	1)
+		: happy
+		;;
+	*)
+		echo >&2 "why status $status!!!"
+		false
+		;;
+	esac
+'
+
+test_expect_success 'merge-recursive d/f conflict result' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 1	a"
+		echo "100644 $o1 2	a"
+		echo "100644 $o4 0	a/c"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'merge-recursive d/f conflict the other way' '
+
+	rm -fr [Aabd] &&
+	git reset --hard &&
+	git checkout -f "$c4" &&
+
+	git-merge-recursive "$c0" -- "$c4" "$c1"
+	status=$?
+	case "$status" in
+	1)
+		: happy
+		;;
+	*)
+		echo >&2 "why status $status!!!"
+		false
+		;;
+	esac
+'
+
+test_expect_success 'merge-recursive d/f conflict result the other way' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 1	a"
+		echo "100644 $o1 3	a"
+		echo "100644 $o4 0	a/c"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'merge-recursive d/f conflict' '
+
+	rm -fr [Aabd] &&
+	git reset --hard &&
+	git checkout -f "$c1" &&
+
+	git-merge-recursive "$c0" -- "$c1" "$c6"
+	status=$?
+	case "$status" in
+	1)
+		: happy
+		;;
+	*)
+		echo >&2 "why status $status!!!"
+		false
+		;;
+	esac
+'
+
+test_expect_success 'merge-recursive d/f conflict result' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o1 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o6 3	d"
+		echo "100644 $o0 1	d/e"
+		echo "100644 $o1 2	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'merge-recursive d/f conflict' '
+
+	rm -fr [Aabd] &&
+	git reset --hard &&
+	git checkout -f "$c6" &&
+
+	git-merge-recursive "$c0" -- "$c6" "$c1"
+	status=$?
+	case "$status" in
+	1)
+		: happy
+		;;
+	*)
+		echo >&2 "why status $status!!!"
+		false
+		;;
+	esac
+'
+
+test_expect_success 'merge-recursive d/f conflict result' '
+
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o1 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o6 2	d"
+		echo "100644 $o0 1	d/e"
+		echo "100644 $o1 3	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_expect_success 'reset and 3-way merge' '
+
+	git reset --hard "$c2" &&
+	git read-tree -m "$c0" "$c2" "$c1"
+
+'
+
+test_expect_success 'reset and bind merge' '
+
+	git reset --hard master &&
+	git read-tree --prefix=M/ master &&
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	M/A"
+		echo "100644 $o1 0	M/a"
+		echo "100644 $o0 0	M/b"
+		echo "100644 $o1 0	M/d/e"
+		echo "100644 $o1 0	a"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual &&
+
+	git read-tree --prefix=a1/ master &&
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	M/A"
+		echo "100644 $o1 0	M/a"
+		echo "100644 $o0 0	M/b"
+		echo "100644 $o1 0	M/d/e"
+		echo "100644 $o1 0	a"
+		echo "100644 $o0 0	a1/A"
+		echo "100644 $o1 0	a1/a"
+		echo "100644 $o0 0	a1/b"
+		echo "100644 $o1 0	a1/d/e"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+	) >expected &&
+	git diff -u expected actual
+
+	git read-tree --prefix=z/ master &&
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o0 0	A"
+		echo "100644 $o0 0	M/A"
+		echo "100644 $o1 0	M/a"
+		echo "100644 $o0 0	M/b"
+		echo "100644 $o1 0	M/d/e"
+		echo "100644 $o1 0	a"
+		echo "100644 $o0 0	a1/A"
+		echo "100644 $o1 0	a1/a"
+		echo "100644 $o0 0	a1/b"
+		echo "100644 $o1 0	a1/d/e"
+		echo "100644 $o0 0	b"
+		echo "100644 $o1 0	d/e"
+		echo "100644 $o0 0	z/A"
+		echo "100644 $o1 0	z/a"
+		echo "100644 $o0 0	z/b"
+		echo "100644 $o1 0	z/d/e"
+	) >expected &&
+	git diff -u expected actual
+
+'
+
+test_done
+
diff --git a/unpack-trees.c b/unpack-trees.c
index a0b676903..513948135 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -665,7 +665,6 @@ int threeway_merge(struct cache_entry **stages,
 	int count;
 	int head_match = 0;
 	int remote_match = 0;
-	const char *path = NULL;
 
 	int df_conflict_head = 0;
 	int df_conflict_remote = 0;
@@ -675,13 +674,10 @@ int threeway_merge(struct cache_entry **stages,
 	int i;
 
 	for (i = 1; i < o->head_idx; i++) {
-		if (!stages[i])
+		if (!stages[i] || stages[i] == o->df_conflict_entry)
 			any_anc_missing = 1;
-		else {
-			if (!path)
-				path = stages[i]->name;
+		else
 			no_anc_exists = 0;
-		}
 	}
 
 	index = stages[0];
@@ -697,13 +693,6 @@ int threeway_merge(struct cache_entry **stages,
 		remote = NULL;
 	}
 
-	if (!path && index)
-		path = index->name;
-	if (!path && head)
-		path = head->name;
-	if (!path && remote)
-		path = remote->name;
-
 	/* First, if there's a #16 situation, note that to prevent #13
 	 * and #14.
 	 */
@@ -755,6 +744,23 @@ int threeway_merge(struct cache_entry **stages,
 	if (o->aggressive) {
 		int head_deleted = !head && !df_conflict_head;
 		int remote_deleted = !remote && !df_conflict_remote;
+		const char *path = NULL;
+
+		if (index)
+			path = index->name;
+		else if (head)
+			path = head->name;
+		else if (remote)
+			path = remote->name;
+		else {
+			for (i = 1; i < o->head_idx; i++) {
+				if (stages[i] && stages[i] != o->df_conflict_entry) {
+					path = stages[i]->name;
+					break;
+				}
+			}
+		}
+
 		/*
 		 * Deleted in both.
 		 * Deleted in one and unchanged in the other.
@@ -786,11 +792,11 @@ int threeway_merge(struct cache_entry **stages,
 
 	o->nontrivial_merge = 1;
 
-	/* #2, #3, #4, #6, #7, #9, #11. */
+	/* #2, #3, #4, #6, #7, #9, #10, #11. */
 	count = 0;
 	if (!head_match || !remote_match) {
 		for (i = 1; i < o->head_idx; i++) {
-			if (stages[i]) {
+			if (stages[i] && stages[i] != o->df_conflict_entry) {
 				keep_entry(stages[i], o);
 				count++;
 				break;