Re: [PATCH] attr: attr.allowInvalidSource config to allow invalid revision

> On Thu, Sep 21, 2023 at 01:52:52AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> In an empty repository, "git log" will die anyway. So I think the more
>>> interesting case is "I have a repository with stuff in it, but HEAD
>>> points to an unborn branch". So:
>>>
>>>   git --attr-source=HEAD diff foo^ foo
>>
>> This still looks like a made-up example.  Who in the right mind
>> would specify HEAD when both of the revs involved in the operation
>> are from branch 'foo'?  The history of HEAD may not have anything
>> common with the operand of the operation 'foo' (or its parent), or
>> worse, it may not even exist.
>
> I think it's unlikely for a user to write that. But if you are running a
> server full of bare repositories that does diffs, you might end up with
> a script that sticks "--attr-source=HEAD" at the beginning of every
> command.
>
> It is true that HEAD may not be related. But that is what we use if you
> are in a non-bare repository and run "git diff foo^ foo".
>
> Arguably:
>
>   git --attr-source=$to diff $from $to
>
> is a better default for this command. But something like "git log -p" is
> trickier, as you have many commits to show. You can try to use the tip
> of the traversal, but there may be multiple. Using the attributes from
> the destination of each commit is the most likely to avoid divergence
> between the attributes and the code, but it may also not be what people
> want. Using the modern attributes from the working tree often makes
> showing historical commits much nicer.
>
> So I dunno. I could see arguments in both directions, but I think in
> general people have been OK with pulling attributes from the working
> tree. And --attr-source=HEAD is the bare equivalent.
>
>> But your "in this repository we never trust attributes from working
>> tree, take it instead from this file or from this blob" example does
>> make a lot more sense as a use case.
>
> I don't know that it was my example. :) But yes, if you do
> "--attr-source=$to", you're overriding even the non-bare case. That may
> be what you want for some cases, but as above, I think it's hard to
> apply consistently (or even what you'd want for the general case).
>
>>> And there you really are saying "if there are attributes in HEAD, use
>>> them; otherwise, don't worry about it". This is exactly what we do with
>>> mailmap.blob: in a bare repository it is set to HEAD by default, but if
>>> HEAD does not resolve, we just ignore it (just like a HEAD that does not
>>> contain a .mailmap file). And those match the non-bare cases, where we'd
>>> read those files from the working tree instead.
>>
>> "HEAD" -> "HEAD:.mailmap" if I recall correctly.
>
> True, yeah. We can't do that here because attributes are spread across
> the tree. So all my mentions of attr.blob would really be attr.tree.
>
>> And if HEAD does not resolve, we pretend as if HEAD is an empty
>> tree-ish (hence HEAD:.mailmap is missing).  It becomes very tempting
>> to do the same for the attribute sources and treat unborn HEAD as if
>> it specifies an empty tree-ish, without any configuration or an
>> extra option.
>>
>> Such a change would be an end-user observable behaviour change, but
>> nobody sane would be running "git --attr-source=HEAD diff HEAD^ HEAD"
>> to check and detect an unborn HEAD for its error exit code, so I do
>> not think it is a horribly wrong thing to do.
>
> Yeah, that is basically what I am proposing. It sounds from the
> discussion here that there are two interesting cases:
>
>   1. You want to use --attr-source=HEAD because you are trying to make a
>      bare repo behave like a non-bare one. You probably want the "don't
>      complain if it is missing" behavior.

>
>   2. You are trying to use the attributes from one side of the diff to
>      override the worktree ones (because the two trees are unrelated).
>      In which case it does not really matter if --attr-source complains,
>      because the diff will likewise complain if the tree cannot be
>      resolved.
>
> Just trying to play devil's advocate, though, I guess you could run a
> non-diff operation like say:
>
>   git --attr-source=my-branch check-attr foo
>
> and then you probably _do_ want to know if that source was ignored or
> typo'd.
>
>> But again, as you said, --attr-source=<tree-ish> does not sound like
>> a good fit for bare-repository hosted environment and a tentative
>> hack waiting for a proper attr.blob support, or something like that,
>> to appear.
>
> I think folks mentioned mailmap.blob in the original discussion for
> --attr-source. I don't remember why the patch went with --attr-source
> there. Maybe John can speak to that.