Re: [PATCH v2] gpg-interface: lazily initialize and read the configuration

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sun, Feb 26 2023, Jeff King wrote:

> On Thu, Feb 09, 2023 at 12:24:14PM -0800, Junio C Hamano wrote:
>
>> Instead of forcing the porcelain commands to always read the
>> configuration variables related to the signing and verifying
>> signatures, lazily initialize the necessary subsystem on demand upon
>> the first use.
>> 
>> This hopefully would make it more future-proof as we do not have to
>> think and decide whether we should call git_gpg_config() in the
>> git_config() callback for each command.
>
> Sorry, I seem to have missed this when you originally posted it. And I
> saw it marked as "will merge to next?" in the latest what's cooking. It
> looks good to me, and I think we can proceed with it (though of course
> it is not urgent and can probably wait until post-2.40).
>
>> A few git_config() callback functions that used to be custom
>> callbacks are now just a thin wrapper around git_default_config().
>> We could further remove, git_FOO_config and replace calls to
>> git_config(git_FOO_config) with git_config(git_default_config), but
>> to make it clear which ones are affected and the effect is only the
>> removal of git_gpg_config(), it is vastly preferred not to do such a
>> change in this step (they can be done on top once the dust settled).
>
> Yes, I think it is good not to do so in this patch. If we want to do it
> now on top, here's a patch. Though I could also see the argument for
> just leaving them.
>
> -- >8 --
> Subject: [PATCH] drop pure pass-through config callbacks
>
> Commit fd2d4c135e (gpg-interface: lazily initialize and read the
> configuration, 2023-02-09) shrunk a few custom config callbacks so that
> they are just one-liners of:
>
>   return git_default_config(...);
>
> We can drop them entirely and replace them direct calls of
> git_default_config() intead. This makes the code a little shorter and
> easier to understand (with the downside being that if they do grow
> custom options again later, we'll have to recreate the functions).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I looked over the output of:
>
>   git grep --function-context 'return git_default_config'
>
> to see if there were other cases, not caused by fd2d4c135e. But I didn't
> see any.

As added review: This is the same patch diff as I sent on February 9th:
https://lore.kernel.org/git/patch-1.2-d93c160dcbc-20230209T142225Z-avarab@gmail.com/;
my local range-diff to my previously submitted topic & next being:
	
	229:  cc5d1d32fd4 !   2:  d93c160dcbc drop pure pass-through config callbacks
	    @@
	      ## Metadata ##
	    -Author: Jeff King <peff@peff.net>
	    +Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
	
	      ## Commit message ##
	    -    drop pure pass-through config callbacks
	    +    {am,commit-tree,verify-{commit,tag}}: refactor away config wrapper
	
	    -    Commit fd2d4c135e (gpg-interface: lazily initialize and read the
	    -    configuration, 2023-02-09) shrunk a few custom config callbacks so that
	    -    they are just one-liners of:
	    +    In the preceding commit these config functions became mere wrappers
	    +    for git_default_config(), so let's invoke it directly instead.
	
	    -      return git_default_config(...);
	    -
	    -    We can drop them entirely and replace them direct calls of
	    -    git_default_config() intead. This makes the code a little shorter and
	    -    easier to understand (with the downside being that if they do grow
	    -    custom options again later, we'll have to recreate the functions).
	    -
	    -    Signed-off-by: Jeff King <peff@peff.net>
	    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
	    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
	
	      ## builtin/am.c ##  
	     @@ builtin/am.c: static int parse_opt_show_current_patch(const struct option *opt, const char *ar
	  -:  ----------- >   3:  c099d48b4bf gpg-interface.c: lazily get GPG config variables on demand

So this LGTM.