threads / patch / 59244

patch, 2 partsMake GPG errors less puzzling

Subject: [PATCH 0/2] Make GPG errors less puzzling

## tl;dr

4 messages between Feb 15, 2023 and Feb 15, 2023. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Johannes Schindelin via GitGitGadget· Feb 15, 2023, 05:58 UTC · lore

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

Johannes Schindelin via GitGitGadget· Feb 15, 2023, 05:58 UTC · re: Johannes Schindelin via GitGitGadget · lore

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

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(-)

Show changes to 2 files +15 −3

gpg-interface.c, t/t7510-signed-commit.sh

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

Junio C Hamano· Feb 15, 2023, 17:20 UTC · re: Johannes Schindelin via GitGitGadget · lore

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

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

Show 10 quoted lines

>  	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.

Johannes Schindelin via GitGitGadget· Feb 15, 2023, 05:58 UTC · re: Johannes Schindelin via GitGitGadget · lore

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

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(+)

Show changes to t/t7510-signed-commit.sh +36 −0

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