[PATCH v1 0/2] Be nicer to the user on tracked/untracked merge conflicts

[Jonathan <git.jonathan.bressat@gmail.com>]

Sorry for being a bit slow to answer.

Junio C Hamano <gitster@pobox.com> wrote:
> 	if (result) {
> 		...
> -	}
> +	} else if (!ie_modified(...)) {
> +		return 0;
> +	}
> 	return add_rejected_path(...);
> 
> is what you want, perhaps?

Yes, but that made us ask if it can be a good idea to extend our patch
to overwrite all unstaged file. However if we want to overwrite all
unstaged file even tracked one just this may not be enough:

if (result) {
	...
}

+if (!ie_modified(...)) {
+	return 0;
+}

Because with this merge still fail for unstaged file that has the same
content, because unstaged file are not exactly treated the same way.

Our patch broke some test in t6436-merge-overwrite.sh so we think that
we need to modify those tests to make them follow the patch.

Thanks for your reviews

Jonathan (2):
  t7615: test how merge behave when there is untracked file
  merge with untracked file that are the same without failure

 t/t7615-merge-untracked.sh | 63 ++++++++++++++++++++++++++++++++++++++
 unpack-trees.c             |  5 ++-
 2 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100755 t/t7615-merge-untracked.sh

Interdiff vs v0 :
diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
index 71a34041d2..99f8bae4c0 100755
--- a/t/t7615-merge-untracked.sh
+++ b/t/t7615-merge-untracked.sh
@@ -2,78 +2,62 @@
 
 test_description='test when merge with untracked file'
 
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
 . ./test-lib.sh
 
+test_expect_success 'setup' '
+	test_commit "init" README.md "content" &&
+	git checkout -b A
+'
+
+test_expect_success 'fastforward overwrite untracked file that has the same content' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	git merge B
+'
 
-test_expect_success 'overwrite the file when fastforward and the same content' '
-    echo content >README.md &&
-    test_commit "init" README.md &&
-    git branch A &&
-    git checkout -b B &&
-    echo content >file &&
-    git add file &&
-    git commit -m "tracked" &&
-    git switch A &&
-    echo content >file &&
-    git merge B
+test_expect_success 'fastforward fail when untracked file has different content' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" &&
+	git switch A &&
+	echo other >file &&
+	test_must_fail git merge B
 '
 
-test_expect_success 'merge fail with fastforward and different content' '
-    rm * &&
-    rm -r .git &&
-    git init &&
-    echo content >README.md &&
-    test_commit "init" README.md &&
-    git branch A &&
-    git checkout -b B &&
-    echo content >file &&
-    git add file &&
-    git commit -m "tracked" &&
-    git switch A &&
-    echo dif >file &&
-    test_must_fail git merge B
+test_expect_success 'normal merge overwrite untracked file that has the same content' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" fileB "content" &&
+	git switch A &&
+	test_commit --no-tag "exA" fileA "content" &&
+	echo content >file &&
+	git merge B
 '
 
-test_expect_success 'normal merge with untracked with the same content' '
-    rm * &&
-    rm -r .git &&
-    git init &&
-    echo content >README.md &&
-    test_commit "init" README.md &&
-    git branch A &&
-    git checkout -b B &&
-    echo content >fileB &&
-    echo content >file &&
-    git add fileB &&
-    git add file &&
-    git commit -m "tracked" &&
-    git switch A &&
-    echo content >fileA &&
-    git add fileA &&
-    git commit -m "exA" &&
-    echo content >file &&
-    git merge B -m "merge"
+test_expect_success 'normal merge fail when untracked file has different content' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" fileB "content" &&
+	git switch A &&
+	test_commit --no-tag "exA" fileA "content" &&
+	echo dif >file &&
+	test_must_fail git merge B
 '
 
-test_expect_success 'normal merge fail when untracked with different content' '
-    rm * &&
-    rm -r .git &&
-    git init &&
-    echo content >README.md &&
-    test_commit "init" README.md &&
-    git branch A &&
-    git checkout -b B &&
-    echo content >fileB &&
-    echo content >file &&
-    git add fileB &&
-    git add file &&
-    git commit -m "tracked" &&
-    git switch A &&
-    echo content >fileA &&
-    git add fileA &&
-    git commit -m "exA" &&
-    echo dif >file &&
-    test_must_fail git merge B -m "merge"
+test_expect_success 'merge fail when tracked file modification is unstaged' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	test_commit --no-tag "unstaged" file "other" &&
+	git checkout -b B &&
+	test_commit --no-tag "staged" file "content" &&
+	git switch A &&
+	echo content >file &&
+	test_must_fail git merge B
 '
 
-test_done
\ No newline at end of file
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 834aca0da9..61e06c04be 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2257,18 +2257,17 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 	if (result) {
 		if (result->ce_flags & CE_REMOVE)
 			return 0;
-	}
-
-	if (!ie_modified(&o->result, ce, st, 0))
+	} else if (!ie_modified(&o->result, ce, st, 0)) {
 		return 0;
-
+	}
 
 	return add_rejected_path(o, error_type, name);
 }
 
 /*
  * We do not want to remove or overwrite a working tree file that
- * is not tracked, unless it is ignored.
+ * is not tracked, unless it is ignored and unless it has the same
+ * content than the merged file.
  */
 static int verify_absent_1(const struct cache_entry *ce,
 			   enum unpack_trees_error_types error_type,
-- 
2.35.1.7.gc8609858e0.dirty