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

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> One thing left un-noted here is that this will defer any errors in the
> config now until use (or lazy init), so e.g.:
>
> 	git -c gpg.mintrustlevel=bad show --show-signature
>
> Used to exit with code 128 and an error, but will now (at least for me)
> proceed to run show successfully.

That one is probably a good thing.  We shouldn't interrupt users for
a misspelt configuration value that the user is not using.

>> @@ -632,6 +644,8 @@ int check_signature(struct signature_check *sigc,
>>  	struct gpg_format *fmt;
>>  	int status;
>>  
>> +	gpg_interface_lazy_init();
>> +
>>  	sigc->result = 'N';
>>  	sigc->trust_level = -1;
>
> This is needed, because we need "configured_min_trust_level" populated.

I specifically did not want anybody to start doing this line of
analysis, because it will add unnecessary bugs, and also introduce
maintenance problems.  You may be able to grab the current state of
the code, but that will get stale and won't be a good guide to keep
our code robust.

>> @@ -695,11 +709,13 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct
>>  
>>  void set_signing_key(const char *key)
>>  {
>> +	gpg_interface_lazy_init();
>> +
>>  	free(configured_signing_key);
>>  	configured_signing_key = xstrdup(key);
>>  }
>
> But this is not, we could say that we're doing it for good measure, but
> it's hard to imagine a scenario where we would end up actually needing
> lazy init here. we'll just set a variable here, which...

And especially this one, we must have init or we'll be incorrect, I
think.  There is a direct set_signing_key() caller (I think in "git
tag") that does not come from the git_config() callback route.
Without the lazy initialization, we'd get configured_signing_key
from the config because early in the start-up sequence of the
command we would do git_gpg_config() via git_config(), and then try
to process "-u keyid" by calling this one again.

If you forget to lazily initialize here, configured_signing_key gets
the keyid obtained via "-u keyid", and then when control reaches the
real signing function, we'd realize that we still haven't
initialized ourselves.  And we call lazy init, which finds
configured keyID, which is very likely different from "-u keyid"
(the user would not be passing "-u keyid" from the command line to
override, if that is the same as the configured one), and clobber
it.

Thanks.