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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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);
>  }