From: Junio C Hamano Date: Tue, 14 Apr 2026 17:15:40 GMT Subject: Re: [PATCH v2 1/8] environment: move "trust_ctime" into `struct repo_config_values` Message-ID: In-Reply-To: <53f43b85-b274-4352-938b-d40f942bfb2d@gmail.com> Phillip Wood 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.