Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.

Mike Hommey [off-list ref] writes:

On Wed, Jan 25, 2017 at 03:04:38PM -0800, Junio C Hamano wrote:
...

quoted

Overall I think this is a good thing to do.  Instead of eating the
status output, showing what we got, especially when we know the
command failed, would make the bug-hunting of user's environment
easier, like you showed above.

The only thing in the design that makes me wonder is the filtering
out based on "[GNUPG:]" prefix.  Why do we need to filter them out?

The [GNUPG:] lines are part of the status-fd protocol. They show details
that don't really seem interesting to the user. In fact, they even
contain the signed message (yes, in my case, it turns out gpg was
actually still signing, but returned an error code...).

OK, that detail was what was missing in the proposed log message.
Without that "why do we filter?", readers cannot correctly assess
why it is a good idea.  I wasn't arguing against filtering it; I
just wanted to make sure "git log" readers (and "git show" user
after "git blame" identifies this change in the history) will know
how we decided to filter lines with the prefix.  

With that information recorded in the log (or in-code comment, or
both), if it turns out that some lines with the prefix are useful
(or some other lines without the prefix are not very useful), they
can tweak the filtering criteria as appropriate, with confidence
that they _know_ for what purpose the initial "filter lines with the
prefix" was trying to serve, and their update is still in the same
spirit as the original, only executed better.

Thanks.

On Wed, Jan 25, 2017 at 06:37:55PM -0800, Junio C Hamano wrote:

Mike Hommey [off-list ref] writes:

quoted

On Wed, Jan 25, 2017 at 03:04:38PM -0800, Junio C Hamano wrote:
...

quoted

Overall I think this is a good thing to do.  Instead of eating the
status output, showing what we got, especially when we know the
command failed, would make the bug-hunting of user's environment
easier, like you showed above.

The only thing in the design that makes me wonder is the filtering
out based on "[GNUPG:]" prefix.  Why do we need to filter them out?

The [GNUPG:] lines are part of the status-fd protocol. They show details
that don't really seem interesting to the user. In fact, they even
contain the signed message (yes, in my case, it turns out gpg was
actually still signing, but returned an error code...).

OK, that detail was what was missing in the proposed log message.
Without that "why do we filter?", readers cannot correctly assess
why it is a good idea.  I wasn't arguing against filtering it; I
just wanted to make sure "git log" readers (and "git show" user
after "git blame" identifies this change in the history) will know
how we decided to filter lines with the prefix.  

With that information recorded in the log (or in-code comment, or
both), if it turns out that some lines with the prefix are useful
(or some other lines without the prefix are not very useful), they
can tweak the filtering criteria as appropriate, with confidence
that they _know_ for what purpose the initial "filter lines with the
prefix" was trying to serve, and their update is still in the same
spirit as the original, only executed better.

Come to think of it, and considering that mutt happily signs emails in
the same conditions, maybe it would make sense to just ignore gpg return
code as long as there is a SIG_CREATED message...

Mike