{"thread":{"id":"59244","subject":"[PATCH 0/2] Make GPG errors less puzzling","startedAt":"2023-02-15T05:58:40Z","lastAt":"2023-02-15T17:20:53Z","messageCount":4,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"472123","messageId":"pull.1480.git.1676440714.gitgitgadget@gmail.com","threadId":"59244","inReplyTo":null,"subject":"[PATCH 0/2] Make GPG errors less puzzling","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-15T05:58:32Z","receivedAt":"2023-02-15T05:58:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"For one times too many, I was asked to help with a commit signing problem\nwhere the only error message was an unhelpful:\n\nerror: gpg failed to sign the data\nfatal: failed to write commit object\n\n\nThat was it. No further indication what went wrong. And certainly not that\nwonderful error message that the helper that was configured as gpg.program\nwrote to its stderr.\n\nLet's show whatever GPG (or any alternative configured via gpg.program) had\nto say when signing failed.\n\nJohannes Schindelin (2):\n  t7510: add a test case that does not need gpg\n  gpg: do show gpg's error message upon failure\n\n gpg-interface.c          |  8 ++++++--\n t/t7510-signed-commit.sh | 44 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 50 insertions(+), 2 deletions(-)\n\n\nbase-commit: 23c56f7bd5f1667f8b793d796bf30e39545920f6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1480%2Fdscho%2Fmake-gpg-errors-less-puzzling-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1480/dscho/make-gpg-errors-less-puzzling-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1480\n-- \ngitgitgadget\n"},{"id":"472124","messageId":"ead90d343b1f4f4ce8998b2f31558fd30d7d2675.1676440714.git.gitgitgadget@gmail.com","threadId":"59244","inReplyTo":"pull.1480.git.1676440714.gitgitgadget@gmail.com","subject":"[PATCH 2/2] gpg: do show gpg's error message upon failure","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-15T05:58:34Z","receivedAt":"2023-02-15T05:58:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThere are few things more frustrating when signing a commit fails than\nreading a terse \"error: gpg failed to sign the data\" message followed by\nthe unsurprising \"fatal: failed to write commit object\" message.\n\nIn many cases where signing a commit or tag fails, `gpg` actually said\nsomething helpful, on its stderr, and Git even consumed that, but then\nkeeps mum about it.\n\nTeach Git to stop withholding that rather important information.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n gpg-interface.c          |  8 ++++++--\n t/t7510-signed-commit.sh | 10 +++++++++-\n 2 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 687236430bf..5cd66d3a786 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -977,9 +977,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\tbreak; /* found */\n \t}\n \tret |= !cp;\n+\tif (ret) {\n+\t\terror(_(\"gpg failed to sign the data:\\n%s\"),\n+\t\t      gpg_status.len ? gpg_status.buf : \"(no gpg output)\");\n+\t\tstrbuf_release(&gpg_status);\n+\t\treturn -1;\n+\t}\n \tstrbuf_release(&gpg_status);\n-\tif (ret)\n-\t\treturn error(_(\"gpg failed to sign the data\"));\n \n \t/* Strip CR from the line endings, in case we are on Windows. */\n \tremove_cr_after(signature, bottom);\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex ec07c8550f5..48f86cb3678 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -399,6 +399,10 @@ test_expect_success 'custom `gpg.program`' '\n \n \tcase \"$1\" in\n \t-bsau)\n+\t\ttest -z \"$LET_GPG_PROGRAM_FAIL\" || {\n+\t\t\techo \"zOMG signing failed!\" >&2\n+\t\t\texit 1\n+\t\t}\n \t\tcat >sign.file\n \t\techo \"[GNUPG:] SIG_CREATED $args\" >&2\n \t\techo \"-----BEGIN PGP MESSAGE-----\"\n@@ -420,7 +424,11 @@ test_expect_success 'custom `gpg.program`' '\n \tgit commit -S --allow-empty -m signed-commit &&\n \ttest_path_exists sign.file &&\n \tgit show --show-signature &&\n-\ttest_path_exists verify.file\n+\ttest_path_exists verify.file &&\n+\n+\ttest_must_fail env LET_GPG_PROGRAM_FAIL=1 \\\n+\tgit commit -S --allow-empty -m must-fail 2>err &&\n+\tgrep zOMG err\n '\n \n test_done\n-- \ngitgitgadget\n"},{"id":"472125","messageId":"fe55654b993c84b59f3d00e418d662189325eb3a.1676440714.git.gitgitgadget@gmail.com","threadId":"59244","inReplyTo":"pull.1480.git.1676440714.gitgitgadget@gmail.com","subject":"[PATCH 1/2] t7510: add a test case that does not need gpg","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-15T05:58:33Z","receivedAt":"2023-02-15T05:58:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis test case not only increases test coverage in setups without\nworking gpg, but also prepares for verifying that the error message of\n`gpg.program` is shown upon failure.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t7510-signed-commit.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex bc7a31ba3e7..ec07c8550f5 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -387,4 +387,40 @@ test_expect_success GPG 'verify-commit verifies multiply signed commits' '\n \t! grep \"BAD signature from\" actual\n '\n \n+test_expect_success 'custom `gpg.program`' '\n+\twrite_script fake-gpg <<-\\EOF &&\n+\targs=\"$*\"\n+\n+\t# skip uninteresting options\n+\twhile case \"$1\" in\n+\t--status-fd=*|--keyid-format=*) ;; # skip\n+\t*) break;;\n+\tesac; do shift; done\n+\n+\tcase \"$1\" in\n+\t-bsau)\n+\t\tcat >sign.file\n+\t\techo \"[GNUPG:] SIG_CREATED $args\" >&2\n+\t\techo \"-----BEGIN PGP MESSAGE-----\"\n+\t\techo \"$args\"\n+\t\techo \"-----END PGP MESSAGE-----\"\n+\t\t;;\n+\t--verify)\n+\t\tcat \"$2\" >verify.file\n+\t\texit 0\n+\t\t;;\n+\t*)\n+\t\techo \"Unhandled args: $*\" >&2\n+\t\texit 1\n+\t\t;;\n+\tesac\n+\tEOF\n+\n+\ttest_config gpg.program \"$(pwd)/fake-gpg\" &&\n+\tgit commit -S --allow-empty -m signed-commit &&\n+\ttest_path_exists sign.file &&\n+\tgit show --show-signature &&\n+\ttest_path_exists verify.file\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"472146","messageId":"xmqqzg9ezvyo.fsf@gitster.g","threadId":"59244","inReplyTo":"ead90d343b1f4f4ce8998b2f31558fd30d7d2675.1676440714.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] gpg: do show gpg's error message upon failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-15T17:20:47Z","receivedAt":"2023-02-15T17:20:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n>  \tret |= !cp;\n> +\tif (ret) {\n> +\t\terror(_(\"gpg failed to sign the data:\\n%s\"),\n> +\t\t      gpg_status.len ? gpg_status.buf : \"(no gpg output)\");\n> +\t\tstrbuf_release(&gpg_status);\n> +\t\treturn -1;\n> +\t}\n>  \tstrbuf_release(&gpg_status);\n> -\tif (ret)\n> -\t\treturn error(_(\"gpg failed to sign the data\"));\n\nGood.  As we are worried about error messages that are too terse,\ndumping everything to the output would be a vast improvement.\nHopefully gpg_status.len would to be thousands of bytes long, and\nthis is not a codepath that is triggered remotely anyway.\n\nWill queue.  Thanks.\n"}]}