{"thread":{"id":"50032","subject":"[PATCH 0/4] Expose gpgsig in pretty-print","startedAt":"2018-12-13T21:23:03Z","lastAt":"2018-12-21T13:52:19Z","messageCount":14,"participants":["John Passaro","Michał Górny","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"365304","messageId":"20181213212256.48122-1-john.a.passaro@gmail.com","threadId":"50032","inReplyTo":null,"subject":"[PATCH 0/4] Expose gpgsig in pretty-print","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-13T21:22:52Z","receivedAt":"2018-12-13T21:23:03Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"Currently, users who do not have GPG installed have no way to discern\nsigned from unsigned commits without examining raw commit data. I\npropose two new pretty-print placeholders to expose this information:\n\n%GR: full (\"R\"aw) contents of gpgsig header\n%G+: Y/N if the commit has nonempty gpgsig header or not\n\nThe second is of course much more likely to be used, but having exposed\nthe one, exposing the other too adds almost no complexity.\n\nI'm open to suggestion on the names of these placeholders.\n\nThis commit is based on master but e5a329a279 (\"run-command: report exec\nfailure\" 2018-12-11) is required for the tests to pass.\n\nOne note is that this change touches areas of the pretty-format\ndocumentation that are radically revamped in aw/pretty-trailers: see\n42617752d4 (\"doc: group pretty-format.txt placeholders descriptions\"\n2018-12-08). I have another version of this branch based on that branch\nas well, so you can use that in case conflicts with aw/pretty-trailers\narise.\n\nSee:\n- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig\n- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers\n\nJohn Passaro (4):\n  pretty: expose raw commit signature\n  t/t7510-signed-commit.sh: test new placeholders\n  doc, tests: pretty behavior when gpg missing\n  docs/pretty-formats: add explanation + copy edits\n\n Documentation/pretty-formats.txt |  21 ++++--\n pretty.c                         |  36 ++++++++-\n t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--\n 3 files changed, 167 insertions(+), 15 deletions(-)\n\n\nbase-commit: 5d826e972970a784bd7a7bdf587512510097b8c7\nprerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db\n-- \n2.19.1\n\n"},{"id":"365305","messageId":"20181213212256.48122-2-john.a.passaro@gmail.com","threadId":"50032","inReplyTo":"20181213212256.48122-1-john.a.passaro@gmail.com","subject":"[PATCH 1/4] pretty: expose raw commit signature","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-13T21:22:53Z","receivedAt":"2018-12-13T21:23:11Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"Add new pretty-format placeholders %GR and %G+ to support inspecting\ngpgsig commit header in pretty format, even if GPG is not available.\n\nSigned-off-by: John Passaro <john.a.passaro@gmail.com>\n---\n Documentation/pretty-formats.txt |  2 ++\n pretty.c                         | 36 ++++++++++++++++++++++++++++++--\n 2 files changed, 36 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 417b638cd8..582454a4f7 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -142,6 +142,8 @@ The placeholders are:\n ifndef::git-rev-list[]\n - '%N': commit notes\n endif::git-rev-list[]\n+- '%GR': contents of the commits signature (blank when unsigned)\n+- '%G+': \"Y\" if the commit is signed, \"N\" otherwise\n - '%GG': raw verification message from GPG for a signed commit\n - '%G?': show \"G\" for a good (valid) signature,\n   \"B\" for a bad signature,\ndiff --git a/pretty.c b/pretty.c\nindex b83a3ecd23..d142b457b5 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -768,6 +768,9 @@ struct format_commit_context {\n \tunsigned commit_header_parsed:1;\n \tunsigned commit_message_parsed:1;\n \tstruct signature_check signature_check;\n+\tunsigned signature_checked:2;\n+\tstruct strbuf signature;\n+\tstruct strbuf signature_payload;\n \tenum flush_type flush_type;\n \tenum trunc_type truncate;\n \tconst char *message;\n@@ -1228,8 +1231,30 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t}\n \n \tif (placeholder[0] == 'G') {\n-\t\tif (!c->signature_check.result)\n-\t\t\tcheck_commit_signature(c->commit, &(c->signature_check));\n+\t\tif (!c->signature_checked) {\n+\t\t\tparse_signed_commit(c->commit, &(c->signature_payload), &(c->signature));\n+\t\t\tc->signature_checked = 1;\n+\t\t}\n+\t\tswitch (placeholder[1]) {\n+\t\tcase 'R':\n+\t\t\tstrbuf_addbuf(sb, &(c->signature));\n+\t\t\tbreak;\n+\t\tcase '+':\n+\t\t\tstrbuf_addch(sb, c->signature.len ? 'Y' : 'N');\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tgoto do_signature_check;\n+\t\t}\n+\t\treturn 2;\n+\n+do_signature_check:\n+\t\tif (c->signature_checked < 2) {\n+\t\t\tif (c->signature.len)\n+\t\t\t\tcheck_signature(c->signature_payload.buf, c->signature_payload.len,\n+\t\t\t\t\t\tc->signature.buf, c->signature.len,\n+\t\t\t\t\t\t&(c->signature_check));\n+\t\t\tc->signature_checked = 2;\n+\t\t}\n \t\tswitch (placeholder[1]) {\n \t\tcase 'G':\n \t\t\tif (c->signature_check.gpg_output)\n@@ -1246,6 +1271,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\tcase 'Y':\n \t\t\tcase 'R':\n \t\t\t\tstrbuf_addch(sb, c->signature_check.result);\n+\t\t\t\tbreak;\n+\t\t\tcase 0: // i.e. no signature so we never ran the check\n+\t\t\t\tstrbuf_addch(sb, 'N');\n \t\t\t}\n \t\t\tbreak;\n \t\tcase 'S':\n@@ -1527,6 +1555,8 @@ void format_commit_message(const struct commit *commit,\n \tcontext.commit = commit;\n \tcontext.pretty_ctx = pretty_ctx;\n \tcontext.wrap_start = sb->len;\n+\tstrbuf_init(&context.signature, 0);\n+\tstrbuf_init(&context.signature_payload, 0);\n \t/*\n \t * convert a commit message to UTF-8 first\n \t * as far as 'format_commit_item' assumes it in UTF-8\n@@ -1556,6 +1586,8 @@ void format_commit_message(const struct commit *commit,\n \t\t\tstrbuf_attach(sb, out, outsz, outsz + 1);\n \t}\n \n+\tstrbuf_release(&context.signature);\n+\tstrbuf_release(&context.signature_payload);\n \tfree(context.commit_encoding);\n \tunuse_commit_buffer(commit, context.message);\n }\n-- \n2.19.1\n\n"},{"id":"365306","messageId":"20181213212256.48122-3-john.a.passaro@gmail.com","threadId":"50032","inReplyTo":"20181213212256.48122-1-john.a.passaro@gmail.com","subject":"[PATCH 2/4] t/t7510-signed-commit.sh: test new placeholders","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-13T21:22:54Z","receivedAt":"2018-12-13T21:23:19Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"Test that %GR output (\"Raw\" contents of \"gpgsig\" header) looks like\nASCII-armored GPG signature.\n\nTest %G+ (Y/N for presence/absence of \"gpgsig\" header) by adding it to\nexisting format tests for signed commits.\n\nSigned-off-by: John Passaro <john.a.passaro@gmail.com>\n---\n t/t7510-signed-commit.sh | 30 +++++++++++++++++++++++++-----\n 1 file changed, 25 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 86d3f93fa2..aff6b1eb3a 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -177,8 +177,9 @@ test_expect_success GPG 'show good signature with custom format' '\n \tC O Mitter <committer@example.com>\n \t73D758744BE721698EC54E8713B6F51ECDDE430D\n \t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tY\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP\" sixth-signed >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" sixth-signed >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -189,8 +190,9 @@ test_expect_success GPG 'show bad signature with custom format' '\n \tC O Mitter <committer@example.com>\n \n \n+\tY\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP\" $(cat forged1.commit) >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" $(cat forged1.commit) >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -201,8 +203,9 @@ test_expect_success GPG 'show untrusted signature with custom format' '\n \tEris Discordia <discord@example.net>\n \tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n \tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tY\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP\" eighth-signed-alt >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" eighth-signed-alt >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -213,8 +216,9 @@ test_expect_success GPG 'show unknown signature with custom format' '\n \n \n \n+\tY\n \tEOF\n-\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP\" eighth-signed-alt >actual &&\n+\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" eighth-signed-alt >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -225,11 +229,27 @@ test_expect_success GPG 'show lack of signature with custom format' '\n \n \n \n+\tN\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP\" seventh-unsigned >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" seventh-unsigned >actual &&\n \ttest_cmp expect actual\n '\n \n+test_expect_success GPG 'show lack of raw signature with custom format' '\n+\tgit log -1 --format=format:\"%GR\" seventh-unsigned > actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success GPG 'show raw signature with custom format' '\n+\tgit log -1 --format=format:\"%GR\" sixth-signed >output &&\n+\tcat output &&\n+\thead -n1 output | grep -q \"^---*BEGIN PGP SIGNATURE---*$\" &&\n+\tsed 1d output | grep -q \"^$\" &&\n+\tsed \"1,/^$/d\" output | grep -q \"^[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$\" &&\n+\ttail -n2 output | head -n1 | grep -q \"^=[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$\" &&\n+\ttail -n1 output | grep -q \"^---*END PGP SIGNATURE---*$\"\n+'\n+\n test_expect_success GPG 'log.showsignature behaves like --show-signature' '\n \ttest_config log.showsignature true &&\n \tgit show initial >actual &&\n-- \n2.19.1\n\n"},{"id":"365307","messageId":"20181213212256.48122-4-john.a.passaro@gmail.com","threadId":"50032","inReplyTo":"20181213212256.48122-1-john.a.passaro@gmail.com","subject":"[PATCH 3/4] doc, tests: pretty behavior when gpg missing","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-13T21:22:55Z","receivedAt":"2018-12-13T21:23:22Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"Test that when GPG cannot be run, new placeholders %GR and %G+ are\nunaffected, %G? always returns 'N', and other GPG-related placeholders\nreturn blank.\n\nAs of e5a329a279 (\"run-command: report exec failure\" 2018-12-11), if GPG\ncannot be run and placeholders requiring GPG are given, git complains to\nstderr that GPG cannot be found. That commit included low-level tests\nfor this behavior. Now, test it also at the level of everyday user\ncommands.\n\nSigned-off-by: John Passaro <john.a.passaro@gmail.com>\n---\n Documentation/pretty-formats.txt |  6 +-\n t/t7510-signed-commit.sh         | 95 ++++++++++++++++++++++++++++++++\n 2 files changed, 99 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 582454a4f7..4a83796250 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -144,7 +144,9 @@ ifndef::git-rev-list[]\n endif::git-rev-list[]\n - '%GR': contents of the commits signature (blank when unsigned)\n - '%G+': \"Y\" if the commit is signed, \"N\" otherwise\n-- '%GG': raw verification message from GPG for a signed commit\n+- '%GG': raw verification message from GPG for a signed commit.\n+  This and all the other %G* placeholders, other than %GR, %G+, and\n+  %G?, return blank if GPG cannot be run.\n - '%G?': show \"G\" for a good (valid) signature,\n   \"B\" for a bad signature,\n   \"U\" for a good signature with unknown validity,\n@@ -152,7 +154,7 @@ endif::git-rev-list[]\n   \"Y\" for a good signature made by an expired key,\n   \"R\" for a good signature made by a revoked key,\n   \"E\" if the signature cannot be checked (e.g. missing key)\n-  and \"N\" for no signature\n+  and \"N\" for no signature (e.g. unsigned, or GPG cannot be run)\n - '%GS': show the name of the signer for a signed commit\n - '%GK': show the key used to sign a signed commit\n - '%GF': show the fingerprint of the key used to sign a signed commit\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex aff6b1eb3a..d65425eddc 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -170,6 +170,48 @@ test_expect_success GPG 'amending already signed commit' '\n \t! grep \"BAD signature from\" actual\n '\n \n+test_expect_success GPG 'show custom format fields for signed commit if gpg is missing' '\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\n+\n+\tY\n+\tEOF\n+\ttest_config gpg.program this-is-not-a-program &&\n+\tgit log -n1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" sixth-signed >actual 2>/dev/null &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show custom format fields for unsigned commit if gpg is missing' '\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\n+\n+\tN\n+\tEOF\n+\ttest_config gpg.program this-is-not-a-program &&\n+\tgit log -n1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" seventh-unsigned >actual 2>/dev/null &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show error for custom format fields on signed commit if gpg is missing' '\n+\ttest_config gpg.program this-is-not-a-program &&\n+\tgit log -n1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" sixth-signed >/dev/null 2>errors &&\n+\ttest $(wc -l <errors) = 1 &&\n+\ttest_i18ngrep \"^error: \" errors &&\n+\tgrep this-is-not-a-program errors\n+'\n+\n+test_expect_success GPG 'do not run gpg at all for unsigned commit' '\n+\ttest_config gpg.program this-is-not-a-program &&\n+\tgit log -n1 --format=\"%G?%n%GK%n%GS%n%GF%n%GP%n%G+\" seventh-unsigned >/dev/null 2>errors &&\n+\ttest_must_be_empty errors\n+'\n+\n test_expect_success GPG 'show good signature with custom format' '\n \tcat >expect <<-\\EOF &&\n \tG\n@@ -240,6 +282,14 @@ test_expect_success GPG 'show lack of raw signature with custom format' '\n \ttest_must_be_empty actual\n '\n \n+test_expect_success GPG 'show lack of raw signature with custom format without running GPG' '\n+\techo N > expected &&\n+\ttest_config gpg.program this-is-not-a-program &&\n+\tgit log -1 --format=\"%G+%GR\" seventh-unsigned >actual 2>errors &&\n+\ttest_cmp expected actual &&\n+\ttest_must_be_empty errors\n+'\n+\n test_expect_success GPG 'show raw signature with custom format' '\n \tgit log -1 --format=format:\"%GR\" sixth-signed >output &&\n \tcat output &&\n@@ -250,6 +300,51 @@ test_expect_success GPG 'show raw signature with custom format' '\n \ttail -n1 output | grep -q \"^---*END PGP SIGNATURE---*$\"\n '\n \n+test_expect_success GPG 'show raw signature with custom format without running GPG' '\n+\ttest_config gpg.program this-is-not-a-program &&\n+\tgit log -1 --format=format:\"%GR\" sixth-signed >rawsig 2>errors &&\n+\tcat rawsig &&\n+\thead -n1 rawsig | grep -q \"^---*BEGIN PGP SIGNATURE---*$\" &&\n+\tsed 1d rawsig | grep -q \"^$\" &&\n+\tsed \"1,/^$/d\" rawsig | grep -q \"^[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$\" &&\n+\ttail -n2 rawsig | head -n1 | grep -q \"^=[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$\" &&\n+\ttail -n1 rawsig | grep -q \"^---*END PGP SIGNATURE---*$\" &&\n+\ttest_must_be_empty errors\n+'\n+\n+test_expect_success GPG 'show presence of gpgsig with custom format when gpg is missing without errors' '\n+\techo Y > expected &&\n+\tgit log -1 --format=%G+ sixth-signed >output 2>errors &&\n+\ttest_cmp expected output &&\n+\ttest_must_be_empty errors\n+'\n+\n+test_expect_success GPG 'show presence of invalid gpgsig header' '\n+\tprintf gpgsig >gpgsig-header &&\n+\ttee prank-signature <<-\\EOF | sed \"s/^/ /\" >>gpgsig-header &&\n+\tthis is not a signature but an awful...\n+\t\t\t\t\t   888\n+\t\t\t\t\t   888\n+\t\t\t\t\t   888\n+\t88888b.  888d888  8888b.  88888b.  888  888\n+\t888 \"88b 888P\"       \"88b 888 \"88b 888 .88P\n+\t888  888 888     .d888888 888  888 888888K\n+\t888 d88P 888     888  888 888  888 888 \"88b\n+\t88888P\"  888     \"Y888888 888  888 888  888\n+\t888\n+\t888\n+\t888\n+\tEOF\n+\tgit cat-file commit seventh-unsigned >bare-commit-data &&\n+\tsed \"/^committer/r gpgsig-header\" bare-commit-data >commit-data &&\n+\tgit hash-object -w -t commit commit-data >commit &&\n+\techo Y >expected &&\n+\tcat prank-signature >>expected &&\n+\tgit log -n1 --format=format:%G+%n%GR $(cat commit) >actual 2>errors &&\n+\ttest_cmp expected actual &&\n+\ttest_must_be_empty errors\n+'\n+\n test_expect_success GPG 'log.showsignature behaves like --show-signature' '\n \ttest_config log.showsignature true &&\n \tgit show initial >actual &&\n-- \n2.19.1\n\n"},{"id":"365308","messageId":"20181213212256.48122-5-john.a.passaro@gmail.com","threadId":"50032","inReplyTo":"20181213212256.48122-1-john.a.passaro@gmail.com","subject":"[PATCH 4/4] docs/pretty-formats: add explanation + copy edits","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-13T21:22:56Z","receivedAt":"2018-12-13T21:23:27Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"Clarify description of %G? = \"U\" to say it can mean good signature but\nuntrusted key.\n\nMake wording consistent between %G* placeholders and other placeholders\nby removing the verb \"show\".\n\nSigned-off-by: John Passaro <john.a.passaro@gmail.com>\n---\n Documentation/pretty-formats.txt | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 4a83796250..32c2f75060 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -147,18 +147,19 @@ endif::git-rev-list[]\n - '%GG': raw verification message from GPG for a signed commit.\n   This and all the other %G* placeholders, other than %GR, %G+, and\n   %G?, return blank if GPG cannot be run.\n-- '%G?': show \"G\" for a good (valid) signature,\n+- '%G?': \"G\" for a good (valid) signature,\n   \"B\" for a bad signature,\n-  \"U\" for a good signature with unknown validity,\n+  \"U\" for a good signature with unknown validity (e.g. key is known but\n+  not trusted),\n   \"X\" for a good signature that has expired,\n   \"Y\" for a good signature made by an expired key,\n   \"R\" for a good signature made by a revoked key,\n   \"E\" if the signature cannot be checked (e.g. missing key)\n   and \"N\" for no signature (e.g. unsigned, or GPG cannot be run)\n-- '%GS': show the name of the signer for a signed commit\n-- '%GK': show the key used to sign a signed commit\n-- '%GF': show the fingerprint of the key used to sign a signed commit\n-- '%GP': show the fingerprint of the primary key whose subkey was used\n+- '%GS': name of the signer for a signed commit\n+- '%GK': key used to sign a signed commit\n+- '%GF': fingerprint of the key used to sign a signed commit\n+- '%GP': fingerprint of the primary key whose subkey was used\n   to sign a signed commit\n - '%gD': reflog selector, e.g., `refs/stash@{1}` or\n   `refs/stash@{2 minutes ago`}; the format follows the rules described\n-- \n2.19.1\n\n"},{"id":"365325","messageId":"1544760713.970.1.camel@gentoo.org","threadId":"50032","inReplyTo":"20181213212256.48122-1-john.a.passaro@gmail.com","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"Michał Górny","fromEmail":"mgorny@gentoo.org","sentAt":"2018-12-14T04:11:53Z","receivedAt":"2018-12-14T04:12:02Z","isPatch":true,"sender":{"key":"mgorny@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/110765?v=4"},"body":"On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote:\n> Currently, users who do not have GPG installed have no way to discern\n> signed from unsigned commits without examining raw commit data. I\n> propose two new pretty-print placeholders to expose this information:\n> \n> %GR: full (\"R\"aw) contents of gpgsig header\n> %G+: Y/N if the commit has nonempty gpgsig header or not\n> \n> The second is of course much more likely to be used, but having exposed\n> the one, exposing the other too adds almost no complexity.\n> \n> I'm open to suggestion on the names of these placeholders.\n> \n> This commit is based on master but e5a329a279 (\"run-command: report exec\n> failure\" 2018-12-11) is required for the tests to pass.\n> \n> One note is that this change touches areas of the pretty-format\n> documentation that are radically revamped in aw/pretty-trailers: see\n> 42617752d4 (\"doc: group pretty-format.txt placeholders descriptions\"\n> 2018-12-08). I have another version of this branch based on that branch\n> as well, so you can use that in case conflicts with aw/pretty-trailers\n> arise.\n> \n> See:\n> - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig\n> - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers\n> \n> John Passaro (4):\n>   pretty: expose raw commit signature\n>   t/t7510-signed-commit.sh: test new placeholders\n>   doc, tests: pretty behavior when gpg missing\n>   docs/pretty-formats: add explanation + copy edits\n> \n>  Documentation/pretty-formats.txt |  21 ++++--\n>  pretty.c                         |  36 ++++++++-\n>  t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--\n>  3 files changed, 167 insertions(+), 15 deletions(-)\n> \n> \n> base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7\n> prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db\n\nJust a suggestion: since the raw signature is not very useful without\nthe commit data to check it against, and the commit data is non-trivial\nto construct (requires mangling raw data anyway), maybe you could either\nadd another placeholder to get the data for signature verification, or\n(alternatively or simultaneously) add a placeholder that prints both\ndata and signature in the OpenPGP message format (i.e. something you can\npass straight to 'gpg --verify').\n\n-- \nBest regards,\nMichał Górny\n"},{"id":"365355","messageId":"CAJdN7KjExd6T+H4-wEupO2dg_mMWzeA22oYaskkfhz+GuFbfRQ@mail.gmail.com","threadId":"50032","inReplyTo":"1544760713.970.1.camel@gentoo.org","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-14T16:07:03Z","receivedAt":"2018-12-14T16:07:44Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote:\n>\n> On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote:\n> > Currently, users who do not have GPG installed have no way to discern\n> > signed from unsigned commits without examining raw commit data. I\n> > propose two new pretty-print placeholders to expose this information:\n> >\n> > %GR: full (\"R\"aw) contents of gpgsig header\n> > %G+: Y/N if the commit has nonempty gpgsig header or not\n> >\n> > The second is of course much more likely to be used, but having exposed\n> > the one, exposing the other too adds almost no complexity.\n> >\n> > I'm open to suggestion on the names of these placeholders.\n> >\n> > This commit is based on master but e5a329a279 (\"run-command: report exec\n> > failure\" 2018-12-11) is required for the tests to pass.\n> >\n> > One note is that this change touches areas of the pretty-format\n> > documentation that are radically revamped in aw/pretty-trailers: see\n> > 42617752d4 (\"doc: group pretty-format.txt placeholders descriptions\"\n> > 2018-12-08). I have another version of this branch based on that branch\n> > as well, so you can use that in case conflicts with aw/pretty-trailers\n> > arise.\n> >\n> > See:\n> > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig\n> > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers\n> >\n> > John Passaro (4):\n> >   pretty: expose raw commit signature\n> >   t/t7510-signed-commit.sh: test new placeholders\n> >   doc, tests: pretty behavior when gpg missing\n> >   docs/pretty-formats: add explanation + copy edits\n> >\n> >  Documentation/pretty-formats.txt |  21 ++++--\n> >  pretty.c                         |  36 ++++++++-\n> >  t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--\n> >  3 files changed, 167 insertions(+), 15 deletions(-)\n> >\n> >\n> > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7\n> > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db\n>\n> Just a suggestion: since the raw signature is not very useful without\n> the commit data to check it against, and the commit data is non-trivial\n> to construct (requires mangling raw data anyway), maybe you could either\n> add another placeholder to get the data for signature verification, or\n> (alternatively or simultaneously) add a placeholder that prints both\n> data and signature in the OpenPGP message format (i.e. something you can\n> pass straight to 'gpg --verify').\n>\n\nThat's a great idea!\n\nThen I might rename the other new placeholders too:\n\n%Gs: signed commit signature (blank when unsigned)\n%Gp: signed commit payload (i.e. in practice minus the gpgsig header;\nalso blank when unsigned as well)\n%Gq: query/question whether is signed commit (\"Y\"/\"N\")\n\nThus establishing %G<lowercase> as the gpg-related placeholders that\ndo not actually require gpg.\n\nAnd add a test that %Gp%n%Gs or the like passes gpg --verify.\n\nI'll put in a v2 later today or tomorrow. Thank you for the feedback!\n\n--\nJP\n"},{"id":"365357","messageId":"1544806139.7371.1.camel@gentoo.org","threadId":"50032","inReplyTo":"CAJdN7KjExd6T+H4-wEupO2dg_mMWzeA22oYaskkfhz+GuFbfRQ@mail.gmail.com","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"Michał Górny","fromEmail":"mgorny@gentoo.org","sentAt":"2018-12-14T16:48:59Z","receivedAt":"2018-12-14T16:49:06Z","isPatch":true,"sender":{"key":"mgorny@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/110765?v=4"},"body":"On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote:\n> On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote:\n> > \n> > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote:\n> > > Currently, users who do not have GPG installed have no way to discern\n> > > signed from unsigned commits without examining raw commit data. I\n> > > propose two new pretty-print placeholders to expose this information:\n> > > \n> > > %GR: full (\"R\"aw) contents of gpgsig header\n> > > %G+: Y/N if the commit has nonempty gpgsig header or not\n> > > \n> > > The second is of course much more likely to be used, but having exposed\n> > > the one, exposing the other too adds almost no complexity.\n> > > \n> > > I'm open to suggestion on the names of these placeholders.\n> > > \n> > > This commit is based on master but e5a329a279 (\"run-command: report exec\n> > > failure\" 2018-12-11) is required for the tests to pass.\n> > > \n> > > One note is that this change touches areas of the pretty-format\n> > > documentation that are radically revamped in aw/pretty-trailers: see\n> > > 42617752d4 (\"doc: group pretty-format.txt placeholders descriptions\"\n> > > 2018-12-08). I have another version of this branch based on that branch\n> > > as well, so you can use that in case conflicts with aw/pretty-trailers\n> > > arise.\n> > > \n> > > See:\n> > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig\n> > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers\n> > > \n> > > John Passaro (4):\n> > >   pretty: expose raw commit signature\n> > >   t/t7510-signed-commit.sh: test new placeholders\n> > >   doc, tests: pretty behavior when gpg missing\n> > >   docs/pretty-formats: add explanation + copy edits\n> > > \n> > >  Documentation/pretty-formats.txt |  21 ++++--\n> > >  pretty.c                         |  36 ++++++++-\n> > >  t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--\n> > >  3 files changed, 167 insertions(+), 15 deletions(-)\n> > > \n> > > \n> > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7\n> > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db\n> > \n> > Just a suggestion: since the raw signature is not very useful without\n> > the commit data to check it against, and the commit data is non-trivial\n> > to construct (requires mangling raw data anyway), maybe you could either\n> > add another placeholder to get the data for signature verification, or\n> > (alternatively or simultaneously) add a placeholder that prints both\n> > data and signature in the OpenPGP message format (i.e. something you can\n> > pass straight to 'gpg --verify').\n> > \n> \n> That's a great idea!\n> \n> Then I might rename the other new placeholders too:\n> \n> %Gs: signed commit signature (blank when unsigned)\n> %Gp: signed commit payload (i.e. in practice minus the gpgsig header;\n> also blank when unsigned as well)\n> %Gq: query/question whether is signed commit (\"Y\"/\"N\")\n> \n> Thus establishing %G<lowercase> as the gpg-related placeholders that\n> do not actually require gpg.\n> \n> And add a test that %Gp%n%Gs or the like passes gpg --verify.\n> \n> I'll put in a v2 later today or tomorrow. Thank you for the feedback!\n> \n\nTechnically speaking, '%Gp%n%Gs' sounds a bit odd, given that\nthe payload needs to be preceded by the PGP message header but instead\nof footer it has the signature's header.  Also note that some lines in\nthe payload may need to be escaped.\n\n-- \nBest regards,\nMichał Górny\n"},{"id":"365388","messageId":"CAJdN7KgWQyjrfifqNEr5SeHM0F1KzrKyoK0gy0AeTd-jPvMtCw@mail.gmail.com","threadId":"50032","inReplyTo":"1544806139.7371.1.camel@gentoo.org","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-14T23:10:41Z","receivedAt":"2018-12-14T23:11:23Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"On Fri, Dec 14, 2018 at 11:49 AM Michał Górny <mgorny@gentoo.org> wrote:\n>\n> On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote:\n> > On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote:\n> > >\n> > > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote:\n> > > > Currently, users who do not have GPG installed have no way to discern\n> > > > signed from unsigned commits without examining raw commit data. I\n> > > > propose two new pretty-print placeholders to expose this information:\n> > > >\n> > > > %GR: full (\"R\"aw) contents of gpgsig header\n> > > > %G+: Y/N if the commit has nonempty gpgsig header or not\n> > > >\n> > > > The second is of course much more likely to be used, but having exposed\n> > > > the one, exposing the other too adds almost no complexity.\n> > > >\n> > > > I'm open to suggestion on the names of these placeholders.\n> > > >\n> > > > This commit is based on master but e5a329a279 (\"run-command: report exec\n> > > > failure\" 2018-12-11) is required for the tests to pass.\n> > > >\n> > > > One note is that this change touches areas of the pretty-format\n> > > > documentation that are radically revamped in aw/pretty-trailers: see\n> > > > 42617752d4 (\"doc: group pretty-format.txt placeholders descriptions\"\n> > > > 2018-12-08). I have another version of this branch based on that branch\n> > > > as well, so you can use that in case conflicts with aw/pretty-trailers\n> > > > arise.\n> > > >\n> > > > See:\n> > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig\n> > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers\n> > > >\n> > > > John Passaro (4):\n> > > >   pretty: expose raw commit signature\n> > > >   t/t7510-signed-commit.sh: test new placeholders\n> > > >   doc, tests: pretty behavior when gpg missing\n> > > >   docs/pretty-formats: add explanation + copy edits\n> > > >\n> > > >  Documentation/pretty-formats.txt |  21 ++++--\n> > > >  pretty.c                         |  36 ++++++++-\n> > > >  t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--\n> > > >  3 files changed, 167 insertions(+), 15 deletions(-)\n> > > >\n> > > >\n> > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7\n> > > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db\n> > >\n> > > Just a suggestion: since the raw signature is not very useful without\n> > > the commit data to check it against, and the commit data is non-trivial\n> > > to construct (requires mangling raw data anyway), maybe you could either\n> > > add another placeholder to get the data for signature verification, or\n> > > (alternatively or simultaneously) add a placeholder that prints both\n> > > data and signature in the OpenPGP message format (i.e. something you can\n> > > pass straight to 'gpg --verify').\n> > >\n> >\n> > That's a great idea!\n> >\n> > Then I might rename the other new placeholders too:\n> >\n> > %Gs: signed commit signature (blank when unsigned)\n> > %Gp: signed commit payload (i.e. in practice minus the gpgsig header;\n> > also blank when unsigned as well)\n> > %Gq: query/question whether is signed commit (\"Y\"/\"N\")\n> >\n> > Thus establishing %G<lowercase> as the gpg-related placeholders that\n> > do not actually require gpg.\n> >\n> > And add a test that %Gp%n%Gs or the like passes gpg --verify.\n> >\n> > I'll put in a v2 later today or tomorrow. Thank you for the feedback!\n> >\n>\n> Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that\n> the payload needs to be preceded by the PGP message header but instead\n> of footer it has the signature's header.  Also note that some lines in\n> the payload may need to be escaped.\n\nIt's indeed failing with\n\"-----BEGIN PGP SIGNED MESSAGE-----%n%Gp%n%Gs\"\n\nThis appears to be a misunderstanding on my part of how cleartext GPG\nmessages work, as this message seems to fail verification even when I\nconstruct it manually:\n\n-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\ntree 3820419b50e9827493beedf6a1423e7d9c7edf3b\nparent 356f37356edf1a45c8494870e1c051e2fbe529fa\nauthor A U Thor <author@example.com> 1112912413 -0700\ncommitter C O Mitter <committer@example.com> 1112912533 -0700\n\nsixth\n-----BEGIN PGP SIGNATURE-----\n\niHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCXBQzKRYcY29tbWl0dGVy\nQGV4YW1wbGUuY29tAAoJEBO29R7N3kMN2QYAnA2A/VpD4qMI9rJlIYnyyO32rTXz\nAJ0drWlJsASMcf6AEX6nSQPxWq81Fg==\n=fr2F\n-----END PGP SIGNATURE-----\n\nI got this by running tho following inside the trash directory for t7510,\nwhich as far as I can tell is roughly equivalent to\nThe gpg --verify fails.\n{\n  echo \"-----BEGIN PGP SIGNED MESSAGE-----\"\n  echo Hash: SHA256\n  echo\n  git cat-file commit sixth-signed | perl -ne '/^(?:gpgsig)? / || print'\n  git cat-file commit sixth-signed | perl -ne 's/^(?:gpgsig)? // && print'\n} | tee commit-as-signed-message | gpg --verify\n\nAll seems to work fine when I treat %Gs as a detached signature.\nHow should the combined message be constructed properly? (Goes\nto usefulness of printing signature payload, and indeed of raw crypto\ndata in general.)\n\n\n> --\n> Best regards,\n> Michał Górny\n"},{"id":"365389","messageId":"CAJdN7KggRedPRfGRExTy8q6rGAAgbTCBc+_tdzSwF5kEYom9fg@mail.gmail.com","threadId":"50032","inReplyTo":"CAJdN7KgWQyjrfifqNEr5SeHM0F1KzrKyoK0gy0AeTd-jPvMtCw@mail.gmail.com","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-14T23:13:48Z","receivedAt":"2018-12-14T23:14:30Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"On Fri, Dec 14, 2018 at 6:10 PM John Passaro <john.a.passaro@gmail.com> wrote:\n>\n> On Fri, Dec 14, 2018 at 11:49 AM Michał Górny <mgorny@gentoo.org> wrote:\n> >\n> > On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote:\n> > > On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote:\n> > > >\n> > > > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote:\n> > > > > Currently, users who do not have GPG installed have no way to discern\n> > > > > signed from unsigned commits without examining raw commit data. I\n> > > > > propose two new pretty-print placeholders to expose this information:\n> > > > >\n> > > > > %GR: full (\"R\"aw) contents of gpgsig header\n> > > > > %G+: Y/N if the commit has nonempty gpgsig header or not\n> > > > >\n> > > > > The second is of course much more likely to be used, but having exposed\n> > > > > the one, exposing the other too adds almost no complexity.\n> > > > >\n> > > > > I'm open to suggestion on the names of these placeholders.\n> > > > >\n> > > > > This commit is based on master but e5a329a279 (\"run-command: report exec\n> > > > > failure\" 2018-12-11) is required for the tests to pass.\n> > > > >\n> > > > > One note is that this change touches areas of the pretty-format\n> > > > > documentation that are radically revamped in aw/pretty-trailers: see\n> > > > > 42617752d4 (\"doc: group pretty-format.txt placeholders descriptions\"\n> > > > > 2018-12-08). I have another version of this branch based on that branch\n> > > > > as well, so you can use that in case conflicts with aw/pretty-trailers\n> > > > > arise.\n> > > > >\n> > > > > See:\n> > > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig\n> > > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers\n> > > > >\n> > > > > John Passaro (4):\n> > > > >   pretty: expose raw commit signature\n> > > > >   t/t7510-signed-commit.sh: test new placeholders\n> > > > >   doc, tests: pretty behavior when gpg missing\n> > > > >   docs/pretty-formats: add explanation + copy edits\n> > > > >\n> > > > >  Documentation/pretty-formats.txt |  21 ++++--\n> > > > >  pretty.c                         |  36 ++++++++-\n> > > > >  t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--\n> > > > >  3 files changed, 167 insertions(+), 15 deletions(-)\n> > > > >\n> > > > >\n> > > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7\n> > > > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db\n> > > >\n> > > > Just a suggestion: since the raw signature is not very useful without\n> > > > the commit data to check it against, and the commit data is non-trivial\n> > > > to construct (requires mangling raw data anyway), maybe you could either\n> > > > add another placeholder to get the data for signature verification, or\n> > > > (alternatively or simultaneously) add a placeholder that prints both\n> > > > data and signature in the OpenPGP message format (i.e. something you can\n> > > > pass straight to 'gpg --verify').\n> > > >\n> > >\n> > > That's a great idea!\n> > >\n> > > Then I might rename the other new placeholders too:\n> > >\n> > > %Gs: signed commit signature (blank when unsigned)\n> > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header;\n> > > also blank when unsigned as well)\n> > > %Gq: query/question whether is signed commit (\"Y\"/\"N\")\n> > >\n> > > Thus establishing %G<lowercase> as the gpg-related placeholders that\n> > > do not actually require gpg.\n> > >\n> > > And add a test that %Gp%n%Gs or the like passes gpg --verify.\n> > >\n> > > I'll put in a v2 later today or tomorrow. Thank you for the feedback!\n> > >\n> >\n> > Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that\n> > the payload needs to be preceded by the PGP message header but instead\n> > of footer it has the signature's header.  Also note that some lines in\n> > the payload may need to be escaped.\n>\n> It's indeed failing with\n> \"-----BEGIN PGP SIGNED MESSAGE-----%n%Gp%n%Gs\"\n>\n> This appears to be a misunderstanding on my part of how cleartext GPG\n> messages work, as this message seems to fail verification even when I\n> construct it manually:\n>\n> -----BEGIN PGP SIGNED MESSAGE-----\n> Hash: SHA256\n>\n> tree 3820419b50e9827493beedf6a1423e7d9c7edf3b\n> parent 356f37356edf1a45c8494870e1c051e2fbe529fa\n> author A U Thor <author@example.com> 1112912413 -0700\n> committer C O Mitter <committer@example.com> 1112912533 -0700\n>\n> sixth\n> -----BEGIN PGP SIGNATURE-----\n>\n> iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCXBQzKRYcY29tbWl0dGVy\n> QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN2QYAnA2A/VpD4qMI9rJlIYnyyO32rTXz\n> AJ0drWlJsASMcf6AEX6nSQPxWq81Fg==\n> =fr2F\n> -----END PGP SIGNATURE-----\n>\n> I got this by running tho following inside the trash directory for t7510,\n> which as far as I can tell is roughly equivalent to\n> The gpg --verify fails.\n> {\n>   echo \"-----BEGIN PGP SIGNED MESSAGE-----\"\n>   echo Hash: SHA256\n>   echo\n>   git cat-file commit sixth-signed | perl -ne '/^(?:gpgsig)? / || print'\n>   git cat-file commit sixth-signed | perl -ne 's/^(?:gpgsig)? // && print'\n> } | tee commit-as-signed-message | gpg --verify\n\nI should add that when I took out Hash: SHA256 (don't ask), gpg went\nfrom saying \"signature digest conflict\" to saying \"BAD signature\". Not\nquite as bad but still confusing.\n>\n> All seems to work fine when I treat %Gs as a detached signature.\n> How should the combined message be constructed properly? (Goes\n> to usefulness of printing signature payload, and indeed of raw crypto\n> data in general.)\n>\n>\n> > --\n> > Best regards,\n> > Michał Górny\n"},{"id":"365420","messageId":"xmqqa7l7d48r.fsf@gitster-ct.c.googlers.com","threadId":"50032","inReplyTo":"1544760713.970.1.camel@gentoo.org","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-15T00:26:44Z","receivedAt":"2018-12-15T00:26:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michał Górny <mgorny@gentoo.org> writes:\n\n> Just a suggestion: since the raw signature is not very useful without\n> the commit data to check it against, and the commit data is non-trivial\n> to construct (requires mangling raw data anyway), maybe you could either\n> add another placeholder to get the data for signature verification, or\n> (alternatively or simultaneously) add a placeholder that prints both\n> data and signature in the OpenPGP message format (i.e. something you can\n> pass straight to 'gpg --verify').\n\nYeah, the last would be the most usable; anything short of that, I\nhave to suspect that going from \"cat-file commit\", rather than using\nthis new %Gsomething placeholder, would be more practical.\n"},{"id":"365494","messageId":"20181217202406.GA12122@sigill.intra.peff.net","threadId":"50032","inReplyTo":"CAJdN7KjExd6T+H4-wEupO2dg_mMWzeA22oYaskkfhz+GuFbfRQ@mail.gmail.com","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-17T20:24:07Z","receivedAt":"2018-12-17T20:24:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote:\n\n> Then I might rename the other new placeholders too:\n> \n> %Gs: signed commit signature (blank when unsigned)\n> %Gp: signed commit payload (i.e. in practice minus the gpgsig header;\n> also blank when unsigned as well)\n\nOne complication: the pretty-printing code sees the commit data in the\ni18n.logOutputEncoding charset (utf8 by default). But the signature will\nbe over the raw commit data. That's also utf8 by default, but there may\nbe an encoding header indicating that it's something else. In that case,\nyou couldn't actually verify the signature from the \"%Gs%Gp\" pair.\n\nI don't think that's insurmountable in the code. You'll have to jump\nthrough a few hoops to make sure you have the _original_ payload, but we\nobviously do have that data. However, it does feel a little weird to\ninclude content from a different encoding in the middle of the log\noutput stream which claims to be i18n.logOutputEncoding.\n\n-Peff\n"},{"id":"365563","messageId":"CAJdN7KixEG+VKJAZz281RFEiVPRpRz_fBy6J2dBJiJMYT1mpBg@mail.gmail.com","threadId":"50032","inReplyTo":"20181217202406.GA12122@sigill.intra.peff.net","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"John Passaro","fromEmail":"john.a.passaro@gmail.com","sentAt":"2018-12-19T05:59:52Z","receivedAt":"2018-12-19T06:00:32Z","isPatch":true,"sender":{"key":"john.a.passaro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6754005?v=4"},"body":"On Fri, Dec 14, 2018 at 6:10 PM John Passaro wrote:\n> All seems to work fine when I treat %Gs as a detached signature.\n\nIn light of this, my best guess as to why the cleartext PGP message\ndidn't verify properly is that the commit data normally doesn't end\nwith \\n, but as far as I can tell there's no way to express that in\nthe cleartext format. I don't see a way around this. However, as long\nas the following works, I think we have proof-of-concept that this\nenhancement allows you to play with signature data however you please\nwithout leaving it to git under the hood:\n\ngpg --verify <(git show -s --format=format:%Gs commit) <(git show -s\n--format=format:%Gp commit)\n\nOn Mon, Dec 17, 2018 at 3:24 PM Jeff King <peff@peff.net> wrote:\n>\n> On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote:\n>\n> > Then I might rename the other new placeholders too:\n> >\n> > %Gs: signed commit signature (blank when unsigned)\n> > %Gp: signed commit payload (i.e. in practice minus the gpgsig header;\n> > also blank when unsigned as well)\n>\n> One complication: the pretty-printing code sees the commit data in the\n> i18n.logOutputEncoding charset (utf8 by default). But the signature will\n> be over the raw commit data. That's also utf8 by default, but there may\n> be an encoding header indicating that it's something else. In that case,\n> you couldn't actually verify the signature from the \"%Gs%Gp\" pair.\n>\n> I don't think that's insurmountable in the code. You'll have to jump\n> through a few hoops to make sure you have the _original_ payload, but we\n> obviously do have that data. However, it does feel a little weird to\n> include content from a different encoding in the middle of the log\n> output stream which claims to be i18n.logOutputEncoding.\n>\n\nThanks for the feedback! This is an interesting conflict. If the user\nrequests %Gp, the payload for the signature, they almost certainly do\nwant it in the original encoding; if i18n.logOutputEncoding is\nsomething incompatible, whether explicitly or by default, that seems\nlike an error. Not much we can do to reconcile the two requests\n(commit encoding vs output encoding) so seems reasonable to treat it\nas fatal.\n\nUpdated patch coming as soon as I work out Peff's aforementioned \"few\nhoops\" to get properly encoded data -- and also how to test success\nand failure!\n"},{"id":"365735","messageId":"1545400330.811.1.camel@gentoo.org","threadId":"50032","inReplyTo":"CAJdN7KixEG+VKJAZz281RFEiVPRpRz_fBy6J2dBJiJMYT1mpBg@mail.gmail.com","subject":"Re: [PATCH 0/4] Expose gpgsig in pretty-print","fromName":"Michał Górny","fromEmail":"mgorny@gentoo.org","sentAt":"2018-12-21T13:52:10Z","receivedAt":"2018-12-21T13:52:19Z","isPatch":true,"sender":{"key":"mgorny@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/110765?v=4"},"body":"On Wed, 2018-12-19 at 00:59 -0500, John Passaro wrote:\n> On Fri, Dec 14, 2018 at 6:10 PM John Passaro wrote:\n> > All seems to work fine when I treat %Gs as a detached signature.\n> \n> In light of this, my best guess as to why the cleartext PGP message\n> didn't verify properly is that the commit data normally doesn't end\n> with \\n, but as far as I can tell there's no way to express that in\n> the cleartext format. I don't see a way around this.\n\nYou are most likely right.  I've just skimmed through RFC 4880\nand indeed it seems to rely on the newline encoding being quite\nnormalized in the message.\n\n> However, as long\n> as the following works, I think we have proof-of-concept that this\n> enhancement allows you to play with signature data however you please\n> without leaving it to git under the hood:\n> \n> gpg --verify <(git show -s --format=format:%Gs commit) <(git show -s\n> --format=format:%Gp commit)\n\nThat's a nice trick.  Thanks for the effort you're putting into this!\n\n> \n> On Mon, Dec 17, 2018 at 3:24 PM Jeff King <peff@peff.net> wrote:\n> > \n> > On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote:\n> > \n> > > Then I might rename the other new placeholders too:\n> > > \n> > > %Gs: signed commit signature (blank when unsigned)\n> > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header;\n> > > also blank when unsigned as well)\n> > \n> > One complication: the pretty-printing code sees the commit data in the\n> > i18n.logOutputEncoding charset (utf8 by default). But the signature will\n> > be over the raw commit data. That's also utf8 by default, but there may\n> > be an encoding header indicating that it's something else. In that case,\n> > you couldn't actually verify the signature from the \"%Gs%Gp\" pair.\n> > \n> > I don't think that's insurmountable in the code. You'll have to jump\n> > through a few hoops to make sure you have the _original_ payload, but we\n> > obviously do have that data. However, it does feel a little weird to\n> > include content from a different encoding in the middle of the log\n> > output stream which claims to be i18n.logOutputEncoding.\n> > \n> \n> Thanks for the feedback! This is an interesting conflict. If the user\n> requests %Gp, the payload for the signature, they almost certainly do\n> want it in the original encoding; if i18n.logOutputEncoding is\n> something incompatible, whether explicitly or by default, that seems\n> like an error. Not much we can do to reconcile the two requests\n> (commit encoding vs output encoding) so seems reasonable to treat it\n> as fatal.\n> \n> Updated patch coming as soon as I work out Peff's aforementioned \"few\n> hoops\" to get properly encoded data -- and also how to test success\n> and failure!\n\n-- \nBest regards,\nMichał Górny\n"}]}