Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
Jeff King writes: > On Mon, Jul 16, 2018 at 02:56:34PM -0700, Junio C Hamano wrote: > >> >> I'm okay with us forcing "openpgp". That seems sane enough for now, and >> >> if people scream loudly, we can loosen it. >> > >> > Well, I specifically meant "are you sure subsections like this are a >> > good idea". But it seems like people think so? >> >> I admit that I did not even consider that there may be better tool >> than using subsections to record this information. What are the >> possibilities you have in mind (if you have one)? > > I don't think there is another tool except two-level options, like > "gpg.openpgpprogram" and "gpg.x509program". > > Although those are a bit ugly, I just wondered if they might make things > simpler, since AFAIK we are not planning to add more config options > here. Like gpg.x509.someotherflag, nor gpg.someothertool.program. > > Of course one reason _for_ the tri-level is that we might one day add > gpg.x509.someotherflag, and this gives us room to do it with less > awkwardness (i.e., a proliferation of gpg.x509someflag options). Yes, and signingtool.. is probably a good (ultra-)long term direction. Preparing the code may be quite a lot of work that nobody may be interested in, and nothing other than the GPG family might materialize for a long time, but if we can cheaply prepare external interface less dependent on GPG/PGP, that by itself would be a good thing to have, I would guess. Thanks.
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
On Mon, Jul 16, 2018 at 02:56:34PM -0700, Junio C Hamano wrote: > >> I'm okay with us forcing "openpgp". That seems sane enough for now, and > >> if people scream loudly, we can loosen it. > > > > Well, I specifically meant "are you sure subsections like this are a > > good idea". But it seems like people think so? > > I admit that I did not even consider that there may be better tool > than using subsections to record this information. What are the > possibilities you have in mind (if you have one)? I don't think there is another tool except two-level options, like "gpg.openpgpprogram" and "gpg.x509program". Although those are a bit ugly, I just wondered if they might make things simpler, since AFAIK we are not planning to add more config options here. Like gpg.x509.someotherflag, nor gpg.someothertool.program. Of course one reason _for_ the tri-level is that we might one day add gpg.x509.someotherflag, and this gives us room to do it with less awkwardness (i.e., a proliferation of gpg.x509someflag options). -Peff
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
Jeff King writes: > On Sat, Jul 14, 2018 at 06:13:47PM +, brian m. carlson wrote: > >> On Tue, Jul 10, 2018 at 12:56:38PM -0400, Jeff King wrote: >> > On Tue, Jul 10, 2018 at 12:54:13PM -0400, Jeff King wrote: >> > >> > > 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. >> > >> > I did just read the discussion in response to v1, where everybody told >> > you the opposite. ;) >> > >> > So I guess my question/points are more for brian and Junio. >> >> I'm okay with us forcing "openpgp". That seems sane enough for now, and >> if people scream loudly, we can loosen it. > > Well, I specifically meant "are you sure subsections like this are a > good idea". But it seems like people think so? I admit that I did not even consider that there may be better tool than using subsections to record this information. What are the possibilities you have in mind (if you have one)? > > If so, then I think the best route is just dictating that yes, > gpg.format is case-sensitive because it is referencing > gpg..program, which is itself case-sensitive (and "openpgp" is > the right spelling). > > -Peff
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
On Sat, Jul 14, 2018 at 06:13:47PM +, brian m. carlson wrote: > On Tue, Jul 10, 2018 at 12:56:38PM -0400, Jeff King wrote: > > On Tue, Jul 10, 2018 at 12:54:13PM -0400, Jeff King wrote: > > > > > 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. > > > > I did just read the discussion in response to v1, where everybody told > > you the opposite. ;) > > > > So I guess my question/points are more for brian and Junio. > > I'm okay with us forcing "openpgp". That seems sane enough for now, and > if people scream loudly, we can loosen it. Well, I specifically meant "are you sure subsections like this are a good idea". But it seems like people think so? If so, then I think the best route is just dictating that yes, gpg.format is case-sensitive because it is referencing gpg..program, which is itself case-sensitive (and "openpgp" is the right spelling). -Peff
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
On Tue, Jul 10, 2018 at 12:56:38PM -0400, Jeff King wrote: > On Tue, Jul 10, 2018 at 12:54:13PM -0400, Jeff King wrote: > > > 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. > > I did just read the discussion in response to v1, where everybody told > you the opposite. ;) > > So I guess my question/points are more for brian and Junio. I'm okay with us forcing "openpgp". That seems sane enough for now, and if people scream loudly, we can loosen it. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
Replies to this one have been ignored for v3. I do not know how to proceed here. Henning Am Tue, 10 Jul 2018 10:52:29 +0200 schrieb Henning Schild : > Supporting multiple signing formats we will have the need to > configure a custom program each. Add a new config value to cater for > that. > > Signed-off-by: Henning Schild > --- > Documentation/config.txt | 5 + > gpg-interface.c | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > > 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..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. > + > gui.commitMsgWidth:: > Defines how wide the commit message window is in the > linkgit:git-gui[1]. "75" is the default. > 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(_format, var, value); > } > > - if (!strcmp(var, "gpg.program")) > + if (!strcmp(var, "gpg.program") || !strcmp(var, > "gpg.openpgp.program")) return > git_config_string(_formats[PGP_FMT].program, var, value); > return 0;
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
Jeff King writes: > Should we allow: > > [gpg "OpenPGP"] > program = whatever > > given that we allow: > > [gpg] > format = OpenPGP If the latter is allowed then we should allow the former. But because allowing the former is cumbersome, we may be better off not parsing the value case-insensitively like an earlier step in this series did.
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
On Tue, Jul 10, 2018 at 12:54:13PM -0400, Jeff King wrote: > 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. I did just read the discussion in response to v1, where everybody told you the opposite. ;) So I guess my question/points are more for brian and Junio. -Peff
Re: [PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
On Tue, Jul 10, 2018 at 10:52:29AM +0200, Henning Schild wrote: > 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..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... > 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(_format, var, value); > } > > - if (!strcmp(var, "gpg.program")) > + if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program")) > return git_config_string(_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
[PATCH v2 7/9] gpg-interface: introduce new config to select per gpg format program
Supporting multiple signing formats we will have the need to configure a custom program each. Add a new config value to cater for that. Signed-off-by: Henning Schild --- Documentation/config.txt | 5 + gpg-interface.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) 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..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. + gui.commitMsgWidth:: Defines how wide the commit message window is in the linkgit:git-gui[1]. "75" is the default. 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(_format, var, value); } - if (!strcmp(var, "gpg.program")) + if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program")) return git_config_string(_formats[PGP_FMT].program, var, value); return 0; -- 2.16.4
