{"thread":{"id":"50432","subject":"[PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","startedAt":"2019-02-08T03:17:54Z","lastAt":"2019-02-14T06:32:14Z","messageCount":15,"participants":["Todd Zullinger","Henning Schild","Junio C Hamano","SZEDER Gábor","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"368771","messageId":"20190208031746.22683-1-tmz@pobox.com","threadId":"50432","inReplyTo":null,"subject":"[PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-02-08T03:17:44Z","receivedAt":"2019-02-08T03:17:54Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nLooking through the build logs for the fedora git packages, I noticed it\nwas missing the GPGSM prereq.  I added the necessary package to the\nbuild requirements but GPGSM was still failing to be set.  This turned\nout to be due to a use of ${GNUPGHOME} without quoting, which leads to a\nnon-zero exit from echo and the end of the happy && chain when using\nbash as the test shell.  Fixing this allows the GPGSM test prereq to be\nset.\n\nWhile I was poking around I also saw an extra gpgconf call to kill\ngpg-agent.  This was copied from the GPG block earlier in lib-gpg.sh,\nbut should not be needed (as far as I can tell).  I don't think it can\ncause any real harm apart from causing gpg and gpgsm to start the agent\nmore often than necessary.  But I didn't run the tests with the --stress\noption to look for potential issues that could be more serious.\n\nLastly, the GPG test prereq was failing in two of the tests where it was\nused, t5573-pull-verify-signatures and t7612-merge-verify-signatures.  I\ntracked this down to an annoying issue with gnugp-2¹, which recently\nbecame the default /bin/gpg in fedora².\n\nUsing gnupg2 as /bin/gpg means using gpg-agent by default.  When using a\nnon-standard GNUPGHOME, gpg-agent defaults to putting its socket files\nin GNUPGHOME and fails if the path for any of them is longer than\nsun_path (108 chars on linux, 104 on OpenBSD and FreeBSD, and likely\nsimilar on other unices).\n\nWhen building in the typical fedora build tool (mock), the path to the\ngit test dir is \"/builddir/build/BUILD/git-2.20.1/t.\"  That path then\nhas \"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\" appended and a\n\"gpghome\" directory within.  For t5573 and t7612, the gpg-agent socket\npath for S.gpg-agent.browser exceeds the sun_path limit and gpg-agent\nfails to start.  Sadly, this is handled poorly by gpg and makes the\ntests fail to set either the GPG or GPGSM prereqs.\n\nFor the fedora packages, I decided to pass --root=/tmp/git-t.XXXX (via\nmktemp, of course) to the test suite which ensures a path short enough\nto keep gpg-agent happy.\n\nI don't know if there are other packagers or builders who run into this,\nso maybe it's not worth much effort to try and have the test suite cope\nbetter.  It took me longer than I would have liked to track it down, so\nI thought I'd mention it in case anyone else has run into this or has\nthoughts on how to improve lib-gpg.sh while waiting for GnuPG to improve\nthis area.\n\nA GIT_TEST_GNUPGHOME_ROOT var to set the root path for the GNUPGHOME\ndirs in the tests is one thought I had, but didn't try to put it into\npatch form.  Setting the --root test option is probably enough control\nfor most cases.\n\n¹ https://dev.gnupg.org/T2964\n² https://fedoraproject.org/wiki/Changes/GnuPG2_as_default_GPG_implementation\n\nTodd Zullinger (2):\n  t/lib-gpg: quote path to ${GNUPGHOME}/trustlist.txt\n  t/lib-gpg: drop redundant killing of gpg-agent\n\n t/lib-gpg.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\n-- \nTodd\n"},{"id":"368772","messageId":"20190208031746.22683-3-tmz@pobox.com","threadId":"50432","inReplyTo":"20190208031746.22683-1-tmz@pobox.com","subject":"[PATCH 2/2] t/lib-gpg: drop redundant killing of gpg-agent","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-02-08T03:17:46Z","receivedAt":"2019-02-08T03:17:58Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"In 53fc999306 (\"gpg-interface t: extend the existing GPG tests with\nGPGSM\", 2018-07-20), the gpgconf call which kills gpg-agent was copied\nfrom the existing gpg setup code.\n\nThe reason for killing gpg-agent is given in 29ff1f8f74 (\"t: lib-gpg:\nflush gpg agent on startup\", 2017-07-20):\n\n  When running gpg-relevant tests, a gpg-daemon is spawned for each\n  GNUPGHOME used. This daemon may stay running after the test and cache\n  file descriptors for the trash directories, even after the trash\n  directory is removed. This leads to ENOENT errors when attempting to\n  create files if tests are run multiple times.\n\n  Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME\n  (if any) before setting up the GPG relevant-environment.\n\nKilling gpg-agent once per test is sufficient.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 207009793b..8d28652b72 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -64,7 +64,6 @@ then\n \t\ttr -d '\\n' >\"${GNUPGHOME}/trustlist.txt\" &&\n \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-- \nTodd\n"},{"id":"368773","messageId":"20190208031746.22683-2-tmz@pobox.com","threadId":"50432","inReplyTo":"20190208031746.22683-1-tmz@pobox.com","subject":"[PATCH 1/2] t/lib-gpg: quote path to ${GNUPGHOME}/trustlist.txt","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-02-08T03:17:45Z","receivedAt":"2019-02-08T03:18:00Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"When gpgsm is installed, lib-gpg.sh attempts to update trustlist.txt to\nrelax the checking of some root certificate requirements.  The path to\n\"${GNUPGHOME}\" contains spaces which cause an \"ambiguous redirect\"\nwarning when bash is used to run the tests:\n\n  $ bash t7030-verify-tag.sh\n  /git/t/lib-gpg.sh: line 66: ${GNUPGHOME}/trustlist.txt: ambiguous redirect\n  ok 1 - create signed tags\n  ok 2 # skip create signed tags x509  (missing GPGSM)\n  ...\n\nNo warning is issued when using bash called as /bin/sh, dash, or mksh.\n\nQuote the path to ensure the redirect works as intended and sets the\nGPGSM prereq.  While we're here, drop the space after \">>\".\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 f1277bef4f..207009793b 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -63,7 +63,7 @@ then\n \t\tcut -d\" \" -f4 |\n \t\ttr -d '\\n' >\"${GNUPGHOME}/trustlist.txt\" &&\n \n-\t\techo \" S relax\" >> ${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-- \nTodd\n"},{"id":"368783","messageId":"20190208093324.7b17f270@md1za8fc.ad001.siemens.net","threadId":"50432","inReplyTo":"20190208031746.22683-1-tmz@pobox.com","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Henning Schild","fromEmail":"henning.schild@siemens.com","sentAt":"2019-02-08T08:33:24Z","receivedAt":"2019-02-08T08:39:26Z","isPatch":true,"sender":{"key":"henning.schild@siemens.com","avatar":"https://avatars.githubusercontent.com/u/7237877?v=4"},"body":"Hi,\n\nboth patches look good to me. Killing the agent once should be enough,\ni remember manually killing it many times as i was looking for a way to\ngenerate certs and trust (configure gpgsm for the test). That is\nprobably why i copied it over in the first place.\n\nHenning\n\nAm Thu, 7 Feb 2019 22:17:44 -0500\nschrieb Todd Zullinger <tmz@pobox.com>:\n\n> Hi,\n> \n> Looking through the build logs for the fedora git packages, I noticed\n> it was missing the GPGSM prereq.  I added the necessary package to the\n> build requirements but GPGSM was still failing to be set.  This turned\n> out to be due to a use of ${GNUPGHOME} without quoting, which leads\n> to a non-zero exit from echo and the end of the happy && chain when\n> using bash as the test shell.  Fixing this allows the GPGSM test\n> prereq to be set.\n> \n> While I was poking around I also saw an extra gpgconf call to kill\n> gpg-agent.  This was copied from the GPG block earlier in lib-gpg.sh,\n> but should not be needed (as far as I can tell).  I don't think it can\n> cause any real harm apart from causing gpg and gpgsm to start the\n> agent more often than necessary.  But I didn't run the tests with the\n> --stress option to look for potential issues that could be more\n> serious.\n> \n> Lastly, the GPG test prereq was failing in two of the tests where it\n> was used, t5573-pull-verify-signatures and\n> t7612-merge-verify-signatures.  I tracked this down to an annoying\n> issue with gnugp-2¹, which recently became the default /bin/gpg in\n> fedora².\n> \n> Using gnupg2 as /bin/gpg means using gpg-agent by default.  When\n> using a non-standard GNUPGHOME, gpg-agent defaults to putting its\n> socket files in GNUPGHOME and fails if the path for any of them is\n> longer than sun_path (108 chars on linux, 104 on OpenBSD and FreeBSD,\n> and likely similar on other unices).\n> \n> When building in the typical fedora build tool (mock), the path to the\n> git test dir is \"/builddir/build/BUILD/git-2.20.1/t.\"  That path then\n> has \"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\" appended and a\n> \"gpghome\" directory within.  For t5573 and t7612, the gpg-agent socket\n> path for S.gpg-agent.browser exceeds the sun_path limit and gpg-agent\n> fails to start.  Sadly, this is handled poorly by gpg and makes the\n> tests fail to set either the GPG or GPGSM prereqs.\n> \n> For the fedora packages, I decided to pass --root=/tmp/git-t.XXXX (via\n> mktemp, of course) to the test suite which ensures a path short enough\n> to keep gpg-agent happy.\n> \n> I don't know if there are other packagers or builders who run into\n> this, so maybe it's not worth much effort to try and have the test\n> suite cope better.  It took me longer than I would have liked to\n> track it down, so I thought I'd mention it in case anyone else has\n> run into this or has thoughts on how to improve lib-gpg.sh while\n> waiting for GnuPG to improve this area.\n> \n> A GIT_TEST_GNUPGHOME_ROOT var to set the root path for the GNUPGHOME\n> dirs in the tests is one thought I had, but didn't try to put it into\n> patch form.  Setting the --root test option is probably enough control\n> for most cases.\n> \n> ¹ https://dev.gnupg.org/T2964\n> ²\n> https://fedoraproject.org/wiki/Changes/GnuPG2_as_default_GPG_implementation\n> \n> Todd Zullinger (2):\n>   t/lib-gpg: quote path to ${GNUPGHOME}/trustlist.txt\n>   t/lib-gpg: drop redundant killing of gpg-agent\n> \n>  t/lib-gpg.sh | 3 +--\n>  1 file changed, 1 insertion(+), 2 deletions(-)\n> \n\n"},{"id":"368849","messageId":"xmqqimxukvhl.fsf@gitster-ct.c.googlers.com","threadId":"50432","inReplyTo":"20190208093324.7b17f270@md1za8fc.ad001.siemens.net","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T18:04:06Z","receivedAt":"2019-02-08T18:04:11Z","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> both patches look good to me. Killing the agent once should be enough,\n> i remember manually killing it many times as i was looking for a way to\n> generate certs and trust (configure gpgsm for the test). That is\n> probably why i copied it over in the first place.\n\nThanks, both.  Will queue.\n"},{"id":"368873","messageId":"20190208201105.GA10587@szeder.dev","threadId":"50432","inReplyTo":"20190208031746.22683-2-tmz@pobox.com","subject":"Re: [PATCH 1/2] t/lib-gpg: quote path to ${GNUPGHOME}/trustlist.txt","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-08T20:11:05Z","receivedAt":"2019-02-08T20:11:11Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Feb 07, 2019 at 10:17:45PM -0500, Todd Zullinger wrote:\n> When gpgsm is installed, lib-gpg.sh attempts to update trustlist.txt to\n> relax the checking of some root certificate requirements.  The path to\n> \"${GNUPGHOME}\" contains spaces which cause an \"ambiguous redirect\"\n> warning when bash is used to run the tests:\n\ns/error/warning/\n\n>   $ bash t7030-verify-tag.sh\n>   /git/t/lib-gpg.sh: line 66: ${GNUPGHOME}/trustlist.txt: ambiguous redirect\n>   ok 1 - create signed tags\n>   ok 2 # skip create signed tags x509  (missing GPGSM)\n>   ...\n> \n> No warning is issued when using bash called as /bin/sh, dash, or mksh.\n\nLikewise.\n\nPOSIX says that no field splitting should be performed on the result\nof a parameter expansion that is used as the target of a redirection,\nbut Bash doesn't conform in this respect (unless in POSIX mode).\n\n> Quote the path to ensure the redirect works as intended and sets the\n> GPGSM prereq.  While we're here, drop the space after \">>\".\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 f1277bef4f..207009793b 100755\n> --- a/t/lib-gpg.sh\n> +++ b/t/lib-gpg.sh\n> @@ -63,7 +63,7 @@ then\n>  \t\tcut -d\" \" -f4 |\n>  \t\ttr -d '\\n' >\"${GNUPGHOME}/trustlist.txt\" &&\n>  \n> -\t\techo \" S relax\" >> ${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> -- \n> Todd\n"},{"id":"368879","messageId":"20190208202505.GE30548@zaya.teonanacatl.net","threadId":"50432","inReplyTo":"20190208201105.GA10587@szeder.dev","subject":"Re: [PATCH 1/2] t/lib-gpg: quote path to ${GNUPGHOME}/trustlist.txt","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-02-08T20:25:05Z","receivedAt":"2019-02-08T20:31:52Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"SZEDER Gábor wrote:\n> On Thu, Feb 07, 2019 at 10:17:45PM -0500, Todd Zullinger wrote:\n>> When gpgsm is installed, lib-gpg.sh attempts to update trustlist.txt to\n>> relax the checking of some root certificate requirements.  The path to\n>> \"${GNUPGHOME}\" contains spaces which cause an \"ambiguous redirect\"\n>> warning when bash is used to run the tests:\n> \n> s/error/warning/\n\nDid you mean s/warning/error/ so the sentence reads:\n\n    The path to \"${GNUPGHOME}\" contains spaces which cause\n    an \"ambiguous redirect\" error when bash is used to run\n    the tests\n\n?\n\nIs it worth a resend before Junio queues it?\n\n>>   $ bash t7030-verify-tag.sh\n>>   /git/t/lib-gpg.sh: line 66: ${GNUPGHOME}/trustlist.txt: ambiguous redirect\n>>   ok 1 - create signed tags\n>>   ok 2 # skip create signed tags x509  (missing GPGSM)\n>>   ...\n>> \n>> No warning is issued when using bash called as /bin/sh, dash, or mksh.\n> \n> Likewise.\n> \n> POSIX says that no field splitting should be performed on the result\n> of a parameter expansion that is used as the target of a redirection,\n> but Bash doesn't conform in this respect (unless in POSIX mode).\n\nI wish I'd remembered reading your detailed explanation of\nthis¹ when I was testing and writing the commit message. :)\n\n¹ https://public-inbox.org/git/20180926121107.GH27036@localhost/\n\n-- \nTodd\n"},{"id":"368880","messageId":"20190208203542.GD10587@szeder.dev","threadId":"50432","inReplyTo":"20190208202505.GE30548@zaya.teonanacatl.net","subject":"Re: [PATCH 1/2] t/lib-gpg: quote path to ${GNUPGHOME}/trustlist.txt","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-08T20:35:42Z","receivedAt":"2019-02-08T20:35:48Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Feb 08, 2019 at 03:25:05PM -0500, Todd Zullinger wrote:\n> SZEDER Gábor wrote:\n> > On Thu, Feb 07, 2019 at 10:17:45PM -0500, Todd Zullinger wrote:\n> >> When gpgsm is installed, lib-gpg.sh attempts to update trustlist.txt to\n> >> relax the checking of some root certificate requirements.  The path to\n> >> \"${GNUPGHOME}\" contains spaces which cause an \"ambiguous redirect\"\n> >> warning when bash is used to run the tests:\n> > \n> > s/error/warning/\n> \n> Did you mean s/warning/error/ so the sentence reads:\n> \n>     The path to \"${GNUPGHOME}\" contains spaces which cause\n>     an \"ambiguous redirect\" error when bash is used to run\n>     the tests\n\nOh, wow.  Indeed that's what I meant.\n\n> Is it worth a resend before Junio queues it?\n\nI remember Junio taking care of minor touchups to commit messages like\nthis, so maybe not.\n\n"},{"id":"368920","messageId":"20190209140605.GE10587@szeder.dev","threadId":"50432","inReplyTo":"20190208031746.22683-1-tmz@pobox.com","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-09T14:06:05Z","receivedAt":"2019-02-09T14:06:12Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Feb 07, 2019 at 10:17:44PM -0500, Todd Zullinger wrote:\n> Looking through the build logs for the fedora git packages, I noticed it\n> was missing the GPGSM prereq.\n\nJust curious: how did you noticed the missing GPGSM prereq?\n\nI'm asking because I use a patch for a good couple of months now that\ncollects the prereqs missed by test cases and prints them at the end\nof 'make test'.  Its output looks like this:\n\n  https://travis-ci.org/szeder/git/jobs/490944032#L2358\n\nSince you seem to be interested in that sort of thing as well, perhaps\nit would be worth to have something like this in git.git?  It's just\nthat I have been too wary of potentially annoying other contributors\nby adding (what might be perceived as) clutter to their 'make test'\noutput :)\n\n\n> Lastly, the GPG test prereq was failing in two of the tests where it was\n> used, t5573-pull-verify-signatures and t7612-merge-verify-signatures.  I\n> tracked this down to an annoying issue with gnugp-2¹, which recently\n> became the default /bin/gpg in fedora².\n> \n> Using gnupg2 as /bin/gpg means using gpg-agent by default.  When using a\n> non-standard GNUPGHOME, gpg-agent defaults to putting its socket files\n> in GNUPGHOME and fails if the path for any of them is longer than\n> sun_path (108 chars on linux, 104 on OpenBSD and FreeBSD, and likely\n> similar on other unices).\n> \n> When building in the typical fedora build tool (mock), the path to the\n> git test dir is \"/builddir/build/BUILD/git-2.20.1/t.\"  That path then\n> has \"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\" appended and a\n> \"gpghome\" directory within.  For t5573 and t7612, the gpg-agent socket\n> path for S.gpg-agent.browser exceeds the sun_path limit and gpg-agent\n> fails to start.  Sadly, this is handled poorly by gpg and makes the\n> tests fail to set either the GPG or GPGSM prereqs.\n> \n> For the fedora packages, I decided to pass --root=/tmp/git-t.XXXX (via\n> mktemp, of course) to the test suite which ensures a path short enough\n> to keep gpg-agent happy.\n> \n> I don't know if there are other packagers or builders who run into this,\n> so maybe it's not worth much effort to try and have the test suite cope\n> better.  It took me longer than I would have liked to track it down, so\n> I thought I'd mention it in case anyone else has run into this or has\n> thoughts on how to improve lib-gpg.sh while waiting for GnuPG to improve\n> this area.\n\nI stumbled upon this when Dscho inadvertently broke a test script on\nsetups without gpg last year; sorry for not speaking about it.  I\nnoticed it in our Travis CI builds on macOS, because it (macOS itself\nor Homebrew? I don't know) defaulted to gpg2 already back then, and to\nmake matters worse its sun_path is on the shorter end, and the path\nto the build dir on Travis CI includes the GitHub user/repo as well.\n\n> A GIT_TEST_GNUPGHOME_ROOT var to set the root path for the GNUPGHOME\n> dirs in the tests is one thought I had, but didn't try to put it into\n> patch form.  Setting the --root test option is probably enough control\n> for most cases.\n\nA potential issue I see with GIT_TEST_GNUPGHOME_ROOT is that there are\nseveral test scripts involving gpg, and if GIT_TEST_GNUPGHOME_ROOT is\nset for the whole 'make test', then they might interfere with each\nother when they happen to be run at the same time.\n\nIn the meantime I came up with a '--short-trash-dir' option to\ntest-lib, which turns 'trash directory.t7612-merge-verify-signatures'\ninto 'trash dir.t7612'.  It works, but I don't really like it, and it\nrequired various adjustments to the CI build scripts, notably to the\npart in 'ci/print-test-failures.sh' that includes the trash dir of\nfailed test scripts in the build log.\n\n\n"},{"id":"368947","messageId":"20190209230553.GI30548@zaya.teonanacatl.net","threadId":"50432","inReplyTo":"20190209140605.GE10587@szeder.dev","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-02-09T23:05:53Z","receivedAt":"2019-02-09T23:06:02Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"SZEDER Gábor wrote:\n> On Thu, Feb 07, 2019 at 10:17:44PM -0500, Todd Zullinger wrote:\n>> Looking through the build logs for the fedora git packages, I noticed it\n>> was missing the GPGSM prereq.\n> \n> Just curious: how did you noticed the missing GPGSM prereq?\n\nI just grep the build output for '# SKIP|skipped:' and then\nfilter out those which I expect (thing like MINGW and\nNATIVE_CRLF that aren't likely to be in a Fedora build).\n\nFar more manual than the slick method you have below. :)\n\n> I'm asking because I use a patch for a good couple of months now that\n> collects the prereqs missed by test cases and prints them at the end\n> of 'make test'.  Its output looks like this:\n> \n>   https://travis-ci.org/szeder/git/jobs/490944032#L2358\n> \n> Since you seem to be interested in that sort of thing as well, perhaps\n> it would be worth to have something like this in git.git?  It's just\n> that I have been too wary of potentially annoying other contributors\n> by adding (what might be perceived as) clutter to their 'make test'\n> output :)\n\nIndeed, I think that would be useful.  At the very least,\nthe .missing_prereqs files look quite handy.  I wouldn't\nmind the output from 'make test' either, but building\npackages surely shifts my perspective toward more verbose\nbuild logs than someone hacking on git regularly and reading\nthe 'make test' output.\n\n>> I don't know if there are other packagers or builders who run into this,\n>> so maybe it's not worth much effort to try and have the test suite cope\n>> better.  It took me longer than I would have liked to track it down, so\n>> I thought I'd mention it in case anyone else has run into this or has\n>> thoughts on how to improve lib-gpg.sh while waiting for GnuPG to improve\n>> this area.\n> \n> I stumbled upon this when Dscho inadvertently broke a test script on\n> setups without gpg last year; sorry for not speaking about it.  I\n> noticed it in our Travis CI builds on macOS, because it (macOS itself\n> or Homebrew? I don't know) defaulted to gpg2 already back then, and to\n> make matters worse its sun_path is on the shorter end, and the path\n> to the build dir on Travis CI includes the GitHub user/repo as well.\n\nHeh, I figured it was a rather specific group of folks that\nmight run into this.\n\n>> A GIT_TEST_GNUPGHOME_ROOT var to set the root path for the GNUPGHOME\n>> dirs in the tests is one thought I had, but didn't try to put it into\n>> patch form.  Setting the --root test option is probably enough control\n>> for most cases.\n> \n> A potential issue I see with GIT_TEST_GNUPGHOME_ROOT is that there are\n> several test scripts involving gpg, and if GIT_TEST_GNUPGHOME_ROOT is\n> set for the whole 'make test', then they might interfere with each\n> other when they happen to be run at the same time.\n\nYeah, I was envisioning that var as something which set the\nbase dir, under which the normal test directories would\nlive.  Basically, like setting --root, but only for the\nGnuPG bits.\n\nI'm not impressed by that idea (and I'm even less so after\nrealizing how it would most likely make it harder to gather\nup the results in the CI scripts).  I mainly tossed it out\nin the hope someone would reply with a better method. ;)\n\n> In the meantime I came up with a '--short-trash-dir' option to\n> test-lib, which turns 'trash directory.t7612-merge-verify-signatures'\n> into 'trash dir.t7612'.  It works, but I don't really like it, and it\n> required various adjustments to the CI build scripts, notably to the\n> part in 'ci/print-test-failures.sh' that includes the trash dir of\n> failed test scripts in the build log.\n\nI can certainly live with setting '--root' to a shorter path\nand waiting to see if GnuPG upstream will come up with\nsomething a little more friendly to users like us - running\ngpg in a test suite.  Though if we do just wait it out,\nmaybe we could/should add a note in t/README or t/lib-gpg.sh\nabout this to warn others?\n\n-- \nTodd\n"},{"id":"369027","messageId":"20190211195140.GB31807@szeder.dev","threadId":"50432","inReplyTo":"20190209230553.GI30548@zaya.teonanacatl.net","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-11T19:51:40Z","receivedAt":"2019-02-11T19:51:47Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Feb 09, 2019 at 06:05:53PM -0500, Todd Zullinger wrote:\n> SZEDER Gábor wrote:\n> > Just curious: how did you noticed the missing GPGSM prereq?\n> \n> I just grep the build output for '# SKIP|skipped:' and then\n> filter out those which I expect (thing like MINGW and\n> NATIVE_CRLF that aren't likely to be in a Fedora build).\n> \n> Far more manual than the slick method you have below. :)\n\nYeah, but yours show the SKIP cases, too, i.e. when the whole test is\nskipped by:\n\n  if check-something\n  then\n        skip_all=\"no can do\"\n        test_done\n  fi\n\nI didn't bother with that because in those cases the prereq is not\ndenoted by a single word, but rather by a human-readable phrase, and\nbecase 'prove' runs those skipped test scripts at last when running\nslowest first, so I could already see them anyway.\n\n> > I'm asking because I use a patch for a good couple of months now that\n> > collects the prereqs missed by test cases and prints them at the end\n> > of 'make test'.  Its output looks like this:\n> > \n> >   https://travis-ci.org/szeder/git/jobs/490944032#L2358\n> > \n> > Since you seem to be interested in that sort of thing as well, perhaps\n> > it would be worth to have something like this in git.git?  It's just\n> > that I have been too wary of potentially annoying other contributors\n> > by adding (what might be perceived as) clutter to their 'make test'\n> > output :)\n> \n> Indeed, I think that would be useful.  At the very least,\n> the .missing_prereqs files look quite handy.  I wouldn't\n> mind the output from 'make test' either, but building\n> packages surely shifts my perspective toward more verbose\n> build logs than someone hacking on git regularly and reading\n> the 'make test' output.\n\nThe problem with those files is that a successful 'make test'\nautomatically and unconditionally removes the whole 'test-results'\ndirectory at the end.  So a separate and optional 'make test ; make\nshow-missed-prereqs' wouldn't have worked, that's why I did it this\nway.\n\nI think it would be better if we kept the 'test-results' directory\neven after a successful 'make test', there are some interesting things\nto be found there:\n\n  https://public-inbox.org/git/CAM0VKjkVreBKQsvMZ=pEE0NN5gG0MM+XJ0MzCbw1rxi_pR+FXQ@mail.gmail.com/\n\n\n> >> A GIT_TEST_GNUPGHOME_ROOT var to set the root path for the GNUPGHOME\n> >> dirs in the tests is one thought I had, but didn't try to put it into\n> >> patch form.  Setting the --root test option is probably enough control\n> >> for most cases.\n> > \n> > A potential issue I see with GIT_TEST_GNUPGHOME_ROOT is that there are\n> > several test scripts involving gpg, and if GIT_TEST_GNUPGHOME_ROOT is\n> > set for the whole 'make test', then they might interfere with each\n> > other when they happen to be run at the same time.\n> \n> Yeah, I was envisioning that var as something which set the\n> base dir, under which the normal test directories would\n> live.  Basically, like setting --root, but only for the\n> GnuPG bits.\n> \n> I'm not impressed by that idea (and I'm even less so after\n> realizing how it would most likely make it harder to gather\n> up the results in the CI scripts).  I mainly tossed it out\n> in the hope someone would reply with a better method. ;)\n> \n> > In the meantime I came up with a '--short-trash-dir' option to\n> > test-lib, which turns 'trash directory.t7612-merge-verify-signatures'\n> > into 'trash dir.t7612'.  It works, but I don't really like it, and it\n> > required various adjustments to the CI build scripts, notably to the\n> > part in 'ci/print-test-failures.sh' that includes the trash dir of\n> > failed test scripts in the build log.\n> \n> I can certainly live with setting '--root' to a shorter path\n> and waiting to see if GnuPG upstream will come up with\n> something a little more friendly to users like us - running\n> gpg in a test suite.\n\nAre they aware of the issue?\n\n  https://lists.gnupg.org/pipermail/gnupg-users/2017-January/057451.html\n\nsuggests to put the socket in '/var/run/user/$(id -u)', but that's for\nthe \"regular\" use case, and we should take extra steps to prevent the\ntests' gpg from interfering with the gpg of the user running the\ntests.  Not sure it would work on macOS.  And ultimately it's not much\ndifferent from your GIT_TEST_GNUPGHOME_ROOT suggestion.\n\nThen I stumbled on these patches patches:\n\n  https://lists.zx2c4.com/pipermail/password-store/2017-May/002932.html\n\nsuggesting that at least one other project is working around this\nissue instead of waiting for upstream to come up with something.\n\n> Though if we do just wait it out,\n> maybe we could/should add a note in t/README or t/lib-gpg.sh\n> about this to warn others?\n\nWell, a comment could help others to not waste time on figuring out\nthis \"path is too long for a unix domains socket\" issue...  but now\nthey will be able to find this thread in the list archives as well :)\n\n\n\nOn a related note: did you happen to notice occasional failures with\ngpg2 on Fedora builds?  I observed some lately in tests like\n'./t7004-tag.sh' or 't7030-verify-tag.sh' on the Travis CI macOS\nbuilds: it appears as if the gpg process were to die mid-verification.\nCouldn't make any sense of it yet, though didn't tried particularly\nhard either.\n\n\n"},{"id":"369076","messageId":"20190212004433.GJ13301@sigill.intra.peff.net","threadId":"50432","inReplyTo":"20190209140605.GE10587@szeder.dev","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-12T00:44:33Z","receivedAt":"2019-02-12T00:44:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 09, 2019 at 03:06:05PM +0100, SZEDER Gábor wrote:\n\n> On Thu, Feb 07, 2019 at 10:17:44PM -0500, Todd Zullinger wrote:\n> > Looking through the build logs for the fedora git packages, I noticed it\n> > was missing the GPGSM prereq.\n> \n> Just curious: how did you noticed the missing GPGSM prereq?\n> \n> I'm asking because I use a patch for a good couple of months now that\n> collects the prereqs missed by test cases and prints them at the end\n> of 'make test'.  Its output looks like this:\n> \n>   https://travis-ci.org/szeder/git/jobs/490944032#L2358\n> \n> Since you seem to be interested in that sort of thing as well, perhaps\n> it would be worth to have something like this in git.git?  It's just\n> that I have been too wary of potentially annoying other contributors\n> by adding (what might be perceived as) clutter to their 'make test'\n> output :)\n\nAt first I thought your script found tests which _should_ have been\nmarked with a particular prereq but weren't. And I scratched my head\nabout how you would find that automatically. If we could, that would be\namazing. ;)\n\nBut it looks from the output like it just mentions every prereq that\nwasn't satisfied. I don't think that's particularly useful to show for\nall users, since most of them are platform things that cannot be changed\n(and you'd never get the list to zero, since some of them are mutually\nexclusive).\n\n-Peff\n"},{"id":"369114","messageId":"xmqqlg2lg28m.fsf@gitster-ct.c.googlers.com","threadId":"50432","inReplyTo":"20190212004433.GJ13301@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-12T02:38:33Z","receivedAt":"2019-02-12T02:38:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But it looks from the output like it just mentions every prereq that\n> wasn't satisfied. I don't think that's particularly useful to show for\n> all users, since most of them are platform things that cannot be changed\n> (and you'd never get the list to zero, since some of them are mutually\n> exclusive).\n\nThe only thing that could be of interest is \"We are skipping tests\naround $SVN, because your Git was built without a feature that\nrequires platform support of feature $SVN, which in turn was caused\nbecause NO_$SVN=UnfortunatelyYes was in your config.mak.autogen\nfile.  You could 'apt-get install $SVN' to lift this artificial\nlimitation in your Git if you wanted to\".\n\nBut the test prereq is probably a bit too roundabout way for that\nkind of information ;-)\n"},{"id":"369116","messageId":"20190212024501.GB1622@szeder.dev","threadId":"50432","inReplyTo":"20190212004433.GJ13301@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-12T02:45:01Z","receivedAt":"2019-02-12T02:45:07Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Feb 11, 2019 at 07:44:33PM -0500, Jeff King wrote:\n> On Sat, Feb 09, 2019 at 03:06:05PM +0100, SZEDER Gábor wrote:\n> \n> > On Thu, Feb 07, 2019 at 10:17:44PM -0500, Todd Zullinger wrote:\n> > > Looking through the build logs for the fedora git packages, I noticed it\n> > > was missing the GPGSM prereq.\n> > \n> > Just curious: how did you noticed the missing GPGSM prereq?\n> > \n> > I'm asking because I use a patch for a good couple of months now that\n> > collects the prereqs missed by test cases and prints them at the end\n> > of 'make test'.  Its output looks like this:\n> > \n> >   https://travis-ci.org/szeder/git/jobs/490944032#L2358\n\n> But it looks from the output like it just mentions every prereq that\n> wasn't satisfied. I don't think that's particularly useful to show for\n> all users, since most of them are platform things that cannot be changed\n> (and you'd never get the list to zero, since some of them are mutually\n> exclusive).\n\nThe idea was that people might notice when a new unmet prereq pops up\nall of a sudden, because they modified something on their setup, or\nbecause a new prereq was recently introduced, e.g. PERLJSON.  Or they\nmight notice that a prereq necessary to test a fundamental feature is\nmissing on their setup that they haven't been aware of before, e.g.\nTTY.\n\n"},{"id":"369294","messageId":"20190214063202.GD16125@zaya.teonanacatl.net","threadId":"50432","inReplyTo":"20190211195140.GB31807@szeder.dev","subject":"Re: [PATCH 0/2] t/lib-gpg: a gpgsm fix, a minor improvement, and a question","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-02-14T06:32:02Z","receivedAt":"2019-02-14T06:32:14Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"SZEDER Gábor wrote:\n> On Sat, Feb 09, 2019 at 06:05:53PM -0500, Todd Zullinger wrote:\n>> SZEDER Gábor wrote:\n>>> Just curious: how did you noticed the missing GPGSM prereq?\n>> \n>> I just grep the build output for '# SKIP|skipped:' and then\n>> filter out those which I expect (thing like MINGW and\n>> NATIVE_CRLF that aren't likely to be in a Fedora build).\n>> \n>> Far more manual than the slick method you have below. :)\n> \n> Yeah, but yours show the SKIP cases, too, i.e. when the whole test is\n> skipped by:\n> \n>   if check-something\n>   then\n>         skip_all=\"no can do\"\n>         test_done\n>   fi\n> \n> I didn't bother with that because in those cases the prereq is not\n> denoted by a single word, but rather by a human-readable phrase, and\n> becase 'prove' runs those skipped test scripts at last when running\n> slowest first, so I could already see them anyway.\n\nAhh, good points.\n\n>>> I'm asking because I use a patch for a good couple of months now that\n>>> collects the prereqs missed by test cases and prints them at the end\n>>> of 'make test'.  Its output looks like this:\n>>> \n>>>   https://travis-ci.org/szeder/git/jobs/490944032#L2358\n>>> \n>>> Since you seem to be interested in that sort of thing as well, perhaps\n>>> it would be worth to have something like this in git.git?  It's just\n>>> that I have been too wary of potentially annoying other contributors\n>>> by adding (what might be perceived as) clutter to their 'make test'\n>>> output :)\n>> \n>> Indeed, I think that would be useful.  At the very least,\n>> the .missing_prereqs files look quite handy.  I wouldn't\n>> mind the output from 'make test' either, but building\n>> packages surely shifts my perspective toward more verbose\n>> build logs than someone hacking on git regularly and reading\n>> the 'make test' output.\n> \n> The problem with those files is that a successful 'make test'\n> automatically and unconditionally removes the whole 'test-results'\n> directory at the end.  So a separate and optional 'make test ; make\n> show-missed-prereqs' wouldn't have worked, that's why I did it this\n> way.\n> \n> I think it would be better if we kept the 'test-results' directory\n> even after a successful 'make test', there are some interesting things\n> to be found there:\n> \n>   https://public-inbox.org/git/CAM0VKjkVreBKQsvMZ=pEE0NN5gG0MM+XJ0MzCbw1rxi_pR+FXQ@mail.gmail.com/\n\nMaybe that's something which could be controlled with a make\nvar, to allow folks like us to keep the tests but clean them\nup by default for everyone else?\n\nThough even in the fedora package builds, I don't have\naccess to poke around in test-results and likely wouldn't\nwant to make the effort to extract the results and dump them\ninto the build logs (like ci/util/extract-trash-dirs.sh does\nfor the trash dirs).\n\n>> I can certainly live with setting '--root' to a shorter path\n>> and waiting to see if GnuPG upstream will come up with\n>> something a little more friendly to users like us - running\n>> gpg in a test suite.\n> \n> Are they aware of the issue?\n\nYeah, it was filed as https://dev.gnupg.org/T2964, as a\nresult of the gnupg-users thread you mention below.  There\nhasn't been any progress on it since 2017 though, so it's\ndoubtful that upstream will fix it anytime soon.  If (or\nwhen) it's resolved, I wouldn't be surprised if only gnupg\n>= 2.3 included the fix.  So we'll probably have numerous\nsystems with this issue for quite some time.\n\n>   https://lists.gnupg.org/pipermail/gnupg-users/2017-January/057451.html\n> \n> suggests to put the socket in '/var/run/user/$(id -u)', but that's for\n> the \"regular\" use case, and we should take extra steps to prevent the\n> tests' gpg from interfering with the gpg of the user running the\n> tests.  Not sure it would work on macOS.  And ultimately it's not much\n> different from your GIT_TEST_GNUPGHOME_ROOT suggestion.\n> \n> Then I stumbled on these patches patches:\n> \n>   https://lists.zx2c4.com/pipermail/password-store/2017-May/002932.html\n> \n> suggesting that at least one other project is working around this\n> issue instead of waiting for upstream to come up with something.\n\nHeh, the gnupg ticket mentions that the notmuch project\nsimilarly had to work around gpg2's socket handling for\ntheir tests:\n\n    https://notmuchmail.org/pipermail/notmuch/2017/024148.html\n\n>> Though if we do just wait it out,\n>> maybe we could/should add a note in t/README or t/lib-gpg.sh\n>> about this to warn others?\n> \n> Well, a comment could help others to not waste time on figuring out\n> this \"path is too long for a unix domains socket\" issue...  but now\n> they will be able to find this thread in the list archives as well :)\n\nTrue.  Will they curse us for not adding a comment to save\nthem some effort or can we just say we were waiting for them\nto submit such a patch? ;)\n\n> On a related note: did you happen to notice occasional failures with\n> gpg2 on Fedora builds?  I observed some lately in tests like\n> './t7004-tag.sh' or 't7030-verify-tag.sh' on the Travis CI macOS\n> builds: it appears as if the gpg process were to die mid-verification.\n> Couldn't make any sense of it yet, though didn't tried particularly\n> hard either.\n\nI have not seen any such failures.  I've done a few dozen\ntest builds on fedora systems where /bin/gpg is gnupg-2.2\nand other than the socket path issues, the tests have all\nworked well.  But I'll be sure to mention it if I do run\ninto any such failures.\n\n-- \nTodd\n"}]}