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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.