Re: [PATCH v2] Make 'trust_executable_bit' repository-scoped
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 9, 2026, 15:07 UTC
- Message-ID
- <xmqq1pht6nyx.fsf@gitster.g>
- In-Reply-To
- <6e3d373f2f41232ca9015c39ae0ea67d@purelymail.com>
cat@malon.dev writes:
>> Hi drona, >> >> Thanks for the update! Just a quick heads-up: it looks like >> you forgot to CC Junio (gitster@pobox.com) on this iteration.
No strong need to Cc the maintainer when the patch is not ready to be applied, even though it may be nice. I'll be seeing it either way as I rarely look at my mailbox and use the mailing list archive at lore.kernel.org my primary source of Git patches anyway.
There were discussions on pros and cons moving global recipients of configuration values into a dynamically allocated strucrure, which can change when they are parsed and when bad values in them result in warnings, depending on the way the change is done, and excellent pieces of advice have been given by Phillip Wood. If anything, a change like this should ask for input from him.
>> Additionally, I think it's a good practice to respond to >> reviews before sending new patches.
Absolutely.
Show 22 quoted lines
>>> if (!strcmp(var, "core.filemode")) {
>>> + prepare_repo_settings(the_repository);
>>> the_repository->settings.trust_executable_bit =
>>> git_config_bool(var, value);
>>> return 0;
>>> }
>>
>> Regarding the code, calling 'prepare_repo_settings()' inside
>> 'git_default_core_config()' defeats the purpose of lazy-loading,
>> doesn't it?
>>
>> if (!strcmp(var, "core.filemode")) {
>> prepare_repo_settings(the_repository);
>> the_repository->settings.trust_executable_bit = git_config_bool(var,
>> value);
>> return 0;
>> }
>>
>> I think the standard practice is to drop the variable from
>> 'environment.c' completely and read it directly inside
>> 'repo-settings.c: prepare_repo_settings()' using
>> 'repo_config_get_bool()'.This "v2" applies to a mythical codebase where trust_executable_bit is somehow a member in the settings structure, which I do not think we have.