* [PATCH 0/2] Make GPG errors less puzzling
@ 2023-02-15 5:58 Johannes Schindelin via GitGitGadget
2023-02-15 5:58 ` [PATCH 1/2] t7510: add a test case that does not need gpg Johannes Schindelin via GitGitGadget
2023-02-15 5:58 ` [PATCH 2/2] gpg: do show gpg's error message upon failure Johannes Schindelin via GitGitGadget
0 siblings, 2 replies; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-02-15 5:58 UTC (permalink / raw
To: git; +Cc: Johannes Schindelin
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] t7510: add a test case that does not need gpg
2023-02-15 5:58 [PATCH 0/2] Make GPG errors less puzzling Johannes Schindelin via GitGitGadget
@ 2023-02-15 5:58 ` Johannes Schindelin via GitGitGadget
2023-02-15 5:58 ` [PATCH 2/2] gpg: do show gpg's error message upon failure Johannes Schindelin via GitGitGadget
1 sibling, 0 replies; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-02-15 5:58 UTC (permalink / raw
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] gpg: do show gpg's error message upon failure
2023-02-15 5:58 [PATCH 0/2] Make GPG errors less puzzling Johannes Schindelin via GitGitGadget
2023-02-15 5:58 ` [PATCH 1/2] t7510: add a test case that does not need gpg Johannes Schindelin via GitGitGadget
@ 2023-02-15 5:58 ` Johannes Schindelin via GitGitGadget
2023-02-15 17:20 ` Junio C Hamano
1 sibling, 1 reply; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-02-15 5:58 UTC (permalink / raw
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] gpg: do show gpg's error message upon failure
2023-02-15 5:58 ` [PATCH 2/2] gpg: do show gpg's error message upon failure Johannes Schindelin via GitGitGadget
@ 2023-02-15 17:20 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2023-02-15 17:20 UTC (permalink / raw
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-15 17:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-15 5:58 [PATCH 0/2] Make GPG errors less puzzling Johannes Schindelin via GitGitGadget
2023-02-15 5:58 ` [PATCH 1/2] t7510: add a test case that does not need gpg Johannes Schindelin via GitGitGadget
2023-02-15 5:58 ` [PATCH 2/2] gpg: do show gpg's error message upon failure Johannes Schindelin via GitGitGadget
2023-02-15 17:20 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).