[WIP]: make merge nicer to the user

[WIP]: make merge nicer to the user

[Guillaume Cogoni]

Hi,
We were working on a patch to make merge nicer to the user on
tracked/untracked merge conflicts.
You can see that idea on this page:
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas

When merging a commit which has tracked files with the same name as local
untracked files, Git refuses to proceed.
We want to change this behaviour. The idea is to check if the untracked and
the tracked file has the same content, so we can overwrite it.


Examples of use cases where it can be interesting:
The scenarios are the following:

A team member is modifying the templates for a website we are working on.
They are adding some images to the images directory (but forgets to add
them under source control).
They are sending the images by mail, later, to me.
I'm adding the images under the source control and pushing them to
GitHub together with other changes
They cannot pull updates from GitHub because Git doesn't want to
overwrite their files.
Source : https://stackoverflow.com/questions/1125968/how-do-i-force-git-pull-to-overwrite-local-files

When using rsync to get files from a distant directory, but then those
files are pushed on a repo from the distant directory, you don't want to
reset the change when you just need to pull the repo because files are
the same.


The following parts is our test file:

diff --git a/t/t7615-merge-conflict.sh b/t/t7615-merge-conflict.sh
new file mode 100644
index 0000000000..4d89fe99ed
--- /dev/null
+++ b/t/t7615-merge-conflict.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+#
+# Copyright (c) 2022 Cogoni Guillaume and Bressat Jonathan
+#
+test_description='merge conflitct'
+. ./test-lib.sh
+
+test_expect_success '[FAST_FORWARD] merge conflict when untracked
file and tracked file have the same name and content' '
+ echo content >readme.md &&
+ test_commit "README" readme.md &&
+ git branch B &&
+ git checkout -b A &&
+ echo content >file &&
+ test_commit "tracked_file" file &&
+ git switch B &&
+ echo content >file &&
+ test_merge merge A
+'
+
+test_expect_success '[MERGE] merge conflict when untracked file and
tracked file have the same name and content' '
+ echo content >readme.md &&
+ test_commit "README" readme.md &&
+ git branch A &&
+ git checkout -b B &&
+ echo content1 >file1 &&
+ test_commit "B_tracked_file" file1 &&
+ git checkout A &&
+ echo content2 >file2 &&
+ test_commit "A_tracked_file" file2 &&
+ git switch B &&
+ echo content2 >file2 &&
+ test_merge merge A
+'
+
+test_expect_thatfailure 'merge conflict when untracked file and tracked
file have not the same content but the same name' '
+ echo content >readme.md &&
+ test_commit "README" readme.md &&
+ git branch B &&
+ git checkout -b A &&
+ echo content1 >file &&
+ test_commit "tracked_file" file &&
+ git switch B &&
+ echo content2 >file &&
+ test_merge merge A
+'
+
+test_done
Those tests must have assert in the end but it's just to explain our idea.

Our research lead us to these functions:

verify_absent_1() from /unpack-trees.c seems to be called for all files
and it check if a file from the merged branch exists in the current
branch in regard of the name and the path (In our test above, if a file
from the branch A exist in the branch B.). Then call check_ok_to_remove()
from /unpack-trees.c when an untracked file with the same name than a
tracked file on the merged branch is spotted.

static int verify_absent_1(const struct cache_entry *ce,
enum unpack_trees_error_types error_type,
enum absent_checking_type absent_type,
struct unpack_trees_options *o);

static int check_ok_to_remove(const char *name, int len, int dtype,
const struct cache_entry *ce, struct stat *st,
enum unpack_trees_error_types error_type,
enum absent_checking_type absent_type,
struct unpack_trees_options *o);


We think that a good way to solve this problem is to check the hash
of the tracked and untracked file in check_ok_to_remove and then if
they are similar we can
overwrite it (return 0). The hash is in ce->object_id.
In fact, it's more efficient than decompress the file and check
the content.

Do you think, we are going in the good way, and is it a good idea ?

thanks for your help and review.

Guillaume Cogoni and
Jonathan Bressat

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

[Jonathan]

When doing a merge while there is untracked files with the same name
as merged files, git refuses to proceed. This patch make git overwrite
files if their content are the same.

We added a statement to check_ok_to_remove() (unpack-trees.c) 
with ie_modified() (read-cache.c) to test if the untracked file 
has the same content as the merged one. It seems to work well 
with all three o->result, o->dst_index and o->src_index,
We are not sure of what is the usage of those three, did we used it
properly?

Our tests need some improvement, for example using test_commit,
and testing more possibilities, it's not a real patch, just 
to comfirm if we are on the right track.

The next idea is when it's a fastforward, attempt to merge the
untracked file and the upstream version (like if the file had
just been committed, but without introducing an extra commit).

you can see this idea here: 
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#Be_nicer_to_the_user_on_tracked.2Funtracked_merge_conflicts

Questions:
The old behaviour was here for technical reasons?
The new behavior that we introduce here become the default one?
If the old behavior was important for some people or for some reasons,
we can set a global variable to switch between the old and the new one.
And if we define a global variable, should we print a warning to let 
users know that there is a new behavior when a merge is called and that
he can switch between the old and new one.
For some reason, test_commit make the merge not working like if it's the
old behaviour of merge, I dont understand why ?

Jonathan (1):
  Merge with untracked file that are the same without failure and test

 t/t7615-merge-untracked.sh | 79 ++++++++++++++++++++++++++++++++++++++
 unpack-trees.c             |  4 ++
 2 files changed, 83 insertions(+)
 create mode 100755 t/t7615-merge-untracked.sh

-- 
2.35.1.7.gc8609858e0.dirty

[PATCH 1/1] Merge with untracked file that are the same without failure and test

[Jonathan]

When doing a merge while there is untracked files with the same name
as merged files, git refuses to proceed. This commit change this
behavior and make git overwrite files if their contents are the same.
This new behaviour is more pleasant for a user and will never be a
frustrating moment.

Add a if statement that check if the file has the same content as the
merged file thanks to the function ie_modified() (read-cache.c).
ie_modified () checks the status of both files, if they are different,
it verifies their contents.

Add new tests that need to pass to confirm that the new feature works.

Co-authored-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 t/t7615-merge-untracked.sh | 79 ++++++++++++++++++++++++++++++++++++++
 unpack-trees.c             |  4 ++
 2 files changed, 83 insertions(+)
 create mode 100755 t/t7615-merge-untracked.sh

diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
new file mode 100755
index 0000000000..71a34041d2
--- /dev/null
+++ b/t/t7615-merge-untracked.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+test_description='test when merge with untracked file'
+
+. ./test-lib.sh
+
+
+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 '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 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 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_done
\ No newline at end of file
diff --git a/unpack-trees.c b/unpack-trees.c
index 360844bda3..834aca0da9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2259,6 +2259,10 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 			return 0;
 	}
 
+	if (!ie_modified(&o->result, ce, st, 0))
+		return 0;
+
+
 	return add_rejected_path(o, error_type, name);
 }
 
-- 
2.35.1.7.gc8609858e0.dirty

Re: [PATCH 1/1] Merge with untracked file that are the same without failure and test

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 12 2022, Jonathan wrote:

> When doing a merge while there is untracked files with the same name
> as merged files, git refuses to proceed. This commit change this
> behavior and make git overwrite files if their contents are the same.
> This new behaviour is more pleasant for a user and will never be a
> frustrating moment.
>
> Add a if statement that check if the file has the same content as the
> merged file thanks to the function ie_modified() (read-cache.c).
> ie_modified () checks the status of both files, if they are different,
> it verifies their contents.
>
> Add new tests that need to pass to confirm that the new feature works.
>
> Co-authored-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
> ---
>  t/t7615-merge-untracked.sh | 79 ++++++++++++++++++++++++++++++++++++++
>  unpack-trees.c             |  4 ++
>  2 files changed, 83 insertions(+)
>  create mode 100755 t/t7615-merge-untracked.sh
>
> diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
> new file mode 100755
> index 0000000000..71a34041d2
> --- /dev/null
> +++ b/t/t7615-merge-untracked.sh
> @@ -0,0 +1,79 @@
> +#!/bin/sh
> +
> +test_description='test when merge with untracked file'
> +
> +. ./test-lib.sh
> +
> +

Too much whitespace.

> +test_expect_success 'overwrite the file when fastforward and the same content' '
> +    echo content >README.md &&

The coding style in this project is TAB-indent, not 4 spaces
> +    test_commit "init" README.md &&
> +    git branch A &&
> +    git checkout -b B &&
> +    echo content >file &&
> +    git add file &&
> +    git commit -m "tracked" &&


Can't these and a lot of the test also just use test_commit, you can do
this sort of thing with its multi-param invocation, if you're trying to
specifically avoid tags there's an option for that.

> +    git switch A &&
> +    echo content >file &&
> +    git merge B
> +'
> +
> +test_expect_success 'merge fail with fastforward and different content' '
> +    rm * &&
> +    rm -r .git &&

Can we just set this up in a "git init repo" or whatever instead?

> +    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

And thendo this in a sub-shell?

> +'
> +
> +test_expect_success 'normal merge with untracked with the same content' '
> +    rm * &&
> +    rm -r .git &&

Please use test_when_finished in the tests themselves for teardown,
rather than having the "next test" do the cleanup after the last one.

> +    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 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_done
> \ No newline at end of file

Git's telling you something here... :)

> diff --git a/unpack-trees.c b/unpack-trees.c
> index 360844bda3..834aca0da9 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2259,6 +2259,10 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
>  			return 0;
>  	}
>  
> +	if (!ie_modified(&o->result, ce, st, 0))
> +		return 0;
> +
> +

Too much whitespace.

>  	return add_rejected_path(o, error_type, name);
>  }

Re: [PATCH 0/1] Be nicer to the user on tracked/untracked merge conflicts

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 12 2022, Jonathan wrote:

> When doing a merge while there is untracked files with the same name
> as merged files, git refuses to proceed. This patch make git overwrite
> files if their content are the same.
>
> We added a statement to check_ok_to_remove() (unpack-trees.c) 
> with ie_modified() (read-cache.c) to test if the untracked file 
> has the same content as the merged one. It seems to work well 
> with all three o->result, o->dst_index and o->src_index,
> We are not sure of what is the usage of those three, did we used it
> properly?
>
> Our tests need some improvement, for example using test_commit,
> and testing more possibilities, it's not a real patch, just 
> to comfirm if we are on the right track.
>
> The next idea is when it's a fastforward, attempt to merge the
> untracked file and the upstream version (like if the file had
> just been committed, but without introducing an extra commit).
>
> you can see this idea here: 
> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#Be_nicer_to_the_user_on_tracked.2Funtracked_merge_conflicts

I left some comments on the patch itself, but structurally it wolud be
really nice to make this and similar changes:

 1. Test for current behavior
 2. Change behavior and relevant (new) tests

Rather than the current one-step, that would also communicate that wiki
link (and better) via code.

> Questions:
> The old behaviour was here for technical reasons?
> The new behavior that we introduce here become the default one?
> If the old behavior was important for some people or for some reasons,
> we can set a global variable to switch between the old and the new one.
> And if we define a global variable, should we print a warning to let 
> users know that there is a new behavior when a merge is called and that
> he can switch between the old and new one.

I don't know if we need a config etc., but FWIW my first reaction to
this is that it's a bit iffy/fragile, i.e. before this we'd basically
error out and say "fix your index/working tree".

But now just because the newly merged content happens to be identical
we'll silently merge it over that "staged" content?

Anyway, I can also see how that would be useful for some people.

I've personally been annoyed by a subset of this behavior in the past, I
can't remember if it's with merge or rebase that we'll refuse to do
anything because we have a locally modified/staged (can't remember) file
"X", even though "X" won't be touched at all if the merge/rebase
happens.

But I haven't wanted git to have quite this level of DWYM behavior in
this area, just my 0.02.

> For some reason, test_commit make the merge not working like if it's the
> old behaviour of merge, I dont understand why ?

Ah, I left some comments on "why not test_commit"...

Do you have an example of such a non-working case? I'm not sure why it
wouldn't work.

Re: [PATCH 1/1] Merge with untracked file that are the same without failure and test

[Junio C Hamano]

Jonathan <git.jonathan.bressat@gmail.com> writes:

> diff --git a/unpack-trees.c b/unpack-trees.c
> index 360844bda3..834aca0da9 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2259,6 +2259,10 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
>  			return 0;
>  	}
>  
> +	if (!ie_modified(&o->result, ce, st, 0))
> +		return 0;
> +
> +
>  	return add_rejected_path(o, error_type, name);
>  }

It probably is better to step back a bit and take a wider look at
the original to assess this change.

The only two callers of this function appears in this function.

        /*
         * We do not want to remove or overwrite a working tree file that
         * is not tracked, unless it is ignored.
         */
        static int verify_absent_1(const struct cache_entry *ce,
                                   enum unpack_trees_error_types error_type,
                                   struct unpack_trees_options *o)
        {
		...

Notice what the comment in front says?  Does this patch change the
behaviour from what the comment tells us it does?  We should adjust
the comment to the new world order if it does.

The existing code before the pre-context of the hunk reads like
this:

            /*
             * The previous round may already have decided to
             * delete this path, which is in a subdirectory that
             * is being replaced with a blob.
             */
            result = index_file_exists(&o->result, name, len, 0);
            if (result) {
                    if (result->ce_flags & CE_REMOVE)
                            return 0;
            }

We've called index_file_exists(), and the new code added here does
not take the outcome into account.

If we truly care the case "we have _UNTRACKED_ path and it happens
to be identical to what we are going to resolve to anyway",
shouldn't we be making sure that the <name,len> refers to an
untracked path by checking if result is NULL here?  If so,

	if (result) {
		...
-	}
+	} else if (!ie_modified(...)) {
+		return 0;
+	}
	return add_rejected_path(...);

is what you want, perhaps?

Or if we have cases where we have a tracked path and ask this
function if it is OK to remove, should the same reasoning you
invented to deal with untracked paths equally apply?  If that is the
case, then the code may be OK but the proposed log message to
explain and justify the change needs to be updated to explain what
happens in such a case (or how such a case will not happen).

Thanks.

Re: [PATCH 0/1] Be nicer to the user on tracked/untracked merge conflicts

[Jonathan Bressat]

On Tue, Apr 12 2022, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Do you have an example of such a non-working case? I'm not sure why it
> wouldn't work.

For exemple this test fail:

+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 &&
+ test_commit "tracked" file &&
+ git checkout A &&
+ echo content >file &&
+ git merge B
+'

but this one works:

+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 &&
+ test_commit "tracked" &&
+ git checkout A &&
+ echo content >file &&
+ git merge B
+'

will send you a new version of our patch soon.
Thanks to your reviews and help.

Jonathan BRESSAT and
Guillaume COGONI

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

[Jonathan]

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

[PATCH v1 1/2] t7615: test how merge behave when there is untracked file

[Jonathan]

when there is untracked file that has the same name than file in the
merged branch git refuse to proceed, even when the file has the same
content

t6436 test a similar thing but not especially with same content file

Signed-off-by: Jonathan <git.jonathan.bressat@gmail.com>
Signed-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 t/t7615-merge-untracked.sh | 63 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100755 t/t7615-merge-untracked.sh

diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
new file mode 100755
index 0000000000..053e6b80ee
--- /dev/null
+++ b/t/t7615-merge-untracked.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+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 fail when untracked file 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 &&
+	test_must_fail 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 'normal merge fail when untracked file 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 &&
+	test_must_fail git merge B
+'
+
+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 '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
-- 
2.35.1.7.gc8609858e0.dirty

[PATCH v1 2/2] merge with untracked file that are the same without failure

[Jonathan]

In unpack-trees.c in the check_ok_to_remove() function: add a new
statement, if the file has the same content as the merged file, it
can be removed.

test this new behaviour in t7615.

Signed-off-by: Jonathan <git.jonathan.bressat@gmail.com>
Signed-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 t/t7615-merge-untracked.sh | 8 ++++----
 unpack-trees.c             | 5 ++++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
index 053e6b80ee..99f8bae4c0 100755
--- a/t/t7615-merge-untracked.sh
+++ b/t/t7615-merge-untracked.sh
@@ -12,13 +12,13 @@ test_expect_success 'setup' '
 	git checkout -b A
 '
 
-test_expect_success 'fastforward fail when untracked file has the same content' '
+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 &&
-	test_must_fail git merge B
+	git merge B
 '
 
 test_expect_success 'fastforward fail when untracked file has different content' '
@@ -30,14 +30,14 @@ test_expect_success 'fastforward fail when untracked file has different content'
 	test_must_fail git merge B
 '
 
-test_expect_success 'normal merge fail when untracked file has the same content' '
+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 &&
-	test_must_fail git merge B
+	git merge B
 '
 
 test_expect_success 'normal merge fail when untracked file has different content' '
diff --git a/unpack-trees.c b/unpack-trees.c
index 360844bda3..61e06c04be 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2257,6 +2257,8 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 	if (result) {
 		if (result->ce_flags & CE_REMOVE)
 			return 0;
+	} else if (!ie_modified(&o->result, ce, st, 0)) {
+		return 0;
 	}
 
 	return add_rejected_path(o, error_type, name);
@@ -2264,7 +2266,8 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 
 /*
  * 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

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

[Junio C Hamano]

Jonathan <git.jonathan.bressat@gmail.com> writes:

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

Correct.  If you want to do this correctly, you'd need to make sure
that you'd clobber untracked files ONLY when you are not losing any
information.

And even with that, I think some existing users will be hurt with
this change in a huge way.  They may have untracked change locally
because they are not quite done with it yet, and somebody else
throws a pull request at them that has the same change as the local
modification.

They make a trial merge, look at the result, and discard it because
there are also unwanted changes in the branch they pulled into.

    $ git pull $URL $branch ;# responding to the pull request
    ... examine the result, finding it unsatisfactory ...
    $ git reset --hard ORIG_HEAD
    ... now we are back to where we started; well not really ...

Now, without this change, "git pull" used to stop until they stashed
away the untracked change safely.  But with this change, "git pull"
will succeed, and then "reset --hard" will discard it together with
other changes that came to us from $URL/$branch.  They lost their
local, uncommitted change.

And "but you can pull the equivalent out of $URL/$branch" is not a
good excuse.  They may not notice the lossage long after having
dealt with this pull request (there are busy people who are handling
many pull requests from many people) and they have been relying on
"git pull" that never clobbers their local uncommitted changes.  And
when they noticed the lossage, they may not even remember which one
of pull requests happened to have an identical change as their local
change to cause this lossage, simply because "git pull" that used to
stop just continued without a noise.

So, I am not sure if this is really a good idea to begin with.  It
certainly would make it slightly simpler in a trivial case, but it
surely looks like a dangerous behaviour change, especially if it is
done unconditionally.

> 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.

Wait.  Isn't it backwards?  The existing tests _may_ be casting an
undesirable current behaviour in stone, but most of the time it is
protecting existing user's expectations.  If you have an untracked
file, you can rest assured that they won't be clobbered by a merge.

So we'd need to think twice and carefully examine if it makes sense
to update the expectations.  I haven't read the change to the tests,
so I cannot tell which case it is.

Thanks.

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

[Guillaume Cogoni]

 Junio C Hamano <gitster@pobox.com> writes:

> So, I am not sure if this is really a good idea to begin with.  It
> certainly would make it slightly simpler in a trivial case, but it
> surely looks like a dangerous behaviour change, especially if it is
> done unconditionally.

Can we create a configuration variable to avoid this problem?
We keep the old behavior by default, and make a configuration variable
for people who wants to have this new behavior, but if the user set the variable
a message informs it about the problem that you mention.

Or, we add an option like git pull --doSomething.

Maybe, we can think about another behaviour.
When the user git pull and this error occurs:
error: The following untracked working tree files would be overwritten by merge:
file1.txt
file2.txt
Please move or remove them before you merge.
Aborting
We don't abort, but we prompt a yes/no for each file, if the user
wants to remove it.

We just make suggestions, we will think more about it.

> Wait.  Isn't it backwards?  The existing tests _may_ be casting an
> undesirable current behaviour in stone, but most of the time it is
> protecting existing user's expectations.  If you have an untracked
> file, you can rest assured that they won't be clobbered by a merge.

> So we'd need to think twice and carefully examine if it makes sense
> to update the expectations.  I haven't read the change to the tests,
> so I cannot tell which case it is.

Yes, we'll figure out a solution, if there is one.

Thanks for your review and your quick response, it give us a lot of information,

COGONI Guillaume and BRESSAT Jonathan

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

[Junio C Hamano]

Guillaume Cogoni <cogoni.guillaume@gmail.com> writes:

>> So, I am not sure if this is really a good idea to begin with.  It
>> certainly would make it slightly simpler in a trivial case, but it
>> surely looks like a dangerous behaviour change, especially if it is
>> done unconditionally.
>
> Can we create a configuration variable to avoid this problem?
> We keep the old behavior by default, and make a configuration variable
> for people who wants to have this new behavior, but if the user set the variable
> a message informs it about the problem that you mention.
>
> Or, we add an option like git pull --doSomething.

Probably a command line option ("git merge" would probably want the
same one) plus a configuration varaible to give it the default (the
latter is optional).

> Maybe, we can think about another behaviour.
> When the user git pull and this error occurs:
> error: The following untracked working tree files would be overwritten by merge:
> file1.txt
> file2.txt
> Please move or remove them before you merge.
> Aborting
> We don't abort, but we prompt a yes/no for each file, if the user
> wants to remove it.

I doubt this would fly as-is.  Especially if the action that is
offered by the prompt is "remove", not "move", as that implies we
are not prepared against loss of information.

There is no indication whether the untracked file1.txt matches the
contents we are pulling in.  Most of the time, it is very unlikely
that the contents being lost is identical to what the other side
has, so answering "yes" to the prompt means "No, I do not care about
my garbage, and it is OK that it will forever be lost."  I do not
think we want to be encouraging people to habitually make such a
statement.  If we move (instead of removing) them away to somewhere,
and give users to easily recover them after running "git pull", it
might become more palatable.

I wonder if this whole thing is an attempt to work around whatever
"stash --untracked" fails to do well (or perhaps there are no such
shortcomings, but just the users are not made aware of the command
enough).  If you have these two untracked files (file1.txt and
file2.txt) are "in the way" for a merge to succeed, I have to wonder
if "Please move or remove" message that was introduced by 23cbf11b
(merge-recursive: porcelain messages for checkout, 2010-08-11) is
still giving a good piece of advice to users today.

Would "git stash push -u file1.txt file2.txt" be an easier and safer
alternative that lets you take these files back later?  Back in
2010, when 23cbf11b was current, "git stash" was a shell script and
it seems there was no "untracked" option, so from that point of
view, "move or remove" may have been the best they could do.

Note that I never use "git stash" with "untracked" option, so I do
not know if it works well in this context already, or we need more
work before it becomes usable in this scenario.  But it smells like
it is exactly what we might want to use in such a situation to stash
away these untracked file1.txt and file2.txt while running the
merge, while allowing us to recover them after running the merge or
discarding it.  I dunno.

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

[Matthieu Moy]

On 4/26/22 00:28, Guillaume Cogoni wrote:
>   Junio C Hamano <gitster@pobox.com> writes:
> 
>> So, I am not sure if this is really a good idea to begin with.  It
>> certainly would make it slightly simpler in a trivial case, but it
>> surely looks like a dangerous behaviour change, especially if it is
>> done unconditionally.
> 
> Can we create a configuration variable to avoid this problem?
> We keep the old behavior by default, and make a configuration variable
> for people who wants to have this new behavior, but if the user set the variable
> a message informs it about the problem that you mention.
> 
> Or, we add an option like git pull --doSomething.
> 
> Maybe, we can think about another behaviour.
> When the user git pull and this error occurs:
> error: The following untracked working tree files would be overwritten by merge:
> file1.txt
> file2.txt
> Please move or remove them before you merge.
> Aborting
> We don't abort, but we prompt a yes/no for each file, if the user
> wants to remove it.

Git very rarely goes interactive like this (only a few special command 
like git send-email, git clean -i, git add -i/-p prompt the user).

Prompting the user in the middle of an operation has several drawbacks:

- When the command is launched from a script, the script may work most 
of the time, and sometimes pause on an interactive prompt which wasn't 
expected from the author of the script. This can be a bit nasty when the 
script isn't ran from a place where you can type to the standard input 
of the command or when its output is redirected.

- Asking for each individual file can be tedious when there are many 
files. Similarly, "rm -i" (plain rm, not "git rm") is a nice safety 
measure, but not really convenient to me at least.

In this particular case, actually, I can't imagine a sane behavior when 
the user wants a mix of "yes" / "no". If a single untracked file 
conflicts with what's being merged, the merge aborts, even if you're OK 
to replace other files. So I can only imagine a single yes/no answer. 
And then the question can be replaced with a suggestion to re-run with a 
command-line flag when all the conflicting files are unmodified.

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: [PATCH v1 1/2] t7615: test how merge behave when there is untracked file

[Matthieu Moy]

On 4/25/22 22:27, Jonathan wrote:
> when there is untracked file that has the same name than file in the
> merged branch git refuse to proceed, even when the file has the same
> content
> 
> t6436 test a similar thing but not especially with same content file

Write your commit message like normal english: capitalize start of 
sentence, and period at the end (we omit the period in the subject line, 
though).

> +test_expect_success 'fastforward fail when untracked file has the same content' '

Here and other test names: third person => s (fail_s_, and overwrite_s_ 
in the next patch).

> +	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 &&
> +	test_must_fail git merge B

It would make sense to grep for the correct error message in the output, 
but maybe that's overkill.

-- 
Matthieu Moy
https://matthieu-moy.fr/

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

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@univ-lyon1.fr> writes:

> Git very rarely goes interactive like this (only a few special command
> like git send-email, git clean -i, git add -i/-p prompt the user).
>
> Prompting the user in the middle of an operation has several drawbacks:
> ...
> In this particular case, actually, I can't imagine a sane behavior
> when the user wants a mix of "yes" / "no". If a single untracked file 
> conflicts with what's being merged, the merge aborts, even if you're
> OK to replace other files. So I can only imagine a single yes/no
> answer. And then the question can be replaced with a suggestion to
> re-run with a command-line flag when all the conflicting files are
> unmodified.

Nicely explained.  Thanks.

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

[Jonathan Bressat]

Junio C Hamano <gitster@pobox.com> writes:

> Probably a command line option ("git merge" would probably want the
> same one) plus a configuration varaible to give it the default (the
> latter is optional).

First, we think that add an option to pull and merge is more suited to
our situation, and next, it could be good to add the configuration
variable

In unpack-trees.c there is a list of files that cause problem with merge.
We want to split this list to list files that have the same content, then if
all the files have the same content, we can suggest to use the option
to overwrite those files.
Then we can modify the error message to show the files that have the
same content apart.

> I wonder if this whole thing is an attempt to work around whatever
> "stash --untracked" fails to do well (or perhaps there are no such
> shortcomings, but just the users are not made aware of the command
> enough).  If you have these two untracked files (file1.txt and
> file2.txt) are "in the way" for a merge to succeed, I have to wonder
> if "Please move or remove" message that was introduced by 23cbf11b
> (merge-recursive: porcelain messages for checkout, 2010-08-11) is
> still giving a good piece of advice to users today.

We got a similar idea, but we finally decide that it was not a very good
approach because it's not efficient if we have a lot of files or some big files.
And because if there are files that doesn't block the merge, we treat them
anyway and they will move from the work tree, it's a bit overkill.

> Note that I never use "git stash" with "untracked" option, so I do
>  not know if it works well in this context already, or we need more
> work before it becomes usable in this scenario.  But it smells like
> it is exactly what we might want to use in such a situation to stash
> away these untracked file1.txt and file2.txt while running the
> merge, while allowing us to recover them after running the merge or
> discarding it.  I dunno.

Indeed, git stash works well with this kind of problem, however an option
would be easier in that specific case.

Thanks for you're helpfull review, you always give us a lot of good
information and ideas.

Cogoni Guillaume and
Bressat Jonathan

[PATCH v2 0/4] Be nicer to the user on tracked/untracked merge conflicts

[Jonathan Bressat]

We improve our last patch with adding an option to merge and pull command and a
configuration variable.
This make the user able to modify the behavior of merge when it meet untracked files
that have the same content than files in merged branch.

We kept the old behavior as default.

thanks

Jonathan Bressat and 
Guillaume Cogoni

Jonathan Bressat (4):
  t6436: tests how merge behave when there is untracked file with the
    same content
  merge with untracked file that are the same without failure
  add configuration variable corresponding to --overwrite-same-content
  error message now advice to use the new option

 Documentation/config/merge.txt |  5 ++
 Documentation/git-merge.txt    |  4 ++
 builtin/checkout.c             |  1 +
 builtin/merge.c                |  9 +++-
 builtin/pull.c                 |  8 +++-
 cache.h                        |  3 +-
 merge-ort.c                    |  1 +
 merge-recursive.h              |  1 +
 merge.c                        |  4 +-
 sequencer.c                    |  2 +-
 t/t6436-merge-overwrite.sh     | 34 ++++++++++++++
 t/t7615-merge-untracked.sh     | 84 ++++++++++++++++++++++++++++++++++
 unpack-trees.c                 | 27 +++++++++--
 unpack-trees.h                 |  2 +
 14 files changed, 174 insertions(+), 11 deletions(-)
 create mode 100755 t/t7615-merge-untracked.sh

Interdiff contre v1 :
diff --git a/Documentation/config/merge.txt b/Documentation/config/merge.txt
index 99e83dd36e..2824dd19c7 100644
--- a/Documentation/config/merge.txt
+++ b/Documentation/config/merge.txt
@@ -89,6 +89,11 @@ merge.autoStash::
 	`--autostash` options of linkgit:git-merge[1].
 	Defaults to false.
 
+merge.overwritesamecontent::
+    When set to true, it will modify the behavior of git merge 
+    to overwrite untracked files that have the same name and 
+    content than files in the merged commit.	
+
 merge.tool::
 	Controls which merge tool is used by linkgit:git-mergetool[1].
 	The list below shows the valid built-in values.
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 3125473cc1..ceda0271c2 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -100,6 +100,10 @@ will be appended to the specified message.
 	Silently overwrite ignored files from the merge result. This
 	is the default behavior. Use `--no-overwrite-ignore` to abort.
 
+--overwrite-same-content::
+       Silently overwrite untracked files that have the same content 
+       and name than files in the merged commit from the merge result.
+
 --abort::
 	Abort the current conflict resolution process, and
 	try to reconstruct the pre-merge state. If an autostash entry is
diff --git a/builtin/checkout.c b/builtin/checkout.c
index cc804ba8e1..1b1d1813c7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -760,6 +760,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 				       &new_branch_info->commit->object.oid :
 				       &new_branch_info->oid, NULL);
 		topts.preserve_ignored = !opts->overwrite_ignore;
+		topts.overwrite_same_content = 0;/* FIXME: opts->overwrite_same_content */
 		tree = parse_tree_indirect(old_branch_info->commit ?
 					   &old_branch_info->commit->object.oid :
 					   the_hash_algo->empty_tree);
diff --git a/builtin/merge.c b/builtin/merge.c
index 74e53cf20a..936cb8480d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -68,6 +68,7 @@ static int option_edit = -1;
 static int allow_trivial = 1, have_message, verify_signatures;
 static int check_trust_level = 1;
 static int overwrite_ignore = 1;
+static int overwrite_same_content;
 static struct strbuf merge_msg = STRBUF_INIT;
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
@@ -305,6 +306,7 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
 	OPT_BOOL(0, "signoff", &signoff, N_("add a Signed-off-by trailer")),
 	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
+	OPT_BOOL(0, "overwrite-same-content", &overwrite_same_content, N_("overwrite untracked file with the same content and name")),
 	OPT_END()
 };
 
@@ -656,6 +658,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	} else if (!strcmp(k, "merge.autostash")) {
 		autostash = git_config_bool(k, v);
 		return 0;
+	} else if (!strcmp(k,"merge.overwritesamecontent")) {
+		overwrite_same_content = git_config_bool(k, v);
 	}
 
 	status = fmt_merge_msg_config(k, v, cb);
@@ -684,6 +688,7 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 	opts.trivial_merges_only = 1;
 	opts.merge = 1;
 	opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
+	opts.overwrite_same_content = overwrite_same_content;
 	trees[nr_trees] = parse_tree_indirect(common);
 	if (!trees[nr_trees++])
 		return -1;
@@ -746,6 +751,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		o.branch1 = head_arg;
 		o.branch2 = merge_remote_util(remoteheads->item)->name;
+		o.overwrite_same_content = overwrite_same_content;
 
 		for (j = common; j; j = j->next)
 			commit_list_insert(j->item, &reversed);
@@ -1573,7 +1579,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (checkout_fast_forward(the_repository,
 					  &head_commit->object.oid,
 					  &commit->object.oid,
-					  overwrite_ignore)) {
+					  overwrite_ignore,
+					  overwrite_same_content)) {
 			apply_autostash(git_path_merge_autostash(the_repository));
 			ret = 1;
 			goto done;
diff --git a/builtin/pull.c b/builtin/pull.c
index 100cbf9fb8..46ef68e721 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -92,6 +92,7 @@ static struct strvec opt_strategies = STRVEC_INIT;
 static struct strvec opt_strategy_opts = STRVEC_INIT;
 static char *opt_gpg_sign;
 static int opt_allow_unrelated_histories;
+static int opt_overwrite_same_content;
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -182,6 +183,7 @@ static struct option pull_options[] = {
 	OPT_SET_INT(0, "allow-unrelated-histories",
 		    &opt_allow_unrelated_histories,
 		    N_("allow merging unrelated histories"), 1),
+	OPT_BOOL(0, "overwrite-same-content", &opt_overwrite_same_content, N_("overwrite untracked file with the same content and name")),
 
 	/* Options passed to git-fetch */
 	OPT_GROUP(N_("Options related to fetching")),
@@ -612,7 +614,7 @@ static int pull_into_void(const struct object_id *merge_head,
 	 */
 	if (checkout_fast_forward(the_repository,
 				  the_hash_algo->empty_tree,
-				  merge_head, 0))
+				  merge_head, 0, opt_overwrite_same_content))
 		return 1;
 
 	if (update_ref("initial pull", "HEAD", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR))
@@ -679,6 +681,8 @@ static int run_merge(void)
 		strvec_pushf(&args, "--cleanup=%s", cleanup_arg);
 	if (opt_ff)
 		strvec_push(&args, opt_ff);
+	if (opt_overwrite_same_content)
+		strvec_push(&args, "--overwrite-same-content");
 	if (opt_verify)
 		strvec_push(&args, opt_verify);
 	if (opt_verify_signatures)
@@ -1078,7 +1082,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			"commit %s."), oid_to_hex(&orig_head));
 
 		if (checkout_fast_forward(the_repository, &orig_head,
-					  &curr_head, 0))
+					  &curr_head, 0, opt_overwrite_same_content))
 			die(_("Cannot fast-forward your working tree.\n"
 				"After making sure that you saved anything precious from\n"
 				"$ git diff %s\n"
diff --git a/cache.h b/cache.h
index 281f00ab1b..163367ab41 100644
--- a/cache.h
+++ b/cache.h
@@ -1858,7 +1858,8 @@ int try_merge_command(struct repository *r,
 int checkout_fast_forward(struct repository *r,
 			  const struct object_id *from,
 			  const struct object_id *to,
-			  int overwrite_ignore);
+			  int overwrite_ignore,
+			  int overwrite_same_content);
 
 
 int sane_execvp(const char *file, char *const argv[]);
diff --git a/merge-ort.c b/merge-ort.c
index c319797021..a8d1496b4a 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4066,6 +4066,7 @@ static int checkout(struct merge_options *opt,
 	unpack_opts.verbose_update = (opt->verbosity > 2);
 	unpack_opts.fn = twoway_merge;
 	unpack_opts.preserve_ignored = 0; /* FIXME: !opts->overwrite_ignore */
+	unpack_opts.overwrite_same_content = opt->overwrite_same_content;
 	parse_tree(prev);
 	init_tree_desc(&trees[0], prev->buffer, prev->size);
 	parse_tree(next);
diff --git a/merge-recursive.h b/merge-recursive.h
index 0795a1d3ec..6f8c5678e0 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -46,6 +46,7 @@ struct merge_options {
 	/* miscellaneous control options */
 	const char *subtree_shift;
 	unsigned renormalize : 1;
+	int overwrite_same_content;
 
 	/* internal fields used by the implementation */
 	struct merge_options_internal *priv;
diff --git a/merge.c b/merge.c
index 2382ff66d3..410c92d235 100644
--- a/merge.c
+++ b/merge.c
@@ -47,7 +47,8 @@ int try_merge_command(struct repository *r,
 int checkout_fast_forward(struct repository *r,
 			  const struct object_id *head,
 			  const struct object_id *remote,
-			  int overwrite_ignore)
+			  int overwrite_ignore,
+			  int overwrite_same_content)
 {
 	struct tree *trees[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;
@@ -80,6 +81,7 @@ int checkout_fast_forward(struct repository *r,
 
 	memset(&opts, 0, sizeof(opts));
 	opts.preserve_ignored = !overwrite_ignore;
+	opts.overwrite_same_content = overwrite_same_content;
 
 	opts.head_idx = 1;
 	opts.src_index = r->index;
diff --git a/sequencer.c b/sequencer.c
index 5213d16e97..d11802c542 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -529,7 +529,7 @@ static int fast_forward_to(struct repository *r,
 	struct strbuf err = STRBUF_INIT;
 
 	repo_read_index(r);
-	if (checkout_fast_forward(r, from, to, 1))
+	if (checkout_fast_forward(r, from, to, 1, 0))
 		return -1; /* the callee should have complained already */
 
 	strbuf_addf(&sb, _("%s: fast-forward"), _(action_name(opts)));
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index c0b7bd7c3f..bb323b1ee3 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -204,4 +204,38 @@ test_expect_success 'will not clobber WT/index when merging into unborn' '
 	grep bar untracked-file
 '
 
+test_expect_success 'create branch A' '
+	git reset --hard c0 &&
+	git checkout -b A
+'
+
+test_expect_success 'fastforward will not overwrite untracked file with the same content' '
+	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	test_must_fail git merge B
+'
+
+test_expect_success 'will not overwrite untracked file with the same content' '
+	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" fileB "content" &&
+	git checkout A &&
+	test_commit --no-tag "exA" fileA "content" &&
+	echo content >file &&
+	test_must_fail git merge B
+'
+
+test_expect_success 'will not overwrite unstaged file with the same content' '
+	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
+	test_commit --no-tag "unstaged" file "other" &&
+	git checkout -b B &&
+	test_commit --no-tag "staged" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	test_must_fail git merge B
+'
+
 test_done
diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
index 99f8bae4c0..cfefd8f473 100755
--- a/t/t7615-merge-untracked.sh
+++ b/t/t7615-merge-untracked.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test when merge with untracked file'
+test_description='test when merge with untracked files and the option --overwrite-same-content'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
@@ -18,7 +18,7 @@ test_expect_success 'fastforward overwrite untracked file that has the same cont
 	test_commit --no-tag "tracked" file "content" &&
 	git checkout A &&
 	echo content >file &&
-	git merge B
+	git merge --overwrite-same-content B
 '
 
 test_expect_success 'fastforward fail when untracked file has different content' '
@@ -27,7 +27,7 @@ test_expect_success 'fastforward fail when untracked file has different content'
 	test_commit --no-tag "tracked" file "content" &&
 	git switch A &&
 	echo other >file &&
-	test_must_fail git merge B
+	test_must_fail git merge --overwrite-same-content B
 '
 
 test_expect_success 'normal merge overwrite untracked file that has the same content' '
@@ -37,7 +37,7 @@ test_expect_success 'normal merge overwrite untracked file that has the same con
 	git switch A &&
 	test_commit --no-tag "exA" fileA "content" &&
 	echo content >file &&
-	git merge B
+	git merge --overwrite-same-content B
 '
 
 test_expect_success 'normal merge fail when untracked file has different content' '
@@ -47,7 +47,7 @@ test_expect_success 'normal merge fail when untracked file has different content
 	git switch A &&
 	test_commit --no-tag "exA" fileA "content" &&
 	echo dif >file &&
-	test_must_fail git merge B
+	test_must_fail git merge --overwrite-same-content B
 '
 
 test_expect_success 'merge fail when tracked file modification is unstaged' '
@@ -57,7 +57,28 @@ test_expect_success 'merge fail when tracked file modification is unstaged' '
 	test_commit --no-tag "staged" file "content" &&
 	git switch A &&
 	echo content >file &&
-	test_must_fail git merge B
+	test_must_fail git merge --overwrite-same-content B
+'
+
+test_expect_success 'fastforward overwrite untracked file that has the same content with the configuration variable' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	test_config merge.overwritesamecontent true &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	git merge B
+'
+
+test_expect_success 'normal merge overwrite untracked file that has the same content with the configuration variable' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	test_config merge.overwritesamecontent true &&
+	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_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 61e06c04be..6c660b084b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -158,17 +158,17 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 	if (!strcmp(cmd, "checkout"))
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("The following untracked working tree files would be overwritten by checkout:\n%%s"
-			  "Please move or remove them before you switch branches.")
+			  "Please move or remove them before you switch branches.%%s")
 		      : _("The following untracked working tree files would be overwritten by checkout:\n%%s");
 	else if (!strcmp(cmd, "merge"))
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("The following untracked working tree files would be overwritten by merge:\n%%s"
-			  "Please move or remove them before you merge.")
+			  "Please move or remove them before you merge.%%s")
 		      : _("The following untracked working tree files would be overwritten by merge:\n%%s");
 	else
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("The following untracked working tree files would be overwritten by %s:\n%%s"
-			  "Please move or remove them before you %s.")
+			  "Please move or remove them before you %s.%%s")
 		      : _("The following untracked working tree files would be overwritten by %s:\n%%s");
 	msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] =
 		strvec_pushf(&opts->msgs_to_free, msg, cmd, cmd);
@@ -251,6 +251,14 @@ static void display_error_msgs(struct unpack_trees_options *o)
 {
 	int e;
 	unsigned error_displayed = 0;
+	const char *can_overwrite_msg;
+
+	if (o->can_overwrite) {
+		can_overwrite_msg = _("\nYou can also rerun the command with --overwrite-same-content to overwrite files with same content.");
+	} else {
+		can_overwrite_msg = "";
+	}
+
 	for (e = 0; e < NB_UNPACK_TREES_ERROR_TYPES; e++) {
 		struct string_list *rejects = &o->unpack_rejects[e];
 
@@ -261,7 +269,8 @@ static void display_error_msgs(struct unpack_trees_options *o)
 			error_displayed = 1;
 			for (i = 0; i < rejects->nr; i++)
 				strbuf_addf(&path, "\t%s\n", rejects->items[i].string);
-			error(ERRORMSG(o, e), super_prefixed(path.buf));
+
+			error(ERRORMSG(o, e), super_prefixed(path.buf), can_overwrite_msg);
 			strbuf_release(&path);
 		}
 		string_list_clear(rejects, 0);
@@ -1711,6 +1720,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	struct pattern_list pl;
 	int free_pattern_list = 0;
 	struct dir_struct dir = DIR_INIT;
+	o->can_overwrite = 1;
 
 	if (o->reset == UNPACK_RESET_INVALID)
 		BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
@@ -2257,8 +2267,12 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 	if (result) {
 		if (result->ce_flags & CE_REMOVE)
 			return 0;
-	} else if (!ie_modified(&o->result, ce, st, 0)) {
-		return 0;
+	} else if (ce && !ie_modified(o->src_index, ce, st, 0)) {
+		if(o->overwrite_same_content) {
+			return 0;
+		}
+	} else {
+		o->can_overwrite = 0;
 	}
 
 	return add_rejected_path(o, error_type, name);
@@ -2266,8 +2280,8 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 
 /*
  * We do not want to remove or overwrite a working tree file that
- * is not tracked, unless it is ignored and unless it has the same
- * content than the merged file.
+ * is not tracked, unless it is ignored or it has the same content
+ * than the merged file with the option --overwrite_same_content.
  */
 static int verify_absent_1(const struct cache_entry *ce,
 			   enum unpack_trees_error_types error_type,
diff --git a/unpack-trees.h b/unpack-trees.h
index efb9edfbb2..2be74ce5bf 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -71,6 +71,8 @@ struct unpack_trees_options {
 		     quiet,
 		     exiting_early,
 		     show_all_errors,
+		     overwrite_same_content,
+		     can_overwrite,
 		     dry_run;
 	enum unpack_trees_reset_type reset;
 	const char *prefix;
-- 
2.35.1.10.g88248585b1.dirty

[PATCH v2 1/4] t6436: tests how merge behave when there is untracked file with the same content

[Jonathan Bressat]

add test to show explicitly that merge doesn't overwrite untracked files
or unstaged even when they have the same content than files int the
merged commit

Signed-off-by: Jonathan Bressat <git.jonathan.bressat@gmail.com>
Signed-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 t/t6436-merge-overwrite.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index c0b7bd7c3f..bb323b1ee3 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -204,4 +204,38 @@ test_expect_success 'will not clobber WT/index when merging into unborn' '
 	grep bar untracked-file
 '
 
+test_expect_success 'create branch A' '
+	git reset --hard c0 &&
+	git checkout -b A
+'
+
+test_expect_success 'fastforward will not overwrite untracked file with the same content' '
+	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	test_must_fail git merge B
+'
+
+test_expect_success 'will not overwrite untracked file with the same content' '
+	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" fileB "content" &&
+	git checkout A &&
+	test_commit --no-tag "exA" fileA "content" &&
+	echo content >file &&
+	test_must_fail git merge B
+'
+
+test_expect_success 'will not overwrite unstaged file with the same content' '
+	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
+	test_commit --no-tag "unstaged" file "other" &&
+	git checkout -b B &&
+	test_commit --no-tag "staged" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	test_must_fail git merge B
+'
+
 test_done
-- 
2.35.1.10.g88248585b1.dirty

[PATCH v2 2/4] merge with untracked file that are the same without failure

[Jonathan Bressat]

Keep the old behavior as default.

Add the option --overwrite-same-content, when this option is used merge
will overwrite untracked file that have the same content.

It make the merge nicer to the user, usefull for a simple utilisation,
for exemple if you copy and paste files from another project and then
you decide to pull this project, git will not proceed even if you didn't
modify those files.

t7615 tests this new behavior.

Signed-off-by: Jonathan Bressat <git.jonathan.bressat@gmail.com>
Signed-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 Documentation/git-merge.txt |  4 +++
 builtin/merge.c             |  7 ++++-
 builtin/pull.c              |  8 +++--
 cache.h                     |  3 +-
 merge-ort.c                 |  1 +
 merge-recursive.h           |  1 +
 merge.c                     |  4 ++-
 sequencer.c                 |  2 +-
 t/t7615-merge-untracked.sh  | 63 +++++++++++++++++++++++++++++++++++++
 unpack-trees.c              |  7 ++++-
 unpack-trees.h              |  1 +
 11 files changed, 94 insertions(+), 7 deletions(-)
 create mode 100755 t/t7615-merge-untracked.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 3125473cc1..ceda0271c2 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -100,6 +100,10 @@ will be appended to the specified message.
 	Silently overwrite ignored files from the merge result. This
 	is the default behavior. Use `--no-overwrite-ignore` to abort.
 
+--overwrite-same-content::
+       Silently overwrite untracked files that have the same content 
+       and name than files in the merged commit from the merge result.
+
 --abort::
 	Abort the current conflict resolution process, and
 	try to reconstruct the pre-merge state. If an autostash entry is
diff --git a/builtin/merge.c b/builtin/merge.c
index 74e53cf20a..fffae81068 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -68,6 +68,7 @@ static int option_edit = -1;
 static int allow_trivial = 1, have_message, verify_signatures;
 static int check_trust_level = 1;
 static int overwrite_ignore = 1;
+static int overwrite_same_content;
 static struct strbuf merge_msg = STRBUF_INIT;
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
@@ -305,6 +306,7 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
 	OPT_BOOL(0, "signoff", &signoff, N_("add a Signed-off-by trailer")),
 	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
+	OPT_BOOL(0, "overwrite-same-content", &overwrite_same_content, N_("overwrite untracked file with the same content and name")),
 	OPT_END()
 };
 
@@ -684,6 +686,7 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 	opts.trivial_merges_only = 1;
 	opts.merge = 1;
 	opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
+	opts.overwrite_same_content = overwrite_same_content;
 	trees[nr_trees] = parse_tree_indirect(common);
 	if (!trees[nr_trees++])
 		return -1;
@@ -746,6 +749,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		o.branch1 = head_arg;
 		o.branch2 = merge_remote_util(remoteheads->item)->name;
+		o.overwrite_same_content = overwrite_same_content;
 
 		for (j = common; j; j = j->next)
 			commit_list_insert(j->item, &reversed);
@@ -1573,7 +1577,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (checkout_fast_forward(the_repository,
 					  &head_commit->object.oid,
 					  &commit->object.oid,
-					  overwrite_ignore)) {
+					  overwrite_ignore,
+					  overwrite_same_content)) {
 			apply_autostash(git_path_merge_autostash(the_repository));
 			ret = 1;
 			goto done;
diff --git a/builtin/pull.c b/builtin/pull.c
index 100cbf9fb8..46ef68e721 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -92,6 +92,7 @@ static struct strvec opt_strategies = STRVEC_INIT;
 static struct strvec opt_strategy_opts = STRVEC_INIT;
 static char *opt_gpg_sign;
 static int opt_allow_unrelated_histories;
+static int opt_overwrite_same_content;
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -182,6 +183,7 @@ static struct option pull_options[] = {
 	OPT_SET_INT(0, "allow-unrelated-histories",
 		    &opt_allow_unrelated_histories,
 		    N_("allow merging unrelated histories"), 1),
+	OPT_BOOL(0, "overwrite-same-content", &opt_overwrite_same_content, N_("overwrite untracked file with the same content and name")),
 
 	/* Options passed to git-fetch */
 	OPT_GROUP(N_("Options related to fetching")),
@@ -612,7 +614,7 @@ static int pull_into_void(const struct object_id *merge_head,
 	 */
 	if (checkout_fast_forward(the_repository,
 				  the_hash_algo->empty_tree,
-				  merge_head, 0))
+				  merge_head, 0, opt_overwrite_same_content))
 		return 1;
 
 	if (update_ref("initial pull", "HEAD", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR))
@@ -679,6 +681,8 @@ static int run_merge(void)
 		strvec_pushf(&args, "--cleanup=%s", cleanup_arg);
 	if (opt_ff)
 		strvec_push(&args, opt_ff);
+	if (opt_overwrite_same_content)
+		strvec_push(&args, "--overwrite-same-content");
 	if (opt_verify)
 		strvec_push(&args, opt_verify);
 	if (opt_verify_signatures)
@@ -1078,7 +1082,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			"commit %s."), oid_to_hex(&orig_head));
 
 		if (checkout_fast_forward(the_repository, &orig_head,
-					  &curr_head, 0))
+					  &curr_head, 0, opt_overwrite_same_content))
 			die(_("Cannot fast-forward your working tree.\n"
 				"After making sure that you saved anything precious from\n"
 				"$ git diff %s\n"
diff --git a/cache.h b/cache.h
index 281f00ab1b..163367ab41 100644
--- a/cache.h
+++ b/cache.h
@@ -1858,7 +1858,8 @@ int try_merge_command(struct repository *r,
 int checkout_fast_forward(struct repository *r,
 			  const struct object_id *from,
 			  const struct object_id *to,
-			  int overwrite_ignore);
+			  int overwrite_ignore,
+			  int overwrite_same_content);
 
 
 int sane_execvp(const char *file, char *const argv[]);
diff --git a/merge-ort.c b/merge-ort.c
index c319797021..a8d1496b4a 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4066,6 +4066,7 @@ static int checkout(struct merge_options *opt,
 	unpack_opts.verbose_update = (opt->verbosity > 2);
 	unpack_opts.fn = twoway_merge;
 	unpack_opts.preserve_ignored = 0; /* FIXME: !opts->overwrite_ignore */
+	unpack_opts.overwrite_same_content = opt->overwrite_same_content;
 	parse_tree(prev);
 	init_tree_desc(&trees[0], prev->buffer, prev->size);
 	parse_tree(next);
diff --git a/merge-recursive.h b/merge-recursive.h
index 0795a1d3ec..6f8c5678e0 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -46,6 +46,7 @@ struct merge_options {
 	/* miscellaneous control options */
 	const char *subtree_shift;
 	unsigned renormalize : 1;
+	int overwrite_same_content;
 
 	/* internal fields used by the implementation */
 	struct merge_options_internal *priv;
diff --git a/merge.c b/merge.c
index 2382ff66d3..410c92d235 100644
--- a/merge.c
+++ b/merge.c
@@ -47,7 +47,8 @@ int try_merge_command(struct repository *r,
 int checkout_fast_forward(struct repository *r,
 			  const struct object_id *head,
 			  const struct object_id *remote,
-			  int overwrite_ignore)
+			  int overwrite_ignore,
+			  int overwrite_same_content)
 {
 	struct tree *trees[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;
@@ -80,6 +81,7 @@ int checkout_fast_forward(struct repository *r,
 
 	memset(&opts, 0, sizeof(opts));
 	opts.preserve_ignored = !overwrite_ignore;
+	opts.overwrite_same_content = overwrite_same_content;
 
 	opts.head_idx = 1;
 	opts.src_index = r->index;
diff --git a/sequencer.c b/sequencer.c
index 5213d16e97..d11802c542 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -529,7 +529,7 @@ static int fast_forward_to(struct repository *r,
 	struct strbuf err = STRBUF_INIT;
 
 	repo_read_index(r);
-	if (checkout_fast_forward(r, from, to, 1))
+	if (checkout_fast_forward(r, from, to, 1, 0))
 		return -1; /* the callee should have complained already */
 
 	strbuf_addf(&sb, _("%s: fast-forward"), _(action_name(opts)));
diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
new file mode 100755
index 0000000000..05a34cf03f
--- /dev/null
+++ b/t/t7615-merge-untracked.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='test when merge with untracked files and the option --overwrite-same-content'
+
+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 --overwrite-same-content 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 --overwrite-same-content 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 --overwrite-same-content B
+'
+
+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 --overwrite-same-content B
+'
+
+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 --overwrite-same-content B
+'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 360844bda3..1a52be723e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2257,6 +2257,10 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 	if (result) {
 		if (result->ce_flags & CE_REMOVE)
 			return 0;
+	} else if (ce && !ie_modified(o->src_index, ce, st, 0)) {
+		if(o->overwrite_same_content) {
+			return 0;
+		}
 	}
 
 	return add_rejected_path(o, error_type, name);
@@ -2264,7 +2268,8 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 
 /*
  * 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 or it has the same content
+ * than the merged file with the option --overwrite_same_content.
  */
 static int verify_absent_1(const struct cache_entry *ce,
 			   enum unpack_trees_error_types error_type,
diff --git a/unpack-trees.h b/unpack-trees.h
index efb9edfbb2..ebe4be0b35 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -71,6 +71,7 @@ struct unpack_trees_options {
 		     quiet,
 		     exiting_early,
 		     show_all_errors,
+		     overwrite_same_content,
 		     dry_run;
 	enum unpack_trees_reset_type reset;
 	const char *prefix;
-- 
2.35.1.10.g88248585b1.dirty

[PATCH v2 3/4] add configuration variable corresponding to --overwrite-same-content

[Jonathan Bressat]

Configuration variable merge.overwritesamecontent corresponding to
the --overwrite-same-content option.

This allow merge to overwrite untracked files that have the same
content, a configuration variable is interressant because some people
may want this activated as default to not have to use the option every
time

Signed-off-by: Jonathan Bressat <git.jonathan.bressat@gmail.com>
Signed-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 Documentation/config/merge.txt |  5 +++++
 builtin/merge.c                |  2 ++
 t/t7615-merge-untracked.sh     | 21 +++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/Documentation/config/merge.txt b/Documentation/config/merge.txt
index 99e83dd36e..2824dd19c7 100644
--- a/Documentation/config/merge.txt
+++ b/Documentation/config/merge.txt
@@ -89,6 +89,11 @@ merge.autoStash::
 	`--autostash` options of linkgit:git-merge[1].
 	Defaults to false.
 
+merge.overwritesamecontent::
+    When set to true, it will modify the behavior of git merge 
+    to overwrite untracked files that have the same name and 
+    content than files in the merged commit.	
+
 merge.tool::
 	Controls which merge tool is used by linkgit:git-mergetool[1].
 	The list below shows the valid built-in values.
diff --git a/builtin/merge.c b/builtin/merge.c
index fffae81068..936cb8480d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -658,6 +658,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	} else if (!strcmp(k, "merge.autostash")) {
 		autostash = git_config_bool(k, v);
 		return 0;
+	} else if (!strcmp(k,"merge.overwritesamecontent")) {
+		overwrite_same_content = git_config_bool(k, v);
 	}
 
 	status = fmt_merge_msg_config(k, v, cb);
diff --git a/t/t7615-merge-untracked.sh b/t/t7615-merge-untracked.sh
index 05a34cf03f..cfefd8f473 100755
--- a/t/t7615-merge-untracked.sh
+++ b/t/t7615-merge-untracked.sh
@@ -60,4 +60,25 @@ test_expect_success 'merge fail when tracked file modification is unstaged' '
 	test_must_fail git merge --overwrite-same-content B
 '
 
+test_expect_success 'fastforward overwrite untracked file that has the same content with the configuration variable' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	test_config merge.overwritesamecontent true &&
+	git checkout -b B &&
+	test_commit --no-tag "tracked" file "content" &&
+	git checkout A &&
+	echo content >file &&
+	git merge B
+'
+
+test_expect_success 'normal merge overwrite untracked file that has the same content with the configuration variable' '
+	test_when_finished "git branch -D B && git reset --hard init && git clean --force" &&
+	test_config merge.overwritesamecontent true &&
+	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_done
-- 
2.35.1.10.g88248585b1.dirty

[PATCH v2 4/4] error message now advice to use the new option

[Jonathan Bressat]

When all the untracked files in the working tree have the same content
than the files in merged branch then the error message advice to use
the --overwrite-same-content option.

Signed-off-by: Jonathan Bressat <git.jonathan.bressat@gmail.com>
Signed-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>
---
 builtin/checkout.c |  1 +
 unpack-trees.c     | 20 ++++++++++++++++----
 unpack-trees.h     |  1 +
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index cc804ba8e1..1b1d1813c7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -760,6 +760,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 				       &new_branch_info->commit->object.oid :
 				       &new_branch_info->oid, NULL);
 		topts.preserve_ignored = !opts->overwrite_ignore;
+		topts.overwrite_same_content = 0;/* FIXME: opts->overwrite_same_content */
 		tree = parse_tree_indirect(old_branch_info->commit ?
 					   &old_branch_info->commit->object.oid :
 					   the_hash_algo->empty_tree);
diff --git a/unpack-trees.c b/unpack-trees.c
index 1a52be723e..6c660b084b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -158,17 +158,17 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 	if (!strcmp(cmd, "checkout"))
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("The following untracked working tree files would be overwritten by checkout:\n%%s"
-			  "Please move or remove them before you switch branches.")
+			  "Please move or remove them before you switch branches.%%s")
 		      : _("The following untracked working tree files would be overwritten by checkout:\n%%s");
 	else if (!strcmp(cmd, "merge"))
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("The following untracked working tree files would be overwritten by merge:\n%%s"
-			  "Please move or remove them before you merge.")
+			  "Please move or remove them before you merge.%%s")
 		      : _("The following untracked working tree files would be overwritten by merge:\n%%s");
 	else
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("The following untracked working tree files would be overwritten by %s:\n%%s"
-			  "Please move or remove them before you %s.")
+			  "Please move or remove them before you %s.%%s")
 		      : _("The following untracked working tree files would be overwritten by %s:\n%%s");
 	msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] =
 		strvec_pushf(&opts->msgs_to_free, msg, cmd, cmd);
@@ -251,6 +251,14 @@ static void display_error_msgs(struct unpack_trees_options *o)
 {
 	int e;
 	unsigned error_displayed = 0;
+	const char *can_overwrite_msg;
+
+	if (o->can_overwrite) {
+		can_overwrite_msg = _("\nYou can also rerun the command with --overwrite-same-content to overwrite files with same content.");
+	} else {
+		can_overwrite_msg = "";
+	}
+
 	for (e = 0; e < NB_UNPACK_TREES_ERROR_TYPES; e++) {
 		struct string_list *rejects = &o->unpack_rejects[e];
 
@@ -261,7 +269,8 @@ static void display_error_msgs(struct unpack_trees_options *o)
 			error_displayed = 1;
 			for (i = 0; i < rejects->nr; i++)
 				strbuf_addf(&path, "\t%s\n", rejects->items[i].string);
-			error(ERRORMSG(o, e), super_prefixed(path.buf));
+
+			error(ERRORMSG(o, e), super_prefixed(path.buf), can_overwrite_msg);
 			strbuf_release(&path);
 		}
 		string_list_clear(rejects, 0);
@@ -1711,6 +1720,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	struct pattern_list pl;
 	int free_pattern_list = 0;
 	struct dir_struct dir = DIR_INIT;
+	o->can_overwrite = 1;
 
 	if (o->reset == UNPACK_RESET_INVALID)
 		BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
@@ -2261,6 +2271,8 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 		if(o->overwrite_same_content) {
 			return 0;
 		}
+	} else {
+		o->can_overwrite = 0;
 	}
 
 	return add_rejected_path(o, error_type, name);
diff --git a/unpack-trees.h b/unpack-trees.h
index ebe4be0b35..2be74ce5bf 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -72,6 +72,7 @@ struct unpack_trees_options {
 		     exiting_early,
 		     show_all_errors,
 		     overwrite_same_content,
+		     can_overwrite,
 		     dry_run;
 	enum unpack_trees_reset_type reset;
 	const char *prefix;
-- 
2.35.1.10.g88248585b1.dirty

Re: [PATCH v2 1/4] t6436: tests how merge behave when there is untracked file with the same content

[Matthieu Moy]

On 5/27/22 21:55, Jonathan Bressat wrote:
> add test to show explicitly that merge doesn't overwrite untracked files
> or unstaged even when they have the same content than files int the
> merged commit

Nit: capital at the beginning of the sentence, period at the end.

"untracked files or unstaged" -> "untracked or unstaged files"

> +test_expect_success 'create branch A' '
> +	git reset --hard c0 &&
> +	git checkout -b A
> +'
> +
> +test_expect_success 'fastforward will not overwrite untracked file with the same content' '

Git usually spells fast-forward with a hyphen, not fastforward.

> +	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
> +	git checkout -b B &&
> +	test_commit --no-tag "tracked" file "content" &&
> +	git checkout A &&
> +	echo content >file &&
> +	test_must_fail git merge B

Other tests in the same file test a bit more: the file mustn't be 
touched. It's a very important thing with Git: 99% of the times, when an 
operation fails, it fails before starting any change on-disk, as opposed 
to "I started messing up with your repo, I can't go further, go fix the 
mess yourself" ;-).

The way it's done is by creating a file with the content, using "cp" 
instead of "echo >" and "test_cmp" to check the content.

Other tests also check the absence of .git/MERGE_HEAD, which seems to be 
a sensible thing to do.

> +test_expect_success 'will not overwrite untracked file with the same content' '
> +	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
> +	git checkout -b B &&
> +	test_commit --no-tag "tracked" file "content" fileB "content" &&
> +	git checkout A &&
> +	test_commit --no-tag "exA" fileA "content" &&
> +	echo content >file &&
> +	test_must_fail git merge B
> +'
> +
> +test_expect_success 'will not overwrite unstaged file with the same content' '
> +	test_when_finished "git branch -D B && git reset --hard c0 && git clean --force" &&
> +	test_commit --no-tag "unstaged" file "other" &&
> +	git checkout -b B &&
> +	test_commit --no-tag "staged" file "content" &&
> +	git checkout A &&
> +	echo content >file &&
> +	test_must_fail git merge B
> +'

As discussed IRL, I think two more cases should be tested:

- index matches commit being merged, but the worktree file doesn't
- worktree file doesn't match content, but index does

in both cases, I'd expect the old and the new behavior to abort the 
merge. Perhaps there are use-cases where one would expect a successful 
merge silently, but for rare corner-cases, it's safe to ask the user to 
fix the situation manually and too much magic can only confuse the user.

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: [PATCH v2 2/4] merge with untracked file that are the same without failure

[Matthieu Moy]

On 5/27/22 21:55, Jonathan Bressat wrote:
> Keep the old behavior as default.
> 
> Add the option --overwrite-same-content, when this option is used merge
> will overwrite untracked file that have the same content.
> 
> It make the merge nicer to the user, usefull for a simple utilisation,

make_s_

usefull -> useful

utilisation -> use

> for exemple if you copy and paste files from another project and then

ex_a_mple.

> you decide to pull this project, git will not proceed even if you didn't
> modify those files.

I'd avoid saying "you" in a commit message. "the user" seems clearer to me.

Also, don't use the future to talk about the behavior before the patch, 
it's really confusing. Actually, the commit message talks about the 
previous behavior, but doesn't really document the new one.

> +--overwrite-same-content::
> +       Silently overwrite untracked files that have the same content
> +       and name than files in the merged commit from the merge result.

I don't understand what "in the merged commit from the merge result" means.

Perhaps "overwrite" is not the best name. We actually re-use the file 
without touching it.

> --- /dev/null
> +++ b/t/t7615-merge-untracked.sh

Why a new file? These are minor variants of the ones you just added in 
the previous commit, and would deserve being written next to them.

> +test_expect_success 'fastforward overwrite untracked file that has the same content' '
> +test_expect_success 'fastforward fail when untracked file has different content' '
> +test_expect_success 'normal merge overwrite untracked file that has the same content' '
> +test_expect_success 'normal merge fail when untracked file has different content' '
> +test_expect_success 'merge fail when tracked file modification is unstaged' '

We're making a lot of tests, very similar to each other and very similar 
to other existing ones. I think we've reached the point where we need to 
refactor a bit and write one generic function that covers

- index state : same / different
- worktree state : same / different
- --overwrite-untracked : present / absent
- kind of merge : fast-forward / real merge

and then call this function with the appropriate set of parameters. 
Either the function can be called within tests (each test becoming a 
one-liner), or perhaps the function can call test_expect_success and 
then we can write stg like

for index in same different
do
	for worktree in same different
	do
	...
		run_test_merge $index $worktree ....
	done
done

> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2257,6 +2257,10 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
>   	if (result) {
>   		if (result->ce_flags & CE_REMOVE)
>   			return 0;
> +	} else if (ce && !ie_modified(o->src_index, ce, st, 0)) {
> +		if(o->overwrite_same_content) {
> +			return 0;
> +		}

This looks good, but honestly I'm a bit lost between o->src_index, 
o->dst_index and o.result, so the review of someone more familiar with 
this part of the codebase would be welcome.

> + * is not tracked, unless it is ignored or it has the same content
> + * than the merged file with the option --overwrite_same_content.

"same content _as_".

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: [PATCH v2 4/4] error message now advice to use the new option

[Matthieu Moy]

On 5/27/22 21:55, Jonathan Bressat wrote:
 > Subject: Re: [PATCH v2 4/4] error message now advice to use the new 
option

Commit messages are usually written with imperative tone. The "now" 
doesn't add information, the reader can guess that the commit message 
describes the new behavior.

"suggest --overwrite-same-content in error message when appropriate" ?

> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -760,6 +760,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
>   				       &new_branch_info->commit->object.oid :
>   				       &new_branch_info->oid, NULL);
>   		topts.preserve_ignored = !opts->overwrite_ignore;
> +		topts.overwrite_same_content = 0;/* FIXME: opts->overwrite_same_content */

Why not use opts->overwrite_same_content in the code rather than saying 
you should in a comment?

Actually, doesn't this hunk belong to the previous commit?

The rest of the patch looks good to me.

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: [PATCH v2 3/4] add configuration variable corresponding to --overwrite-same-content

[Matthieu Moy]

On 5/27/22 21:55, Jonathan Bressat wrote:
> Configuration variable merge.overwritesamecontent corresponding to

We usually write them as camelCase for readability (but options are 
case-insensitive).

> +merge.overwritesamecontent::
> +    When set to true, it will modify the behavior of git merge

"When set to true, modify", the future is not needed.

> +    to overwrite untracked files that have the same name and
> +    content than files in the merged commit.	

The doc should mention the command-line option too, and say which one 
takes precedence when both are specified (as it is the case for many 
other options).

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: [PATCH v2 4/4] error message now advice to use the new option

[Guillaume Cogoni]

Hello,

Thanks for your reviews.
We will take in consideration what you say for the next version.

Sincerely,
Guillaume COGONI and Jonathan BRESSAT

On Sat, Jun 4, 2022 at 4:57 PM Matthieu Moy <Matthieu.Moy@univ-lyon1.fr> wrote:
>
> On 5/27/22 21:55, Jonathan Bressat wrote:
>  > Subject: Re: [PATCH v2 4/4] error message now advice to use the new
> option
>
> Commit messages are usually written with imperative tone. The "now"
> doesn't add information, the reader can guess that the commit message
> describes the new behavior.
>
> "suggest --overwrite-same-content in error message when appropriate" ?
>
> > --- a/builtin/checkout.c
> > +++ b/builtin/checkout.c
> > @@ -760,6 +760,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
> >                                      &new_branch_info->commit->object.oid :
> >                                      &new_branch_info->oid, NULL);
> >               topts.preserve_ignored = !opts->overwrite_ignore;
> > +             topts.overwrite_same_content = 0;/* FIXME: opts->overwrite_same_content */
>
> Why not use opts->overwrite_same_content in the code rather than saying
> you should in a comment?
>
> Actually, doesn't this hunk belong to the previous commit?
>
> The rest of the patch looks good to me.
>
> --
> Matthieu Moy
> https://matthieu-moy.fr/