Re: [PATCH v2] gpg-interface: lazily initialize and read the configuration
On Sun, Feb 26 2023, Jeff King wrote:
Show 50 quoted lines
> 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 demandSo this LGTM.
Ævar Arnfjörð Bjarmason <avarab@gmail.com>