Re: [PATCH] gpg-interface: use more status letters

2016-09-27 Thread Junio C Hamano
Michael J Gruber  writes:

> According to gpg2's doc/DETAILS:
> "For each signature only one of the codes GOODSIG, BADSIG, EXPSIG,
> EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted."
>
> gpg1 ("classic") behaves the same (although doc/DETAILS
> differs).
>
> Currently, we parse gpg's status output for GOODSIG, BADSIG and trust
> information and translate that into status codes G, B, U, N for the %G?
> format specifier.
>
> git-verify-* returns success in the GOODSIG case only. This is somewhat in
> disagreement with gpg, which considers the first 5 of the 6 above as VALIDSIG,
> but we err on the very safe side.
>
> Introduce additional status codes E, X, R for ERRSIG, EXP*SIG, REVKEYSIG
> so that a user of %G? gets more information about the absence of a 'G'
> on first glance.
>
> Reported-by: Alex 
> Signed-off-by: Michael J Gruber 

That probably was requested-by, but that's OK.

> I'd be happy to learn are more portable/safer/cooler way to make gpg forget
> that key in the added test...

We seem to set GNUPGHOME to $HOME/gnupg-home-not-used in test-lib.sh
to say "No gnupg keys for you!" for all the tests by default, which
is overriden by the signature tests like 7510.  I do not know if
that is more portable/safer/cooler than setting it to /dev/null but
imitating it might be a way for you to push the potential problem
away to other people ;-)  If it becomes an issue to set it to a
directory that does not exist with an updated future version of GPG,
this new test will share the same problem with everybody else, and
hopefully the solution would be the same ;-)

Having said that, if GNUPGHOME=/dev/null works for you, that's good
enough for now, so that people on other platforms can test it and
report.

Thanks.

> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index 6e839f5..fd22742 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -190,7 +190,7 @@ test_expect_success GPG 'show bad signature with custom 
> format' '
>   test_cmp expect actual
>  '
>  
> -test_expect_success GPG 'show unknown signature with custom format' '
> +test_expect_success GPG 'show untrusted signature with custom format' '
>   cat >expect <<-\EOF &&
>   U
>   61092E85B7227189
> @@ -200,6 +200,15 @@ test_expect_success GPG 'show unknown signature with 
> custom format' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success GPG 'show unknown signature with custom format' '
> + cat >expect <<-\EOF &&
> + E
> + 61092E85B7227189
> + EOF
> + GNUPGHOME=/dev/null git log -1 --format="%G?%n%GK" eighth-signed-alt 
> >actual &&
> + test_cmp expect actual
> +'
> +
>  test_expect_success GPG 'show lack of signature with custom format' '
>   cat >expect <<-\EOF &&
>   N


[PATCH] gpg-interface: use more status letters

2016-09-27 Thread Michael J Gruber
According to gpg2's doc/DETAILS:
"For each signature only one of the codes GOODSIG, BADSIG, EXPSIG,
EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted."

gpg1 ("classic") behaves the same (although doc/DETAILS
differs).

Currently, we parse gpg's status output for GOODSIG, BADSIG and trust
information and translate that into status codes G, B, U, N for the %G?
format specifier.

git-verify-* returns success in the GOODSIG case only. This is somewhat in
disagreement with gpg, which considers the first 5 of the 6 above as VALIDSIG,
but we err on the very safe side.

Introduce additional status codes E, X, R for ERRSIG, EXP*SIG, REVKEYSIG
so that a user of %G? gets more information about the absence of a 'G'
on first glance.

Reported-by: Alex 
Signed-off-by: Michael J Gruber 
---
I'd be happy to learn are more portable/safer/cooler way to make gpg forget
that key in the added test...

 Documentation/pretty-formats.txt |  9 +++--
 gpg-interface.c  |  4 
 pretty.c |  3 +++
 t/t7510-signed-commit.sh | 11 ++-
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a942d57..806b47f 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -143,8 +143,13 @@ ifndef::git-rev-list[]
 - '%N': commit notes
 endif::git-rev-list[]
 - '%GG': raw verification message from GPG for a signed commit
-- '%G?': show "G" for a good (valid) signature, "B" for a bad signature,
-  "U" for a good signature with unknown validity and "N" for no signature
+- '%G?': show "G" for a good (rather: valid) signature,
+  "B" for a bad signature,
+  "U" for a good signature with unknown validity,
+  "X" for a good expired signature, or good signature made by an expired key,
+  "R" for a good signature made by a revoked key,
+  "E" if the signature cannot be checked (e.g. missing key)
+  and "N" for no signature
 - '%GS': show the name of the signer for a signed commit
 - '%GK': show the key used to sign a signed commit
 - '%gD': reflog selector, e.g., `refs/stash@{1}` or
diff --git a/gpg-interface.c b/gpg-interface.c
index 8672eda..8a3e245 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -33,6 +33,10 @@ static struct {
{ 'B', "\n[GNUPG:] BADSIG " },
{ 'U', "\n[GNUPG:] TRUST_NEVER" },
{ 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
+   { 'E', "\n[GNUPG:] ERRSIG "},
+   { 'X', "\n[GNUPG:] EXPSIG "},
+   { 'X', "\n[GNUPG:] EXPKEYSIG "},
+   { 'R', "\n[GNUPG:] REVKEYSIG "},
 };
 
 void parse_gpg_output(struct signature_check *sigc)
diff --git a/pretty.c b/pretty.c
index 493edb0..39a36cd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1232,8 +1232,11 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
switch (c->signature_check.result) {
case 'G':
case 'B':
+   case 'E':
case 'U':
case 'N':
+   case 'X':
+   case 'R':
strbuf_addch(sb, c->signature_check.result);
}
break;
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 6e839f5..fd22742 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -190,7 +190,7 @@ test_expect_success GPG 'show bad signature with custom 
format' '
test_cmp expect actual
 '
 
-test_expect_success GPG 'show unknown signature with custom format' '
+test_expect_success GPG 'show untrusted signature with custom format' '
cat >expect <<-\EOF &&
U
61092E85B7227189
@@ -200,6 +200,15 @@ test_expect_success GPG 'show unknown signature with 
custom format' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'show unknown signature with custom format' '
+   cat >expect <<-\EOF &&
+   E
+   61092E85B7227189
+   EOF
+   GNUPGHOME=/dev/null git log -1 --format="%G?%n%GK" eighth-signed-alt 
>actual &&
+   test_cmp expect actual
+'
+
 test_expect_success GPG 'show lack of signature with custom format' '
cat >expect <<-\EOF &&
N
-- 
2.10.0.527.gbcb6904