Re: [PATCH v2 1/8] environment: move "trust_ctime" into `struct repo_config_values`

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 14, 2026, 17:15 UTC
Message-ID: <xmqqa4v5bgzn.fsf@gitster.g>
In-Reply-To: <53f43b85-b274-4352-938b-d40f942bfb2d@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 15 quoted lines

> On 14/04/2026 09:52, Karthik Nayak wrote:
> ...
>> Nit: I was hoping you'd also shed light on why this can go into
>> `repo_config_values()`. Does it need to be eagerly parsed? If so, why?
>
> If trust_ctime was lazily parsed where it is used we'd end up dying in 
> match_stat_data() which would be quite unexpected, make it very hard to 
> reason about the code, and hamper the libification efforts. I'd much 
> rather we put the onus on patch authors to justify any conversion from 
> eager parsing to lazy parsing rather than forcing them to justify 
> continuing to parse settings eagerly.
>
> Thanks
>
> Phillip

I too often get confused while looking at these "global static variables holding parsed configuration values are bad, let's move it elsewhere" patches between the on-demand and upfront parsing.

I agree that what has traditionally been parsed upfront are mostly fundamental things (e.g., in core.* namespace) that is better parsed upfront, and what has been parsed on-demand are often very operation specific thing whose misspelt values do not matter when we are not running that specific operation, so it is better parsed on-demand. There may be exceptions and some variables that the current code parses upfront might be better parsed on-demand and vice versa, but the default for these rewrite effort ought to be to keep the existing semantics unless there is a good justification for changing it.

Thanks for injecting a dose of sanity so clearly.