Re: [PATCH v2] Make 'trust_executable_bit' repository-scoped
- From
- Dronaraj Gyawali <dronarajgyawali@gmail.com>
- Date
- Mar 9, 2026, 17:51 UTC
- Message-ID
- <CAJtK1FO56BhCo7DgtFVgMRi9yNv92_jV1i1LfEx_G2uauR+jnw@mail.gmail.com>
- In-Reply-To
- <xmqq1pht6nyx.fsf@gitster.g>
Hi Junio,
I have sent a new updated series of changes that aligns with the actual code structure. There was some confusion regarding work flow by my side. I am still learning..
Thanks for earlier review and guidance.
Best regards, dorna
On Mon, 9 Mar 2026 at 20:52, Junio C Hamano <gitster@pobox.com> wrote:
Show 52 quoted lines
>
> 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.
>
> >>> 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.
>