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

[Phillip Wood <phillip.wood123@gmail.com>]

On 14/04/2026 09:52, Karthik Nayak wrote:
> 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

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