Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program

From: Jeff King <peff@peff.net>
Date: Jul 10, 2018, 16:54 UTC
Message-ID: <20180710165412.GE23624@sigill.intra.peff.net>
In-Reply-To: <4905c1907a866c0fd1a4dac978dd6ca3e468ac43.1531208187.git.henning.schild@siemens.com>

On Tue, Jul 10, 2018 at 10:52:29AM +0200, Henning Schild wrote:

Show 12 quoted lines

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ac373e3f4..c0bd80954 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1832,6 +1832,11 @@ gpg.format::
>  	Specifies which key format to use when signing with `--gpg-sign`.
>  	Default is "openpgp", that is also the only supported value.
>  
> +gpg.<format>.program::
> +	Use this to customize the program used for the signing format you
> +	chose. (see gpg.program) gpg.openpgp.program is a synonym for the
> +	legacy gpg.program.

This seems like a good step forward. This is similar to the signingtool.$name.program I proposed earlier, but keeping it specific to gpg, which makes sense.

On the other hand, do we anticipate the user ever being able to add gpg.foo.program? I don't think so; we'll provide a limited set of options. So we _could_ go with "gpg.openpgpProgram" or similar, and later add "gpg.x509Program".

And one reason to do so might be...

Show 12 quoted lines

> diff --git a/gpg-interface.c b/gpg-interface.c
> index ac2df498d..65098430f 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -179,7 +179,7 @@ int git_gpg_config(const char *var, const char *value, void *cb)
>  		return git_config_string(&gpg_format, var, value);
>  	}
>  
> -	if (!strcmp(var, "gpg.program"))
> +	if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program"))
>  		return git_config_string(&gpg_formats[PGP_FMT].program, var,
>  					 value);

We normally match config keys with strcmp() because the config machinery will have already normalized them to lowercase. But in Git's config format, the subsection (the middle in a three-dot name) is less restricted and is case-sensitive.

Should we allow:

  [gpg "OpenPGP"]
  program = whatever

given that we allow:

  [gpg]
  format = OpenPGP

? I think just using strcasecmp() here would be sufficient. But I wonder if it is a symptom of using the wrong tool (subsections) when we don't need it.

-Peff