Re: [PATCH v7 3/4] environment: move trust_executable_bit into repo_config_values
- From
Tian Yuchen <cat@malon.dev>
- Date
- Jul 20, 2026, 10:09 UTC
- Message-ID
- <3cd5e386-aaa4-4a96-92e1-1ed244d39c9e@malon.dev>
- In-Reply-To
- <xmqq8q7961xe.fsf@gitster.g>
On 7/18/26 00:01, Junio C Hamano wrote:
Show 42 quoted lines
> Tian Yuchen <cat@malon.dev> writes:
>
>> diff --git a/environment.c b/environment.c
>> index fc3ed8bb1c..75069a884d 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -41,7 +41,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;
>> @@ -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;
>> +}
>> +
>> int have_git_dir(void)
>> {
>> return startup_info->have_repository
>
> Two comments.
>
> * Missing SP before '?'. It may be easier to read if it is written
> like this:
>
> return repo->gitdir
> ? repo_config_values(repo)->trust_executable_bit
> : 1;
>
> which more clearly highlights the ternary structure. If you tilt
> your head 90 degrees to the left, you can almost see the parse
> tree of the expression.
> Okay.
Show 7 quoted lines
> * Does it make sense to protect against a NULL 'repo' case, as > repo_protect_ntfs() and repo_protect_hfs() helpers do? Or is it > better to crash loudly with a segfault to let the developer know > they have a bug to fix? I lean toward the latter myself, and if > we go that route, we should probably stop using 'repo && > repo->gitdir' elsewhere, rather than sweeping the problem under > the rug with defensive checks.
Sounds sensible. I will drop the checks and adjust the calls themselves instead.
Note that repo_protect_ntfs() looks like this:
int repo_protect_ntfs(struct repository *repo)
{
return (repo && repo->initialized) ?
repo_config_values(repo)->protect_ntfs :
PROTECT_NTFS_DEFAULT;
}Do we need another fixup commit for it?
Regards, yuchen