Re: [PATCH 4/5] merge: verify signatures if gpg.verifySignatures is true

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

Hans Jerry Illikainen <hji@dyntopia.com> writes:

> @@ -610,6 +610,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
>  		show_diffstat = git_config_bool(k, v);
>  	else if (!strcmp(k, "merge.verifysignatures"))
>  		verify_signatures = git_config_bool(k, v);
> +	else if (!strcmp(k, "gpg.verifysignatures") && verify_signatures < 0)
> +		verify_signatures = git_config_bool(k, v);

Hmm, the next person who attempts to generalize the mechanism
further would have a hard time introducing a fallback configuration
that is even common than "gpg" when s/he has to start with this
code, no?  That is, this patch introduced "gpg.verifysignatures is
used when merge.verifysignatures is not defined" and with the two
level override the code works OK, but to implement "if neither gpg.*
or merge.* is defined, common.verifysignatures is used instead", the
above part needs to be dismantled and redone.

Keeping the "initialize verify_signatures to -1 (unspecified)" from
this patch, setting a separate gpg_verify_signatures variable upon
seeing gpg.verifysignatures, and consolidating the final finding
after git_config(git_merge_config, NULL) returns into verify_signatures
like so:

	init_diff_ui_defaults();
	git_config(git_merge_config, NULL);

+	if (verify_signatures < 0)
+		verify_signatures = (0 <= gpg_verify_signatures) 
+				  ? gpg_verify_signatures 
+				  : 0;

would be more in line with the way we arrange multiple configuration
variables to serve as fallback defaults.  And that is more easily
extensible.

Also with such an arrangement, "if (verify_signatures == 1)" we see
below will become unnecessary, which is another plus.

Thanks.

>  	else if (!strcmp(k, "pull.twohead"))
>  		return git_config_string(&pull_twohead, k, v);
>  	else if (!strcmp(k, "pull.octopus"))
> @@ -1399,7 +1401,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		if (remoteheads->next)
>  			die(_("Can merge only exactly one commit into empty head"));
>  
> -		if (verify_signatures &&
> +		if (verify_signatures == 1 &&
>  		    gpg_verify_commit(&remoteheads->item->object.oid, NULL,
>  				      NULL, gpg_flags))
>  			die(_("Signature verification failed"));
> @@ -1423,7 +1425,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		usage_with_options(builtin_merge_usage,
>  			builtin_merge_options);
>  
> -	if (verify_signatures) {
> +	if (verify_signatures == 1) {
>  		for (p = remoteheads; p; p = p->next) {
>  			if (gpg_verify_commit(&p->item->object.oid, NULL, NULL,
>  					      gpg_flags))