From: John Cai <johncai86@gmail.com>
Date: Thu, 05 Oct 2023 19:46:46 GMT
Subject: Re: [PATCH v2 1/2] attr: add attr.tree for setting the treeish to read attributes from
Message-ID: <BC4BEE47-2BE1-44F8-B368-6E858D98D416@gmail.com>
In-Reply-To: <20231005170703.GA975921@coredump.intra.peff.net>

Hi Peff,

On 5 Oct 2023, at 13:07, Jeff King wrote:

> On Wed, Oct 04, 2023 at 12:58:43PM -0700, Junio C Hamano wrote:
>
>> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> From: John Cai <johncai86@gmail.com>
>>>
>>> 44451a2e5e (attr: teach "--attr-source=<tree>" global option to "git",
>>> 2023-05-06) provided the ability to pass in a treeish as the attr
>>> source. In the context of serving Git repositories as bare repos like we
>>> do at GitLab however, it would be easier to point --attr-source to HEAD
>>> for all commands by setting it once.
>>>
>>> Add a new config attr.tree that allows this.
>>
>> Hmph, I wonder if we want to go all the way to emulate how the
>> mailmap.blob was done, including
>>
>>  - Default the value of attr.tree to HEAD in a bare repository;
>>
>>  - Notice but ignore errors if the attr.tree does not point at a
>>    tree object, and pretend as if attr.tree specified an empty tree;
>>
>> which does not seem to be in this patch.  With such a change,
>> probably we do not even need [2/2] of the series, perhaps?
>
> Oh good, this was exactly what I was going to write in a review, so now
> I don't have to. :)
>
> Even though it creates behavior differences between attr.tree and
> --attr-source, I think that is justifiable. Config options apply across
> a wider set of contexts, so loosening the error handling may make sense
> where it would not for a command-line option.

Makes sense. Will adjust in the next version.

> But we should document both that, and also how the two interact (I assume
> "git --attr-source" would override attr.tree completely).

Yes, --attr-source would take precedence over attr.tree

>
> -Peff

thanks!
John