Re: [PATCH v2 1/2] attr: add attr.tree for setting the treeish to read attributes from

From: Jeff King <peff@peff.net>
Date: Oct 5, 2023, 17:07 UTC
Message-ID: <20231005170703.GA975921@coredump.intra.peff.net>
In-Reply-To: <xmqqfs2qp3bg.fsf@gitster.g>

On Wed, Oct 04, 2023 at 12:58:43PM -0700, Junio C Hamano wrote:

Show 22 quoted lines

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

-Peff