[PATCH 0/2] Make GPG errors less puzzling

[PATCH 0/2] Make GPG errors less puzzling

[Johannes Schindelin via GitGitGadget]

For one times too many, I was asked to help with a commit signing problem
where the only error message was an unhelpful:

error: gpg failed to sign the data
fatal: failed to write commit object


That was it. No further indication what went wrong. And certainly not that
wonderful error message that the helper that was configured as gpg.program
wrote to its stderr.

Let's show whatever GPG (or any alternative configured via gpg.program) had
to say when signing failed.

Johannes Schindelin (2):
  t7510: add a test case that does not need gpg
  gpg: do show gpg's error message upon failure

 gpg-interface.c          |  8 ++++++--
 t/t7510-signed-commit.sh | 44 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)


base-commit: 23c56f7bd5f1667f8b793d796bf30e39545920f6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1480%2Fdscho%2Fmake-gpg-errors-less-puzzling-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1480/dscho/make-gpg-errors-less-puzzling-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1480
-- 
gitgitgadget

[PATCH 1/2] t7510: add a test case that does not need gpg

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This test case not only increases test coverage in setups without
working gpg, but also prepares for verifying that the error message of
`gpg.program` is shown upon failure.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t7510-signed-commit.sh | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index bc7a31ba3e7..ec07c8550f5 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -387,4 +387,40 @@ test_expect_success GPG 'verify-commit verifies multiply signed commits' '
 	! grep "BAD signature from" actual
 '
 
+test_expect_success 'custom `gpg.program`' '
+	write_script fake-gpg <<-\EOF &&
+	args="$*"
+
+	# skip uninteresting options
+	while case "$1" in
+	--status-fd=*|--keyid-format=*) ;; # skip
+	*) break;;
+	esac; do shift; done
+
+	case "$1" in
+	-bsau)
+		cat >sign.file
+		echo "[GNUPG:] SIG_CREATED $args" >&2
+		echo "-----BEGIN PGP MESSAGE-----"
+		echo "$args"
+		echo "-----END PGP MESSAGE-----"
+		;;
+	--verify)
+		cat "$2" >verify.file
+		exit 0
+		;;
+	*)
+		echo "Unhandled args: $*" >&2
+		exit 1
+		;;
+	esac
+	EOF
+
+	test_config gpg.program "$(pwd)/fake-gpg" &&
+	git commit -S --allow-empty -m signed-commit &&
+	test_path_exists sign.file &&
+	git show --show-signature &&
+	test_path_exists verify.file
+'
+
 test_done
-- 
gitgitgadget

[PATCH 2/2] gpg: do show gpg's error message upon failure

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

There are few things more frustrating when signing a commit fails than
reading a terse "error: gpg failed to sign the data" message followed by
the unsurprising "fatal: failed to write commit object" message.

In many cases where signing a commit or tag fails, `gpg` actually said
something helpful, on its stderr, and Git even consumed that, but then
keeps mum about it.

Teach Git to stop withholding that rather important information.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 gpg-interface.c          |  8 ++++++--
 t/t7510-signed-commit.sh | 10 +++++++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 687236430bf..5cd66d3a786 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -977,9 +977,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
 			break; /* found */
 	}
 	ret |= !cp;
+	if (ret) {
+		error(_("gpg failed to sign the data:\n%s"),
+		      gpg_status.len ? gpg_status.buf : "(no gpg output)");
+		strbuf_release(&gpg_status);
+		return -1;
+	}
 	strbuf_release(&gpg_status);
-	if (ret)
-		return error(_("gpg failed to sign the data"));
 
 	/* Strip CR from the line endings, in case we are on Windows. */
 	remove_cr_after(signature, bottom);
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index ec07c8550f5..48f86cb3678 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -399,6 +399,10 @@ test_expect_success 'custom `gpg.program`' '
 
 	case "$1" in
 	-bsau)
+		test -z "$LET_GPG_PROGRAM_FAIL" || {
+			echo "zOMG signing failed!" >&2
+			exit 1
+		}
 		cat >sign.file
 		echo "[GNUPG:] SIG_CREATED $args" >&2
 		echo "-----BEGIN PGP MESSAGE-----"
@@ -420,7 +424,11 @@ test_expect_success 'custom `gpg.program`' '
 	git commit -S --allow-empty -m signed-commit &&
 	test_path_exists sign.file &&
 	git show --show-signature &&
-	test_path_exists verify.file
+	test_path_exists verify.file &&
+
+	test_must_fail env LET_GPG_PROGRAM_FAIL=1 \
+	git commit -S --allow-empty -m must-fail 2>err &&
+	grep zOMG err
 '
 
 test_done
-- 
gitgitgadget

Re: [PATCH 2/2] gpg: do show gpg's error message upon failure

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>  	ret |= !cp;
> +	if (ret) {
> +		error(_("gpg failed to sign the data:\n%s"),
> +		      gpg_status.len ? gpg_status.buf : "(no gpg output)");
> +		strbuf_release(&gpg_status);
> +		return -1;
> +	}
>  	strbuf_release(&gpg_status);
> -	if (ret)
> -		return error(_("gpg failed to sign the data"));

Good.  As we are worried about error messages that are too terse,
dumping everything to the output would be a vast improvement.
Hopefully gpg_status.len would to be thousands of bytes long, and
this is not a codepath that is triggered remotely anyway.

Will queue.  Thanks.