Re: [PATCH v2 1/8] environment: move "trust_ctime" into `struct repo_config_values`
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Apr 14, 2026, 09:35 UTC
- Message-ID
- <53f43b85-b274-4352-938b-d40f942bfb2d@gmail.com>
- In-Reply-To
- <CAOLa=ZTD+qqgyB4Pn4bcOfP+Ks8Zch+AWZkzhrRRbk-eJvS-mg@mail.gmail.com>
On 14/04/2026 09:52, Karthik Nayak wrote:
Show 16 quoted lines
> Olamide Caleb Bello <belkid98@gmail.com> writes: > >> The `core.trustctime` configuration is currently stored in the global >> variable `trust_ctime`, which makes it shared across repository >> instances in a single process. >> >> Store it instead in `repo_config_values`, so the value is tied to the >> repository from which it was read. This preserves existing behavior >> while avoiding cross-repository state leakage and continues the effort >> to reduce reliance on global configuration state. >> >> Update all references to use repo_config_values(). >> > > 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
Show 51 quoted lines
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
>> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
>> ---
>> environment.c | 4 ++--
>> environment.h | 2 +-
>> statinfo.c | 6 ++++--
>> 3 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/environment.c b/environment.c
>> index fc3ed8bb1c..0a9067729e 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -42,7 +42,6 @@ static int pack_compression_seen;
>> static int zlib_compression_seen;
>>
>> int trust_executable_bit = 1;
>> -int trust_ctime = 1;
>> int check_stat = 1;
>> int has_symlinks = 1;
>> int minimum_abbrev = 4, default_abbrev = -1;
>> @@ -309,7 +308,7 @@ int git_default_core_config(const char *var, const char *value,
>> return 0;
>> }
>> if (!strcmp(var, "core.trustctime")) {
>> - trust_ctime = git_config_bool(var, value);
>> + cfg->trust_ctime = git_config_bool(var, value);
>> return 0;
>> }
>> if (!strcmp(var, "core.checkstat")) {
>> @@ -721,4 +720,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
>> cfg->attributes_file = NULL;
>> cfg->apply_sparse_checkout = 0;
>> cfg->branch_track = BRANCH_TRACK_REMOTE;
>> + cfg->trust_ctime = 1;
>> }
>> diff --git a/environment.h b/environment.h
>> index 123a71cdc8..64d537686e 100644
>> --- a/environment.h
>> +++ b/environment.h
>> @@ -91,6 +91,7 @@ struct repo_config_values {
>> /* section "core" config values */
>> char *attributes_file;
>> int apply_sparse_checkout;
>> + int trust_ctime;
>>
>
> Since we parse it as a bool, perhaps we can make the variable to be of
> type bool?
>
> [snip]