Re: [PATCH 1/2] environment: move ignore_case into repo_config_values

[Tian Yuchen <cat@malon.dev>]

On 6/18/26 01:16, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> Note that the newly introduced getter, 'repo_get_ignore_case()',
>> intentionally avoids checking 'repo->gitdir'. This could safely
>> accommodates early dynamic probing of the filesystem during
>> 'git init' or clone operations, where the 'gitdir' might not be fully
>> initialized but the filesystem capability must be recorded.
> 
> Why "could"?  It either "safely accommodates" or it doesn't.
> 

Okay, will change in the next reroll.

> I do not quite understand the logic behind this part.  Why is it OK
> to punt until .gitdir is ready for trust-executable-bit, like it is
> done in f951ed98 (environment: move trust_executable_bit into
> repo_config_values, 2026-06-13)
> 
> diff --git a/environment.c b/environment.c
> index fc3ed8bb1c..75069a884d 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -142,6 +141,13 @@ int is_bare_repository(void)
>   	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>   }
>   
> +int repo_trust_executable_bit(struct repository *repo)
> +{
> +	return repo->gitdir?
> +		repo_config_values(repo)->trust_executable_bit :
> +		1;
> +}
> +
> 
> or hfs/ntfs in 71386c21 (environment: move 'protect_hfs' and
> 'protect_ntfs' into 'repo_config_values', 2026-06-10)
> 
> diff --git a/environment.c b/environment.c
> index fc3ed8bb1c..683fe1b4d3 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -142,6 +140,20 @@ int is_bare_repository(void)
>   	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>   }
>   
> +int repo_protect_ntfs(struct repository *repo)
> +{
> +	return repo->gitdir ?
> +		repo_config_values(repo)->protect_ntfs :
> +		PROTECT_NTFS_DEFAULT;
> +}
> +
> +int repo_protect_hfs(struct repository *repo)
> +{
> +	return repo->gitdir ?
> +		repo_config_values(repo)->protect_hfs :
> +		PROTECT_HFS_DEFAULT;
> +}
> +
>   int have_git_dir(void)
>   {
>   	return startup_info->have_repository
> 
> but not for this bit?

You're right, I made a mistake.

Earlier, when I was testing using 'repo->gitdir', the CI tests failed, 
and I thought it was because the test script was forcing initial values 
too early. I just realized the error was somewhere else. I'll fix it.

> 
>> +int repo_get_ignore_case(struct repository *repo)
>> +{
>> +	if (repo)
>> +		return repo_config_values(repo)->ignore_case;
>> +	return 0;
>> +}
> 
> What makes ignore-case so special?  Doesn't the same logic apply to
> the other three bits?
> 
> Or use a more direct
> 
> 	if (repo && repo->initialized)
> 		...;
> 
> for all three, as repo_config_values(repo) barfs when repo is not
> initialized?
> 
> I dunno.

That makes some sense. The reasoning behind using 'repo->gitdir' is that 
"as long as I know where .git is, I assume the repository has been 
initialized," which may indeed be less precise than 'repo->initialized'.

Btw, I think it's better to change the getter's name from 
'repo_get_ignore_case()' to 'repo_ignore_case()'. This way, it will be 
consistent with the previous flags.

Thanks, yuchen