Re: [PATCH v5 0/2] attr: add attr.tree config

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 13, 2023, 20:47 UTC
Message-ID: <xmqqzg0mxnaj.fsf@gitster.g>
In-Reply-To: <xmqqa5smz2lk.fsf@gitster.g>

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

> So in a sense, for !!ignore_bad_attr_tree case, the code ends up
> doing the right thing.  But if !ignore_bad_attr_tree is true, i.e.,
> a blob object name is given via --attr-source or GIT_ATTR_SOURCE,
> then the bug will be uncovered.

Having said all that, I suspect that this problem is not new and certainly not caused by this topic. We should have unconditionally died when GIT_ATTR_SOURCE gave a blob object name, but pretended as if an empty tree was given. There may even be existing users who now assume that is working as intended and depend on this bug.

So, let's leave it as a "possible bug" that we might want to fix in the future, outside the scope of this series.

Thanks.

Show 26 quoted lines

>  t/t0003-attributes.sh | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git c/t/t0003-attributes.sh w/t/t0003-attributes.sh
> index ecf43ab545..0f02f22171 100755
> --- c/t/t0003-attributes.sh
> +++ w/t/t0003-attributes.sh
> @@ -394,6 +394,18 @@ test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success '--attr-source that points at a non-treeish' '
> +	test_when_finished rm -rf empty &&
> +	git init empty &&
> +	(
> +		cd empty &&
> +		echo "$bad_attr_source_err" >expect_err &&
> +		H=$(git hash-object -t blob --stdin -w </dev/null) &&
> +		test_must_fail git --attr-source=$H check-attr test -- f/path 2>err &&
> +		test_cmp expect_err err
> +	)
> +'
> +
>  test_expect_success 'precedence of --attr-source, GIT_ATTR_SOURCE, then attr.tree' '
>  	test_when_finished rm -rf empty &&
>  	git init empty &&