{"thread":{"id":"44019","subject":"[PATCH] Unbreak interactive GPG prompt upon signing","startedAt":"2016-09-06T08:02:30Z","lastAt":"2018-10-02T13:02:20Z","messageCount":16,"participants":["Johannes Schindelin","Michael J Gruber","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"301177","messageId":"0b8196564ac9f1db7c59b42d7e0973ba1399ec52.1473148900.git.johannes.schindelin@gmx.de","threadId":"44019","inReplyTo":null,"subject":"[PATCH] Unbreak interactive GPG prompt upon signing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-06T08:01:59Z","receivedAt":"2016-09-06T08:02:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"With the recent update in efee955 (gpg-interface: check gpg signature\ncreation status, 2016-06-17), we ask GPG to send all status updates to\nstderr, and then catch the stderr in an strbuf.\n\nBut GPG might fail, and send error messages to stderr. And we simply\ndo not show them to the user.\n\nEven worse: this swallows any interactive prompt for a passphrase. And\ndetaches stderr from the tty so that the passphrase cannot be read.\n\nSo while the first problem could be fixed (by printing the captured\nstderr upon error), the second problem cannot be easily fixed, and\npresents a major regression.\n\nSo let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.\n\nThis fixes https://github.com/git-for-windows/git/issues/871\n\nCc: Michael J Gruber \nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/fix-gpg-v1\nFetch-It-Via: git fetch https://github.com/dscho/git fix-gpg-v1\n\n gpg-interface.c | 8 ++------\n t/t7004-tag.sh | 9 +--------\n 2 files changed, 3 insertions(+), 14 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 8672eda..3f3a3f7 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -153,11 +153,9 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n \tsize_t i, j, bottom;\n-\tstruct strbuf gpg_status = STRBUF_INIT;\n \n \targv_array_pushl(&gpg.args,\n \t\t\t gpg_program,\n-\t\t\t \"--status-fd=2\",\n \t\t\t \"-bsau\", signing_key,\n \t\t\t NULL);\n \n@@ -169,12 +167,10 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \t */\n \tsigchain_push(SIGPIPE, SIG_IGN);\n \tret = pipe_command(&gpg, buffer->buf, buffer->len,\n-\t\t\t signature, 1024, &gpg_status, 0);\n+\t\t\t signature, 1024, NULL, 0);\n \tsigchain_pop(SIGPIPE);\n \n-\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n-\tstrbuf_release(&gpg_status);\n-\tif (ret)\n+\tif (ret || signature->len == bottom)\n \t\treturn error(_(\"gpg failed to sign the data\"));\n \n \t/* Strip CR from the line endings, in case we are on Windows. */\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 8b0f71a..f9b7d79 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1202,17 +1202,10 @@ test_expect_success GPG,RFC1991 \\\n # try to sign with bad user.signingkey\n git config user.signingkey BobTheMouse\n test_expect_success GPG \\\n-\t'git tag -s fails if gpg is misconfigured (bad key)' \\\n+\t'git tag -s fails if gpg is misconfigured' \\\n \t'test_must_fail git tag -s -m tail tag-gpg-failure'\n git config --unset user.signingkey\n \n-# try to produce invalid signature\n-test_expect_success GPG \\\n-\t'git tag -s fails if gpg is misconfigured (bad signature format)' \\\n-\t'test_config gpg.program echo &&\n-\t test_must_fail git tag -s -m tail tag-gpg-failure'\n-\n-\n # try to verify without gpg:\n \n rm -rf gpghome\n-- \n2.10.0.windows.1.6.gc4f481a\n\nbase-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b\n"},{"id":"301184","messageId":"ced7502d-0095-bd90-19e3-c14d0e4d4f07@drmicha.warpmail.net","threadId":"44019","inReplyTo":"0b8196564ac9f1db7c59b42d7e0973ba1399ec52.1473148900.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] Unbreak interactive GPG prompt upon signing","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-09-06T12:32:40Z","receivedAt":"2016-09-06T12:32:58Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Schindelin venit, vidit, dixit 06.09.2016 10:01:\n> With the recent update in efee955 (gpg-interface: check gpg signature\n> creation status, 2016-06-17), we ask GPG to send all status updates to\n> stderr, and then catch the stderr in an strbuf.\n> \n> But GPG might fail, and send error messages to stderr. And we simply\n> do not show them to the user.\n> \n> Even worse: this swallows any interactive prompt for a passphrase. And\n> detaches stderr from the tty so that the passphrase cannot be read.\n> \n> So while the first problem could be fixed (by printing the captured\n> stderr upon error), the second problem cannot be easily fixed, and\n> presents a major regression.\n\nMy Git has that commit and does ask me for the passphrase on the tty.\nAlso, I do get error messages:\n\ngit tag -u pebcak -s testt -m m\nerror: gpg failed to sign the data\nerror: unable to sign the tag\n\nwhich we could (maybe should) amend by gpg's stderr.\n\n> So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.\n\nThat \"just\" reintroduces the problem that the orignal patch solves.\n\nThe passphrase/tty issue must be Windows specific - or the non-issue\nLinux-specific, if you prefer.\n\n> This fixes https://github.com/git-for-windows/git/issues/871\n> \n> Cc: Michael J Gruber \n> Signed-off-by: Johannes Schindelin \n> ---\n> Published-As: https://github.com/dscho/git/releases/tag/fix-gpg-v1\n> Fetch-It-Via: git fetch https://github.com/dscho/git fix-gpg-v1\n> \n> gpg-interface.c | 8 ++------\n> t/t7004-tag.sh | 9 +--------\n> 2 files changed, 3 insertions(+), 14 deletions(-)\n> \n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 8672eda..3f3a3f7 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -153,11 +153,9 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n> \tstruct child_process gpg = CHILD_PROCESS_INIT;\n> \tint ret;\n> \tsize_t i, j, bottom;\n> -\tstruct strbuf gpg_status = STRBUF_INIT;\n> \n> \targv_array_pushl(&gpg.args,\n> \t\t\t gpg_program,\n> -\t\t\t \"--status-fd=2\",\n> \t\t\t \"-bsau\", signing_key,\n> \t\t\t NULL);\n> \n> @@ -169,12 +167,10 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n> \t */\n> \tsigchain_push(SIGPIPE, SIG_IGN);\n> \tret = pipe_command(&gpg, buffer->buf, buffer->len,\n> -\t\t\t signature, 1024, &gpg_status, 0);\n> +\t\t\t signature, 1024, NULL, 0);\n> \tsigchain_pop(SIGPIPE);\n> \n> -\tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n> -\tstrbuf_release(&gpg_status);\n> -\tif (ret)\n> +\tif (ret || signature->len == bottom)\n> \t\treturn error(_(\"gpg failed to sign the data\"));\n> \n> \t/* Strip CR from the line endings, in case we are on Windows. */\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 8b0f71a..f9b7d79 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1202,17 +1202,10 @@ test_expect_success GPG,RFC1991 \\\n> # try to sign with bad user.signingkey\n> git config user.signingkey BobTheMouse\n> test_expect_success GPG \\\n> -\t'git tag -s fails if gpg is misconfigured (bad key)' \\\n> +\t'git tag -s fails if gpg is misconfigured' \\\n> \t'test_must_fail git tag -s -m tail tag-gpg-failure'\n> git config --unset user.signingkey\n> \n> -# try to produce invalid signature\n> -test_expect_success GPG \\\n> -\t'git tag -s fails if gpg is misconfigured (bad signature format)' \\\n> -\t'test_config gpg.program echo &&\n> -\t test_must_fail git tag -s -m tail tag-gpg-failure'\n> -\n> -\n> # try to verify without gpg:\n> \n> rm -rf gpghome\n> \n\n"},{"id":"301186","messageId":"18a7e2984121d988137c135ec560fee56506981b.1473167263.git.git@drmicha.warpmail.net","threadId":"44019","inReplyTo":"ced7502d-0095-bd90-19e3-c14d0e4d4f07@drmicha.warpmail.net","subject":"[PATCH] gpg-interface: reflect stderr to stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-09-06T13:13:03Z","receivedAt":"2016-09-06T13:13:09Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"efee955 (\"gpg-interface: check gpg signature creation status\",\n2016-06-17) used stderr to capture gpg's status output, which is the\nonly reliable way for status checks. As a side effect, stderr was not\nshown to the user any more.\n\nIn case of a gpg error, reflect the whole captured buffer to stderr.\n\nSigned-off-by: Michael J Gruber \n---\nA full blown approach would use --status-fd=4 or such rather than hijacking stderr.\nThis would require an extension of pipe_command() etc. to handle yet another fd.\n\n gpg-interface.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 8672eda..cf35bca 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -173,9 +173,12 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tsigchain_pop(SIGPIPE);\n \n \tret |= !strstr(gpg_status.buf, \"\\n[GNUPG:] SIG_CREATED \");\n-\tstrbuf_release(&gpg_status);\n-\tif (ret)\n+\tif (ret) {\n+\t\tfputs(gpg_status.buf, stderr);\n+\t\tstrbuf_release(&gpg_status);\n \t\treturn error(_(\"gpg failed to sign the data\"));\n+\t}\n+\tstrbuf_release(&gpg_status);\n \n \t/* Strip CR from the line endings, in case we are on Windows. */\n \tfor (i = j = bottom; i < signature->len; i++)\n-- \n2.10.0.rc2.333.g8ef2d05\n\n"},{"id":"301205","messageId":"alpine.DEB.2.20.1609061827290.129229@virtualbox","threadId":"44019","inReplyTo":"18a7e2984121d988137c135ec560fee56506981b.1473167263.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-06T16:30:16Z","receivedAt":"2016-09-06T16:30:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Tue, 6 Sep 2016, Michael J Gruber wrote:\n\n> efee955 (\"gpg-interface: check gpg signature creation status\",\n> 2016-06-17) used stderr to capture gpg's status output, which is the\n> only reliable way for status checks. As a side effect, stderr was not\n> shown to the user any more.\n> \n> In case of a gpg error, reflect the whole captured buffer to stderr.\n> \n> Signed-off-by: Michael J Gruber \n> ---\n> A full blown approach would use --status-fd=4 or such rather than hijacking stderr.\n> This would require an extension of pipe_command() etc. to handle yet another fd.\n\nAs I indicated in my patch, this is not enough on Windows. In fact, my\nfirst version of a patch tried to do exactly what you presented here, and\nall it did was make the error message a bit more verbose:\n\n-- snip --\nerror: gpg failed to sign the data:\n[GNUPG:] USERID_HINT Johannes Schindelin \n[GNUPG:] NEED_PASSPHRASE 1 0\ngpg: cannot open tty `no tty': No such file or directory\n\nerror: unable to sign the tag\n-- snap --\n\nThis is not a fix for the issue reported on the Git for Windows, but only\nhalf a fix.\n\nCiao,\nDscho\n"},{"id":"301207","messageId":"alpine.DEB.2.20.1609061834490.129229@virtualbox","threadId":"44019","inReplyTo":"ced7502d-0095-bd90-19e3-c14d0e4d4f07@drmicha.warpmail.net","subject":"Re: [PATCH] Unbreak interactive GPG prompt upon signing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-06T16:39:06Z","receivedAt":"2016-09-06T16:43:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Tue, 6 Sep 2016, Michael J Gruber wrote:\n\n> Johannes Schindelin venit, vidit, dixit 06.09.2016 10:01:\n> > With the recent update in efee955 (gpg-interface: check gpg signature\n> > creation status, 2016-06-17), we ask GPG to send all status updates to\n> > stderr, and then catch the stderr in an strbuf.\n> > \n> > But GPG might fail, and send error messages to stderr. And we simply\n> > do not show them to the user.\n> > \n> > Even worse: this swallows any interactive prompt for a passphrase. And\n> > detaches stderr from the tty so that the passphrase cannot be read.\n> > \n> > So while the first problem could be fixed (by printing the captured\n> > stderr upon error), the second problem cannot be easily fixed, and\n> > presents a major regression.\n> \n> My Git has that commit and does ask me for the passphrase on the tty.\n> Also, I do get error messages:\n> \n> git tag -u pebcak -s testt -m m\n> error: gpg failed to sign the data\n> error: unable to sign the tag\n\nThat is not GPG's error message. It just leaves users puzzled, is what it\ndoes.\n\n> which we could (maybe should) amend by gpg's stderr.\n\nRight. But then we still do not solve the problem. The problem being that\nsome platforms cannot use getpass(prompt): it simply does not exist.\n\nOn Windows, we do not even have a /dev/tty (technically, GPG, being an\nMSYS2 program, knows about /dev/tty, but we spawn it from a non-MSYS2\nprogram, so there is a disconnect).\n\n> > So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.\n> \n> That \"just\" reintroduces the problem that the orignal patch solves.\n\nRight. Which is: when some user misconfigures gpg, causing Git to run\nsomething different that simply succeeds, there is no signature.\n\nThis is a minor issue, as it requires a user to configure gpg, and do a\nbad job at it.\n\nNot being able to input the passphrase on Windows is a major issue, as the\nuser has done nothing wrong.\n\n> The passphrase/tty issue must be Windows specific - or the non-issue\n> Linux-specific, if you prefer.\n\nSure. Let's talk about semantics. Oh wait, maybe we should work on\nresolving the issue instead.\n\n> > This fixes https://github.com/git-for-windows/git/issues/871\n\nTo reiterate: this is the problem I need to see solved.\n\nCiao,\nDscho\n"},{"id":"301208","messageId":"alpine.DEB.2.20.1609061839370.129229@virtualbox","threadId":"44019","inReplyTo":"alpine.DEB.2.20.1609061827290.129229@virtualbox","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-06T16:42:20Z","receivedAt":"2016-09-06T16:43:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Tue, 6 Sep 2016, Johannes Schindelin wrote:\n\n> On Tue, 6 Sep 2016, Michael J Gruber wrote:\n> \n> > A full blown approach would use --status-fd=4 or such rather than hijacking stderr.\n> > This would require an extension of pipe_command() etc. to handle yet another fd.\n\nFor the record: I do not know whether that would work, either. So unless\nwe are fairly certain that it *would* work, I'd rather not spend the time.\n\nYour original issue seemed to be that the gpg command could succeed, but\nstill no signature be seen. There *must* be a way to test whether the\ncalled program added a signature, simply by testing whether *any*\ncharacters were written.\n\nAnd if characters were written that were not actually a GPG signature,\nmaybe the enterprisey user who configured the gpg command to be her magic\nscript actually meant something else than a GPG signature to be added?\n\nCiao,\nDscho\n"},{"id":"301209","messageId":"alpine.DEB.2.20.1609061843120.129229@virtualbox","threadId":"44019","inReplyTo":"alpine.DEB.2.20.1609061839370.129229@virtualbox","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-06T16:43:53Z","receivedAt":"2016-09-06T16:44:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nokay, final mail on this issue today:\n\nOn Tue, 6 Sep 2016, Johannes Schindelin wrote:\n\n> Your original issue seemed to be that the gpg command could succeed, but\n> still no signature be seen. There *must* be a way to test whether the\n> called program added a signature, simply by testing whether *any*\n> characters were written.\n> \n> And if characters were written that were not actually a GPG signature,\n> maybe the enterprisey user who configured the gpg command to be her magic\n> script actually meant something else than a GPG signature to be added?\n\nI actually just saw that this is *precisely* what the code does already:\n\n if (ret || signature->len == bottom)\n return error(_(\"gpg failed to sign the data\"));\n\nWhy is this not good enough?\n\nCiao,\nDscho\n"},{"id":"301246","messageId":"655b42d8-baa9-e649-2b3c-5b7bfc914bc5@drmicha.warpmail.net","threadId":"44019","inReplyTo":"alpine.DEB.2.20.1609061843120.129229@virtualbox","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-09-07T08:27:34Z","receivedAt":"2016-09-07T08:27:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Schindelin venit, vidit, dixit 06.09.2016 18:43:\n> Hi Michael,\n> \n> okay, final mail on this issue today:\n> \n> On Tue, 6 Sep 2016, Johannes Schindelin wrote:\n> \n>> Your original issue seemed to be that the gpg command could succeed, but\n>> still no signature be seen. There *must* be a way to test whether the\n>> called program added a signature, simply by testing whether *any*\n>> characters were written.\n>>\n>> And if characters were written that were not actually a GPG signature,\n>> maybe the enterprisey user who configured the gpg command to be her magic\n>> script actually meant something else than a GPG signature to be added?\n> \n> I actually just saw that this is *precisely* what the code does already:\n> \n> if (ret || signature->len == bottom)\n> return error(_(\"gpg failed to sign the data\"));\n> \n> Why is this not good enough?\n\n\nAssuming \"this\" refers to error():\n\nYou said it's not good enough - because gpg's stderr is not displayed.\n\nAnd I agree with you on that point.\n\nAssuming \"this\" refers to the exit code check:\n\ngpg documentation says so over and over again: do not rely on exit\ncodes, parse status-fd instead. (An often ignored advice, oh well.)\n\nAs for most of your other remarks, I would appreciate if you could take\na breath and reread what I wrote and what you wrote - before you send it\n- and curb your remarks about who is working on resolving issues.\n\nSo, trying again to structure the issues and solutions, there are three\nissues:\n\nA) relying on gpg's exit code (and stdout) is not enough. Secure use of\ngpg requires checking status-fd.\n\nThis is what my old patch solved.\n\nB) \"gpg --status-fd=2\" and swallowing stderr hides usual stderr from the\nuser.\n\nThis is what my old patch introduced and a Windows user reported. It is\nsolved by my new additional patch.\n\nC) With that old patch, that Windows user is not asked for a passphrase\non tty any more.\n\nReverting my patches appears to solve C on Windows and reintroduces A on\nall platforms, obviously. C is not present on Linux. B is solved either way.\n\nNow, I can't reproduce C on Linux[*], so there is more involved. It\ncould be that my patch just exposes a problem in our start_command()\netc.: run-command.c contains a lot of ifdefing, so possibly quite\ndifferent code is run on different platforms.\n\nIt would be great if someone with a Windows environment could help our\nefforts in resolving issue C, by checking what is actually behind[**]: I\ncan't believe that capturing stderr keeps gpg from reading stdin, but\nwho knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.\n\nMichael\n\n[*] Maybe that even depends on Linux environments (terminal emulator),\nso input from others would be helpful, too:\n\nWithout a passphrase-agent/wallet etc, does \"git tag -s -m test test\"\nask you for a passphrase on the terminal?\n\nI does for me with this stack:\n\nX11->i3->st->tmux->bash->git->gpg\n\n[**] \"--status-fd=3\" instead of \"--status-fd=2\" in my old patch would be\na check whether our capturing of stderr is creating problems on Windows\nor gpg's writing status to stderr (which --status-fd=3 would change, at\nthe expense of breaking the final check): Does gpg ask for the\npassphrase now?\n"},{"id":"301247","messageId":"20160907083947.b7q7ebe62xsr6447@sigill.intra.peff.net","threadId":"44019","inReplyTo":"655b42d8-baa9-e649-2b3c-5b7bfc914bc5@drmicha.warpmail.net","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-07T08:39:47Z","receivedAt":"2016-09-07T08:39:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:\n\n> Now, I can't reproduce C on Linux[*], so there is more involved. It\n> could be that my patch just exposes a problem in our start_command()\n> etc.: run-command.c contains a lot of ifdefing, so possibly quite\n> different code is run on different platforms.\n\nMaybe, though my blind guess is that it is simply that on Linux we can\nopen /dev/tty directly, and console-IO on Windows is a bit more\ncomplicated.\n\nYou might also check your GPG versions; between gpg1.x and gpg2, the\npassphrase input handling has been completely revamped.\n\n> It would be great if someone with a Windows environment could help our\n> efforts in resolving issue C, by checking what is actually behind[**]: I\n> can't believe that capturing stderr keeps gpg from reading stdin, but\n> who knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.\n\nI know nothing about Windows, but I'd be surprised if gpg is reading\nfrom stdin, as opposed to /dev/tty. It's probably more to do with how\ngpg finds the \"tty\" on Windows (presumably it looks at stderr for that).\n\nAnyway, I wrote pipe_command() in such a way as to be prepared for\nexactly this kind of thing, so it would be trivial to extend it to an\nextra descriptor. The trouble is that run_command() doesn't understand\nanything except stdin/stdout/stderr. We can open an extra pipe() before\ncalling run_command(), and make sure it is not marked CLOEXEC. I don't\nknow if there are other portability concerns, though.\n\n-Peff\n"},{"id":"301250","messageId":"225f43eb-d708-dd9b-77d9-aa8cbe2cd9f1@drmicha.warpmail.net","threadId":"44019","inReplyTo":"20160907083947.b7q7ebe62xsr6447@sigill.intra.peff.net","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-09-07T09:32:53Z","receivedAt":"2016-09-07T09:33:01Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 07.09.2016 10:39:\n> On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:\n> \n>> Now, I can't reproduce C on Linux[*], so there is more involved. It\n>> could be that my patch just exposes a problem in our start_command()\n>> etc.: run-command.c contains a lot of ifdefing, so possibly quite\n>> different code is run on different platforms.\n> \n> Maybe, though my blind guess is that it is simply that on Linux we can\n> open /dev/tty directly, and console-IO on Windows is a bit more\n> complicated.\n> \n> You might also check your GPG versions; between gpg1.x and gpg2, the\n> passphrase input handling has been completely revamped.\n\nThat's a good point to note.\n\ngpg1 asks for the passphrase (without use-agent), gpg2 always delegates\nto gpg-agent (and starts it on demand).\n\nOn Linux, gpg-agent says you should\n\nexport GPG_TTY=$(tty)\n\nto make gpg-agent find the tty, and claims it's not necessary on Win.\n\nIn fact, it's not necessary on Linux either unless you want to use\npinentry-curses.\n\nAlas, be it gpg1.4.21 or gpg2.1.13, whatever pinentry, I do get the\npassphrase prompt, even with curses (if GPG_TTY is set, which was\nnecessary before any patches already).\n\nI put up a request for more input from the reporters in the github\nissue. I guess that's the best way to reach them.\n\n>> It would be great if someone with a Windows environment could help our\n>> efforts in resolving issue C, by checking what is actually behind[**]: I\n>> can't believe that capturing stderr keeps gpg from reading stdin, but\n>> who knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.\n> \n> I know nothing about Windows, but I'd be surprised if gpg is reading\n> from stdin, as opposed to /dev/tty. It's probably more to do with how\n> gpg finds the \"tty\" on Windows (presumably it looks at stderr for that).\n> \n> Anyway, I wrote pipe_command() in such a way as to be prepared for\n> exactly this kind of thing, so it would be trivial to extend it to an\n> extra descriptor. The trouble is that run_command() doesn't understand\n> anything except stdin/stdout/stderr. We can open an extra pipe() before\n> calling run_command(), and make sure it is not marked CLOEXEC. I don't\n> know if there are other portability concerns, though.\n\nMy suggestion to try \"--status-fd=3\" was meant to test whether the above\ncould help: If fd=3 helps, then our capturing stderr is not the problem.\n(If fd=3 does not help then we still don't know...)\n\nMichael\n\n"},{"id":"301415","messageId":"xmqqwpimgso6.fsf@gitster.mtv.corp.google.com","threadId":"44019","inReplyTo":"20160907083947.b7q7ebe62xsr6447@sigill.intra.peff.net","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T18:20:09Z","receivedAt":"2016-09-08T18:20:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:\n>\n>> Now, I can't reproduce C on Linux[*], so there is more involved. It\n>> could be that my patch just exposes a problem in our start_command()\n>> etc.: run-command.c contains a lot of ifdefing, so possibly quite\n>> different code is run on different platforms.\n>\n> Maybe, though my blind guess is that it is simply that on Linux we can\n> open /dev/tty directly, and console-IO on Windows is a bit more\n> complicated.\n\nTrue.\n\nEven though this patch is fixing only one of the two issues, I am\ntempted to say that we should queue it for now, as it does so\nwithout breaking a bigger gain made by the original, i.e. we learn\nthe status of verification in a way the authors of GPG wants us to,\nwhile somebody figuires out what the best way is to show the prompt\nto the console on Windows.\n\n"},{"id":"301440","messageId":"20160908200305.okeeh35xmrvcveyg@sigill.intra.peff.net","threadId":"44019","inReplyTo":"xmqqwpimgso6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-08T20:03:05Z","receivedAt":"2016-09-08T20:03:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2016 at 11:20:09AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:\n> >\n> >> Now, I can't reproduce C on Linux[*], so there is more involved. It\n> >> could be that my patch just exposes a problem in our start_command()\n> >> etc.: run-command.c contains a lot of ifdefing, so possibly quite\n> >> different code is run on different platforms.\n> >\n> > Maybe, though my blind guess is that it is simply that on Linux we can\n> > open /dev/tty directly, and console-IO on Windows is a bit more\n> > complicated.\n> \n> True.\n> \n> Even though this patch is fixing only one of the two issues, I am\n> tempted to say that we should queue it for now, as it does so\n> without breaking a bigger gain made by the original, i.e. we learn\n> the status of verification in a way the authors of GPG wants us to,\n> while somebody figuires out what the best way is to show the prompt\n> to the console on Windows.\n\nThat's OK by me, but I don't know if we can put off the \"best way to\nshow the prompt\" fix. It seems like a pretty serious regression for\npeople on Windows.\n\n-Peff\n"},{"id":"301455","messageId":"xmqqzinidqfi.fsf@gitster.mtv.corp.google.com","threadId":"44019","inReplyTo":"20160908200305.okeeh35xmrvcveyg@sigill.intra.peff.net","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T21:36:49Z","receivedAt":"2016-09-08T21:36:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Even though this patch is fixing only one of the two issues, I am\n>> tempted to say that we should queue it for now, as it does so\n>> without breaking a bigger gain made by the original, i.e. we learn\n>> the status of verification in a way the authors of GPG wants us to,\n>> while somebody figuires out what the best way is to show the prompt\n>> to the console on Windows.\n>\n> That's OK by me, but I don't know if we can put off the \"best way to\n> show the prompt\" fix. It seems like a pretty serious regression for\n> people on Windows.\n\nYes, I am not saying that it is OK to keep Windows users broken.\n\nAs I understand what Dscho said correctly, his users are covered by\na reversion of the \"read the GPG status correctly\" patch, i.e. with\na different trade-off between the correctness of GPG status vs\nusability of the prompt, he will ship with Git for Windows, and that\nstop-gap measure will last only until developers who can do Windows\n(which excludes you, me, and Michael it seems) comes up with a\nsolution that satisfies both.\n\nI consider that an approach that is perfectly fine.\n\n\n"},{"id":"301464","messageId":"alpine.DEB.2.20.1609090925150.129229@virtualbox","threadId":"44019","inReplyTo":"xmqqwpimgso6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-09T07:28:11Z","receivedAt":"2016-09-09T07:28:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 8 Sep 2016, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:\n> >\n> >> Now, I can't reproduce C on Linux[*], so there is more involved. It\n> >> could be that my patch just exposes a problem in our start_command()\n> >> etc.: run-command.c contains a lot of ifdefing, so possibly quite\n> >> different code is run on different platforms.\n> >\n> > Maybe, though my blind guess is that it is simply that on Linux we can\n> > open /dev/tty directly, and console-IO on Windows is a bit more\n> > complicated.\n> \n> True.\n> \n> Even though this patch is fixing only one of the two issues, I am\n> tempted to say that we should queue it for now, as it does so\n> without breaking a bigger gain made by the original, i.e. we learn\n> the status of verification in a way the authors of GPG wants us to,\n> while somebody figuires out what the best way is to show the prompt\n> to the console on Windows.\n\nBetween protecting users from their own mis-configurations and allowing\nthem to enter their passphrase interactively on Windows, I would argue\nthat the more important thing to have working is the latter, because it is\nnot at all the user's fault that they cannot enter their passphrase\ncurrently, and they cannot fix it by fixing their mis-configuration.\n\nUnfortunately, you obviously disagree with this assessement.\n\nAs I *need* to fix this *major* bug for Windows users, you basically put\nan additional burden on me by applying Michael's patch, which not only\ndoes not fix the problem for Windows users, but conflicts with my\nwork-around.\n\nPity,\nDscho\n"},{"id":"301722","messageId":"3b2f13c5-c315-7156-5126-8f469956d645@drmicha.warpmail.net","threadId":"44019","inReplyTo":"xmqqzinidqfi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-09-12T13:39:55Z","receivedAt":"2016-09-12T13:40:03Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 08.09.2016 23:36:\n> Jeff King writes:\n> \n>>> Even though this patch is fixing only one of the two issues, I am\n>>> tempted to say that we should queue it for now, as it does so\n>>> without breaking a bigger gain made by the original, i.e. we learn\n>>> the status of verification in a way the authors of GPG wants us to,\n>>> while somebody figuires out what the best way is to show the prompt\n>>> to the console on Windows.\n>>\n>> That's OK by me, but I don't know if we can put off the \"best way to\n>> show the prompt\" fix. It seems like a pretty serious regression for\n>> people on Windows.\n> \n> Yes, I am not saying that it is OK to keep Windows users broken.\n> \n> As I understand what Dscho said correctly, his users are covered by\n> a reversion of the \"read the GPG status correctly\" patch, i.e. with\n> a different trade-off between the correctness of GPG status vs\n> usability of the prompt, he will ship with Git for Windows, and that\n> stop-gap measure will last only until developers who can do Windows\n> (which excludes you, me, and Michael it seems) comes up with a\n> solution that satisfies both.\n\nUnfortunately \"I can't do Windows\". Also, I'm not sure \"I can do pipes\",\nbut it's really the ifdeffing that keeps me from even trying: Nothing is\ngained for Windows users if I extend the Linux code to use an extra file\nhandle for status-fd - which would be the clean and correct solution,\nbut which would need to be implemented twice.\n\n> I consider that an approach that is perfectly fine.\n\nAs a side note, I'm wondering why MSYS-gpg version 1 is bundled with\nnon-MSYS-git. It's an honest question - there must be good reasons for\nthat, and git should work with gpg 1, 2 (maybe) and 2.1, of course. I'm\njust trying to understand the situation, and the question goes both ways:\n\n- some Windows user(s) in the Github issue apparantly had wrong\nassumptions about which gpg they're using (via git) - why bundle it at all?\n\n- If bundling it to get a known working setup, why not gpg 2(.1) which\nruns gpg-agent automatically, giving a more Windows-like experience for\nthe passphrase-prompt?\n\nOn Fedora, with some things still requiring gpg 1, gpg 2.1 installed in\nparallel, things can become confusing quickly because of the 1-time\n1-way migration of the secret key store. It's probably the same on\nWindows with those two gpg's used in parallel (and probably answers my\nsecond question).\n\nMichael\n"},{"id":"359413","messageId":"nycvar.QRO.7.76.6.1810021501420.2034@tvgsbejvaqbjf.bet","threadId":"44019","inReplyTo":"3b2f13c5-c315-7156-5126-8f469956d645@drmicha.warpmail.net","subject":"Re: [PATCH] gpg-interface: reflect stderr to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-02T13:02:03Z","receivedAt":"2018-10-02T13:02:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\n[blast from the past]\n\nOn Mon, 12 Sep 2016, Michael J Gruber wrote:\n\n> As a side note, I'm wondering why MSYS-gpg version 1 is bundled with\n> non-MSYS-git.\n\nThose are two questions:\n\n- an MSYS version of GPG is bundled because it was the stable one\n available at the time when I had to decide: in summer 2015.\n\n- GPG v1.x was bundled because, again, this was the version bundled by\n MSYS2 (and I have to rely heavily on what is bundled with MSYS2 by\n default; I could not run Git for Windows if I had to build all of the\n components from scratch, and maintain them, in addition to Git\n itself).\n\nThe good news (ahem, see below) is that GPG v2.x is now bundled in\nMSYS2.\n\nThe not-so-good news is that this can break existing setups. My own\nsetup, for example, was broken by this under-announced upgrade.\n\nBut other things, such as GPG-signing commits in a rebase, should\nactually be *fixed* by this upgrade, as GPG v2.x has a working GUI even\non Windows (in contrast to GPG v1.x, as shipped before).\n\n> It's an honest question - there must be good reasons for that, and git\n> should work with gpg 1, 2 (maybe) and 2.1, of course. I'm just trying\n> to understand the situation, and the question goes both ways:\n> \n> - some Windows user(s) in the Github issue apparantly had wrong\n> assumptions about which gpg they're using (via git) - why bundle it at\n> all?\n\nIt was bundled with Git for Windows v1.x. Skipping it would have meant\nregressing existing users' setups. I am not willing to do that\nwillfully.\n\n> - If bundling it to get a known working setup, why not gpg 2(.1) which\n> runs gpg-agent automatically, giving a more Windows-like experience for\n> the passphrase-prompt?\n\nAgain, this option was not available at the time.\n\n> On Fedora, with some things still requiring gpg 1, gpg 2.1 installed in\n> parallel, things can become confusing quickly because of the 1-time\n> 1-way migration of the secret key store. It's probably the same on\n> Windows with those two gpg's used in parallel (and probably answers my\n> second question).\n\nWell, to be quite honest, I am still not convinced that GPG v2.x has a\ntruly Windows-like experience, as it does not integrate e.g. with the\nWindows Credential Store. But it is understandable: traditionally,\nWindows and GNU-licensed programs were at odds. I think that changed in\nthe meantime, so who knows? Maybe GPG v2.x will sprout support for the\nWindows Credential Store after all...\n\nCiao,\nDscho\n"}]}