# [PATCH 0/2] Make GPG errors less puzzling 4 messages from 2023-02-15 to 2023-02-15. Participants: Johannes Schindelin via GitGitGadget, Junio C Hamano. Thread: https://gitlist.dev/t/59244 ## Johannes Schindelin via GitGitGadget, 2023-02-15 05:58 Subject: [PATCH 0/2] Make GPG errors less puzzling Message-ID: URL: https://gitlist.dev/e/pull.1480.git.1676440714.gitgitgadget%40gmail.com ``` 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, 2023-02-15 05:58 Subject: [PATCH 2/2] gpg: do show gpg's error message upon failure Message-ID: URL: https://gitlist.dev/e/ead90d343b1f4f4ce8998b2f31558fd30d7d2675.1676440714.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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 --- 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 ``` ## Johannes Schindelin via GitGitGadget, 2023-02-15 05:58 Subject: [PATCH 1/2] t7510: add a test case that does not need gpg Message-ID: URL: https://gitlist.dev/e/fe55654b993c84b59f3d00e418d662189325eb3a.1676440714.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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 --- 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 ``` ## Junio C Hamano, 2023-02-15 17:20 Subject: Re: [PATCH 2/2] gpg: do show gpg's error message upon failure Message-ID: URL: https://gitlist.dev/e/xmqqzg9ezvyo.fsf%40gitster.g In-Reply-To: ``` "Johannes Schindelin via GitGitGadget" 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. ```