Subject: Re: [PATCH v5 07/18] merge-tree tests: test for the mode comparison in same_entry()

From: Junio C Hamano <[email-masked]>
Date: Wed, 31 Mar 2021 16:12:33 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Add a test to stress the "a->mode == b->mode" comparison in
> merge-tree.c's same_entry().
>
> That code was initially added by Linus in 33deb63a36f (Add
> "merge-tree" helper program. Maybe it's retarded, maybe it's helpful.,
> 2005-04-14), and then again in its current form in
> 492e0759bfe (Handling large files with GIT, 2006-02-14).
>
> However, nothing was testing that we handled this case
> correctly. Simply removing the mode comparison left all tests passing,
> but as seen here it's important that we don't think a path with the
> same content but different modes is the same_entry().

Good thinking.

> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  t/t4300-merge-tree.sh | 44 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
> index e59601e5fe9..f783d784d02 100755
> --- a/t/t4300-merge-tree.sh
> +++ b/t/t4300-merge-tree.sh
> @@ -40,6 +40,25 @@ test_expect_success 'file add A, B (same)' '
>  	test_must_be_empty actual
>  '
>  
> +test_expect_success 'file add A, B (different mode)' '
> +	git reset --hard initial &&
> +	test_commit "add-a-b-same-diff-mode-A" "ONE" "AAA" &&
> +	git reset --hard initial &&
> +	echo AAA >ONE &&
> +	test_chmod +x ONE &&
> +	test_tick &&
> +	git commit -m"add-a-b-same-diff-mode-B" &&
> +	git tag "add-a-b-same-diff-mode-B" HEAD &&
> +	git merge-tree initial add-a-b-same-diff-mode-A add-a-b-same-diff-mode-B >actual &&
> +	cat >expected <<EXPECTED &&
> +added in both
> +  our    100644 $(git rev-parse add-a-b-same-diff-mode-A:ONE) ONE
> +  their  100755 $(git rev-parse add-a-b-same-diff-mode-B:ONE) ONE
> +EXPECTED

Can we HT-indent the here text with "<<-EOF", unless existing tests
in the same script predate the more recent convention?

    ... goes and looks and realizes it is full of unindented
    expectation ...

Nah, let's keep it that way.

> +	test_cmp expected actual
> +'
> +
>  test_expect_success 'file add A, B (different)' '
>  	git reset --hard initial &&
>  	test_commit "add-a-b-diff-A" "ONE" "AAA" &&
> @@ -61,6 +80,31 @@ EXPECTED
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'file add A, B (different and different mode)' '
> +	git reset --hard initial &&
> +	test_commit "add-a-b-diff-diff-mode-A" "ONE" "AAA" &&
> +	git reset --hard initial &&
> +	echo BBB >ONE &&
> +	test_chmod +x ONE &&
> +	test_tick &&
> +	git commit -m"add-a-b-diff-diff-mode-B" &&
> +	git tag "add-a-b-diff-diff-mode-B" &&
> +	git merge-tree initial add-a-b-diff-diff-mode-A add-a-b-diff-diff-mode-B >actual &&
> +	cat >expected <<EXPECTED &&
> +added in both
> +  our    100644 $(git rev-parse add-a-b-diff-diff-mode-A:ONE) ONE
> +  their  100755 $(git rev-parse add-a-b-diff-diff-mode-B:ONE) ONE
> +@@ -1 +1,5 @@
> ++<<<<<<< .our
> + AAA
> ++=======
> ++BBB
> ++>>>>>>> .their
> +EXPECTED

Nice.  So the mode-only conflict and mode plus contents conflict are
both shown in a sensible way.

Looks good.
Thanks.