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

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

From
JJonathan <git.jonathan.bressat@gmail.com>
Date
Apr 25, 2022, 20:27 UTC
Message-ID
<20220425202721.20066-1-git.jonathan.bressat@gmail.com>
In-Reply-To
<xmqqfsmg97ac.fsf@gitster.g>
Sorry for being a bit slow to answer.
Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> 	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
Previous: Junio C HamanoNext: Jonathan
Message 6 of 27 in “[WIP]: make merge nicer to the user”
  1. Guillaume CogoniMar 27, 2022
  2. 0/1 Be nicer to the user on tracked/untracked merge conflictsJonathan, Apr 12, 2022
  3. 1/1 Merge with untracked file that are the same without failure and testJonathan, Apr 12, 2022
  4. Ævar Arnfjörð BjarmasonApr 12, 2022
  5. Junio C HamanoApr 13, 2022
  6. 0/2 Be nicer to the user on tracked/untracked merge conflictsJonathan, Apr 25, 2022
  7. 1/2 t7615: test how merge behave when there is untracked fileJonathan, Apr 25, 2022
  8. 2/2 merge with untracked file that are the same without failureJonathan, Apr 25, 2022
  9. Junio C HamanoApr 25, 2022
  10. Guillaume CogoniApr 25, 2022
  11. Junio C HamanoApr 25, 2022
  12. Ævar Arnfjörð BjarmasonApr 12, 2022
  13. Jonathan BressatApr 14, 2022
  14. Matthieu MoyApr 26, 2022
  15. Junio C HamanoApr 26, 2022
  16. Jonathan BressatApr 28, 2022
  17. 0/4 Be nicer to the user on tracked/untracked merge conflictsJonathan Bressat, May 27, 2022
  18. 1/4 t6436: tests how merge behave when there is untracked file with the same contentJonathan Bressat, May 27, 2022
  19. 2/4 merge with untracked file that are the same without failureJonathan Bressat, May 27, 2022
  20. 3/4 add configuration variable corresponding to --overwrite-same-contentJonathan Bressat, May 27, 2022
  21. 4/4 error message now advice to use the new optionJonathan Bressat, May 27, 2022
  22. Matthieu MoyApr 26, 2022
  23. Matthieu MoyJun 4, 2022
  24. Guillaume CogoniJun 10, 2022
  25. Matthieu MoyJun 4, 2022
  26. Matthieu MoyJun 4, 2022
  27. Matthieu MoyJun 4, 2022

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.