[PATCH] Unbreak interactive GPG prompt upon signing

[PATCH] Unbreak interactive GPG prompt upon signing

[Johannes Schindelin]

With the recent update in efee955 (gpg-interface: check gpg signature
creation status, 2016-06-17), we ask GPG to send all status updates to
stderr, and then catch the stderr in an strbuf.

But GPG might fail, and send error messages to stderr. And we simply
do not show them to the user.

Even worse: this swallows any interactive prompt for a passphrase. And
detaches stderr from the tty so that the passphrase cannot be read.

So while the first problem could be fixed (by printing the captured
stderr upon error), the second problem cannot be easily fixed, and
presents a major regression.

So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.

This fixes https://github.com/git-for-windows/git/issues/871

Cc: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/fix-gpg-v1
Fetch-It-Via: git fetch https://github.com/dscho/git fix-gpg-v1

 gpg-interface.c | 8 ++------
 t/t7004-tag.sh  | 9 +--------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 8672eda..3f3a3f7 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -153,11 +153,9 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	struct child_process gpg = CHILD_PROCESS_INIT;
 	int ret;
 	size_t i, j, bottom;
-	struct strbuf gpg_status = STRBUF_INIT;
 
 	argv_array_pushl(&gpg.args,
 			 gpg_program,
-			 "--status-fd=2",
 			 "-bsau", signing_key,
 			 NULL);
 
@@ -169,12 +167,10 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	 */
 	sigchain_push(SIGPIPE, SIG_IGN);
 	ret = pipe_command(&gpg, buffer->buf, buffer->len,
-			   signature, 1024, &gpg_status, 0);
+			   signature, 1024, NULL, 0);
 	sigchain_pop(SIGPIPE);
 
-	ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
-	strbuf_release(&gpg_status);
-	if (ret)
+	if (ret || signature->len == bottom)
 		return error(_("gpg failed to sign the data"));
 
 	/* Strip CR from the line endings, in case we are on Windows. */
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..f9b7d79 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1202,17 +1202,10 @@ test_expect_success GPG,RFC1991 \
 # try to sign with bad user.signingkey
 git config user.signingkey BobTheMouse
 test_expect_success GPG \
-	'git tag -s fails if gpg is misconfigured (bad key)' \
+	'git tag -s fails if gpg is misconfigured' \
 	'test_must_fail git tag -s -m tail tag-gpg-failure'
 git config --unset user.signingkey
 
-# try to produce invalid signature
-test_expect_success GPG \
-	'git tag -s fails if gpg is misconfigured (bad signature format)' \
-	'test_config gpg.program echo &&
-	 test_must_fail git tag -s -m tail tag-gpg-failure'
-
-
 # try to verify without gpg:
 
 rm -rf gpghome
-- 
2.10.0.windows.1.6.gc4f481a

base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b

Re: [PATCH] Unbreak interactive GPG prompt upon signing

[Michael J Gruber]

Johannes Schindelin venit, vidit, dixit 06.09.2016 10:01:
> With the recent update in efee955 (gpg-interface: check gpg signature
> creation status, 2016-06-17), we ask GPG to send all status updates to
> stderr, and then catch the stderr in an strbuf.
> 
> But GPG might fail, and send error messages to stderr. And we simply
> do not show them to the user.
> 
> Even worse: this swallows any interactive prompt for a passphrase. And
> detaches stderr from the tty so that the passphrase cannot be read.
> 
> So while the first problem could be fixed (by printing the captured
> stderr upon error), the second problem cannot be easily fixed, and
> presents a major regression.

My Git has that commit and does ask me for the passphrase on the tty.
Also, I do get error messages:

git tag -u pebcak -s testt -m m
error: gpg failed to sign the data
error: unable to sign the tag

which we could (maybe should) amend by gpg's stderr.

> So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.

That "just" reintroduces the problem that the orignal patch solves.

The passphrase/tty issue must be Windows specific - or the non-issue
Linux-specific, if you prefer.

> This fixes https://github.com/git-for-windows/git/issues/871
> 
> Cc: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/fix-gpg-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git fix-gpg-v1
> 
>  gpg-interface.c | 8 ++------
>  t/t7004-tag.sh  | 9 +--------
>  2 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 8672eda..3f3a3f7 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -153,11 +153,9 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	struct child_process gpg = CHILD_PROCESS_INIT;
>  	int ret;
>  	size_t i, j, bottom;
> -	struct strbuf gpg_status = STRBUF_INIT;
>  
>  	argv_array_pushl(&gpg.args,
>  			 gpg_program,
> -			 "--status-fd=2",
>  			 "-bsau", signing_key,
>  			 NULL);
>  
> @@ -169,12 +167,10 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	 */
>  	sigchain_push(SIGPIPE, SIG_IGN);
>  	ret = pipe_command(&gpg, buffer->buf, buffer->len,
> -			   signature, 1024, &gpg_status, 0);
> +			   signature, 1024, NULL, 0);
>  	sigchain_pop(SIGPIPE);
>  
> -	ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
> -	strbuf_release(&gpg_status);
> -	if (ret)
> +	if (ret || signature->len == bottom)
>  		return error(_("gpg failed to sign the data"));
>  
>  	/* Strip CR from the line endings, in case we are on Windows. */
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 8b0f71a..f9b7d79 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1202,17 +1202,10 @@ test_expect_success GPG,RFC1991 \
>  # try to sign with bad user.signingkey
>  git config user.signingkey BobTheMouse
>  test_expect_success GPG \
> -	'git tag -s fails if gpg is misconfigured (bad key)' \
> +	'git tag -s fails if gpg is misconfigured' \
>  	'test_must_fail git tag -s -m tail tag-gpg-failure'
>  git config --unset user.signingkey
>  
> -# try to produce invalid signature
> -test_expect_success GPG \
> -	'git tag -s fails if gpg is misconfigured (bad signature format)' \
> -	'test_config gpg.program echo &&
> -	 test_must_fail git tag -s -m tail tag-gpg-failure'
> -
> -
>  # try to verify without gpg:
>  
>  rm -rf gpghome
> 

[PATCH] gpg-interface: reflect stderr to stderr

[Michael J Gruber]

efee955 ("gpg-interface: check gpg signature creation status",
2016-06-17) used stderr to capture gpg's status output, which is the
only reliable way for status checks. As a side effect, stderr was not
shown to the user any more.

In case of a gpg error, reflect the whole captured buffer to stderr.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
A full blown approach would use --status-fd=4 or such rather than hijacking stderr.
This would require an extension of pipe_command() etc. to handle yet another fd.

 gpg-interface.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 8672eda..cf35bca 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -173,9 +173,12 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	sigchain_pop(SIGPIPE);
 
 	ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
-	strbuf_release(&gpg_status);
-	if (ret)
+	if (ret) {
+		fputs(gpg_status.buf, stderr);
+		strbuf_release(&gpg_status);
 		return error(_("gpg failed to sign the data"));
+	}
+	strbuf_release(&gpg_status);
 
 	/* Strip CR from the line endings, in case we are on Windows. */
 	for (i = j = bottom; i < signature->len; i++)
-- 
2.10.0.rc2.333.g8ef2d05

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Johannes Schindelin]

Hi Michael,

On Tue, 6 Sep 2016, Michael J Gruber wrote:

> efee955 ("gpg-interface: check gpg signature creation status",
> 2016-06-17) used stderr to capture gpg's status output, which is the
> only reliable way for status checks. As a side effect, stderr was not
> shown to the user any more.
> 
> In case of a gpg error, reflect the whole captured buffer to stderr.
> 
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> A full blown approach would use --status-fd=4 or such rather than hijacking stderr.
> This would require an extension of pipe_command() etc. to handle yet another fd.

As I indicated in my patch, this is not enough on Windows. In fact, my
first version of a patch tried to do exactly what you presented here, and
all it did was make the error message a bit more verbose:

-- snip --
error: gpg failed to sign the data:
[GNUPG:] USERID_HINT <key> Johannes Schindelin <johannes.schindelin@gmx.de>
[GNUPG:] NEED_PASSPHRASE <key> <key2> 1 0
gpg: cannot open tty `no tty': No such file or directory

error: unable to sign the tag
-- snap --

This is not a fix for the issue reported on the Git for Windows, but only
half a fix.

Ciao,
Dscho

Re: [PATCH] Unbreak interactive GPG prompt upon signing

[Johannes Schindelin]

Hi Michael,

On Tue, 6 Sep 2016, Michael J Gruber wrote:

> Johannes Schindelin venit, vidit, dixit 06.09.2016 10:01:
> > With the recent update in efee955 (gpg-interface: check gpg signature
> > creation status, 2016-06-17), we ask GPG to send all status updates to
> > stderr, and then catch the stderr in an strbuf.
> > 
> > But GPG might fail, and send error messages to stderr. And we simply
> > do not show them to the user.
> > 
> > Even worse: this swallows any interactive prompt for a passphrase. And
> > detaches stderr from the tty so that the passphrase cannot be read.
> > 
> > So while the first problem could be fixed (by printing the captured
> > stderr upon error), the second problem cannot be easily fixed, and
> > presents a major regression.
> 
> My Git has that commit and does ask me for the passphrase on the tty.
> Also, I do get error messages:
> 
> git tag -u pebcak -s testt -m m
> error: gpg failed to sign the data
> error: unable to sign the tag

That is not GPG's error message. It just leaves users puzzled, is what it
does.

> which we could (maybe should) amend by gpg's stderr.

Right. But then we still do not solve the problem. The problem being that
some platforms cannot use getpass(prompt): it simply does not exist.

On Windows, we do not even have a /dev/tty (technically, GPG, being an
MSYS2 program, knows about /dev/tty, but we spawn it from a non-MSYS2
program, so there is a disconnect).

> > So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.
> 
> That "just" reintroduces the problem that the orignal patch solves.

Right. Which is: when some user misconfigures gpg, causing Git to run
something different that simply succeeds, there is no signature.

This is a minor issue, as it requires a user to configure gpg, and do a
bad job at it.

Not being able to input the passphrase on Windows is a major issue, as the
user has done nothing wrong.

> The passphrase/tty issue must be Windows specific - or the non-issue
> Linux-specific, if you prefer.

Sure. Let's talk about semantics. Oh wait, maybe we should work on
resolving the issue instead.

> > This fixes https://github.com/git-for-windows/git/issues/871

To reiterate: this is the problem I need to see solved.

Ciao,
Dscho

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Johannes Schindelin]

Hi Michael,

On Tue, 6 Sep 2016, Johannes Schindelin wrote:

> On Tue, 6 Sep 2016, Michael J Gruber wrote:
> 
> > A full blown approach would use --status-fd=4 or such rather than hijacking stderr.
> > This would require an extension of pipe_command() etc. to handle yet another fd.

For the record: I do not know whether that would work, either. So unless
we are fairly certain that it *would* work, I'd rather not spend the time.

Your original issue seemed to be that the gpg command could succeed, but
still no signature be seen. There *must* be a way to test whether the
called program added a signature, simply by testing whether *any*
characters were written.

And if characters were written that were not actually a GPG signature,
maybe the enterprisey user who configured the gpg command to be her magic
script actually meant something else than a GPG signature to be added?

Ciao,
Dscho

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Johannes Schindelin]

Hi Michael,

okay, final mail on this issue today:

On Tue, 6 Sep 2016, Johannes Schindelin wrote:

> Your original issue seemed to be that the gpg command could succeed, but
> still no signature be seen. There *must* be a way to test whether the
> called program added a signature, simply by testing whether *any*
> characters were written.
> 
> And if characters were written that were not actually a GPG signature,
> maybe the enterprisey user who configured the gpg command to be her magic
> script actually meant something else than a GPG signature to be added?

I actually just saw that this is *precisely* what the code does already:

        if (ret || signature->len == bottom)
                return error(_("gpg failed to sign the data"));

Why is this not good enough?

Ciao,
Dscho

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Michael J Gruber]

Johannes Schindelin venit, vidit, dixit 06.09.2016 18:43:
> Hi Michael,
> 
> okay, final mail on this issue today:
> 
> On Tue, 6 Sep 2016, Johannes Schindelin wrote:
> 
>> Your original issue seemed to be that the gpg command could succeed, but
>> still no signature be seen. There *must* be a way to test whether the
>> called program added a signature, simply by testing whether *any*
>> characters were written.
>>
>> And if characters were written that were not actually a GPG signature,
>> maybe the enterprisey user who configured the gpg command to be her magic
>> script actually meant something else than a GPG signature to be added?
> 
> I actually just saw that this is *precisely* what the code does already:
> 
>         if (ret || signature->len == bottom)
>                 return error(_("gpg failed to sign the data"));
> 
> Why is this not good enough?


Assuming "this" refers to error():

You said it's not good enough - because gpg's stderr is not displayed.

And I agree with you on that point.

Assuming "this" refers to the exit code check:

gpg documentation says so over and over again: do not rely on exit
codes, parse status-fd instead. (An often ignored advice, oh well.)

As for most of your other remarks, I would appreciate if you could take
a breath and reread what I wrote and what you wrote - before you send it
- and curb your remarks about who is working on resolving issues.

So, trying again to structure the issues and solutions, there are three
issues:

A) relying on gpg's exit code (and stdout) is not enough. Secure use of
gpg requires checking status-fd.

This is what my old patch solved.

B) "gpg --status-fd=2" and swallowing stderr hides usual stderr from the
user.

This is what my old patch introduced and a Windows user reported. It is
solved by my new additional patch.

C) With that old patch, that Windows user is not asked for a passphrase
on tty any more.

Reverting my patches appears to solve C on Windows and reintroduces A on
all platforms, obviously. C is not present on Linux. B is solved either way.

Now, I can't reproduce C on Linux[*], so there is more involved. It
could be that my patch just exposes a problem in our start_command()
etc.: run-command.c contains a lot of ifdefing, so possibly quite
different code is run on different platforms.

It would be great if someone with a Windows environment could help our
efforts in resolving issue C, by checking what is actually behind[**]: I
can't believe that capturing stderr keeps gpg from reading stdin, but
who knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.

Michael

[*] Maybe that even depends on Linux environments (terminal emulator),
so input from others would be helpful, too:

Without a passphrase-agent/wallet etc, does "git tag -s -m test test"
ask you for a passphrase on the terminal?

I does for me with this stack:

X11->i3->st->tmux->bash->git->gpg

[**] "--status-fd=3" instead of "--status-fd=2" in my old patch would be
a check whether our capturing of stderr is creating problems on Windows
or gpg's writing status to stderr (which --status-fd=3 would change, at
the expense of breaking the final check): Does gpg ask for the
passphrase now?

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Jeff King]

On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:

> Now, I can't reproduce C on Linux[*], so there is more involved. It
> could be that my patch just exposes a problem in our start_command()
> etc.: run-command.c contains a lot of ifdefing, so possibly quite
> different code is run on different platforms.

Maybe, though my blind guess is that it is simply that on Linux we can
open /dev/tty directly, and console-IO on Windows is a bit more
complicated.

You might also check your GPG versions; between gpg1.x and gpg2, the
passphrase input handling has been completely revamped.

> It would be great if someone with a Windows environment could help our
> efforts in resolving issue C, by checking what is actually behind[**]: I
> can't believe that capturing stderr keeps gpg from reading stdin, but
> who knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.

I know nothing about Windows, but I'd be surprised if gpg is reading
from stdin, as opposed to /dev/tty. It's probably more to do with how
gpg finds the "tty" on Windows (presumably it looks at stderr for that).

Anyway, I wrote pipe_command() in such a way as to be prepared for
exactly this kind of thing, so it would be trivial to extend it to an
extra descriptor. The trouble is that run_command() doesn't understand
anything except stdin/stdout/stderr. We can open an extra pipe() before
calling run_command(), and make sure it is not marked CLOEXEC. I don't
know if there are other portability concerns, though.

-Peff

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Michael J Gruber]

Jeff King venit, vidit, dixit 07.09.2016 10:39:
> On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:
> 
>> Now, I can't reproduce C on Linux[*], so there is more involved. It
>> could be that my patch just exposes a problem in our start_command()
>> etc.: run-command.c contains a lot of ifdefing, so possibly quite
>> different code is run on different platforms.
> 
> Maybe, though my blind guess is that it is simply that on Linux we can
> open /dev/tty directly, and console-IO on Windows is a bit more
> complicated.
> 
> You might also check your GPG versions; between gpg1.x and gpg2, the
> passphrase input handling has been completely revamped.

That's a good point to note.

gpg1 asks for the passphrase (without use-agent), gpg2 always delegates
to gpg-agent (and starts it on demand).

On Linux, gpg-agent says you should

export GPG_TTY=$(tty)

to make gpg-agent find the tty, and claims it's not necessary on Win.

In fact, it's not necessary on Linux either unless you want to use
pinentry-curses.

Alas, be it gpg1.4.21 or gpg2.1.13, whatever pinentry, I do get the
passphrase prompt, even with curses (if GPG_TTY is set, which was
necessary before any patches already).

I put up a request for more input from the reporters in the github
issue. I guess that's the best way to reach them.

>> It would be great if someone with a Windows environment could help our
>> efforts in resolving issue C, by checking what is actually behind[**]: I
>> can't believe that capturing stderr keeps gpg from reading stdin, but
>> who knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.
> 
> I know nothing about Windows, but I'd be surprised if gpg is reading
> from stdin, as opposed to /dev/tty. It's probably more to do with how
> gpg finds the "tty" on Windows (presumably it looks at stderr for that).
> 
> Anyway, I wrote pipe_command() in such a way as to be prepared for
> exactly this kind of thing, so it would be trivial to extend it to an
> extra descriptor. The trouble is that run_command() doesn't understand
> anything except stdin/stdout/stderr. We can open an extra pipe() before
> calling run_command(), and make sure it is not marked CLOEXEC. I don't
> know if there are other portability concerns, though.

My suggestion to try "--status-fd=3" was meant to test whether the above
could help: If fd=3 helps, then our capturing stderr is not the problem.
(If fd=3 does not help then we still don't know...)

Michael

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:
>
>> Now, I can't reproduce C on Linux[*], so there is more involved. It
>> could be that my patch just exposes a problem in our start_command()
>> etc.: run-command.c contains a lot of ifdefing, so possibly quite
>> different code is run on different platforms.
>
> Maybe, though my blind guess is that it is simply that on Linux we can
> open /dev/tty directly, and console-IO on Windows is a bit more
> complicated.

True.

Even though this patch is fixing only one of the two issues, I am
tempted to say that we should queue it for now, as it does so
without breaking a bigger gain made by the original, i.e. we learn
the status of verification in a way the authors of GPG wants us to,
while somebody figuires out what the best way is to show the prompt
to the console on Windows.

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Jeff King]

On Thu, Sep 08, 2016 at 11:20:09AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:
> >
> >> Now, I can't reproduce C on Linux[*], so there is more involved. It
> >> could be that my patch just exposes a problem in our start_command()
> >> etc.: run-command.c contains a lot of ifdefing, so possibly quite
> >> different code is run on different platforms.
> >
> > Maybe, though my blind guess is that it is simply that on Linux we can
> > open /dev/tty directly, and console-IO on Windows is a bit more
> > complicated.
> 
> True.
> 
> Even though this patch is fixing only one of the two issues, I am
> tempted to say that we should queue it for now, as it does so
> without breaking a bigger gain made by the original, i.e. we learn
> the status of verification in a way the authors of GPG wants us to,
> while somebody figuires out what the best way is to show the prompt
> to the console on Windows.

That's OK by me, but I don't know if we can put off the "best way to
show the prompt" fix. It seems like a pretty serious regression for
people on Windows.

-Peff

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Even though this patch is fixing only one of the two issues, I am
>> tempted to say that we should queue it for now, as it does so
>> without breaking a bigger gain made by the original, i.e. we learn
>> the status of verification in a way the authors of GPG wants us to,
>> while somebody figuires out what the best way is to show the prompt
>> to the console on Windows.
>
> That's OK by me, but I don't know if we can put off the "best way to
> show the prompt" fix. It seems like a pretty serious regression for
> people on Windows.

Yes, I am not saying that it is OK to keep Windows users broken.

As I understand what Dscho said correctly, his users are covered by
a reversion of the "read the GPG status correctly" patch, i.e. with
a different trade-off between the correctness of GPG status vs
usability of the prompt, he will ship with Git for Windows, and that
stop-gap measure will last only until developers who can do Windows
(which excludes you, me, and Michael it seems) comes up with a
solution that satisfies both.

I consider that an approach that is perfectly fine.

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Johannes Schindelin]

Hi Junio,

On Thu, 8 Sep 2016, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Wed, Sep 07, 2016 at 10:27:34AM +0200, Michael J Gruber wrote:
> >
> >> Now, I can't reproduce C on Linux[*], so there is more involved. It
> >> could be that my patch just exposes a problem in our start_command()
> >> etc.: run-command.c contains a lot of ifdefing, so possibly quite
> >> different code is run on different platforms.
> >
> > Maybe, though my blind guess is that it is simply that on Linux we can
> > open /dev/tty directly, and console-IO on Windows is a bit more
> > complicated.
> 
> True.
> 
> Even though this patch is fixing only one of the two issues, I am
> tempted to say that we should queue it for now, as it does so
> without breaking a bigger gain made by the original, i.e. we learn
> the status of verification in a way the authors of GPG wants us to,
> while somebody figuires out what the best way is to show the prompt
> to the console on Windows.

Between protecting users from their own mis-configurations and allowing
them to enter their passphrase interactively on Windows, I would argue
that the more important thing to have working is the latter, because it is
not at all the user's fault that they cannot enter their passphrase
currently, and they cannot fix it by fixing their mis-configuration.

Unfortunately, you obviously disagree with this assessement.

As I *need* to fix this *major* bug for Windows users, you basically put
an additional burden on me by applying Michael's patch, which not only
does not fix the problem for Windows users, but conflicts with my
work-around.

Pity,
Dscho

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 08.09.2016 23:36:
> Jeff King <peff@peff.net> writes:
> 
>>> Even though this patch is fixing only one of the two issues, I am
>>> tempted to say that we should queue it for now, as it does so
>>> without breaking a bigger gain made by the original, i.e. we learn
>>> the status of verification in a way the authors of GPG wants us to,
>>> while somebody figuires out what the best way is to show the prompt
>>> to the console on Windows.
>>
>> That's OK by me, but I don't know if we can put off the "best way to
>> show the prompt" fix. It seems like a pretty serious regression for
>> people on Windows.
> 
> Yes, I am not saying that it is OK to keep Windows users broken.
> 
> As I understand what Dscho said correctly, his users are covered by
> a reversion of the "read the GPG status correctly" patch, i.e. with
> a different trade-off between the correctness of GPG status vs
> usability of the prompt, he will ship with Git for Windows, and that
> stop-gap measure will last only until developers who can do Windows
> (which excludes you, me, and Michael it seems) comes up with a
> solution that satisfies both.

Unfortunately "I can't do Windows". Also, I'm not sure "I can do pipes",
but it's really the ifdeffing that keeps me from even trying: Nothing is
gained for Windows users if I extend the Linux code to use an extra file
handle for status-fd - which would be the clean and correct solution,
but which would need to be implemented twice.

> I consider that an approach that is perfectly fine.

As a side note, I'm wondering why MSYS-gpg version 1 is bundled with
non-MSYS-git. It's an honest question - there must be good reasons for
that, and git should work with gpg 1, 2 (maybe) and 2.1, of course. I'm
just trying to understand the situation, and the question goes both ways:

- some Windows user(s) in the Github issue apparantly had wrong
assumptions about which gpg they're using (via git) - why bundle it at all?

- If bundling it to get a known working setup, why not gpg 2(.1) which
runs gpg-agent automatically, giving a more Windows-like experience for
the passphrase-prompt?

On Fedora, with some things still requiring gpg 1, gpg 2.1 installed in
parallel, things can become confusing quickly because  of the 1-time
1-way migration of the secret key store. It's probably the same on
Windows with those two gpg's used in parallel (and probably answers my
second question).

Michael

Re: [PATCH] gpg-interface: reflect stderr to stderr

[Johannes Schindelin]

Hi Michael,

[blast from the past]

On Mon, 12 Sep 2016, Michael J Gruber wrote:

> As a side note, I'm wondering why MSYS-gpg version 1 is bundled with
> non-MSYS-git.

Those are two questions:

- an MSYS version of GPG is bundled because it was the stable one
  available at the time when I had to decide: in summer 2015.

- GPG v1.x was bundled because, again, this was the version bundled by
  MSYS2 (and I have to rely heavily on what is bundled with MSYS2 by
  default; I could not run Git for Windows if I had to build all of the
  components from scratch, and maintain them, in addition to Git
  itself).

The good news (ahem, see below) is that GPG v2.x is now bundled in
MSYS2.

The not-so-good news is that this can break existing setups. My own
setup, for example, was broken by this under-announced upgrade.

But other things, such as GPG-signing commits in a rebase, should
actually be *fixed* by this upgrade, as GPG v2.x has a working GUI even
on Windows (in contrast to GPG v1.x, as shipped before).

> It's an honest question - there must be good reasons for that, and git
> should work with gpg 1, 2 (maybe) and 2.1, of course. I'm just trying
> to understand the situation, and the question goes both ways:
> 
> - some Windows user(s) in the Github issue apparantly had wrong
> assumptions about which gpg they're using (via git) - why bundle it at
> all?

It was bundled with Git for Windows v1.x. Skipping it would have meant
regressing existing users' setups. I am not willing to do that
willfully.

> - If bundling it to get a known working setup, why not gpg 2(.1) which
> runs gpg-agent automatically, giving a more Windows-like experience for
> the passphrase-prompt?

Again, this option was not available at the time.

> On Fedora, with some things still requiring gpg 1, gpg 2.1 installed in
> parallel, things can become confusing quickly because  of the 1-time
> 1-way migration of the secret key store. It's probably the same on
> Windows with those two gpg's used in parallel (and probably answers my
> second question).

Well, to be quite honest, I am still not convinced that GPG v2.x has a
truly Windows-like experience, as it does not integrate e.g. with the
Windows Credential Store. But it is understandable: traditionally,
Windows and GNU-licensed programs were at odds. I think that changed in
the meantime, so who knows? Maybe GPG v2.x will sprout support for the
Windows Credential Store after all...

Ciao,
Dscho