* [PATCH 1/5] t7510: stop referring to master in later tests
2014-06-16 23:59 ` [PATCH 0/5] --format=%G tests and fixes Jeff King
@ 2014-06-16 23:59 ` Jeff King
2014-06-17 0:03 ` [PATCH 2/5] t7510: use consistent &&-chains in loop Jeff King
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Jeff King @ 2014-06-16 23:59 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Junio C Hamano, Michael J Gruber
Our setup creates a sequence of commits, each with its own
tag. However, we sometimes refer to "seventh-signed" as
"master". This works, since it is at the tip of the created
branch, but is brittle if new tests need to add more
commits. Let's use its tag name to be unambiguous.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7510-signed-commit.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 5ddac1a..37c3778 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -47,7 +47,7 @@ test_expect_success GPG 'create signed commits' '
test_expect_success GPG 'show signatures' '
(
- for commit in initial second merge fourth-signed fifth-signed sixth-signed master
+ for commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed
do
git show --pretty=short --show-signature $commit >actual &&
grep "Good signature from" actual || exit 1
@@ -67,7 +67,7 @@ test_expect_success GPG 'show signatures' '
'
test_expect_success GPG 'detect fudged signature' '
- git cat-file commit master >raw &&
+ git cat-file commit seventh-signed >raw &&
sed -e "s/seventh/7th forged/" raw >forged1 &&
git hash-object -w -t commit forged1 >forged1.commit &&
@@ -77,7 +77,7 @@ test_expect_success GPG 'detect fudged signature' '
'
test_expect_success GPG 'detect fudged signature with NUL' '
- git cat-file commit master >raw &&
+ git cat-file commit seventh-signed >raw &&
cat raw >forged2 &&
echo Qwik | tr "Q" "\000" >>forged2 &&
git hash-object -w -t commit forged2 >forged2.commit &&
--
2.0.0.566.gfe3e6b2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] t7510: use consistent &&-chains in loop
2014-06-16 23:59 ` [PATCH 0/5] --format=%G tests and fixes Jeff King
2014-06-16 23:59 ` [PATCH 1/5] t7510: stop referring to master in later tests Jeff King
@ 2014-06-17 0:03 ` Jeff King
2014-06-17 0:05 ` [PATCH 3/5] t7510: test a commit signed by an unknown key Jeff King
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Jeff King @ 2014-06-17 0:03 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Junio C Hamano, Michael J Gruber
From: Michael J Gruber <git@drmicha.warpmail.net>
We check multiple commits in a loop. Because we want to
break out of the loop if any single iteration fails, we use
a subshell/exit like:
(
for i in $stuff
do
do-something $i || exit 1
done
)
However, we are inconsistent in our loop body. Some commands
get their own "|| exit 1", and others try to chain to the
next command with "&&", like:
X &&
Y || exit 1
Z || exit 1
This is a little hard to read and follow, because X and Y
are treated differently for no good reason. But much worse,
the second loop follows a similar pattern and gets it wrong.
"Y" is expected to fail, so we use "&& exit 1", giving us:
X &&
Y && exit 1
Z || exit 1
That gets the test for X wrong (we do not exit unless both X
fails and Y unexpectedly succeeds, but we would want to exit
if _either_ is wrong). We can write this clearly and
correctly by consistently using "&&", followed by a single
"|| exit 1", and negating Y with "!" (as we would in a
normal &&-chain). Like:
X &&
! Y &&
Z || exit 1
Signed-off-by: Jeff King <peff@peff.net>
---
I listed Michael as the author here, because this is the patch that I
expected to come in his next re-roll, based on our discussion. I'm
including it here because I'm about to touch the same area (and Michael,
you'd probably want to rebase on top of this series anyway. Assuming I
haven't screwed it up :) ).
t/t7510-signed-commit.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 37c3778..cdffcbd 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -50,18 +50,18 @@ test_expect_success GPG 'show signatures' '
for commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed
do
git show --pretty=short --show-signature $commit >actual &&
- grep "Good signature from" actual || exit 1
- ! grep "BAD signature from" actual || exit 1
- echo $commit OK
+ grep "Good signature from" actual &&
+ ! grep "BAD signature from" actual &&
+ echo $commit OK || exit 1
done
) &&
(
for commit in merge^2 fourth-unsigned sixth-unsigned seventh-unsigned
do
git show --pretty=short --show-signature $commit >actual &&
- grep "Good signature from" actual && exit 1
- ! grep "BAD signature from" actual || exit 1
- echo $commit OK
+ ! grep "Good signature from" actual &&
+ ! grep "BAD signature from" actual &&
+ echo $commit OK || exit 1
done
)
'
--
2.0.0.566.gfe3e6b2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] t7510: test a commit signed by an unknown key
2014-06-16 23:59 ` [PATCH 0/5] --format=%G tests and fixes Jeff King
2014-06-16 23:59 ` [PATCH 1/5] t7510: stop referring to master in later tests Jeff King
2014-06-17 0:03 ` [PATCH 2/5] t7510: use consistent &&-chains in loop Jeff King
@ 2014-06-17 0:05 ` Jeff King
2014-06-17 0:06 ` [PATCH 4/5] t7510: check %G* pretty-format output Jeff King
2014-06-17 0:07 ` [PATCH 5/5] pretty: avoid reading past end-of-string with "%G" Jeff King
4 siblings, 0 replies; 10+ messages in thread
From: Jeff King @ 2014-06-17 0:05 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Junio C Hamano, Michael J Gruber
We tested both good and bad signatures, but not ones made
correctly but with a key for which we have no trust.
Signed-off-by: Jeff King <peff@peff.net>
---
I'm not happy about grepping more gpg output, but perhaps this "not
certified" is no worse than the current 'Good signature from" greps we
have?
The internal code uses --status-fd, which is presumably more robust to
changes. I dunno. I'd be inclined to go with this, and if it becomes a
problem in a future gpg release, we can deal with it then.
t/t7510-signed-commit.sh | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index cdffcbd..04fc2c5 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -43,6 +43,9 @@ test_expect_success GPG 'create signed commits' '
test_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&
git tag seventh-signed
+
+ echo 8 >file && test_tick && git commit -a -m eighth -SB7227189 &&
+ git tag eighth-signed-alt
'
test_expect_success GPG 'show signatures' '
@@ -63,6 +66,16 @@ test_expect_success GPG 'show signatures' '
! grep "BAD signature from" actual &&
echo $commit OK || exit 1
done
+ ) &&
+ (
+ for commit in eighth-signed-alt
+ do
+ git show --pretty=short --show-signature $commit >actual &&
+ grep "Good signature from" actual &&
+ ! grep "BAD signature from" actual &&
+ grep "not certified" actual &&
+ echo $commit OK || exit 1
+ done
)
'
--
2.0.0.566.gfe3e6b2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] t7510: check %G* pretty-format output
2014-06-16 23:59 ` [PATCH 0/5] --format=%G tests and fixes Jeff King
` (2 preceding siblings ...)
2014-06-17 0:05 ` [PATCH 3/5] t7510: test a commit signed by an unknown key Jeff King
@ 2014-06-17 0:06 ` Jeff King
2014-06-17 0:07 ` [PATCH 5/5] pretty: avoid reading past end-of-string with "%G" Jeff King
4 siblings, 0 replies; 10+ messages in thread
From: Jeff King @ 2014-06-17 0:06 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Junio C Hamano, Michael J Gruber
We do not check these along with the other pretty-format
placeholders in t6006, because we need signed commits to
make them interesting. t7510 has such commits, and can
easily exercise them in addition to the regular
--show-signature code path.
Signed-off-by: Jeff King <peff@peff.net>
---
Similar to before, but with the 'U' test added in.
t/t7510-signed-commit.sh | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 04fc2c5..e97477a 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -107,4 +107,44 @@ test_expect_success GPG 'amending already signed commit' '
! grep "BAD signature from" actual
'
+test_expect_success GPG 'show good signature with custom format' '
+ cat >expect <<-\EOF &&
+ G
+ 13B6F51ECDDE430D
+ C O Mitter <committer@example.com>
+ EOF
+ git log -1 --format="%G?%n%GK%n%GS" sixth-signed >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success GPG 'show bad signature with custom format' '
+ cat >expect <<-\EOF &&
+ B
+ 13B6F51ECDDE430D
+ C O Mitter <committer@example.com>
+ EOF
+ git log -1 --format="%G?%n%GK%n%GS" $(cat forged1.commit) >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+ cat >expect <<-\EOF &&
+ U
+ 61092E85B7227189
+ Eris Discordia <discord@example.net>
+ EOF
+ git log -1 --format="%G?%n%GK%n%GS" eighth-signed-alt >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+ cat >expect <<-\EOF &&
+ N
+
+
+ EOF
+ git log -1 --format="%G?%n%GK%n%GS" seventh-unsigned >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.0.0.566.gfe3e6b2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] pretty: avoid reading past end-of-string with "%G"
2014-06-16 23:59 ` [PATCH 0/5] --format=%G tests and fixes Jeff King
` (3 preceding siblings ...)
2014-06-17 0:06 ` [PATCH 4/5] t7510: check %G* pretty-format output Jeff King
@ 2014-06-17 0:07 ` Jeff King
4 siblings, 0 replies; 10+ messages in thread
From: Jeff King @ 2014-06-17 0:07 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Junio C Hamano, Michael J Gruber
If the user asks for --format=%G with nothing else, we
correctly realize that "%G" is not a valid placeholder (it
should be "%G?", "%GK", etc). But we still tell the
strbuf_expand code that we consumed 2 characters, causing it
to jump over the trailing NUL and output garbage.
This also fixes the case where "%GX" would be consumed (and
produce no output). In other cases, we pass unrecognized
placeholders through to the final string.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before, but with the test included along with the fix, rather
than in another commit.
pretty.c | 2 ++
t/t7510-signed-commit.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/pretty.c b/pretty.c
index e1e2cad..70d8776 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1267,6 +1267,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (c->signature_check.key)
strbuf_addstr(sb, c->signature_check.key);
break;
+ default:
+ return 0;
}
return 2;
}
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index e97477a..9810242 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -147,4 +147,10 @@ test_expect_success GPG 'show lack of signature with custom format' '
test_cmp expect actual
'
+test_expect_success 'unused %G placeholders are passed through' '
+ echo "%GX %G" >expect &&
+ git log -1 --format="%GX %G" >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.0.0.566.gfe3e6b2
^ permalink raw reply related [flat|nested] 10+ messages in thread