{"thread":{"id":"48876","subject":"[PATCH v3 2/7] t/t7510: check the validation of the new config gpg.format","startedAt":"2018-07-13T08:41:48Z","lastAt":"2018-07-17T12:50:31Z","messageCount":19,"participants":["Henning Schild","Junio C Hamano","Jeff King","brian m. carlson"],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"352454","messageId":"e3fc5449ef1fe1d107e11c77f2e66d8d47a451db.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 2/7] t/t7510: check the validation of the new config gpg.format","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:24Z","receivedAt":"2018-07-13T08:41:48Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Test setting gpg.format to both invalid and valid values.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n t/t7510-signed-commit.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 6e2015ed9..1b6a1dd90 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -227,4 +227,13 @@ test_expect_success GPG 'log.showsignature behaves like --show-signature' '\n \tgrep \"gpg: Good signature\" actual\n '\n \n+test_expect_success GPG 'check config gpg.format values' '\n+\ttest_config gpg.format openpgp &&\n+\tgit commit -S --amend -m \"success\" &&\n+\ttest_config gpg.format OpEnPgP &&\n+\tgit commit -S --amend -m \"success\" &&\n+\ttest_config gpg.format malformed &&\n+\ttest_must_fail git commit -S --amend -m \"fail\"\n+'\n+\n test_done\n-- \n2.16.4\n\n"},{"id":"352457","messageId":"5e88da5eac1e1aea8a946960d455ba321e3e42b1.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 1/7] gpg-interface: add new config to select how to sign a commit","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:23Z","receivedAt":"2018-07-13T08:41:51Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Add \"gpg.format\" where the user can specify which type of signature to\nuse for commits. At the moment only \"openpgp\" is supported and the value is\nnot even used. This commit prepares for a new types of signatures.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n Documentation/config.txt | 4 ++++\n gpg-interface.c          | 7 +++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1cc18a828..ac373e3f4 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1828,6 +1828,10 @@ gpg.program::\n \tsigned, and the program is expected to send the result to its\n \tstandard output.\n \n+gpg.format::\n+\tSpecifies which key format to use when signing with `--gpg-sign`.\n+\tDefault is \"openpgp\", that is also the only supported value.\n+\n gui.commitMsgWidth::\n \tDefines how wide the commit message window is in the\n \tlinkgit:git-gui[1]. \"75\" is the default.\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 09ddfbc26..960c40086 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -7,6 +7,7 @@\n #include \"tempfile.h\"\n \n static char *configured_signing_key;\n+static const char *gpg_format = \"openpgp\";\n static const char *gpg_program = \"gpg\";\n \n #define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n@@ -138,6 +139,12 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"gpg.format\")) {\n+\t\tif (value && strcasecmp(value, \"openpgp\"))\n+\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n+\t\treturn git_config_string(&gpg_format, var, value);\n+\t}\n+\n \tif (!strcmp(var, \"gpg.program\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-- \n2.16.4\n\n"},{"id":"352459","messageId":"6fb618aefcf34695cd46b3135829b5ac3f703573.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 4/7] gpg-interface: do not hardcode the key string len anymore","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:26Z","receivedAt":"2018-07-13T08:41:54Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"gnupg does print the keyid followed by a space and the signer comes\nnext. The same pattern is also used in gpgsm, but there the key length\nwould be 40 instead of 16. Instead of hardcoding the expected length,\nfind the first space and calculate it.\nInput that does not match the expected format will be ignored now,\nbefore we jumped to found+17 which might have been behind the end of an\nunexpected string.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n gpg-interface.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 699651fd9..93bd0fb32 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -89,10 +89,11 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t\tsigc->result = sigcheck_gpg_status[i].result;\n \t\t/* The trust messages are not followed by key/signer information */\n \t\tif (sigc->result != 'U') {\n-\t\t\tsigc->key = xmemdupz(found, 16);\n+\t\t\tnext = strchrnul(found, ' ');\n+\t\t\tsigc->key = xmemdupz(found, next - found);\n \t\t\t/* The ERRSIG message is not followed by signer information */\n-\t\t\tif (sigc-> result != 'E') {\n-\t\t\t\tfound += 17;\n+\t\t\tif (*next && sigc-> result != 'E') {\n+\t\t\t\tfound = next + 1;\n \t\t\t\tnext = strchrnul(found, '\\n');\n \t\t\t\tsigc->signer = xmemdupz(found, next - found);\n \t\t\t}\n-- \n2.16.4\n\n"},{"id":"352458","messageId":"38e2eb70233709aa73345d64c94d59d4ccc681ec.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 3/7] gpg-interface: introduce an abstraction for multiple gpg formats","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:25Z","receivedAt":"2018-07-13T08:41:55Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Create a struct that holds the format details for the supported formats.\nAt the moment that is still just \"openpgp\". This commit prepares for the\nintroduction of more formats, that might use other programs and match\nother signatures.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n gpg-interface.c | 84 ++++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 63 insertions(+), 21 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 960c40086..699651fd9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -7,11 +7,46 @@\n #include \"tempfile.h\"\n \n static char *configured_signing_key;\n-static const char *gpg_format = \"openpgp\";\n-static const char *gpg_program = \"gpg\";\n+struct gpg_format {\n+\tconst char *name;\n+\tconst char *program;\n+\tconst char **extra_args_verify;\n+\tconst char **sigs;\n+};\n+\n+static const char *openpgp_verify_args[] = { \"--keyid-format=long\", NULL };\n+static const char *openpgp_sigs[] = {\n+\t\"-----BEGIN PGP SIGNATURE-----\",\n+\t\"-----BEGIN PGP MESSAGE-----\", NULL };\n+\n+static struct gpg_format gpg_formats[] = {\n+\t{ .name = \"openpgp\", .program = \"gpg\",\n+\t  .extra_args_verify = openpgp_verify_args,\n+\t  .sigs = openpgp_sigs\n+\t},\n+};\n+static struct gpg_format *current_format = &gpg_formats[0];\n+\n+static struct gpg_format *get_format_by_name(const char *str)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < ARRAY_SIZE(gpg_formats); i++)\n+\t\tif (!strcasecmp(gpg_formats[i].name, str))\n+\t\t\treturn gpg_formats + i;\n+\treturn NULL;\n+}\n \n-#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n-#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n+static struct gpg_format *get_format_by_sig(const char *sig)\n+{\n+\tint i, j;\n+\n+\tfor (i = 0; i < ARRAY_SIZE(gpg_formats); i++)\n+\t\tfor (j = 0; gpg_formats[i].sigs[j]; j++)\n+\t\t\tif (starts_with(sig, gpg_formats[i].sigs[j]))\n+\t\t\t\treturn gpg_formats + i;\n+\treturn NULL;\n+}\n \n void signature_check_clear(struct signature_check *sigc)\n {\n@@ -102,12 +137,6 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-static int is_gpg_start(const char *line)\n-{\n-\treturn starts_with(line, PGP_SIGNATURE) ||\n-\t\tstarts_with(line, PGP_MESSAGE);\n-}\n-\n size_t parse_signature(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n@@ -115,7 +144,7 @@ size_t parse_signature(const char *buf, size_t size)\n \twhile (len < size) {\n \t\tconst char *eol;\n \n-\t\tif (is_gpg_start(buf + len))\n+\t\tif (get_format_by_sig(buf + len))\n \t\t\tmatch = len;\n \n \t\teol = memchr(buf + len, '\\n', size - len);\n@@ -132,6 +161,9 @@ void set_signing_key(const char *key)\n \n int git_gpg_config(const char *var, const char *value, void *cb)\n {\n+\tstruct gpg_format *fmt = NULL;\n+\tchar *fmtname = NULL;\n+\n \tif (!strcmp(var, \"user.signingkey\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -140,18 +172,23 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \t}\n \n \tif (!strcmp(var, \"gpg.format\")) {\n-\t\tif (value && strcasecmp(value, \"openpgp\"))\n-\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n-\t\treturn git_config_string(&gpg_format, var, value);\n-\t}\n-\n-\tif (!strcmp(var, \"gpg.program\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tgpg_program = xstrdup(value);\n+\t\tfmt = get_format_by_name(value);\n+\t\tif (!fmt)\n+\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n+\t\tcurrent_format = fmt;\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"gpg.program\"))\n+\t\tfmtname = \"openpgp\";\n+\n+\tif (fmtname) {\n+\t\tfmt = get_format_by_name(fmtname);\n+\t\treturn git_config_string(&fmt->program, var, value);\n+\t}\n+\n \treturn 0;\n }\n \n@@ -170,7 +207,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \targv_array_pushl(&gpg.args,\n-\t\t\t gpg_program,\n+\t\t\t current_format->program,\n \t\t\t \"--status-fd=2\",\n \t\t\t \"-bsau\", signing_key,\n \t\t\t NULL);\n@@ -208,6 +245,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n+\tstruct gpg_format *fmt;\n \tstruct tempfile *temp;\n \tint ret;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -223,10 +261,14 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\treturn -1;\n \t}\n \n+\tfmt = get_format_by_sig(signature);\n+\tif (!fmt)\n+\t\tBUG(\"bad signature '%s'\", signature);\n+\n+\targv_array_push(&gpg.args, fmt->program);\n+\targv_array_pushv(&gpg.args, fmt->extra_args_verify);\n \targv_array_pushl(&gpg.args,\n-\t\t\t gpg_program,\n \t\t\t \"--status-fd=1\",\n-\t\t\t \"--keyid-format=long\",\n \t\t\t \"--verify\", temp->filename.buf, \"-\",\n \t\t\t NULL);\n \n-- \n2.16.4\n\n"},{"id":"352460","messageId":"8e6f4f547475e3ce26799901e255a78ea74dac8e.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 5/7] gpg-interface: introduce new config to select per gpg format program","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:27Z","receivedAt":"2018-07-13T08:41:55Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Supporting multiple signing formats we will have the need to configure a\ncustom program each. Add a new config value to cater for that.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n Documentation/config.txt | 5 +++++\n gpg-interface.c          | 2 +-\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ac373e3f4..c0bd80954 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1832,6 +1832,11 @@ gpg.format::\n \tSpecifies which key format to use when signing with `--gpg-sign`.\n \tDefault is \"openpgp\", that is also the only supported value.\n \n+gpg.<format>.program::\n+\tUse this to customize the program used for the signing format you\n+\tchose. (see gpg.program) gpg.openpgp.program is a synonym for the\n+\tlegacy gpg.program.\n+\n gui.commitMsgWidth::\n \tDefines how wide the commit message window is in the\n \tlinkgit:git-gui[1]. \"75\" is the default.\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 93bd0fb32..f3c22b551 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -182,7 +182,7 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"gpg.program\"))\n+\tif (!strcmp(var, \"gpg.program\") || !strcmp(var, \"gpg.openpgp.program\"))\n \t\tfmtname = \"openpgp\";\n \n \tif (fmtname) {\n-- \n2.16.4\n\n"},{"id":"352461","messageId":"003d08b067b239a060641eadcba913e2aedb5cba.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 6/7] gpg-interface: introduce new signature format \"x509\" using gpgsm","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:28Z","receivedAt":"2018-07-13T08:41:57Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"This commit allows git to create and check x509 type signatures using\ngpgsm.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n Documentation/config.txt | 5 +++--\n gpg-interface.c          | 9 +++++++++\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c0bd80954..7cfce4dc8 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1830,12 +1830,13 @@ gpg.program::\n \n gpg.format::\n \tSpecifies which key format to use when signing with `--gpg-sign`.\n-\tDefault is \"openpgp\", that is also the only supported value.\n+\tDefault is \"openpgp\" and another possible value is \"x509\".\n \n gpg.<format>.program::\n \tUse this to customize the program used for the signing format you\n \tchose. (see gpg.program) gpg.openpgp.program is a synonym for the\n-\tlegacy gpg.program.\n+\tlegacy gpg.program, while the default gpg.x509.program is \"gpgsm\".\n+\n \n gui.commitMsgWidth::\n \tDefines how wide the commit message window is in the\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex f3c22b551..62a9acf68 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -18,12 +18,18 @@ static const char *openpgp_verify_args[] = { \"--keyid-format=long\", NULL };\n static const char *openpgp_sigs[] = {\n \t\"-----BEGIN PGP SIGNATURE-----\",\n \t\"-----BEGIN PGP MESSAGE-----\", NULL };\n+static const char *x509_verify_args[] = { NULL };\n+static const char *x509_sigs[] = { \"-----BEGIN SIGNED MESSAGE-----\", NULL };\n \n static struct gpg_format gpg_formats[] = {\n \t{ .name = \"openpgp\", .program = \"gpg\",\n \t  .extra_args_verify = openpgp_verify_args,\n \t  .sigs = openpgp_sigs\n \t},\n+\t{ .name = \"x509\", .program = \"gpgsm\",\n+\t  .extra_args_verify = x509_verify_args,\n+\t  .sigs = x509_sigs\n+\t},\n };\n static struct gpg_format *current_format = &gpg_formats[0];\n \n@@ -185,6 +191,9 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"gpg.program\") || !strcmp(var, \"gpg.openpgp.program\"))\n \t\tfmtname = \"openpgp\";\n \n+\tif (!strcmp(var, \"gpg.x509.program\"))\n+\t\tfmtname = \"x509\";\n+\n \tif (fmtname) {\n \t\tfmt = get_format_by_name(fmtname);\n \t\treturn git_config_string(&fmt->program, var, value);\n-- \n2.16.4\n\n"},{"id":"352462","messageId":"b442be519b16b92ad9b5e564e8b761506c8af79b.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":"cover.1531470729.git.henning.schild@siemens.com","subject":"[PATCH v3 7/7] gpg-interface t: extend the existing GPG tests with GPGSM","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:29Z","receivedAt":"2018-07-13T08:41:59Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Add test cases to cover the new X509/gpgsm support. Most of them\nresemble existing ones. They just switch the format to x509 and set the\nsigningkey when creating signatures. Validation of signatures does not\nneed any configuration of git, it does need gpgsm to be configured to\ntrust the key(-chain).\nSeveral of the testcases build on top of existing gpg testcases.\nThe commit ships a self-signed key for committer@example.com and\nconfigures gpgsm to trust it.\n\nSigned-off-by: Henning Schild <henning.schild@siemens.com>\n---\n t/lib-gpg.sh               |  28 +++++++++++++++++++++++-\n t/lib-gpg/gpgsm-gen-key.in |   8 +++++++\n t/lib-gpg/gpgsm_cert.p12   | Bin 0 -> 2652 bytes\n t/t4202-log.sh             |  39 ++++++++++++++++++++++++++++++++++\n t/t5534-push-signed.sh     |  52 +++++++++++++++++++++++++++++++++++++++++++++\n t/t7004-tag.sh             |  13 ++++++++++++\n t/t7030-verify-tag.sh      |  33 ++++++++++++++++++++++++++++\n 7 files changed, 172 insertions(+), 1 deletion(-)\n create mode 100644 t/lib-gpg/gpgsm-gen-key.in\n create mode 100644 t/lib-gpg/gpgsm_cert.p12\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex a5d3b2cba..3fe02876c 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -38,7 +38,33 @@ then\n \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/ownertrust &&\n \t\tgpg --homedir \"${GNUPGHOME}\" </dev/null >/dev/null 2>&1 \\\n \t\t\t--sign -u committer@example.com &&\n-\t\ttest_set_prereq GPG\n+\t\ttest_set_prereq GPG &&\n+\t\t# Available key info:\n+\t\t# * see t/lib-gpg/gpgsm-gen-key.in\n+\t\t# To generate new certificate:\n+\t\t#  * no passphrase\n+\t\t#\tgpgsm --homedir /tmp/gpghome/ \\\n+\t\t#\t\t-o /tmp/gpgsm.crt.user \\\n+\t\t#\t\t--generate-key \\\n+\t\t#\t\t--batch t/lib-gpg/gpgsm-gen-key.in\n+\t\t# To import certificate:\n+\t\t#\tgpgsm --homedir /tmp/gpghome/ \\\n+\t\t#\t\t--import /tmp/gpgsm.crt.user\n+\t\t# To export into a .p12 we can later import:\n+\t\t#\tgpgsm --homedir /tmp/gpghome/ \\\n+\t\t#\t\t-o t/lib-gpg/gpgsm_cert.p12 \\\n+\t\t#\t\t--export-secret-key-p12 \"committer@example.com\"\n+\t\techo | gpgsm --homedir \"${GNUPGHOME}\" 2>/dev/null \\\n+\t\t\t--passphrase-fd 0 --pinentry-mode loopback \\\n+\t\t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n+\t\tgpgsm --homedir \"${GNUPGHOME}\" 2>/dev/null -K \\\n+\t\t\t| grep fingerprint: | cut -d\" \" -f4 | tr -d '\\n' > \\\n+\t\t\t${GNUPGHOME}/trustlist.txt &&\n+\t\techo \" S relax\" >> ${GNUPGHOME}/trustlist.txt &&\n+\t\t(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&\n+\t\techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n+\t\t\t-u committer@example.com -o /dev/null --sign - 2>&1 &&\n+\t\ttest_set_prereq GPGSM\n \t\t;;\n \tesac\n fi\ndiff --git a/t/lib-gpg/gpgsm-gen-key.in b/t/lib-gpg/gpgsm-gen-key.in\nnew file mode 100644\nindex 000000000..a7fd87c06\n--- /dev/null\n+++ b/t/lib-gpg/gpgsm-gen-key.in\n@@ -0,0 +1,8 @@\n+Key-Type: RSA\n+Key-Length: 2048\n+Key-Usage: sign\n+Serial: random\n+Name-DN: CN=C O Mitter, O=Example, SN=C O, GN=Mitter\n+Name-Email: committer@example.com\n+Not-Before: 1970-01-01 00:00:00\n+Not-After: 3000-01-01 00:00:00\ndiff --git a/t/lib-gpg/gpgsm_cert.p12 b/t/lib-gpg/gpgsm_cert.p12\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..94ffad0d31a3b6c4e849b29d960762e485ba7ea8\nGIT binary patch\nliteral 2652\nzcmY+Ec{mh|8pQ{*m?2^;S+ivrVaAfiQnHSnA|b+zofwQYAI3haA=!75ZX!!|$-Y*$\nzWS5XVvW}3h?sM<`?)~F^&U?;zp7ZAqMS|U-rJ+NSVEkYxG8!9AJx2qf$s@s-fg~8i\nzSqwpufKGo`;5-uW&RJwiO9MC)gTEUZ6fYR|?*&F0Fp3FCzs?3aZK`58prxe;gpq&(\nzm?nam#=;<-Y>ihd?+EMByF}ef4%bKLwbO{e4U=0LG4jP<8x$`N#5#&@0as@!cSSK>\nzh}oaTUi^XPMV(3;kKAjFB5Yq)zn_vnExr>`s&?M}i{A>>$j-{u*Jo)riK{fW!LXet\nz)hkG3xgvIpDClY8=o4q?_bD%htwXG!_=;NxVjU=3#{TU0Q@=Z<lvT*(k5Ms*ff#-&\nz2TqRbeV}33XyuPwvZ-I?sZHaFvamGBC4C2!1f881gBH^>Mc%6wes*Uy&CAPf=JB(a\nzb<9VW4e)@R+qGEgfnewABCPf}mtJOYxL>*jtu9yKk?Fu3UWr~zJ#<*I<w-ZK|I*LI\nzZ;8rw5`)@*7P)PnF1bA8hxp<r)dua;u5}jN6VsL8hh!>(Ey>fhA6~_gUvg(8n<*TM\nzdXo$0jMn2G$GcM484J8MUx*x=@XWq^uvCnI1jp>#Y_2Y<+T_*yX!uTA$vm7Ugs56O\nz@!~>Y?E@FA@{5@ZDHD`TkEeHQ{M2n_Slp%By{89uVia3%>w?IZdvBBX$K*3UzCM7`\nzxGZ-dbh^KlF#?RZaS4_^R|%@F5xX0j)vdE&#M1Ch^W+WzQjVH^m|vu9jE@aRtlbcP\nz^N<e=%>xHpt!%n59KJ!@-IHzjWw9l;)N}J^LLwLT#Xz!Tq4vWkHTO5#j_1_nvA>#u\nzBX5jy&r_5h7PKFH26B<j-};r)Q!~uebz+5*c1Mg6QJ(F}k;)Pa%tHDpyB+C=<T0f@\nzfwFZ6@ul;*T2N!H{aNTXl07=zM1?d4W1Mdoev(Ci*EFxl3Kn9{2}%Z4G?!nQiB0g~\nzK!#@?oC9CCQGKFoVAa_6+`A{@B5hpW5W9}S5wbSPpAH!LbwNDTkuW=uJj2PG5rxVP\nzOCm<+$);+++TyYtcm)5^Bh?n-7yi!ZN#Cz1QN$-pc#5NOa7XPAMXEeas&`7eOn?i*\nz`I~LT4t9a!(9AVh`MH;Y67Sxsw6oB&EpdFg^!Wggy4jDhAn44Q^XU0%RWlJGQ^(`*\nzNds|sQ@XQ+C*v4Z0aEU6d{m?%#@>8-1BMryQYu8HiW#k_Nq!qB4-5#YH|RDUL_&Ug\nz+Y4DwCgmPTpSqu5lU#dxgcf>N8-aXsM}TDNd_~tW$92abZG)0q=Xzr)t;M39D`u~9\nzdbi)dKxTXdE3OHN@sCmOGfdNEKC*BgS&ku$LNb1M>x6#MZ_37F<5gz)d_&6)%S0zP\nz5_lFp_A5n(wMlQ+_2n%!z1tomK+~=DQ9MC|QDxZ1?2?A{(Dqi(*TE;q4g+&HH;vvX\nzI%M?`5*^D+i5eC~tth)c%;)Q^n`$=Nt8=_jLdLg)*d^-BKJ2siLCaLDMJwSxfF|uO\nzc}bc4oW*xw>!m!o6BG%Q_CG+$BZ1<8Bv8~@9Da5oV21zT1x7=A#-YtK0ImHWb?E+3\nz$NK7MLMBq{?jPy^Nx&Yxu7#tyt@t3@=F07Bf;BHh5A|9FYRL9bIo$1a44s9^Wk41>\nzq-0_p!?=F1wYc$wC8sgycQ~IHO0x4@BODYL?(uFu5qaXSm+YP$_!_RrSrzJ#*f(fy\nzF)|i|!Ac}}R(rT~U3@SDll^wcX#c1XS5VcvP3@>e?Qfbr5MU*ox<E&+__5!tZR3)5\nzgK#+MtH30K<aigPK3I)-o1VspNklu-;b#OuvTW7P`E0*iRGje(=vIgGIK+a12|DRj\nzfC{mBlKTVfl0mvi2CwE6^<p}QRoY2?Gq0v*@Ew|@PzC!4y26O^I^!1>y@`fDnT0wr\nzw!P*fjd#ErDfYvcz210jWuOc?HIcQE!NG<w`@a6Z=I{{~h6%^La$;>$>ZY5Qu3~pF\nz|FQ2yc>7;Y5xC)^=~dQ}KtYR4*&rVa1DmU#%D2UhSl&nsVlEYKn?mrFq8i;s)M4>G\nz4;6*=?}rOkcdcDRaf<>E0U#CGt3E0T>H?%&l4K?}KWWdEY9T@1e#=y%n#{&;nJFED\nzBsOEs9+}{*EbQQ(!m^<q3U{D4tRpf$#VcmEKg;z4Ra)@4jna!9H%oKlO|x#sXvzQe\nz*vV|Gg1MBxCfjLJP*p(~7HWGeV{EUa6&oNlsGd_pNi^M~sGf_a{Ur6qxWq!OXCcF>\nz+dYSTfqtiD?X&)@FqTX~m6?3L?tY*9;5BW226VPX+9op7qeeHfpjs}=?&h@GRCCd@\nz&h^*W0M;K21!e@=8*%ZH?2o$A`Zq+PIfeOX!~-sSbVb~2g#60Z>?)Oyn5oPKq`BHq\nz`~$#(bI&)uDgU7H&mS6iGL956F*bgsukzPoWrMXT(`r7P+s2ko%&VRpliG!Jo{7?}\nz!tynAjw*)`T!(JD2<#co`t}vIgGVQH>jj+<lN#PsBUY1^*HL&GBQ3DX@W&J$mbD&8\nzO2<?O%%qCYJpAGs0qBqM%x_hXt>^5%f|VBU(NJc0ZC5!CG-MLweP2q)96vc#dbiR3\nzF+XSQ1x@jSwy&aL9(N0O{DYng^x^cb=~p@DYoE?yxUBp-THVPLpA(9Ra0&9ym-^L%\nzhCQR`rL^4;vnbkrFspSSO?-`-WPju-6x)QWQ|(#C=P{1o;;duD7@nlHC_ut{hb2sg\nzv+qeIg!>GP2aP1wtrcgD{Cs2XPc&xTbQHet!LYBJBSWLzl`>2e5_uKD7X~n0Y`-xM\nzDeIHEns4J&=JW#V%Q(+^Lyo^~CaXzLIAja$+W19T?_SGvr$XHY>PJhcKWD(yYhr2f\nzuKfh_v-BO^a)hB5BVL(UVQkSh-I~z$?rUqy-Vu@G>NMGN?h_Mb>|uGv_u>*ARI%-?\nzoOK~Ngifc+#%cv9>CKeu+e3ne_$`6)B5+|`&uK6m!!n<`r0%jOZZ<aI-RsYh=!?eW\nzHPO<?)H9C@$qo_6fimc<BgQ9QETCl(5>w%`()rP>%$po??KNxfLz&E$-4+w7_Nx&2\nz`AkPchqf!cr7A&%vX6pmCm^C^gK^CXkFgwWIN`hbhu<a@kKJITk{QB~MV4=u@0EMz\nzcHzUc+t<F?$;hsB|HdC^l+}Na4$?KQ*iQLy9`^vFv}^q^DP=d?h#CC=W%ge>LfOz*\nzel37D;68u|Fg{ZOfCC@^;05pmcmsUTe*9U2JJUbT1>ksgIDp;18i6xs2ao~Co+%!1\nz1t5<S{(F`k%>iUPd}lxBq5^&zj`N46L%Rcygnv1p&?rF^6bzDJqNC*o0f21jBIX5I\ng0antNnlKhoxNVUlcvb_D9`D2<lq)wC^9R2F1d`soXaE2J\n\nliteral 0\nHcmV?d00001\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 25b1f8cc7..f57781e39 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed branch' '\n \tgit commit -S -m signed_commit\n '\n \n+test_expect_success GPGSM 'setup signed branch x509' '\n+\ttest_when_finished \"git reset --hard && git checkout master\" &&\n+\tgit checkout -b signed-x509 master &&\n+\techo foo >foo &&\n+\tgit add foo &&\n+\ttest_config gpg.format x509 &&\n+\ttest_config user.signingkey $GIT_COMMITTER_EMAIL &&\n+\tgit commit -S -m signed_commit\n+'\n+\n test_expect_success GPG 'log --graph --show-signature' '\n \tgit log --graph --show-signature -n1 signed >actual &&\n \tgrep \"^| gpg: Signature made\" actual &&\n \tgrep \"^| gpg: Good signature\" actual\n '\n \n+test_expect_success GPGSM 'log --graph --show-signature x509' '\n+\tgit log --graph --show-signature -n1 signed-x509 >actual &&\n+\tgrep \"^| gpgsm: Signature made\" actual &&\n+\tgrep \"^| gpgsm: Good signature\" actual\n+'\n+\n test_expect_success GPG 'log --graph --show-signature for merged tag' '\n \ttest_when_finished \"git reset --hard && git checkout master\" &&\n \tgit checkout -b plain master &&\n@@ -1581,6 +1597,29 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '\n \tgrep \"^| | gpg: Good signature\" actual\n '\n \n+test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '\n+\ttest_when_finished \"git reset --hard && git checkout master\" &&\n+\ttest_config gpg.format x509 &&\n+\ttest_config user.signingkey $GIT_COMMITTER_EMAIL &&\n+\tgit checkout -b plain-x509 master &&\n+\techo aaa >bar &&\n+\tgit add bar &&\n+\tgit commit -m bar_commit &&\n+\tgit checkout -b tagged-x509 master &&\n+\techo bbb >baz &&\n+\tgit add baz &&\n+\tgit commit -m baz_commit &&\n+\tgit tag -s -m signed_tag_msg signed_tag_x509 &&\n+\tgit checkout plain-x509 &&\n+\tgit merge --no-ff -m msg signed_tag_x509 &&\n+\tgit log --graph --show-signature -n1 plain-x509 >actual &&\n+\tgrep \"^|\\\\\\  merged tag\" actual &&\n+\tgrep \"^| | gpgsm: Signature made\" actual &&\n+\tgrep \"^| | gpgsm: Good signature\" actual &&\n+\tgit config --unset gpg.format &&\n+\tgit config --unset user.signingkey\n+'\n+\n test_expect_success GPG '--no-show-signature overrides --show-signature' '\n \tgit log -1 --show-signature --no-show-signature signed >actual &&\n \t! grep \"^gpg:\" actual\ndiff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh\nindex 1cea758f7..a3a12bd05 100755\n--- a/t/t5534-push-signed.sh\n+++ b/t/t5534-push-signed.sh\n@@ -218,4 +218,56 @@ test_expect_success GPG 'fail without key and heed user.signingkey' '\n \ttest_cmp expect dst/push-cert-status\n '\n \n+test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '\n+\ttest_config gpg.format x509 &&\n+\tenv | grep GIT > envfile &&\n+\tprepare_dst &&\n+\tmkdir -p dst/.git/hooks &&\n+\tgit -C dst config receive.certnonceseed sekrit &&\n+\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n+\t# discard the update list\n+\tcat >/dev/null\n+\t# record the push certificate\n+\tif test -n \"${GIT_PUSH_CERT-}\"\n+\tthen\n+\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n+\tfi &&\n+\n+\tcat >../push-cert-status <<E_O_F\n+\tSIGNER=${GIT_PUSH_CERT_SIGNER-nobody}\n+\tKEY=${GIT_PUSH_CERT_KEY-nokey}\n+\tSTATUS=${GIT_PUSH_CERT_STATUS-nostatus}\n+\tNONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}\n+\tNONCE=${GIT_PUSH_CERT_NONCE-nononce}\n+\tE_O_F\n+\n+\tEOF\n+\tunset GIT_COMMITTER_EMAIL &&\n+\tgit config user.email hasnokey@nowhere.com &&\n+\tgit config user.signingkey \"\" &&\n+\ttest_must_fail git push --signed dst noop ff +noff &&\n+\tgit config user.signingkey committer@example.com &&\n+\tgit push --signed dst noop ff +noff &&\n+\n+\t(\n+\t\tcat <<-\\EOF &&\n+\t\tSIGNER=/CN=C O Mitter/O=Example/SN=C O/GN=Mitter\n+\t\tKEY=\n+\t\tSTATUS=G\n+\t\tNONCE_STATUS=OK\n+\t\tEOF\n+\t\tsed -n -e \"s/^nonce /NONCE=/p\" -e \"/^$/q\" dst/push-cert\n+\t) >expect.in &&\n+\tkey=$(cat \"${GNUPGHOME}/trustlist.txt\" | cut -d\" \" -f1 | tr -d \":\") &&\n+\tsed -e \"s/^KEY=/KEY=${key}/\" expect.in > expect &&\n+\n+\tnoop=$(git rev-parse noop) &&\n+\tff=$(git rev-parse ff) &&\n+\tnoff=$(git rev-parse noff) &&\n+\tgrep \"$noop $ff refs/heads/ff\" dst/push-cert &&\n+\tgrep \"$noop $noff refs/heads/noff\" dst/push-cert &&\n+\ttest_cmp expect dst/push-cert-status\n+'\n+\n+\n test_done\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex d7b319e91..2147938aa 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1354,6 +1354,19 @@ test_expect_success GPG \\\n \t'test_config gpg.program echo &&\n \t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n+# try to sign with bad user.signingkey\n+test_expect_success GPGSM \\\n+\t'git tag -s fails if gpgsm is misconfigured (bad key)' \\\n+\t'test_config user.signingkey BobTheMouse &&\n+\t test_config gpg.format x509 &&\n+\t test_must_fail git tag -s -m tail tag-gpg-failure'\n+\n+# try to produce invalid signature\n+test_expect_success GPGSM \\\n+\t'git tag -s fails if gpgsm is misconfigured (bad signature format)' \\\n+\t'test_config gpg.x509.program echo &&\n+\t test_config gpg.format x509 &&\n+\t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n # try to verify without gpg:\n \ndiff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh\nindex 291a1e2b0..db6295d65 100755\n--- a/t/t7030-verify-tag.sh\n+++ b/t/t7030-verify-tag.sh\n@@ -41,6 +41,13 @@ test_expect_success GPG 'create signed tags' '\n \tgit tag -uB7227189 -m eighth eighth-signed-alt\n '\n \n+test_expect_success GPGSM 'create signed tags x509 ' '\n+\ttest_config gpg.format x509 &&\n+\ttest_config user.signingkey $GIT_COMMITTER_EMAIL &&\n+\techo 9 >file && test_tick && git commit -a -m \"nineth gpgsm-signed\" &&\n+\tgit tag -s -m nineth nineth-signed-x509\n+'\n+\n test_expect_success GPG 'verify and show signatures' '\n \t(\n \t\tfor tag in initial second merge fourth-signed sixth-signed seventh-signed\n@@ -72,6 +79,13 @@ test_expect_success GPG 'verify and show signatures' '\n \t)\n '\n \n+test_expect_success GPGSM 'verify and show signatures x509' '\n+\tgit verify-tag nineth-signed-x509 2>actual &&\n+\tgrep \"Good signature from\" actual &&\n+\t! grep \"BAD signature from\" actual &&\n+\techo nineth-signed-x509 OK\n+'\n+\n test_expect_success GPG 'detect fudged signature' '\n \tgit cat-file tag seventh-signed >raw &&\n \tsed -e \"/^tag / s/seventh/7th forged/\" raw >forged1 &&\n@@ -112,6 +126,12 @@ test_expect_success GPG 'verify signatures with --raw' '\n \t)\n '\n \n+test_expect_success GPGSM 'verify signatures with --raw x509' '\n+\tgit verify-tag --raw nineth-signed-x509 2>actual &&\n+\tgrep \"GOODSIG\" actual &&\n+\t! grep \"BADSIG\" actual &&\n+\techo nineth-signed-x509 OK\n+'\n test_expect_success GPG 'verify multiple tags' '\n \ttags=\"fourth-signed sixth-signed seventh-signed\" &&\n \tfor i in $tags\n@@ -125,6 +145,19 @@ test_expect_success GPG 'verify multiple tags' '\n \ttest_cmp expect.stderr actual.stderr\n '\n \n+test_expect_success GPGSM 'verify multiple tags x509' '\n+\ttags=\"seventh-signed nineth-signed-x509\" &&\n+\tfor i in $tags\n+\tdo\n+\t\tgit verify-tag -v --raw $i || return 1\n+\tdone >expect.stdout 2>expect.stderr.1 &&\n+\tgrep \"^.GNUPG:.\" <expect.stderr.1 >expect.stderr &&\n+\tgit verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&\n+\tgrep \"^.GNUPG:.\" <actual.stderr.1 >actual.stderr &&\n+\ttest_cmp expect.stdout actual.stdout &&\n+\ttest_cmp expect.stderr actual.stderr\n+'\n+\n test_expect_success GPG 'verifying tag with --format' '\n \tcat >expect <<-\\EOF &&\n \ttagname : fourth-signed\n-- \n2.16.4\n\n"},{"id":"352463","messageId":"cover.1531470729.git.henning.schild@siemens.com","threadId":"48876","inReplyTo":null,"subject":"[PATCH v3 0/7] X509 (gpgsm) commit signing support","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-13T08:41:22Z","receivedAt":"2018-07-13T08:42:02Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Changes in v3:\n - drop patches 1 and 2 known from < v3, see pu hs/push-cert-check-cleanup\n - dropped some testcases from p6, added two t7004 bad key/sig\n - ship a binary x509 certificate for tests, no generation anymore\n - silence gpgsm in tests\n - switch all tests to use test_config instead of \"git config\"\n - p4 deal with invalid input\n - p3 several refactorings, see \"PATCH v2 5/9\" discussion\n\nChanges in v2:\n - removed trailing commas in array initializers and add leading space\n - replaced assert(0) with BUG in p5\n - consolidated 2 format lookups reusing get_format_data p5\n - changed from format \"PGP\" to \"openpgp\", later X509 to \"x509\"\n - use strcasecmp instead of strcmp for format matching\n - introduce gpg.<format>.program in p8, no gpg.programX509 anymore\n - moved t/7510 patch inbetween the two patches changing validation code\n - changed t/7510 patch to use test_config and test_must_fail\n\nThis series adds support for signing commits with gpgsm.\n\nThe first 5 patches prepare for the introduction of the actual feature in\npatch 6.\nFinally patch 7 extends the testsuite to cover the new feature.\n\nThis series can be seen as a follow up of a series that appeared under\nthe name \"gpg-interface: Multiple signing tools\" in april 2018 [1]. After\nthat series was not merged i decided to get my patches ready. The\noriginal series aimed at being generic for any sort of signing tool, while\nthis series just introduced the X509 variant of gpg. (gpgsm)\nI collected authors and reviewers of that first series and already put them\non cc.\n\n[1] https://public-inbox.org/git/20180409204129.43537-1-mastahyeti@gmail.com/\n\nHenning Schild (7):\n  gpg-interface: add new config to select how to sign a commit\n  t/t7510: check the validation of the new config gpg.format\n  gpg-interface: introduce an abstraction for multiple gpg formats\n  gpg-interface: do not hardcode the key string len anymore\n  gpg-interface: introduce new config to select per gpg format program\n  gpg-interface: introduce new signature format \"x509\" using gpgsm\n  gpg-interface t: extend the existing GPG tests with GPGSM\n\n Documentation/config.txt   |  10 +++++\n gpg-interface.c            |  95 ++++++++++++++++++++++++++++++++++++---------\n t/lib-gpg.sh               |  28 ++++++++++++-\n t/lib-gpg/gpgsm-gen-key.in |   8 ++++\n t/lib-gpg/gpgsm_cert.p12   | Bin 0 -> 2652 bytes\n t/t4202-log.sh             |  39 +++++++++++++++++++\n t/t5534-push-signed.sh     |  52 +++++++++++++++++++++++++\n t/t7004-tag.sh             |  13 +++++++\n t/t7030-verify-tag.sh      |  33 ++++++++++++++++\n t/t7510-signed-commit.sh   |   9 +++++\n 10 files changed, 268 insertions(+), 19 deletions(-)\n create mode 100644 t/lib-gpg/gpgsm-gen-key.in\n create mode 100644 t/lib-gpg/gpgsm_cert.p12\n\n-- \n2.16.4\n\n"},{"id":"352665","messageId":"xmqqh8kzszth.fsf@gitster-ct.c.googlers.com","threadId":"48876","inReplyTo":"5e88da5eac1e1aea8a946960d455ba321e3e42b1.1531470729.git.henning.schild@siemens.com","subject":"Re: [PATCH v3 1/7] gpg-interface: add new config to select how to sign a commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T20:14:34Z","receivedAt":"2018-07-16T20:14:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Henning Schild <henning.schild@siemens.com> writes:\n\n> Add \"gpg.format\" where the user can specify which type of signature to\n> use for commits. At the moment only \"openpgp\" is supported and the value is\n> not even used. This commit prepares for a new types of signatures.\n>\n> Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> ---\n>  Documentation/config.txt | 4 ++++\n>  gpg-interface.c          | 7 +++++++\n>  2 files changed, 11 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 1cc18a828..ac373e3f4 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1828,6 +1828,10 @@ gpg.program::\n>  \tsigned, and the program is expected to send the result to its\n>  \tstandard output.\n>  \n> +gpg.format::\n> +\tSpecifies which key format to use when signing with `--gpg-sign`.\n> +\tDefault is \"openpgp\", that is also the only supported value.\n> +\n>  gui.commitMsgWidth::\n>  \tDefines how wide the commit message window is in the\n>  \tlinkgit:git-gui[1]. \"75\" is the default.\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 09ddfbc26..960c40086 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -7,6 +7,7 @@\n>  #include \"tempfile.h\"\n>  \n>  static char *configured_signing_key;\n> +static const char *gpg_format = \"openpgp\";\n>  static const char *gpg_program = \"gpg\";\n>  \n>  #define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n> @@ -138,6 +139,12 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif (!strcmp(var, \"gpg.format\")) {\n> +\t\tif (value && strcasecmp(value, \"openpgp\"))\n> +\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n> +\t\treturn git_config_string(&gpg_format, var, value);\n\nI may be mistaken (sorry, I read so many topics X-<) but I think the\nconsensus was to accept only \"openpgp\" so that we can ensure that\n\n\t[gpg \"openpgp\"] program = foo\n\netc. can be parsed more naturally without having to manually special\ncase the subsection name to be case insensitive.  In other words,\nstrcasecmp() should just be strcmp().  Otherwise, people would get a\nwrong expectation that\n\n\t[gpg] format = OpenPGP\n\t[gpg \"openpgp\"] program = foo\n\nshould refer to the same and single OpenPGP, but that would violate\nthe usual configuration syntax rules.\n\nThe structure of checking the error looks quite suboptimal even when\nwe initially support the single \"openpgp\" at this step.  I would\nhave expected you to be writing the above like so:\n\n\tif (!value)\n\t\treturn config_error_nonbool(var);\n\tif (strcmp(value, \"openpgp\"))\n\t\treturn error(\"unsupported value for %s: %s\", var, value);\n\treturn git_config_string(...);\n\nThat would make it more clear that the variable is \"nonbool\", which\ndoes not change across additional support for new formats in later\nsteps.\n\n> +\t}\n> +\n>  \tif (!strcmp(var, \"gpg.program\")) {\n>  \t\tif (!value)\n>  \t\t\treturn config_error_nonbool(var);\n"},{"id":"352666","messageId":"xmqqd0vnszs4.fsf@gitster-ct.c.googlers.com","threadId":"48876","inReplyTo":"e3fc5449ef1fe1d107e11c77f2e66d8d47a451db.1531470729.git.henning.schild@siemens.com","subject":"Re: [PATCH v3 2/7] t/t7510: check the validation of the new config gpg.format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T20:15:23Z","receivedAt":"2018-07-16T20:15:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Henning Schild <henning.schild@siemens.com> writes:\n\n> Test setting gpg.format to both invalid and valid values.\n>\n> Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> ---\n>  t/t7510-signed-commit.sh | 9 +++++++++\n>  1 file changed, 9 insertions(+)\n>\n> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\n> index 6e2015ed9..1b6a1dd90 100755\n> --- a/t/t7510-signed-commit.sh\n> +++ b/t/t7510-signed-commit.sh\n> @@ -227,4 +227,13 @@ test_expect_success GPG 'log.showsignature behaves like --show-signature' '\n>  \tgrep \"gpg: Good signature\" actual\n>  '\n>  \n> +test_expect_success GPG 'check config gpg.format values' '\n> +\ttest_config gpg.format openpgp &&\n> +\tgit commit -S --amend -m \"success\" &&\n> +\ttest_config gpg.format OpEnPgP &&\n> +\tgit commit -S --amend -m \"success\" &&\n> +\ttest_config gpg.format malformed &&\n> +\ttest_must_fail git commit -S --amend -m \"fail\"\n> +'\n> +\n>  test_done\n\nSince we'd be doing case-sensitive value for gpg.format, this would\nhave to change in a later iteration, I guess.\n"},{"id":"352674","messageId":"xmqq4lgyud6n.fsf@gitster-ct.c.googlers.com","threadId":"48876","inReplyTo":"38e2eb70233709aa73345d64c94d59d4ccc681ec.1531470729.git.henning.schild@siemens.com","subject":"Re: [PATCH v3 3/7] gpg-interface: introduce an abstraction for multiple gpg formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T20:40:32Z","receivedAt":"2018-07-16T20:40:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Henning Schild <henning.schild@siemens.com> writes:\n\n> Create a struct that holds the format details for the supported formats.\n> At the moment that is still just \"openpgp\". This commit prepares for the\n> introduction of more formats, that might use other programs and match\n> other signatures.\n>\n> Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> ---\n>  gpg-interface.c | 84 ++++++++++++++++++++++++++++++++++++++++++---------------\n>  1 file changed, 63 insertions(+), 21 deletions(-)\n>\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 960c40086..699651fd9 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -7,11 +7,46 @@\n>  #include \"tempfile.h\"\n>  \n>  static char *configured_signing_key;\n> -static const char *gpg_format = \"openpgp\";\n> -static const char *gpg_program = \"gpg\";\n> +struct gpg_format {\n> +\tconst char *name;\n> +\tconst char *program;\n> +\tconst char **extra_args_verify;\n> +\tconst char **sigs;\n> +};\n> +\n> +static const char *openpgp_verify_args[] = { \"--keyid-format=long\", NULL };\n\nLet's write it like this, even if the current line is short enough:\n\nstatic const char *openpgp_verify_args[] = {\n\t\"--keyid-format=long\",\n\tNULL\n};\n\nDitto for the next one.  Even if you do not expect these two arrays\nto get longer, people tend to copy & paste to imitate existing code\nwhen making new things, and we definitely would not want them to be\ndoing so on a code like the above (or below).  When they need to add\nnew elements to their arrays, having the terminating NULL on its own\nline means they will have to see less patch noise.\n\n> +static const char *openpgp_sigs[] = {\n> +\t\"-----BEGIN PGP SIGNATURE-----\",\n> +\t\"-----BEGIN PGP MESSAGE-----\", NULL };\n> +\n> +static struct gpg_format gpg_formats[] = {\n> +\t{ .name = \"openpgp\", .program = \"gpg\",\n> +\t  .extra_args_verify = openpgp_verify_args,\n\nIf the underlying aray is \"verify_args\" (not \"extra\"), perhaps the\nfield name should also be .verify_args to match.\n\nGiving an array of things a name \"things[]\" is a disease, unless the\narray is very often passed around as a whole to represent the\ncollection of everything.  When the array is mostly accessed one\nelement at a time, being able to say thing[3] to mean the third\nthing is much more pleasant.\n\nSo, from that point of view, I pretty much agree with\nopenpgp_verify_args[] and openpgp_sigs[], but am against\ngpg_formats[].  The last one should be gpg_format[], for which we\nhappen to have only one variant right now.\n\n> +\t  .sigs = openpgp_sigs\n> +\t},\n> +};\n> +static struct gpg_format *current_format = &gpg_formats[0];\n\nHave a blank line before the above one.\n\nWhat does \"current_format\" mean?  Is it different from \"format to be\nused\"?  As we will overwrite the variable upon reading the config,\nwe cannot afford to call it default_format, but somehow \"current\"\nfeels misleading, at least to me.  I expected to find \"old format\"\nelsewhere and there may be some format conversion or something from\nthe variable name.\n\n    static struct gpg_format *use_format = &gpg_format[0];\n\nperhaps?\n\n> +\n> +static struct gpg_format *get_format_by_name(const char *str)\n> +{\n> +\tint i;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(gpg_formats); i++)\n> +\t\tif (!strcasecmp(gpg_formats[i].name, str))\n\nAs [1/7], this would become strcmp(), I presume?\n\n> +\t\t\treturn gpg_formats + i;\n> +\treturn NULL;\n> +}\n>  \n> -#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n> -#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n> +static struct gpg_format *get_format_by_sig(const char *sig)\n> +{\n> +\tint i, j;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(gpg_formats); i++)\n> +\t\tfor (j = 0; gpg_formats[i].sigs[j]; j++)\n> +\t\t\tif (starts_with(sig, gpg_formats[i].sigs[j]))\n> +\t\t\t\treturn gpg_formats + i;\n> +\treturn NULL;\n> +}\n\nOK.\n\n> @@ -140,18 +172,23 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n>  \t}\n>  \n>  \tif (!strcmp(var, \"gpg.format\")) {\n> -\t\tif (value && strcasecmp(value, \"openpgp\"))\n> -\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n> -\t\treturn git_config_string(&gpg_format, var, value);\n> -\t}\n> -\n> -\tif (!strcmp(var, \"gpg.program\")) {\n>  \t\tif (!value)\n>  \t\t\treturn config_error_nonbool(var);\n> -\t\tgpg_program = xstrdup(value);\n> +\t\tfmt = get_format_by_name(value);\n> +\t\tif (!fmt)\n> +\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n\nIf I say \"opongpg\", that probably is not \"malformed\" (it is all\nlowercase ascii alphabet that is very plausible-looking string), but\nsimply \"bad value\".\n\nBut other than the minor nit, yes, this structure is what I expected\nto see from the very first step 1/7.\n\n> +\t\tcurrent_format = fmt;\n>  \t\treturn 0;\n>  \t}\n\n>  \n> +\tif (!strcmp(var, \"gpg.program\"))\n> +\t\tfmtname = \"openpgp\";\n\nOK, this is a backward compatibility measure to treat gpg.program as\nif it were gpg.openpgp.program, which makes sense.\n\n> +\tif (fmtname) {\n> +\t\tfmt = get_format_by_name(fmtname);\n> +\t\treturn git_config_string(&fmt->program, var, value);\n> +\t}\n> +\n>  \treturn 0;\n>  }\n"},{"id":"352675","messageId":"xmqqzhyqsylr.fsf@gitster-ct.c.googlers.com","threadId":"48876","inReplyTo":"6fb618aefcf34695cd46b3135829b5ac3f703573.1531470729.git.henning.schild@siemens.com","subject":"Re: [PATCH v3 4/7] gpg-interface: do not hardcode the key string len anymore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T20:40:48Z","receivedAt":"2018-07-16T20:40:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Henning Schild <henning.schild@siemens.com> writes:\n\n> gnupg does print the keyid followed by a space and the signer comes\n> next. The same pattern is also used in gpgsm, but there the key length\n> would be 40 instead of 16. Instead of hardcoding the expected length,\n> find the first space and calculate it.\n> Input that does not match the expected format will be ignored now,\n> before we jumped to found+17 which might have been behind the end of an\n> unexpected string.\n>\n> Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> ---\n>  gpg-interface.c | 7 ++++---\n>  1 file changed, 4 insertions(+), 3 deletions(-)\n\nVery nice.\n\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 699651fd9..93bd0fb32 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -89,10 +89,11 @@ static void parse_gpg_output(struct signature_check *sigc)\n>  \t\tsigc->result = sigcheck_gpg_status[i].result;\n>  \t\t/* The trust messages are not followed by key/signer information */\n>  \t\tif (sigc->result != 'U') {\n> -\t\t\tsigc->key = xmemdupz(found, 16);\n> +\t\t\tnext = strchrnul(found, ' ');\n> +\t\t\tsigc->key = xmemdupz(found, next - found);\n>  \t\t\t/* The ERRSIG message is not followed by signer information */\n> -\t\t\tif (sigc-> result != 'E') {\n> -\t\t\t\tfound += 17;\n> +\t\t\tif (*next && sigc-> result != 'E') {\n> +\t\t\t\tfound = next + 1;\n>  \t\t\t\tnext = strchrnul(found, '\\n');\n>  \t\t\t\tsigc->signer = xmemdupz(found, next - found);\n>  \t\t\t}\n"},{"id":"352676","messageId":"xmqqva9esydn.fsf@gitster-ct.c.googlers.com","threadId":"48876","inReplyTo":"8e6f4f547475e3ce26799901e255a78ea74dac8e.1531470729.git.henning.schild@siemens.com","subject":"Re: [PATCH v3 5/7] gpg-interface: introduce new config to select per gpg format program","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T20:45:40Z","receivedAt":"2018-07-16T20:45:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Henning Schild <henning.schild@siemens.com> writes:\n\n> +gpg.<format>.program::\n> +\tUse this to customize the program used for the signing format you\n> +\tchose. (see gpg.program) gpg.openpgp.program is a synonym for the\n> +\tlegacy gpg.program.\n\nI _think_ you meant \"see gpg.format\", but I am not 100% sure.\n\nWhen X is a synonym for Y, Y is also a synonym for X, so technically\nspeaking this does not matter, but when we talk about backward\ncompatibility fallback, I think we say \"OLDway is retained as a\nlegacy synonym for NEWway\", i.e. the other way around.\n\nAlso, `typeset in tt` what end-users would type literally, like\nconfiguration variable names, i.e.\n\n\tUse this to customize the rpogram used for the signing\n\tformat you chose (see `gpg.format`).  `gpg.program` can\n\tstill be used as a legacy synonym for `gpg.openpgp.program`.\n\n>  gui.commitMsgWidth::\n>  \tDefines how wide the commit message window is in the\n>  \tlinkgit:git-gui[1]. \"75\" is the default.\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 93bd0fb32..f3c22b551 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -182,7 +182,7 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tif (!strcmp(var, \"gpg.program\"))\n> +\tif (!strcmp(var, \"gpg.program\") || !strcmp(var, \"gpg.openpgp.program\"))\n>  \t\tfmtname = \"openpgp\";\n>  \n>  \tif (fmtname) {\n"},{"id":"352693","messageId":"20180716213843.GK25189@sigill.intra.peff.net","threadId":"48876","inReplyTo":"xmqqh8kzszth.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 1/7] gpg-interface: add new config to select how to sign a commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-16T21:38:43Z","receivedAt":"2018-07-16T21:38:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 16, 2018 at 01:14:34PM -0700, Junio C Hamano wrote:\n\n> >  #define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n> > @@ -138,6 +139,12 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n> >  \t\treturn 0;\n> >  \t}\n> >  \n> > +\tif (!strcmp(var, \"gpg.format\")) {\n> > +\t\tif (value && strcasecmp(value, \"openpgp\"))\n> > +\t\t\treturn error(\"malformed value for %s: %s\", var, value);\n> > +\t\treturn git_config_string(&gpg_format, var, value);\n> \n> I may be mistaken (sorry, I read so many topics X-<) but I think the\n> consensus was to accept only \"openpgp\" so that we can ensure that\n> \n> \t[gpg \"openpgp\"] program = foo\n> \n> etc. can be parsed more naturally without having to manually special\n> case the subsection name to be case insensitive.  In other words,\n> strcasecmp() should just be strcmp().  Otherwise, people would get a\n> wrong expectation that\n> \n> \t[gpg] format = OpenPGP\n> \t[gpg \"openpgp\"] program = foo\n> \n> should refer to the same and single OpenPGP, but that would violate\n> the usual configuration syntax rules.\n\nI was the one who argued the other way. But unless we are going to move\nto a two-level config for all of it (i.e., gpg.openPGPProgram), I think\nwhat you suggest here is the sanest way forward.\n\n> The structure of checking the error looks quite suboptimal even when\n> we initially support the single \"openpgp\" at this step.  I would\n> have expected you to be writing the above like so:\n> \n> \tif (!value)\n> \t\treturn config_error_nonbool(var);\n> \tif (strcmp(value, \"openpgp\"))\n> \t\treturn error(\"unsupported value for %s: %s\", var, value);\n> \treturn git_config_string(...);\n> \n> That would make it more clear that the variable is \"nonbool\", which\n> does not change across additional support for new formats in later\n> steps.\n\nAgreed.\n\n-Peff\n"},{"id":"352728","messageId":"20180717000310.GA980821@genre.crustytoothpaste.net","threadId":"48876","inReplyTo":"5e88da5eac1e1aea8a946960d455ba321e3e42b1.1531470729.git.henning.schild@siemens.com","subject":"Re: [PATCH v3 1/7] gpg-interface: add new config to select how to sign a commit","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-17T00:03:11Z","receivedAt":"2018-07-17T00:03:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jul 13, 2018 at 10:41:23AM +0200, Henning Schild wrote:\n> Add \"gpg.format\" where the user can specify which type of signature to\n> use for commits. At the moment only \"openpgp\" is supported and the value is\n> not even used. This commit prepares for a new types of signatures.\n> \n> Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> ---\n>  Documentation/config.txt | 4 ++++\n>  gpg-interface.c          | 7 +++++++\n>  2 files changed, 11 insertions(+)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 1cc18a828..ac373e3f4 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1828,6 +1828,10 @@ gpg.program::\n>  \tsigned, and the program is expected to send the result to its\n>  \tstandard output.\n>  \n> +gpg.format::\n> +\tSpecifies which key format to use when signing with `--gpg-sign`.\n> +\tDefault is \"openpgp\", that is also the only supported value.\n\nI think, as discussed in the other thread, perhaps a different prefix\nfor these options is in order if we'd like to plan for the future.\nMaybe this could be \"signature.format\", \"sign.format\", \"signing.format\",\nor \"signingtool.format\" (I tend to prefer the former, but not too\nstrongly).\n\nI anticipate that some projects will prefer other formats, and it makes\nit easier if we don't have to maintain two sets of legacy names.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"352737","messageId":"20180717005517.GA23398@sigill.intra.peff.net","threadId":"48876","inReplyTo":"20180717000310.GA980821@genre.crustytoothpaste.net","subject":"Re: [PATCH v3 1/7] gpg-interface: add new config to select how to sign a commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-17T00:55:17Z","receivedAt":"2018-07-17T00:55:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 17, 2018 at 12:03:11AM +0000, brian m. carlson wrote:\n\n> > +gpg.format::\n> > +\tSpecifies which key format to use when signing with `--gpg-sign`.\n> > +\tDefault is \"openpgp\", that is also the only supported value.\n> \n> I think, as discussed in the other thread, perhaps a different prefix\n> for these options is in order if we'd like to plan for the future.\n> Maybe this could be \"signature.format\", \"sign.format\", \"signing.format\",\n> or \"signingtool.format\" (I tend to prefer the former, but not too\n> strongly).\n> \n> I anticipate that some projects will prefer other formats, and it makes\n> it easier if we don't have to maintain two sets of legacy names.\n\nHeh. This is slowly morphing into the original signingtool series.\n\nFor the record (since I think my response is what you meant by the\n\"other thread\"), I'm OK with going down this gpg.* road for now, and\ndealing with other tools if and when they appear (via the extra level of\nindirection).\n\n-Peff\n"},{"id":"352773","messageId":"20180717145006.466277fe@md1pvb1c.ad001.siemens.net","threadId":"48876","inReplyTo":"xmqq4lgyud6n.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 3/7] gpg-interface: introduce an abstraction for multiple gpg formats","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-17T12:50:06Z","receivedAt":"2018-07-17T12:50:27Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Am Mon, 16 Jul 2018 13:40:32 -0700\nschrieb Junio C Hamano <gitster@pobox.com>:\n\n> Henning Schild <henning.schild@siemens.com> writes:\n> \n> > Create a struct that holds the format details for the supported\n> > formats. At the moment that is still just \"openpgp\". This commit\n> > prepares for the introduction of more formats, that might use other\n> > programs and match other signatures.\n> >\n> > Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> > ---\n> >  gpg-interface.c | 84\n> > ++++++++++++++++++++++++++++++++++++++++++--------------- 1 file\n> > changed, 63 insertions(+), 21 deletions(-)\n> >\n> > diff --git a/gpg-interface.c b/gpg-interface.c\n> > index 960c40086..699651fd9 100644\n> > --- a/gpg-interface.c\n> > +++ b/gpg-interface.c\n> > @@ -7,11 +7,46 @@\n> >  #include \"tempfile.h\"\n> >  \n> >  static char *configured_signing_key;\n> > -static const char *gpg_format = \"openpgp\";\n> > -static const char *gpg_program = \"gpg\";\n> > +struct gpg_format {\n> > +\tconst char *name;\n> > +\tconst char *program;\n> > +\tconst char **extra_args_verify;\n> > +\tconst char **sigs;\n> > +};\n> > +\n> > +static const char *openpgp_verify_args[] =\n> > { \"--keyid-format=long\", NULL };  \n> \n> Let's write it like this, even if the current line is short enough:\n> \n> static const char *openpgp_verify_args[] = {\n> \t\"--keyid-format=long\",\n> \tNULL\n> };\n> \n> Ditto for the next one.  Even if you do not expect these two arrays\n> to get longer, people tend to copy & paste to imitate existing code\n> when making new things, and we definitely would not want them to be\n> doing so on a code like the above (or below).  When they need to add\n> new elements to their arrays, having the terminating NULL on its own\n> line means they will have to see less patch noise.\n\nOk, for consistency a later patch will introduce { NULL }; on three\nlines.\n\n> > +static const char *openpgp_sigs[] = {\n> > +\t\"-----BEGIN PGP SIGNATURE-----\",\n> > +\t\"-----BEGIN PGP MESSAGE-----\", NULL };\n> > +\n> > +static struct gpg_format gpg_formats[] = {\n> > +\t{ .name = \"openpgp\", .program = \"gpg\",\n> > +\t  .extra_args_verify = openpgp_verify_args,  \n> \n> If the underlying aray is \"verify_args\" (not \"extra\"), perhaps the\n> field name should also be .verify_args to match.\n\nRenamed extra_args_verify to verify_args.\n\n> Giving an array of things a name \"things[]\" is a disease, unless the\n> array is very often passed around as a whole to represent the\n> collection of everything.  When the array is mostly accessed one\n> element at a time, being able to say thing[3] to mean the third\n> thing is much more pleasant.\n> \n> So, from that point of view, I pretty much agree with\n> openpgp_verify_args[] and openpgp_sigs[], but am against\n> gpg_formats[].  The last one should be gpg_format[], for which we\n> happen to have only one variant right now.\n\nrenamed gpg_formats[] to gpg_format[].\n\n> > +\t  .sigs = openpgp_sigs\n> > +\t},\n> > +};\n> > +static struct gpg_format *current_format = &gpg_formats[0];  \n> \n> Have a blank line before the above one.\n> \n> What does \"current_format\" mean?  Is it different from \"format to be\n> used\"?  As we will overwrite the variable upon reading the config,\n> we cannot afford to call it default_format, but somehow \"current\"\n> feels misleading, at least to me.  I expected to find \"old format\"\n> elsewhere and there may be some format conversion or something from\n> the variable name.\n> \n>     static struct gpg_format *use_format = &gpg_format[0];\n> \n> perhaps?\n\nrenamed current_format to use_format.\n\n> > +\n> > +static struct gpg_format *get_format_by_name(const char *str)\n> > +{\n> > +\tint i;\n> > +\n> > +\tfor (i = 0; i < ARRAY_SIZE(gpg_formats); i++)\n> > +\t\tif (!strcasecmp(gpg_formats[i].name, str))  \n> \n> As [1/7], this would become strcmp(), I presume?\n> \n> > +\t\t\treturn gpg_formats + i;\n> > +\treturn NULL;\n> > +}\n> >  \n> > -#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n> > -#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n> > +static struct gpg_format *get_format_by_sig(const char *sig)\n> > +{\n> > +\tint i, j;\n> > +\n> > +\tfor (i = 0; i < ARRAY_SIZE(gpg_formats); i++)\n> > +\t\tfor (j = 0; gpg_formats[i].sigs[j]; j++)\n> > +\t\t\tif (starts_with(sig,\n> > gpg_formats[i].sigs[j]))\n> > +\t\t\t\treturn gpg_formats + i;\n> > +\treturn NULL;\n> > +}  \n> \n> OK.\n> \n> > @@ -140,18 +172,23 @@ int git_gpg_config(const char *var, const\n> > char *value, void *cb) }\n> >  \n> >  \tif (!strcmp(var, \"gpg.format\")) {\n> > -\t\tif (value && strcasecmp(value, \"openpgp\"))\n> > -\t\t\treturn error(\"malformed value for %s: %s\",\n> > var, value);\n> > -\t\treturn git_config_string(&gpg_format, var, value);\n> > -\t}\n> > -\n> > -\tif (!strcmp(var, \"gpg.program\")) {\n> >  \t\tif (!value)\n> >  \t\t\treturn config_error_nonbool(var);\n> > -\t\tgpg_program = xstrdup(value);\n> > +\t\tfmt = get_format_by_name(value);\n> > +\t\tif (!fmt)\n> > +\t\t\treturn error(\"malformed value for %s: %s\",\n> > var, value);  \n> \n> If I say \"opongpg\", that probably is not \"malformed\" (it is all\n> lowercase ascii alphabet that is very plausible-looking string), but\n> simply \"bad value\".\n\nSwitched to \"unsupported value\", as suggested in another reply. \n\nHenning\n\n> But other than the minor nit, yes, this structure is what I expected\n> to see from the very first step 1/7.\n> \n> > +\t\tcurrent_format = fmt;\n> >  \t\treturn 0;\n> >  \t}  \n> \n> >  \n> > +\tif (!strcmp(var, \"gpg.program\"))\n> > +\t\tfmtname = \"openpgp\";  \n> \n> OK, this is a backward compatibility measure to treat gpg.program as\n> if it were gpg.openpgp.program, which makes sense.\n> \n> > +\tif (fmtname) {\n> > +\t\tfmt = get_format_by_name(fmtname);\n> > +\t\treturn git_config_string(&fmt->program, var,\n> > value);\n> > +\t}\n> > +\n> >  \treturn 0;\n> >  }  \n\n"},{"id":"352774","messageId":"20180717145002.2623286d@md1pvb1c.ad001.siemens.net","threadId":"48876","inReplyTo":"xmqqva9esydn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 5/7] gpg-interface: introduce new config to select per gpg format program","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-17T12:50:02Z","receivedAt":"2018-07-17T12:50:30Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Am Mon, 16 Jul 2018 13:45:40 -0700\nschrieb Junio C Hamano <gitster@pobox.com>:\n\n> Henning Schild <henning.schild@siemens.com> writes:\n> \n> > +gpg.<format>.program::\n> > +\tUse this to customize the program used for the signing\n> > format you\n> > +\tchose. (see gpg.program) gpg.openpgp.program is a synonym\n> > for the\n> > +\tlegacy gpg.program.  \n> \n> I _think_ you meant \"see gpg.format\", but I am not 100% sure.\n\nNo i actually meant program, the next version just refers to both\nconfig options for further reading.\n\n> When X is a synonym for Y, Y is also a synonym for X, so technically\n> speaking this does not matter, but when we talk about backward\n> compatibility fallback, I think we say \"OLDway is retained as a\n> legacy synonym for NEWway\", i.e. the other way around.\n> \n> Also, `typeset in tt` what end-users would type literally, like\n> configuration variable names, i.e.\n> \n> \tUse this to customize the rpogram used for the signing\n> \tformat you chose (see `gpg.format`).  `gpg.program` can\n> \tstill be used as a legacy synonym for `gpg.openpgp.program`.\n\nUsed that second sentence.\n\nHenning\n\n> >  gui.commitMsgWidth::\n> >  \tDefines how wide the commit message window is in the\n> >  \tlinkgit:git-gui[1]. \"75\" is the default.\n> > diff --git a/gpg-interface.c b/gpg-interface.c\n> > index 93bd0fb32..f3c22b551 100644\n> > --- a/gpg-interface.c\n> > +++ b/gpg-interface.c\n> > @@ -182,7 +182,7 @@ int git_gpg_config(const char *var, const char\n> > *value, void *cb) return 0;\n> >  \t}\n> >  \n> > -\tif (!strcmp(var, \"gpg.program\"))\n> > +\tif (!strcmp(var, \"gpg.program\") || !strcmp(var,\n> > \"gpg.openpgp.program\")) fmtname = \"openpgp\";\n> >  \n> >  \tif (fmtname) {  \n\n"},{"id":"352775","messageId":"20180717145009.6cf163f3@md1pvb1c.ad001.siemens.net","threadId":"48876","inReplyTo":"xmqqh8kzszth.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 1/7] gpg-interface: add new config to select how to sign a commit","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2018-07-17T12:50:09Z","receivedAt":"2018-07-17T12:50:31Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Am Mon, 16 Jul 2018 13:14:34 -0700\nschrieb Junio C Hamano <gitster@pobox.com>:\n\n> Henning Schild <henning.schild@siemens.com> writes:\n> \n> > Add \"gpg.format\" where the user can specify which type of signature\n> > to use for commits. At the moment only \"openpgp\" is supported and\n> > the value is not even used. This commit prepares for a new types of\n> > signatures.\n> >\n> > Signed-off-by: Henning Schild <henning.schild@siemens.com>\n> > ---\n> >  Documentation/config.txt | 4 ++++\n> >  gpg-interface.c          | 7 +++++++\n> >  2 files changed, 11 insertions(+)\n> >\n> > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > index 1cc18a828..ac373e3f4 100644\n> > --- a/Documentation/config.txt\n> > +++ b/Documentation/config.txt\n> > @@ -1828,6 +1828,10 @@ gpg.program::\n> >  \tsigned, and the program is expected to send the result to\n> > its standard output.\n> >  \n> > +gpg.format::\n> > +\tSpecifies which key format to use when signing with\n> > `--gpg-sign`.\n> > +\tDefault is \"openpgp\", that is also the only supported\n> > value. +\n> >  gui.commitMsgWidth::\n> >  \tDefines how wide the commit message window is in the\n> >  \tlinkgit:git-gui[1]. \"75\" is the default.\n> > diff --git a/gpg-interface.c b/gpg-interface.c\n> > index 09ddfbc26..960c40086 100644\n> > --- a/gpg-interface.c\n> > +++ b/gpg-interface.c\n> > @@ -7,6 +7,7 @@\n> >  #include \"tempfile.h\"\n> >  \n> >  static char *configured_signing_key;\n> > +static const char *gpg_format = \"openpgp\";\n> >  static const char *gpg_program = \"gpg\";\n> >  \n> >  #define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n> > @@ -138,6 +139,12 @@ int git_gpg_config(const char *var, const char\n> > *value, void *cb) return 0;\n> >  \t}\n> >  \n> > +\tif (!strcmp(var, \"gpg.format\")) {\n> > +\t\tif (value && strcasecmp(value, \"openpgp\"))\n> > +\t\t\treturn error(\"malformed value for %s: %s\",\n> > var, value);\n> > +\t\treturn git_config_string(&gpg_format, var,\n> > value);  \n> \n> I may be mistaken (sorry, I read so many topics X-<) but I think the\n> consensus was to accept only \"openpgp\" so that we can ensure that\n> \n> \t[gpg \"openpgp\"] program = foo\n> \n> etc. can be parsed more naturally without having to manually special\n> case the subsection name to be case insensitive.  In other words,\n> strcasecmp() should just be strcmp().  Otherwise, people would get a\n> wrong expectation that\n> \n> \t[gpg] format = OpenPGP\n> \t[gpg \"openpgp\"] program = foo\n> \n> should refer to the same and single OpenPGP, but that would violate\n> the usual configuration syntax rules.\n\nOk, also having read the other mails i think we are still at\ngpg.format and gpg.<format>.program, so i switched the two patches from\nstrcasecmp back to strcmp.\n\n> The structure of checking the error looks quite suboptimal even when\n> we initially support the single \"openpgp\" at this step.  I would\n> have expected you to be writing the above like so:\n> \n> \tif (!value)\n> \t\treturn config_error_nonbool(var);\n> \tif (strcmp(value, \"openpgp\"))\n> \t\treturn error(\"unsupported value for %s: %s\", var,\n> value); return git_config_string(...);\n> \n> That would make it more clear that the variable is \"nonbool\", which\n> does not change across additional support for new formats in later\n> steps.\n\nRight, at one point (not mailed) i had that but expected it would not\npass the review, since git_config_string contains that check as well.\nChanged.\n\nHenning\n \n> > +\t}\n> > +\n> >  \tif (!strcmp(var, \"gpg.program\")) {\n> >  \t\tif (!value)\n> >  \t\t\treturn config_error_nonbool(var);  \n\n"}]}