From: Dronaraj Gyawali <dronarajgyawali@gmail.com>
Date: Mon, 09 Mar 2026 17:51:36 GMT
Subject: Re: [PATCH v2] Make 'trust_executable_bit' repository-scoped
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:
>
> 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.
>