{"thread":{"id":"57363","subject":"[PATCH] gpg-interface: fix for gpgsm v2.3","startedAt":"2022-02-03T12:37:30Z","lastAt":"2022-03-04T10:25:34Z","messageCount":24,"participants":["Fabian Stelzer","Junio C Hamano","Todd Zullinger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"447604","messageId":"20220203123724.47529-1-fs@gigacodes.de","threadId":"57363","inReplyTo":null,"subject":"[PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-03T12:37:24Z","receivedAt":"2022-02-03T12:37:30Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"gpgsm v2.3 changed some details about its output:\n - instead of displaying `fingerprint:` for keys it will print `sha1\n   fpr:` and `sha2 fpr:`\n - some wording of errors has changed\n - signing will omit an extra debug output line before the [GNUPG]: tag\n\nThis change adjusts the gpgsm test prerequisite to work with v2.3 as\nwell by accepting `sha1 fpr:` as well as `fingerprint:` and allows both\nvariants of errors for unknown certs.\nChecking for successful signature creation will omit the leading NL in\nits search string.\n---\n\nI am not a user of gpgsm but noticed that the GPGSM test prereq was disabled \non my runs so i investigated. The `fix` seems rather trivial and I tried to \ntest this as thorough as possible. I ran the test suite on machines \navailable to me (fedora35, centos8) and did a full CI run on github without \nany issues.\nBut if you actually use gpgsm with git please give this a go and let me know \nif I missed anything.\n\n\n gpg-interface.c | 2 +-\n t/lib-gpg.sh    | 4 ++--\n t/t4202-log.sh  | 5 ++++-\n 3 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex b52eb0e2e0..299e7f588a 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -939,7 +939,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   signature, 1024, &gpg_status, 0);\n \tsigchain_pop(SIGPIPE);\n \n-\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n+\tret |= !strstr(gpg_status.buf, \"[GNUPG:] SIG_CREATED \");\n \tstrbuf_release(&gpg_status);\n \tif (ret)\n \t\treturn error(_(\"gpg failed to sign the data\"));\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 3e7ee1386a..6c2dd4b14b 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -73,8 +73,8 @@ test_lazy_prereq GPGSM '\n \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n \n \tgpgsm --homedir \"${GNUPGHOME}\" -K |\n-\tgrep fingerprint: |\n-\tcut -d\" \" -f4 |\n+\tgrep -E \"(fingerprint|sha1 fpr):\" |\n+\tcut -d\":\" -f2- | tr -d \" \" |\n \ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n \n \techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 5049559861..08556493ce 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1931,7 +1931,10 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n \tgrep \"^|\\\\\\  merged tag\" actual &&\n-\tgrep \"^| | gpgsm: certificate not found\" actual\n+\t(\n+\t\tgrep \"^| | gpgsm: certificate not found\" actual ||\n+\t\tgrep \"^| | gpgsm: failed to find the certificate: Not found\" actual\n+\t)\n '\n \n test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '\n-- \n2.34.1\n\n"},{"id":"447638","messageId":"xmqq7dabvkze.fsf@gitster.g","threadId":"57363","inReplyTo":"20220203123724.47529-1-fs@gigacodes.de","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-03T18:55:01Z","receivedAt":"2022-02-03T18:55:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer <fs@gigacodes.de> writes:\n\n> gpgsm v2.3 changed some details about its output:\n>  - instead of displaying `fingerprint:` for keys it will print `sha1\n>    fpr:` and `sha2 fpr:`\n>  - some wording of errors has changed\n>  - signing will omit an extra debug output line before the [GNUPG]: tag\n>\n> This change adjusts the gpgsm test prerequisite to work with v2.3 as\n> well by accepting `sha1 fpr:` as well as `fingerprint:` and allows both\n> variants of errors for unknown certs.\n\nOK, so the change is meant to add support for the new behaviour\nwithout deprecating/removing the support for the older one.  Good.\n\n> Checking for successful signature creation will omit the leading NL in\n> its search string.\n\nI think this is to adjust for \"will omit an extra debug output\"; as\nlong as we still ensure that the \"[GNUPG:] SIG_CREATED\" comes at the\nbeginning of a line with some other means, I think that is a good\nchange.\n\n> I am not a user of gpgsm but noticed that the GPGSM test prereq was disabled \n> on my runs so i investigated. The `fix` seems rather trivial and I tried to \n> test this as thorough as possible. I ran the test suite on machines \n> available to me (fedora35, centos8) and did a full CI run on github without \n> any issues.\n> But if you actually use gpgsm with git please give this a go and let me know \n> if I missed anything.\n\nYup, thanks for a call for help.  I am not gpgsm user either and\ntesting by actual users is very much appreciated.\n\n> @@ -939,7 +939,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n>  \t\t\t   signature, 1024, &gpg_status, 0);\n>  \tsigchain_pop(SIGPIPE);\n>  \n> -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n> +\tret |= !strstr(gpg_status.buf, \"[GNUPG:] SIG_CREATED \");\n\nThis I am not sure about.  I understand that the intention is to\nallow this at the beginning of gpg_status.buf, but not to allow the\nsubstring appear in the middle of an otherwise unrelated line.  I am\nafraid that the new check is a bit too lose for that.\n\nTotally untested but just to illustrate the idea...\n\n gpg-interface.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git c/gpg-interface.c w/gpg-interface.c\nindex b52eb0e2e0..4238e60dfa 100644\n--- c/gpg-interface.c\n+++ w/gpg-interface.c\n@@ -920,6 +920,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n \tsize_t bottom;\n+\tconst char *cp;\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \tstrvec_pushl(&gpg.args,\n@@ -939,7 +940,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   signature, 1024, &gpg_status, 0);\n \tsigchain_pop(SIGPIPE);\n \n-\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n+\tfor (cp = gpg_status.buf;\n+\t     cp && (cp = strstr(cp, \"[GNUPG:] SIG_CREATED \"));\n+\t     cp++) {\n+\t\tif (cp == gpg_status.buf || cp[-1] == '\\n')\n+\t\t\tbreak; /* found */\n+\t}\n+\tret |= !cp;\n \tstrbuf_release(&gpg_status);\n \tif (ret)\n \t\treturn error(_(\"gpg failed to sign the data\"));\n"},{"id":"447648","messageId":"Yfw0kapgSSWO3Pyx@pobox.com","threadId":"57363","inReplyTo":"20220203123724.47529-1-fs@gigacodes.de","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2022-02-03T20:01:21Z","receivedAt":"2022-02-03T20:01:26Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi Fabian,\n\nFabian Stelzer wrote:\n> gpgsm v2.3 changed some details about its output:\n>  - instead of displaying `fingerprint:` for keys it will print `sha1\n>    fpr:` and `sha2 fpr:`\n>  - some wording of errors has changed\n>  - signing will omit an extra debug output line before the [GNUPG]: tag\n\nThanks for sending this.  I noticed these as well, as Fedora\nstarted shipping gnupg-2.3 a few months back.  I have been\ntrying (and failing) to make time to submit (when I know I\nwon't be too distracted to actually converse about them).\nThe commits I made for the tests in Fedora are all here:\n\n    https://src.fedoraproject.org/rpms/git/c/a7d2f7e53\n\nI don't intend that as something anyone here should feel the\nneed to chase down.  But since they provide some additional\ncontext on the changes I made in the same area, it might\nhelp if anyone's curious.\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index b52eb0e2e0..299e7f588a 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -939,7 +939,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n>  \t\t\t   signature, 1024, &gpg_status, 0);\n>  \tsigchain_pop(SIGPIPE);\n>  \n> -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n> +\tret |= !strstr(gpg_status.buf, \"[GNUPG:] SIG_CREATED \");\n>  \tstrbuf_release(&gpg_status);\n>  \tif (ret)\n>  \t\treturn error(_(\"gpg failed to sign the data\"));\n\nAs Junio noted, this loosens the GPG parsing a good bit.  I\nworried it could lead to security issues as well.  The\nsimple fix I made in Fedora was to add a newline to the\ngpg_status string buffer before adding the command output to\nit:\n\n    diff --git a/gpg-interface.c b/gpg-interface.c\n    index 3e7255a2a9..d179dfb3ab 100644\n    --- a/gpg-interface.c\n    +++ b/gpg-interface.c\n    @@ -859,6 +859,12 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n     \n\tbottom = signature->len;\n     \n    +\t/*\n    +\t * Ensure gpg_status begins with a newline or we'll fail to match if\n    +\t * the SIG_CREATED line is at the start of the gpg output.\n    +\t */\n    +\tstrbuf_addch(&gpg_status, '\\n');\n    +\n\t/*\n\t* When the username signingkey is bad, program could be terminated\n\t* because gpg exits without reading and then write gets SIGPIPE.\n\nhttps://src.fedoraproject.org/rpms/git/blob/a7d2f7e53/f/0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch\n\nBut that seemed like a bit of a hack.  What I had queued up\nto submit for discussion (as I'm not sure that it isn't\nentirely horrible) used the string-list API to parse the gpg\noutput:\n\n    diff --git a/gpg-interface.c b/gpg-interface.c\n    index b52eb0e2e0..e63ccdcb11 100644\n    --- a/gpg-interface.c\n    +++ b/gpg-interface.c\n    @@ -921,6 +921,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n     \tint ret;\n     \tsize_t bottom;\n     \tstruct strbuf gpg_status = STRBUF_INIT;\n    +\tstruct string_list lines = { .cmp = starts_with };\n     \n     \tstrvec_pushl(&gpg.args,\n     \t\t     use_format->program,\n    @@ -939,8 +940,11 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n     \t\t\t   signature, 1024, &gpg_status, 0);\n     \tsigchain_pop(SIGPIPE);\n     \n    -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n    +\tstring_list_split_in_place(&lines, gpg_status.buf, '\\n', -1);\n    +\tret |= !unsorted_string_list_has_string(&lines, \"[GNUPG:] SIG_CREATED \");\n     \tstrbuf_release(&gpg_status);\n    +\tstring_list_clear(&lines, 0);\n    +\n     \tif (ret)\n     \t\treturn error(_(\"gpg failed to sign the data\"));\n\nThat's the commit I was most in doubt about though, as my C\n\"skills\" are close to non-existant.  I'd rather have\nsomething ugly and clear (like the `strbuf_addch(...)`\nabove) than clever and wrong in gpg-interface.c.\n\n(To be clear, I mean \"clever and wrong\" in regard to my use\nof the string list API, not anyone else's code.)\n\n> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n> index 3e7ee1386a..6c2dd4b14b 100644\n> --- a/t/lib-gpg.sh\n> +++ b/t/lib-gpg.sh\n> @@ -73,8 +73,8 @@ test_lazy_prereq GPGSM '\n>  \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>  \n>  \tgpgsm --homedir \"${GNUPGHOME}\" -K |\n> -\tgrep fingerprint: |\n> -\tcut -d\" \" -f4 |\n> +\tgrep -E \"(fingerprint|sha1 fpr):\" |\n> +\tcut -d\":\" -f2- | tr -d \" \" |\n>  \ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n\nI think this whole thing can (and should) be simplified by\nusing gpg's --with-colons output which is intended to be\nmachine parsable.\n\nIf we'd been using that previously, we wouldn't need to make\nany further changes here.\n\nI think we're making our lives difficult by screen scraping\nhere wher we don't need to do so.\n\nThe change I made for the Fedora package to fix this does\nit like this:\n\n    --- a/t/lib-gpg.sh\n    +++ b/t/lib-gpg.sh\n    @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n                    --passphrase-fd 0 --pinentry-mode loopback \\\n                    --import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n\n    -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n    -\tgrep fingerprint: |\n    -\tcut -d\" \" -f4 |\n    -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n    +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n    +\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n    +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n\n    -\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n            echo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n                   -u committer@example.com -o /dev/null --sign -\n     '\n\nWith a commit message:\n\n    https://src.fedoraproject.org/rpms/git/blob/a7d2f7e53/f/0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch\n\nI was hoping to submit that small series in the next day or\ntwo, while I've got a few days away from $work.  If doing it\nthat way is appealing, I can submit them.  But only if that\nlooks like a reasonable improvement to you and others.\n\n>  \techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 5049559861..08556493ce 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -1931,7 +1931,10 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n>  \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n>  \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n>  \tgrep \"^|\\\\\\  merged tag\" actual &&\n> -\tgrep \"^| | gpgsm: certificate not found\" actual\n> +\t(\n> +\t\tgrep \"^| | gpgsm: certificate not found\" actual ||\n> +\t\tgrep \"^| | gpgsm: failed to find the certificate: Not found\" actual\n> +\t)\n>  '\n>  \n>  test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '\n\nCan we make this simpler by adjusting the grep pattern?\nIt's certainly a slight trade-off in ease of reading, but it\nsaves a subshell and an extra command:\n\n    -\tgrep \"^| | gpgsm: certificate not found\" actual\n    +\tgrep -Ei \"^| | gpgsm:( failed to find the)? certificate:? not found\" actual\n\nI did that in a separate patch:\n\n    https://src.fedoraproject.org/rpms/git/blob/a7d2f7e53/f/0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch\n\nIMO, this is a bug in gnupg-2.3.  I submitted a patch to\nresolve it back in November, but have not gotten any\nresponse as yet. :(\n\n    https://lists.gnupg.org/pipermail/gnupg-devel/2021-November/034991.html\n\nNot that it will preclude us from having to fix this for the\ntest suite, but it's worth noting why the change is needed\n(and when it will no longer be relevant -- if/when we don't\ncare to support the few early gnupg-2.3.x releases).\n\nThanks,\n\n-- \nTodd\n"},{"id":"447658","messageId":"xmqqsfszr5q6.fsf@gitster.g","threadId":"57363","inReplyTo":"Yfw0kapgSSWO3Pyx@pobox.com","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-03T21:38:09Z","receivedAt":"2022-02-03T21:38:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n>     -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n>     +\tstring_list_split_in_place(&lines, gpg_status.buf, '\\n', -1);\n>     +\tret |= !unsorted_string_list_has_string(&lines, \"[GNUPG:] SIG_CREATED \");\n\nIs \"SIG_CREATED \" supposed to be at the end of that line?  I thought\nthat has_string() asks for an exact match, and unfortunately(?)\nthere is not the string_list_has_string_that_has_this_prefix()\nfunction.  So...\n\n"},{"id":"447676","messageId":"YfxSCbmatoSZlTwB@pobox.com","threadId":"57363","inReplyTo":"xmqqsfszr5q6.fsf@gitster.g","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2022-02-03T22:07:05Z","receivedAt":"2022-02-03T22:07:10Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> Todd Zullinger <tmz@pobox.com> writes:\n> \n>>     -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n>>     +\tstring_list_split_in_place(&lines, gpg_status.buf, '\\n', -1);\n>>     +\tret |= !unsorted_string_list_has_string(&lines, \"[GNUPG:] SIG_CREATED \");\n> \n> Is \"SIG_CREATED \" supposed to be at the end of that line?  I thought\n> that has_string() asks for an exact match, and unfortunately(?)\n> there is not the string_list_has_string_that_has_this_prefix()\n> function.  So...\n\nBy default, yes.  The string_list struct uses strcmp() if no\ncmp function is given.  That's why the previous chunk has:\n\n    struct string_list lines = { .cmp = starts_with };\n\nThere aren't any similar uses in the code, which is just one\nof the reasons I wasn't confident that it was a good idea or\neven a good implementation.\n\n-- \nTodd\n"},{"id":"447680","messageId":"xmqq8rurr2kv.fsf@gitster.g","threadId":"57363","inReplyTo":"YfxSCbmatoSZlTwB@pobox.com","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-03T22:46:08Z","receivedAt":"2022-02-03T22:46:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> By default, yes.  The string_list struct uses strcmp() if no\n> cmp function is given.  That's why the previous chunk has:\n>\n>     struct string_list lines = { .cmp = starts_with };\n\nAhh, I missed that part.  Sounds correct to me, then.\n\nSplitting only to iterate over these lines sounds wasteful to me,\nthough, since we do not need access to these lines only one at a\ntime and there is no need to keep an array of them.\n\nThanks.\n\n"},{"id":"447876","messageId":"20220207105240.dk443kcozynlonpp@fs","threadId":"57363","inReplyTo":"Yfw0kapgSSWO3Pyx@pobox.com","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-07T10:52:40Z","receivedAt":"2022-02-07T10:59:54Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 03.02.2022 15:01, Todd Zullinger wrote:\n>Hi Fabian,\n>\n>Fabian Stelzer wrote:\n>> gpgsm v2.3 changed some details about its output:\n>>  - instead of displaying `fingerprint:` for keys it will print `sha1\n>>    fpr:` and `sha2 fpr:`\n>>  - some wording of errors has changed\n>>  - signing will omit an extra debug output line before the [GNUPG]: tag\n>\n>Thanks for sending this.  I noticed these as well, as Fedora\n>started shipping gnupg-2.3 a few months back.  I have been\n>trying (and failing) to make time to submit (when I know I\n>won't be too distracted to actually converse about them).\n>The commits I made for the tests in Fedora are all here:\n>\n>    https://src.fedoraproject.org/rpms/git/c/a7d2f7e53\n>\n>I don't intend that as something anyone here should feel the\n>need to chase down.  But since they provide some additional\n>context on the changes I made in the same area, it might\n>help if anyone's curious.\n>\n>> diff --git a/gpg-interface.c b/gpg-interface.c\n>> index b52eb0e2e0..299e7f588a 100644\n>> --- a/gpg-interface.c\n>> +++ b/gpg-interface.c\n>> @@ -939,7 +939,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n>>  \t\t\t   signature, 1024, &gpg_status, 0);\n>>  \tsigchain_pop(SIGPIPE);\n>>\n>> -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n>> +\tret |= !strstr(gpg_status.buf, \"[GNUPG:] SIG_CREATED \");\n>>  \tstrbuf_release(&gpg_status);\n>>  \tif (ret)\n>>  \t\treturn error(_(\"gpg failed to sign the data\"));\n>\n>As Junio noted, this loosens the GPG parsing a good bit.  I\n>worried it could lead to security issues as well.  The\n>simple fix I made in Fedora was to add a newline to the\n>gpg_status string buffer before adding the command output to\n>it:\n>\n>    diff --git a/gpg-interface.c b/gpg-interface.c\n>    index 3e7255a2a9..d179dfb3ab 100644\n>    --- a/gpg-interface.c\n>    +++ b/gpg-interface.c\n>    @@ -859,6 +859,12 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n>\n>\tbottom = signature->len;\n>\n>    +\t/*\n>    +\t * Ensure gpg_status begins with a newline or we'll fail to match if\n>    +\t * the SIG_CREATED line is at the start of the gpg output.\n>    +\t */\n>    +\tstrbuf_addch(&gpg_status, '\\n');\n>    +\n>\t/*\n>\t* When the username signingkey is bad, program could be terminated\n>\t* because gpg exits without reading and then write gets SIGPIPE.\n>\n>https://src.fedoraproject.org/rpms/git/blob/a7d2f7e53/f/0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch\n>\n>But that seemed like a bit of a hack.  What I had queued up\n>to submit for discussion (as I'm not sure that it isn't\n>entirely horrible) used the string-list API to parse the gpg\n>output:\n>\n>    diff --git a/gpg-interface.c b/gpg-interface.c\n>    index b52eb0e2e0..e63ccdcb11 100644\n>    --- a/gpg-interface.c\n>    +++ b/gpg-interface.c\n>    @@ -921,6 +921,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n>     \tint ret;\n>     \tsize_t bottom;\n>     \tstruct strbuf gpg_status = STRBUF_INIT;\n>    +\tstruct string_list lines = { .cmp = starts_with };\n>\n>     \tstrvec_pushl(&gpg.args,\n>     \t\t     use_format->program,\n>    @@ -939,8 +940,11 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n>     \t\t\t   signature, 1024, &gpg_status, 0);\n>     \tsigchain_pop(SIGPIPE);\n>\n>    -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n>    +\tstring_list_split_in_place(&lines, gpg_status.buf, '\\n', -1);\n>    +\tret |= !unsorted_string_list_has_string(&lines, \"[GNUPG:] SIG_CREATED \");\n>     \tstrbuf_release(&gpg_status);\n>    +\tstring_list_clear(&lines, 0);\n>    +\n>     \tif (ret)\n>     \t\treturn error(_(\"gpg failed to sign the data\"));\n>\n>That's the commit I was most in doubt about though, as my C\n>\"skills\" are close to non-existant.  I'd rather have\n>something ugly and clear (like the `strbuf_addch(...)`\n>above) than clever and wrong in gpg-interface.c.\n>\n>(To be clear, I mean \"clever and wrong\" in regard to my use\n>of the string list API, not anyone else's code.)\n\nstring_list_split seems a bit like overkill.  My first thought was actually \nsth like:\n\ncp = strstr(gpg_status.buf, \"[GNUPG]: SIG_CREATED\");\nif (cp == gpg_status.buf || --cp == '\\n')\n\t// found\n\nBut that would fail in case the string came up before the actual success \nmessage at the beginning of a line. So Junios variant of using the for() \nloop is more robust and would normally find the correct string on its first \niteration anyway.\n\n>\n>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n>> index 3e7ee1386a..6c2dd4b14b 100644\n>> --- a/t/lib-gpg.sh\n>> +++ b/t/lib-gpg.sh\n>> @@ -73,8 +73,8 @@ test_lazy_prereq GPGSM '\n>>  \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>>\n>>  \tgpgsm --homedir \"${GNUPGHOME}\" -K |\n>> -\tgrep fingerprint: |\n>> -\tcut -d\" \" -f4 |\n>> +\tgrep -E \"(fingerprint|sha1 fpr):\" |\n>> +\tcut -d\":\" -f2- | tr -d \" \" |\n>>  \ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n>\n>I think this whole thing can (and should) be simplified by\n>using gpg's --with-colons output which is intended to be\n>machine parsable.\n\nI looked for sth like this but gpgs --help does not list it so i didn't dig \ndeeper. I've checked the blame and it seems like this was introduced >19 \nyears ago. So i guess we can probably use this ^^\n\n>\n>If we'd been using that previously, we wouldn't need to make\n>any further changes here.\n>\n>I think we're making our lives difficult by screen scraping\n>here wher we don't need to do so.\n>\n>The change I made for the Fedora package to fix this does\n>it like this:\n>\n>    --- a/t/lib-gpg.sh\n>    +++ b/t/lib-gpg.sh\n>    @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n>                    --passphrase-fd 0 --pinentry-mode loopback \\\n>                    --import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>\n>    -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n>    -\tgrep fingerprint: |\n>    -\tcut -d\" \" -f4 |\n>    -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n>    +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n>    +\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n>    +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n\nThis does not quite work for me. It will add the fingerprint without the \ncolons into the trustlist which is not valid :/\nIt would need sth like:\n+       gpgsm --with-colons --homedir \"${GNUPGHOME}\" -K |\n+       awk -F \":\" \"/^(fpr|fingerprint):/ {gsub(/.{2}/, \\\"&:\\\", \\$10); \nprintf \\\"%s S relax\\\\n\\\", substr(\\$10, 1, length(\\$10)-1)}\" \\\n+        >\"${GNUPGHOME}/trustlist.txt\" &&\n\nwhich looks needlessly complicated. There is probably some better way to do \nthis with/without awk.\n\n>\n>    -\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n>            echo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n>                   -u committer@example.com -o /dev/null --sign -\n>     '\n>\n>With a commit message:\n>\n>    https://src.fedoraproject.org/rpms/git/blob/a7d2f7e53/f/0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch\n>\n>I was hoping to submit that small series in the next day or\n>two, while I've got a few days away from $work.  If doing it\n>that way is appealing, I can submit them.  But only if that\n>looks like a reasonable improvement to you and others.\n>\n>>  \techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n>> index 5049559861..08556493ce 100755\n>> --- a/t/t4202-log.sh\n>> +++ b/t/t4202-log.sh\n>> @@ -1931,7 +1931,10 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n>>  \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n>>  \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n>>  \tgrep \"^|\\\\\\  merged tag\" actual &&\n>> -\tgrep \"^| | gpgsm: certificate not found\" actual\n>> +\t(\n>> +\t\tgrep \"^| | gpgsm: certificate not found\" actual ||\n>> +\t\tgrep \"^| | gpgsm: failed to find the certificate: Not found\" actual\n>> +\t)\n>>  '\n>>\n>>  test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '\n>\n>Can we make this simpler by adjusting the grep pattern?\n>It's certainly a slight trade-off in ease of reading, but it\n>saves a subshell and an extra command:\n>\n>    -\tgrep \"^| | gpgsm: certificate not found\" actual\n>    +\tgrep -Ei \"^| | gpgsm:( failed to find the)? certificate:? not found\" actual\n\nThanks, will do.\n\n>\n>I did that in a separate patch:\n>\n>    https://src.fedoraproject.org/rpms/git/blob/a7d2f7e53/f/0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch\n>\n>IMO, this is a bug in gnupg-2.3.  I submitted a patch to\n>resolve it back in November, but have not gotten any\n>response as yet. :(\n>\n>    https://lists.gnupg.org/pipermail/gnupg-devel/2021-November/034991.html\n>\n>Not that it will preclude us from having to fix this for the\n>test suite, but it's worth noting why the change is needed\n>(and when it will no longer be relevant -- if/when we don't\n>care to support the few early gnupg-2.3.x releases).\n>\n>Thanks,\n>\n>-- \n>Todd\n"},{"id":"447891","messageId":"YgFK+F6Ks8FnN5Q6@pobox.com","threadId":"57363","inReplyTo":"20220207105240.dk443kcozynlonpp@fs","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2022-02-07T16:38:16Z","receivedAt":"2022-02-07T16:47:36Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi Fabien,\n\nFabian Stelzer wrote:\n> On 03.02.2022 15:01, Todd Zullinger wrote:\n>> (To be clear, I mean \"clever and wrong\" in regard to my use\n>> of the string list API, not anyone else's code.)\n>\n> string_list_split seems a bit like overkill.\n\nI have little doubt that the string_list_split() method is\nfar from ideal. :)\n\n> I looked for sth like this but gpgs --help does not list it so i didn't dig\n> deeper. I've checked the blame and it seems like this was introduced >19\n> years ago. So i guess we can probably use this ^^\n\nIndeed, the --with-colons output goes much further back in\nthe GnuPG history than Git will ever have to care about.\n\n>>    --- a/t/lib-gpg.sh\n>>    +++ b/t/lib-gpg.sh\n>>    @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n>>                    --passphrase-fd 0 --pinentry-mode loopback \\\n>>                    --import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>> \n>>    -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n>>    -\tgrep fingerprint: |\n>>    -\tcut -d\" \" -f4 |\n>>    -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n>>    +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n>>    +\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n>>    +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n> \n> This does not quite work for me. It will add the fingerprint without the\n> colons into the trustlist which is not valid :/\n\nThe colons are optional, and have been documented as such\nsince cb1840720 ((Agent Configuration): New section.,\n2005-04-20).  The text in the gpg-agent docs from GnuPG 2.2\nsay:\n\n    Colons may optionally be used to separate the bytes of a\n    fingerprint; this enables cutting and pasting the\n    fingerprint from a key listing output.\n\nSource: https://dev.gnupg.org/source/gnupg/browse/STABLE-BRANCH-2-2/doc/gpg-agent.texi;8021fe7670c79d5c698ec3fb600b02a9e5afb415$756?as=source&blame=off\n\nHow did it fail for you?  It passes all the tests when I've\nrun it against Fedora and RHEL-based hosts.  If it's flaky\non other systems, that would put a damper on doing it this\nway.  Though it _should_ work.\n\n[Note to myself] We don't just generate the key data,\ntrustlist, etc. and store it in t/lib-gpg like we do with\nsome other files per b41a36e635 (tests: create gpg homedir\non the fly, 2014-12-12).  That was because the gnupg home\ndirectory layout changed a bit between 2.0 and 2.1.\n\nThanks,\n\n-- \nTodd\n"},{"id":"448036","messageId":"20220209083351.dsoxnhhme3lracck@fs","threadId":"57363","inReplyTo":"YgFK+F6Ks8FnN5Q6@pobox.com","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-09T08:33:51Z","receivedAt":"2022-02-09T08:34:21Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 07.02.2022 11:38, Todd Zullinger wrote:\n>Hi Fabien,\n>\n>Fabian Stelzer wrote:\n>> On 03.02.2022 15:01, Todd Zullinger wrote:\n>>> (To be clear, I mean \"clever and wrong\" in regard to my use\n>>> of the string list API, not anyone else's code.)\n>>\n>> string_list_split seems a bit like overkill.\n>\n>I have little doubt that the string_list_split() method is\n>far from ideal. :)\n>\n>> I looked for sth like this but gpgs --help does not list it so i didn't dig\n>> deeper. I've checked the blame and it seems like this was introduced >19\n>> years ago. So i guess we can probably use this ^^\n>\n>Indeed, the --with-colons output goes much further back in\n>the GnuPG history than Git will ever have to care about.\n>\n>>>    --- a/t/lib-gpg.sh\n>>>    +++ b/t/lib-gpg.sh\n>>>    @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n>>>                    --passphrase-fd 0 --pinentry-mode loopback \\\n>>>                    --import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>>>\n>>>    -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n>>>    -\tgrep fingerprint: |\n>>>    -\tcut -d\" \" -f4 |\n>>>    -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n>>>    +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n>>>    +\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n>>>    +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n>>\n>> This does not quite work for me. It will add the fingerprint without the\n>> colons into the trustlist which is not valid :/\n>\n>The colons are optional, and have been documented as such\n>since cb1840720 ((Agent Configuration): New section.,\n>2005-04-20).  The text in the gpg-agent docs from GnuPG 2.2\n>say:\n>\n>    Colons may optionally be used to separate the bytes of a\n>    fingerprint; this enables cutting and pasting the\n>    fingerprint from a key listing output.\n>\n>Source: https://dev.gnupg.org/source/gnupg/browse/STABLE-BRANCH-2-2/doc/gpg-agent.texi;8021fe7670c79d5c698ec3fb600b02a9e5afb415$756?as=source&blame=off\n>\n>How did it fail for you?  It passes all the tests when I've\n>run it against Fedora and RHEL-based hosts.  If it's flaky\n>on other systems, that would put a damper on doing it this\n>way.  Though it _should_ work.\n\nSorry for the delays, I'm a bit busy with other things at the moment. I did \nget an interactive popup asking if I would like to trust the key when I ran \nthe t4202 test. This never happened with the old variant.\n\n>\n>[Note to myself] We don't just generate the key data,\n>trustlist, etc. and store it in t/lib-gpg like we do with\n>some other files per b41a36e635 (tests: create gpg homedir\n>on the fly, 2014-12-12).  That was because the gnupg home\n>directory layout changed a bit between 2.0 and 2.1.\n>\n>Thanks,\n>\n>-- \n>Todd\n"},{"id":"448049","messageId":"YgPpsJ1UCEI0a4b6@pobox.com","threadId":"57363","inReplyTo":"20220209083351.dsoxnhhme3lracck@fs","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2022-02-09T16:20:00Z","receivedAt":"2022-02-09T16:20:10Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Fabian Stelzer wrote:\n> On 07.02.2022 11:38, Todd Zullinger wrote:\n>> How did it fail for you?  It passes all the tests when I've\n>> run it against Fedora and RHEL-based hosts.  If it's flaky\n>> on other systems, that would put a damper on doing it this\n>> way.  Though it _should_ work.\n> \n> Sorry for the delays, I'm a bit busy with other things at the moment.\n\nNo apologies needed.  This is something I worked on back in\nNovember and had yet to send to the list, so I'm the last\nperson to rush another. :)\n\n> I did get an interactive popup asking if I would like to\n> trust the key when I ran the t4202 test. This never\n> happened with the old variant.\n\nInteresting.  I do have a patch in my gnupg-2.3 series to\nreload the gpg agent after changing the trustlist, as the\nchanges were not picked up prior to that.  In my case, I was\nrunning the tests in an environment where gpg could not\nprompt me.  (It also seems like we should try harder to have\nthe test suite reject such prompts).\n\n--- 8< ---\nSubject: [PATCH] t/lib-gpg: reload gpg components after updating trustlist\n\nWith gpgsm from gnupg-2.3, the changes to the trustlist.txt do not\nappear to be picked up without refreshing the gpg-agent.  Use the 'all'\nkeyword to reload all of the gpg components.  The scdaemon is started as\na child of gpg-agent, for example.\n\nWe used to have a --kill at this spot, but I removed it in 2e285e7803\n(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07).  It seems\nlike it might be necessary (again) for 2.3.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n\nNotes:\n    An alternative to doing this dance with the trustlist.txt and having to\n    kill and/or reload the gpg-agent to pick up the change right after the\n    import in each test might be to make this part of the steps used when\n    adding/updating/removing certificates in t/lib-gpg.\n    \n    If not as a one-time affair when a cert is added/update/removed, then\n    perhaps as a step taken by/in t/lib-gpg.sh only once.  It could populate\n    a gpghome to be copied into the trash dir for each test which used\n    gpg/gpgsm.  I haven't measured the effect of the extra reload precisely,\n    but I'm sure it's not free.\n    \n    (For what it's worth, it didn't add any noticeable amount of time to the\n    full builds/test runs I made while working on this, so it's a seemingly\n    small cost, at least.)\n    \n    Also, hello Henning,\n    \n    Way back, in February 2019¹, when I submitted 2e285e7803 to remove the\n    \"redundant\" killing of the gpg-agent, you said:\n    \n    > Killing the agent once should be enough, i remember manually killing\n    > it many times as i was looking for a way to generate certs and trust\n    > (configure gpgsm for the test). That is probably why i copied it over\n    > in the first place.\n    \n    As I wrote this patch to partially restore the gpg-agent killing (now\n    just a reload), I thought this might have been the sort of issue that\n    you hit while testing.\n    \n    It could be unrelated, but it sounds quite similar to what I found with\n    gnupg-2.3 when trying to get it to pick up the trustlist.txt changes.  I\n    thought you might at least enjoy seeing it come back around.  :)\n    \n    ¹ <20190208093324.7b17f270@md1za8fc.ad001.siemens.net>\n\n t/lib-gpg.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 6bc083ca77..38e2c0f4fb 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '\n \tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n \tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n \t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n+\t(gpgconf --reload all || : ) &&\n \n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n\n--- 8< ---\n\nI have another patch which changes the earlier gpgconf call\nwhich kills gpg-agent to kill all gpg daemons, as there are\nsome others which could potentially interfere with the\ntests:\n\n--- 8< ---\nSubject: [PATCH] t/lib-gpg: kill all gpg components, not just gpg-agent\n\nThe gpg-agent is one of several processes that newer releases of GnuPG\nstart automatically.  Issue a kill to each of them to ensure they do not\naffect separate tests.  (Yes, the separate GNUPGHOME should do that\nalready. If we find that is case, we could drop the --kill entirely.)\n\nIn terms of compatibility, the 'all' keyword was added to the --kill &\n--reload options in GnuPG 2.1.18.  Debian and RHEL are often used as\nindicators of how a change might affect older systems we often try to\nsupport.\n\n    - Debian Strech (old old stable), which has limited security support\n      until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).\n\n    - CentOS/RHEL 7, which is supported until June 2024, has GnuPG\n      2.0.22, which lacks the --kill option, so the change won't have\n      any impact.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex d675698a2d..2bb309a8c1 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -40,7 +40,7 @@ test_lazy_prereq GPG '\n \t\t#\t\t> lib-gpg/ownertrust\n \t\tmkdir \"$GNUPGHOME\" &&\n \t\tchmod 0700 \"$GNUPGHOME\" &&\n-\t\t(gpgconf --kill gpg-agent || : ) &&\n+\t\t(gpgconf --kill all || : ) &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import \\\n \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import-ownertrust \\\n--- 8< ---\n\nI have the series in the gpg-misc-fixes branch:\n\n    https://github.com/tmzullinger/git/commits/gpg-misc-fixes\n\n(That series does differ in that it has `string_list_split()`\ninstead of the simpler `strbuf_addch(&gpg_status, '\\n');` to\nfix the gpgsm parsing.)\n\nIff you think it would be useful or helpful, I can post that\nseries.\n\nThanks,\n\n-- \nTodd\n"},{"id":"448964","messageId":"20220221092234.6kg66c3tuo2pya2a@fs","threadId":"57363","inReplyTo":"YgPpsJ1UCEI0a4b6@pobox.com","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-21T09:22:34Z","receivedAt":"2022-02-21T09:51:16Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 09.02.2022 11:20, Todd Zullinger wrote:\n>Fabian Stelzer wrote:\n>> On 07.02.2022 11:38, Todd Zullinger wrote:\n>>> How did it fail for you?  It passes all the tests when I've\n>>> run it against Fedora and RHEL-based hosts.  If it's flaky\n>>> on other systems, that would put a damper on doing it this\n>>> way.  Though it _should_ work.\n>>\n>> Sorry for the delays, I'm a bit busy with other things at the moment.\n>\n>No apologies needed.  This is something I worked on back in\n>November and had yet to send to the list, so I'm the last\n>person to rush another. :)\n>\n>> I did get an interactive popup asking if I would like to\n>> trust the key when I ran the t4202 test. This never\n>> happened with the old variant.\n>\n>Interesting.  I do have a patch in my gnupg-2.3 series to\n>reload the gpg agent after changing the trustlist, as the\n>changes were not picked up prior to that.  In my case, I was\n>running the tests in an environment where gpg could not\n>prompt me.  (It also seems like we should try harder to have\n>the test suite reject such prompts).\n>\n\nYes, gpg-agent in general can be problematic for the tests. I'm not familiar \nenough with gpg but I don't know if we can get by without it?\n\n>--- 8< ---\n>Subject: [PATCH] t/lib-gpg: reload gpg components after updating trustlist\n>\n>With gpgsm from gnupg-2.3, the changes to the trustlist.txt do not\n>appear to be picked up without refreshing the gpg-agent.  Use the 'all'\n>keyword to reload all of the gpg components.  The scdaemon is started as\n>a child of gpg-agent, for example.\n>\n>We used to have a --kill at this spot, but I removed it in 2e285e7803\n>(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07).  It seems\n>like it might be necessary (again) for 2.3.\n>\n>Signed-off-by: Todd Zullinger <tmz@pobox.com>\n>---\n>\n>Notes:\n>    An alternative to doing this dance with the trustlist.txt and having to\n>    kill and/or reload the gpg-agent to pick up the change right after the\n>    import in each test might be to make this part of the steps used when\n>    adding/updating/removing certificates in t/lib-gpg.\n>\n>    If not as a one-time affair when a cert is added/update/removed, then\n>    perhaps as a step taken by/in t/lib-gpg.sh only once.  It could populate\n>    a gpghome to be copied into the trash dir for each test which used\n>    gpg/gpgsm.  I haven't measured the effect of the extra reload precisely,\n>    but I'm sure it's not free.\n>\n>    (For what it's worth, it didn't add any noticeable amount of time to the\n>    full builds/test runs I made while working on this, so it's a seemingly\n>    small cost, at least.)\n>\n>    Also, hello Henning,\n>\n>    Way back, in February 2019¹, when I submitted 2e285e7803 to remove the\n>    \"redundant\" killing of the gpg-agent, you said:\n>\n>    > Killing the agent once should be enough, i remember manually killing\n>    > it many times as i was looking for a way to generate certs and trust\n>    > (configure gpgsm for the test). That is probably why i copied it over\n>    > in the first place.\n>\n>    As I wrote this patch to partially restore the gpg-agent killing (now\n>    just a reload), I thought this might have been the sort of issue that\n>    you hit while testing.\n>\n>    It could be unrelated, but it sounds quite similar to what I found with\n>    gnupg-2.3 when trying to get it to pick up the trustlist.txt changes.  I\n>    thought you might at least enjoy seeing it come back around.  :)\n>\n>    ¹ <20190208093324.7b17f270@md1za8fc.ad001.siemens.net>\n>\n> t/lib-gpg.sh | 1 +\n> 1 file changed, 1 insertion(+)\n>\n>diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n>index 6bc083ca77..38e2c0f4fb 100644\n>--- a/t/lib-gpg.sh\n>+++ b/t/lib-gpg.sh\n>@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '\n> \tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n> \tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n> \t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n>+\t(gpgconf --reload all || : ) &&\n>\n> \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n> \t       -u committer@example.com -o /dev/null --sign -\n>\n>--- 8< ---\n\nThis patch fixes it for me.\n\n>\n>I have another patch which changes the earlier gpgconf call\n>which kills gpg-agent to kill all gpg daemons, as there are\n>some others which could potentially interfere with the\n>tests:\n>\n>--- 8< ---\n>Subject: [PATCH] t/lib-gpg: kill all gpg components, not just gpg-agent\n>\n>The gpg-agent is one of several processes that newer releases of GnuPG\n>start automatically.  Issue a kill to each of them to ensure they do not\n>affect separate tests.  (Yes, the separate GNUPGHOME should do that\n>already. If we find that is case, we could drop the --kill entirely.)\n>\n>In terms of compatibility, the 'all' keyword was added to the --kill &\n>--reload options in GnuPG 2.1.18.  Debian and RHEL are often used as\n>indicators of how a change might affect older systems we often try to\n>support.\n>\n>    - Debian Strech (old old stable), which has limited security support\n>      until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).\n>\n>    - CentOS/RHEL 7, which is supported until June 2024, has GnuPG\n>      2.0.22, which lacks the --kill option, so the change won't have\n>      any impact.\n>\n>Signed-off-by: Todd Zullinger <tmz@pobox.com>\n>---\n> t/lib-gpg.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n>diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n>index d675698a2d..2bb309a8c1 100644\n>--- a/t/lib-gpg.sh\n>+++ b/t/lib-gpg.sh\n>@@ -40,7 +40,7 @@ test_lazy_prereq GPG '\n> \t\t#\t\t> lib-gpg/ownertrust\n> \t\tmkdir \"$GNUPGHOME\" &&\n> \t\tchmod 0700 \"$GNUPGHOME\" &&\n>-\t\t(gpgconf --kill gpg-agent || : ) &&\n>+\t\t(gpgconf --kill all || : ) &&\n> \t\tgpg --homedir \"${GNUPGHOME}\" --import \\\n> \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n> \t\tgpg --homedir \"${GNUPGHOME}\" --import-ownertrust \\\n>--- 8< ---\n>\n>I have the series in the gpg-misc-fixes branch:\n>\n>    https://github.com/tmzullinger/git/commits/gpg-misc-fixes\n>\n>(That series does differ in that it has `string_list_split()`\n>instead of the simpler `strbuf_addch(&gpg_status, '\\n');` to\n>fix the gpgsm parsing.)\n>\n>Iff you think it would be useful or helpful, I can post that\n>series.\n\nI have prepared the patch with the simple strstr() matching I can post in a \nbit. I would add your two gpg test lib patches to it if thats ok?\n\nThanks\n\n>\n>Thanks,\n>\n>-- \n>Todd\n"},{"id":"449210","messageId":"YhW6Xx5iVqVleUYc@pobox.com","threadId":"57363","inReplyTo":"20220221092234.6kg66c3tuo2pya2a@fs","subject":"Re: [PATCH] gpg-interface: fix for gpgsm v2.3","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2022-02-23T04:38:55Z","receivedAt":"2022-02-23T04:39:03Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Fabian Stelzer wrote:\n> On 09.02.2022 11:20, Todd Zullinger wrote:\n>> Interesting.  I do have a patch in my gnupg-2.3 series to\n>> reload the gpg agent after changing the trustlist, as the\n>> changes were not picked up prior to that.  In my case, I was\n>> running the tests in an environment where gpg could not\n>> prompt me.  (It also seems like we should try harder to have\n>> the test suite reject such prompts).\n>> \n> \n> Yes, gpg-agent in general can be problematic for the tests. I'm not familiar\n> enough with gpg but I don't know if we can get by without it?\n\nWith modern gnupg, the secret keyring access is handled by\ngpg-agent.  So it's no longer optional, which is mildly\nunfortunate for automated tests..\n\n>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n>> index 6bc083ca77..38e2c0f4fb 100644\n>> --- a/t/lib-gpg.sh\n>> +++ b/t/lib-gpg.sh\n>> @@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '\n>> \tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n>> \tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n>> \t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n>> +\t(gpgconf --reload all || : ) &&\n>> \n>> \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n>> \t       -u committer@example.com -o /dev/null --sign -\n>> \n>> --- 8< ---\n> \n> This patch fixes it for me.\n\nExcellent.\n\n> I have prepared the patch with the simple strstr() matching I can post in a\n> bit. I would add your two gpg test lib patches to it if thats ok?\n\nAbsolutely.  Thank you for working on this and pulling it\ntogether.\n\nCheers,\n\n-- \nTodd\n"},{"id":"449421","messageId":"20220224100628.612789-2-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220203123724.47529-1-fs@gigacodes.de","subject":"[PATCH 2/3] t/lib-gpg: reload gpg components after updating trustlist","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-24T10:06:27Z","receivedAt":"2022-02-24T10:06:42Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"From: Todd Zullinger <tmz@pobox.com>\n\nWith gpgsm from gnupg-2.3, the changes to the trustlist.txt do not\nappear to be picked up without refreshing the gpg-agent.  Use the 'all'\nkeyword to reload all of the gpg components.  The scdaemon is started as\na child of gpg-agent, for example.\n\nWe used to have a --kill at this spot, but I removed it in 2e285e7803\n(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07).  It seems\nlike it might be necessary (again) for 2.3.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex e997ce10ea..2bad35e61a 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '\n \tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n \tawk -F \":\" \"/^(fpr|fingerprint):/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n \t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n+\t(gpgconf --reload all || : ) &&\n \n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n-- \n2.35.1\n\n"},{"id":"449422","messageId":"20220224100628.612789-3-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220203123724.47529-1-fs@gigacodes.de","subject":"[PATCH 3/3] t/lib-gpg: kill all gpg components, not just gpg-agent","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-24T10:06:28Z","receivedAt":"2022-02-24T10:06:44Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"From: Todd Zullinger <tmz@pobox.com>\n\nThe gpg-agent is one of several processes that newer releases of GnuPG\nstart automatically.  Issue a kill to each of them to ensure they do not\naffect separate tests.  (Yes, the separate GNUPGHOME should do that\nalready. If we find that is case, we could drop the --kill entirely.)\n\nIn terms of compatibility, the 'all' keyword was added to the --kill &\n--reload options in GnuPG 2.1.18.  Debian and RHEL are often used as\nindicators of how a change might affect older systems we often try to\nsupport.\n\n    - Debian Strech (old old stable), which has limited security support\n      until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).\n\n    - CentOS/RHEL 7, which is supported until June 2024, has GnuPG\n      2.0.22, which lacks the --kill option, so the change won't have\n      any impact.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 2bad35e61a..8b9fb6e932 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -40,7 +40,7 @@ test_lazy_prereq GPG '\n \t\t#\t\t> lib-gpg/ownertrust\n \t\tmkdir \"$GNUPGHOME\" &&\n \t\tchmod 0700 \"$GNUPGHOME\" &&\n-\t\t(gpgconf --kill gpg-agent || : ) &&\n+\t\t(gpgconf --kill all || : ) &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import \\\n \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import-ownertrust \\\n-- \n2.35.1\n\n"},{"id":"449423","messageId":"20220224100628.612789-1-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220203123724.47529-1-fs@gigacodes.de","subject":"[PATCH 1/3] gpg-interface/gpgsm: fix for v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-02-24T10:06:26Z","receivedAt":"2022-02-24T10:06:46Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"gpgsm v2.3 changed some details about its output:\n - instead of displaying `fingerprint:` for keys it will print `sha1\n   fpr:` and `sha2 fpr:`\n - some wording of errors has changed\n - signing will omit an extra debug output line before the [GNUPG]: tag\n\nThis change adjusts the gpgsm test prerequisite to work with v2.3 as\nwell by accepting `sha1 fpr:` as well as `fingerprint:`. To make this\nparsing more robust switch to gpg's `--with-colons` output format.\nAlso allow both variants of errors for unknown certs.\nChecking if signing was successful will now accept '[GNUPG]:\nSIG_CREATED' on any beginning of a line. Not just explictly the second\none anymore.\n\nHelped-By: Junio C Hamano <gitster@pobox.com>\nHelped-By: Todd Zullinger <tmz@pobox.com>\n---\n gpg-interface.c | 9 ++++++++-\n t/lib-gpg.sh    | 8 +++-----\n t/t4202-log.sh  | 2 +-\n 3 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 17b1e44baa..94abb3090b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -934,6 +934,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n \tsize_t bottom;\n+\tconst char *cp;\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \tstrvec_pushl(&gpg.args,\n@@ -953,7 +954,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   signature, 1024, &gpg_status, 0);\n \tsigchain_pop(SIGPIPE);\n \n-\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n+\tfor (cp = gpg_status.buf;\n+\t     cp && (cp = strstr(cp, \"[GNUPG:] SIG_CREATED \"));\n+\t     cp++) {\n+\t\tif (cp == gpg_status.buf || cp[-1] == '\\n')\n+\t\t\tbreak; /* found */\n+\t}\n+\tret |= !cp;\n \tstrbuf_release(&gpg_status);\n \tif (ret)\n \t\treturn error(_(\"gpg failed to sign the data\"));\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 3e7ee1386a..e997ce10ea 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n \t\t--passphrase-fd 0 --pinentry-mode loopback \\\n \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n \n-\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n-\tgrep fingerprint: |\n-\tcut -d\" \" -f4 |\n-\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n+\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n+\tawk -F \":\" \"/^(fpr|fingerprint):/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n+\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n \n-\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n '\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 544f0aa82e..493e376e73 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -2013,7 +2013,7 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n \tgrep \"^|\\\\\\  merged tag\" actual &&\n-\tgrep \"^| | gpgsm: certificate not found\" actual\n+\tgrep -Ei \"^| | gpgsm:( failed to find the)? certificate:? not found\" actual\n '\n \n test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '\n-- \n2.35.1\n\n"},{"id":"449791","messageId":"Yh0NHkyquB7nht3W@pobox.com","threadId":"57363","inReplyTo":"20220224100628.612789-1-fs@gigacodes.de","subject":"Re: [PATCH 1/3] gpg-interface/gpgsm: fix for v2.3","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2022-02-28T17:57:50Z","receivedAt":"2022-02-28T18:22:43Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nFabian Stelzer wrote:\n> gpgsm v2.3 changed some details about its output:\n>  - instead of displaying `fingerprint:` for keys it will print `sha1\n>    fpr:` and `sha2 fpr:`\n>  - some wording of errors has changed\n>  - signing will omit an extra debug output line before the [GNUPG]: tag\n> \n> This change adjusts the gpgsm test prerequisite to work with v2.3 as\n> well by accepting `sha1 fpr:` as well as `fingerprint:`. To make this\n> parsing more robust switch to gpg's `--with-colons` output format.\n> Also allow both variants of errors for unknown certs.\n\nI ran this series through the fedora buildsystem on releases\nwith gnupg 2.2 and 2.3.  All the tests pass, as expected.\n\nI think we may be able to simplify the wording above and the\npatch below regarding the fingerprint/shaN fpr output\nchange, I'll add a comment below the changed hunk.\n\n> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n> index 3e7ee1386a..e997ce10ea 100644\n> --- a/t/lib-gpg.sh\n> +++ b/t/lib-gpg.sh\n> @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n>  \t\t--passphrase-fd 0 --pinentry-mode loopback \\\n>  \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>  \n> -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n> -\tgrep fingerprint: |\n> -\tcut -d\" \" -f4 |\n> -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n> +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n> +\tawk -F \":\" \"/^(fpr|fingerprint):/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n> +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n\nUsing --with-colons to parse the output, we shouldn't be\naffected by the changed output.  The pattern for awk can be\nsimplified to '^fpr:' as older and newer versions of gnupg\nhave used that string in the --with-colons output for many,\nmany years.\n\nPerhaps that allows the commit message to say less about the\nspecific's the gnugp-2.3 output change and just mention that\nit changed and using --with-colons is the preferred way to\nparse the output (where we must parse output at all).\n\n    Switch to gpg's `--with-colons` output format to make\n    parsing more robust.  This avoids issues where the\n    human-readable output from gpg commands changes.\n\nor something?\n\nThanks,\n\n-- \nTodd\n"},{"id":"450068","messageId":"20220302090250.590450-1-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220224100628.612789-1-fs@gigacodes.de","subject":"[PATCH v3 1/3] gpg-interface/gpgsm: fix for v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-02T09:02:48Z","receivedAt":"2022-03-02T09:03:06Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Checking if signing was successful will now accept '[GNUPG]:\nSIG_CREATED' on any beginning of a line. Not just explictly the second\none anymore.\n\nSwitch to gpg's `--with-colons` output format to make\nparsing more robust.  This avoids issues where the\nhuman-readable output from gpg commands changes.\n\nAdjust error messages checking in tests for v2.3 specific output changes.\n\nHelped-By: Junio C Hamano <gitster@pobox.com>\nHelped-By: Todd Zullinger <tmz@pobox.com>\n---\n gpg-interface.c | 9 ++++++++-\n t/lib-gpg.sh    | 8 +++-----\n t/t4202-log.sh  | 2 +-\n 3 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex aa50224e67..280f1fa1a5 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -934,6 +934,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n \tsize_t bottom;\n+\tconst char *cp;\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \tstrvec_pushl(&gpg.args,\n@@ -953,7 +954,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   signature, 1024, &gpg_status, 0);\n \tsigchain_pop(SIGPIPE);\n \n-\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n+\tfor (cp = gpg_status.buf;\n+\t     cp && (cp = strstr(cp, \"[GNUPG:] SIG_CREATED \"));\n+\t     cp++) {\n+\t\tif (cp == gpg_status.buf || cp[-1] == '\\n')\n+\t\t\tbreak; /* found */\n+\t}\n+\tret |= !cp;\n \tstrbuf_release(&gpg_status);\n \tif (ret)\n \t\treturn error(_(\"gpg failed to sign the data\"));\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 3e7ee1386a..6bc083ca77 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n \t\t--passphrase-fd 0 --pinentry-mode loopback \\\n \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n \n-\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n-\tgrep fingerprint: |\n-\tcut -d\" \" -f4 |\n-\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n+\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n+\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n+\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n \n-\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n '\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 55fac64446..d599bf4b11 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -2037,7 +2037,7 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n \tgrep \"^|\\\\\\  merged tag\" actual &&\n-\tgrep \"^| | gpgsm: certificate not found\" actual\n+\tgrep -Ei \"^| | gpgsm:( failed to find the)? certificate:? not found\" actual\n '\n \n test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '\n-- \n2.35.1\n\n"},{"id":"450069","messageId":"20220302090250.590450-2-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220224100628.612789-1-fs@gigacodes.de","subject":"[PATCH v3 2/3] t/lib-gpg: reload gpg components after updating trustlist","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-02T09:02:49Z","receivedAt":"2022-03-02T09:03:10Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"From: Todd Zullinger <tmz@pobox.com>\n\nWith gpgsm from gnupg-2.3, the changes to the trustlist.txt do not\nappear to be picked up without refreshing the gpg-agent.  Use the 'all'\nkeyword to reload all of the gpg components.  The scdaemon is started as\na child of gpg-agent, for example.\n\nWe used to have a --kill at this spot, but I removed it in 2e285e7803\n(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07).  It seems\nlike it might be necessary (again) for 2.3.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 6bc083ca77..38e2c0f4fb 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '\n \tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n \tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n \t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n+\t(gpgconf --reload all || : ) &&\n \n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n-- \n2.35.1\n\n"},{"id":"450070","messageId":"20220302090250.590450-3-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220224100628.612789-1-fs@gigacodes.de","subject":"[PATCH v3 3/3] t/lib-gpg: kill all gpg components, not just gpg-agent","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-02T09:02:50Z","receivedAt":"2022-03-02T09:03:22Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"From: Todd Zullinger <tmz@pobox.com>\n\nThe gpg-agent is one of several processes that newer releases of GnuPG\nstart automatically.  Issue a kill to each of them to ensure they do not\naffect separate tests.  (Yes, the separate GNUPGHOME should do that\nalready. If we find that is case, we could drop the --kill entirely.)\n\nIn terms of compatibility, the 'all' keyword was added to the --kill &\n--reload options in GnuPG 2.1.18.  Debian and RHEL are often used as\nindicators of how a change might affect older systems we often try to\nsupport.\n\n    - Debian Strech (old old stable), which has limited security support\n      until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).\n\n    - CentOS/RHEL 7, which is supported until June 2024, has GnuPG\n      2.0.22, which lacks the --kill option, so the change won't have\n      any impact.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 38e2c0f4fb..114785586a 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -40,7 +40,7 @@ test_lazy_prereq GPG '\n \t\t#\t\t> lib-gpg/ownertrust\n \t\tmkdir \"$GNUPGHOME\" &&\n \t\tchmod 0700 \"$GNUPGHOME\" &&\n-\t\t(gpgconf --kill gpg-agent || : ) &&\n+\t\t(gpgconf --kill all || : ) &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import \\\n \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import-ownertrust \\\n-- \n2.35.1\n\n"},{"id":"450155","messageId":"xmqqh78gkvsm.fsf@gitster.g","threadId":"57363","inReplyTo":"20220302090250.590450-1-fs@gigacodes.de","subject":"Re: [PATCH v3 1/3] gpg-interface/gpgsm: fix for v2.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-02T19:18:33Z","receivedAt":"2022-03-02T19:18:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer <fs@gigacodes.de> writes:\n\n> Checking if signing was successful will now accept '[GNUPG]:\n> SIG_CREATED' on any beginning of a line. Not just explictly the second\n> one anymore.\n\n\"the second or subsequent one\", I would think, but the code change\nlooks correct anyway.\n\n> Switch to gpg's `--with-colons` output format to make\n> parsing more robust.  This avoids issues where the\n> human-readable output from gpg commands changes.\n\nDoes this refer only to how parsing in tests is done?\n\n> Adjust error messages checking in tests for v2.3 specific output changes.\n\nDoes this refer only to the change to 4202 where \"failed to find\nthe\" and the colon after \"certificate\" are made optional, so that\nthe regexp can read messages from both pre- and post-2.3 versions?\n\n> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n> index 3e7ee1386a..6bc083ca77 100644\n> --- a/t/lib-gpg.sh\n> +++ b/t/lib-gpg.sh\n> @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n>  \t\t--passphrase-fd 0 --pinentry-mode loopback \\\n>  \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>  \n> -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n> -\tgrep fingerprint: |\n> -\tcut -d\" \" -f4 |\n> -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n> +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n> +\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n> +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n\nThe old iteration had (fpr|fingerprint) which appeared as if it were\ncatering to both pre- and post-2.3 versions, but \"with colons\", all\nversions we care about would say \"fpr\" and that is the reason why we\nno longer have such an alternative here?  Just checking my\nunderstanding.\n\n> -\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n\nThis removal is because...?  I do not recall seeing the explanation\nin the proposed log message.\n\n>  \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n>  \t       -u committer@example.com -o /dev/null --sign -\n>  '\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 55fac64446..d599bf4b11 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -2037,7 +2037,7 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n>  \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n>  \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n>  \tgrep \"^|\\\\\\  merged tag\" actual &&\n> -\tgrep \"^| | gpgsm: certificate not found\" actual\n> +\tgrep -Ei \"^| | gpgsm:( failed to find the)? certificate:? not found\" actual\n>  '\n\nOK.  It might be easier to read if we give two expressions\nseparately and say \"we can take either of these\", i.e.\n\n\t# the former is from pre-2.3, the latter is from 2.3 and later\n\tgrep -e \"^| | gpgsm: certificate not found\" \\\n\t     -e \"^| | gpgsm: failed to find the certificate: not found\" \\\n\t     actual\n\nThanks for working on this update.\n"},{"id":"450261","messageId":"20220303115114.7pbnggjccku44wqa@fs","threadId":"57363","inReplyTo":"xmqqh78gkvsm.fsf@gitster.g","subject":"Re: [PATCH v3 1/3] gpg-interface/gpgsm: fix for v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-03T11:51:14Z","receivedAt":"2022-03-03T11:51:23Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 02.03.2022 11:18, Junio C Hamano wrote:\n>Fabian Stelzer <fs@gigacodes.de> writes:\n>\n>> Checking if signing was successful will now accept '[GNUPG]:\n>> SIG_CREATED' on any beginning of a line. Not just explictly the second\n>> one anymore.\n>\n>\"the second or subsequent one\", I would think, but the code change\n>looks correct anyway.\n>\n>> Switch to gpg's `--with-colons` output format to make\n>> parsing more robust.  This avoids issues where the\n>> human-readable output from gpg commands changes.\n>\n>Does this refer only to how parsing in tests is done?\n\nIf only refers to the test prerequisite actually. I'll update the message.\n\n>\n>> Adjust error messages checking in tests for v2.3 specific output changes.\n>\n>Does this refer only to the change to 4202 where \"failed to find\n>the\" and the colon after \"certificate\" are made optional, so that\n>the regexp can read messages from both pre- and post-2.3 versions?\n>\n>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n>> index 3e7ee1386a..6bc083ca77 100644\n>> --- a/t/lib-gpg.sh\n>> +++ b/t/lib-gpg.sh\n>> @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n>>  \t\t--passphrase-fd 0 --pinentry-mode loopback \\\n>>  \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n>>\n>> -\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n>> -\tgrep fingerprint: |\n>> -\tcut -d\" \" -f4 |\n>> -\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n>> +\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n>> +\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n>> +\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n>\n>The old iteration had (fpr|fingerprint) which appeared as if it were\n>catering to both pre- and post-2.3 versions, but \"with colons\", all\n>versions we care about would say \"fpr\" and that is the reason why we\n>no longer have such an alternative here?  Just checking my\n>understanding.\n\nCorrect. The `with-colons` always uses fpr pre and post 2.3\n\n>\n>> -\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n>\n>This removal is because...?  I do not recall seeing the explanation\n>in the proposed log message.\n\nSwitching to awk allows us to integrate this trailing info into the awk \nexpression itself making this extra echo unnecessary.\n\n>\n>>  \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n>>  \t       -u committer@example.com -o /dev/null --sign -\n>>  '\n>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n>> index 55fac64446..d599bf4b11 100755\n>> --- a/t/t4202-log.sh\n>> +++ b/t/t4202-log.sh\n>> @@ -2037,7 +2037,7 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n>>  \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n>>  \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n>>  \tgrep \"^|\\\\\\  merged tag\" actual &&\n>> -\tgrep \"^| | gpgsm: certificate not found\" actual\n>> +\tgrep -Ei \"^| | gpgsm:( failed to find the)? certificate:? not found\" actual\n>>  '\n>\n>OK.  It might be easier to read if we give two expressions\n>separately and say \"we can take either of these\", i.e.\n>\n>\t# the former is from pre-2.3, the latter is from 2.3 and later\n>\tgrep -e \"^| | gpgsm: certificate not found\" \\\n>\t     -e \"^| | gpgsm: failed to find the certificate: not found\" \\\n>\t     actual\n>\n>Thanks for working on this update.\n\nEasy enough. Initially I used a subshell and 2 grep calls but this is \nobviously easier. I prefer the static strings over the regex as well.\n\nI'll send a new patch probably tomorrow and try to improve the commit \nmessage.\n\nThanks\n"},{"id":"450368","messageId":"20220304102519.623896-1-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220302090250.590450-1-fs@gigacodes.de","subject":"[PATCH v4 1/3] gpg-interface/gpgsm: fix for v2.3","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-04T10:25:17Z","receivedAt":"2022-03-04T10:25:29Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Checking if signing was successful will now accept '[GNUPG]:\nSIG_CREATED' on the beginning of the first or any subsequent line. Not\njust explictly the second one anymore.\n\nGpgsm v2.3 changed its output when listing keys from `fingerprint` to\n`sha1/2 fpr`. This leads to the gpgsm tests silently not being executed\nbecause of a failed prerequisite.\nSwitch to gpg's `--with-colons` output format when evaluating test\nprerequisites to make parsing more robust. This also allows us to\ncombine the existing grep/cut/tr/echo pipe for writing the trustlist.txt\ninto a single awk expression.\n\nAdjust error message checking in test for v2.3 specific output changes.\n\nHelped-By: Junio C Hamano <gitster@pobox.com>\nHelped-By: Todd Zullinger <tmz@pobox.com>\n---\n gpg-interface.c | 9 ++++++++-\n t/lib-gpg.sh    | 8 +++-----\n t/t4202-log.sh  | 3 ++-\n 3 files changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex aa50224e67..280f1fa1a5 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -934,6 +934,7 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n \tsize_t bottom;\n+\tconst char *cp;\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \tstrvec_pushl(&gpg.args,\n@@ -953,7 +954,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   signature, 1024, &gpg_status, 0);\n \tsigchain_pop(SIGPIPE);\n \n-\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n+\tfor (cp = gpg_status.buf;\n+\t     cp && (cp = strstr(cp, \"[GNUPG:] SIG_CREATED \"));\n+\t     cp++) {\n+\t\tif (cp == gpg_status.buf || cp[-1] == '\\n')\n+\t\t\tbreak; /* found */\n+\t}\n+\tret |= !cp;\n \tstrbuf_release(&gpg_status);\n \tif (ret)\n \t\treturn error(_(\"gpg failed to sign the data\"));\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 3e7ee1386a..6bc083ca77 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '\n \t\t--passphrase-fd 0 --pinentry-mode loopback \\\n \t\t--import \"$TEST_DIRECTORY\"/lib-gpg/gpgsm_cert.p12 &&\n \n-\tgpgsm --homedir \"${GNUPGHOME}\" -K |\n-\tgrep fingerprint: |\n-\tcut -d\" \" -f4 |\n-\ttr -d \"\\\\n\" >\"${GNUPGHOME}/trustlist.txt\" &&\n+\tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n+\tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n+\t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n \n-\techo \" S relax\" >>\"${GNUPGHOME}/trustlist.txt\" &&\n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n '\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 55fac64446..6306e2cbe5 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -2037,7 +2037,8 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss\n \tgit merge --no-ff -m msg signed_tag_x509_nokey &&\n \tGNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&\n \tgrep \"^|\\\\\\  merged tag\" actual &&\n-\tgrep \"^| | gpgsm: certificate not found\" actual\n+\tgrep -e \"^| | gpgsm: certificate not found\" \\\n+\t     -e \"^| | gpgsm: failed to find the certificate: Not found\" actual\n '\n \n test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '\n-- \n2.35.1\n\n"},{"id":"450369","messageId":"20220304102519.623896-3-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220302090250.590450-1-fs@gigacodes.de","subject":"[PATCH v4 3/3] t/lib-gpg: kill all gpg components, not just gpg-agent","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-04T10:25:19Z","receivedAt":"2022-03-04T10:25:31Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"From: Todd Zullinger <tmz@pobox.com>\n\nThe gpg-agent is one of several processes that newer releases of GnuPG\nstart automatically.  Issue a kill to each of them to ensure they do not\naffect separate tests.  (Yes, the separate GNUPGHOME should do that\nalready. If we find that is case, we could drop the --kill entirely.)\n\nIn terms of compatibility, the 'all' keyword was added to the --kill &\n--reload options in GnuPG 2.1.18.  Debian and RHEL are often used as\nindicators of how a change might affect older systems we often try to\nsupport.\n\n    - Debian Strech (old old stable), which has limited security support\n      until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).\n\n    - CentOS/RHEL 7, which is supported until June 2024, has GnuPG\n      2.0.22, which lacks the --kill option, so the change won't have\n      any impact.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 38e2c0f4fb..114785586a 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -40,7 +40,7 @@ test_lazy_prereq GPG '\n \t\t#\t\t> lib-gpg/ownertrust\n \t\tmkdir \"$GNUPGHOME\" &&\n \t\tchmod 0700 \"$GNUPGHOME\" &&\n-\t\t(gpgconf --kill gpg-agent || : ) &&\n+\t\t(gpgconf --kill all || : ) &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import \\\n \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n \t\tgpg --homedir \"${GNUPGHOME}\" --import-ownertrust \\\n-- \n2.35.1\n\n"},{"id":"450370","messageId":"20220304102519.623896-2-fs@gigacodes.de","threadId":"57363","inReplyTo":"20220302090250.590450-1-fs@gigacodes.de","subject":"[PATCH v4 2/3] t/lib-gpg: reload gpg components after updating trustlist","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-03-04T10:25:18Z","receivedAt":"2022-03-04T10:25:34Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"From: Todd Zullinger <tmz@pobox.com>\n\nWith gpgsm from gnupg-2.3, the changes to the trustlist.txt do not\nappear to be picked up without refreshing the gpg-agent.  Use the 'all'\nkeyword to reload all of the gpg components.  The scdaemon is started as\na child of gpg-agent, for example.\n\nWe used to have a --kill at this spot, but I removed it in 2e285e7803\n(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07).  It seems\nlike it might be necessary (again) for 2.3.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 6bc083ca77..38e2c0f4fb 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '\n \tgpgsm --homedir \"${GNUPGHOME}\" -K --with-colons |\n \tawk -F \":\" \"/^fpr:/ {printf \\\"%s S relax\\\\n\\\", \\$10}\" \\\n \t\t>\"${GNUPGHOME}/trustlist.txt\" &&\n+\t(gpgconf --reload all || : ) &&\n \n \techo hello | gpgsm --homedir \"${GNUPGHOME}\" >/dev/null \\\n \t       -u committer@example.com -o /dev/null --sign -\n-- \n2.35.1\n\n"}]}