Hi Junio, On 13 Oct 2023, at 16:47, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> 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. That sounds good--let's fix in a separate series. Thanks for the careful review! thanks John > > >> 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 &&