From 526d56e07200483fc93be9c2dfee81bb87713c9b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 16 Jun 2014 19:59:59 -0400 Subject: [PATCH 1/6] t7510: stop referring to master in later tests 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 Signed-off-by: Junio C Hamano --- 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 5ddac1a9f7..37c3778b29 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 && From 7b1732c11605f84b809bd120401df49866c27141 Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Mon, 16 Jun 2014 20:03:43 -0400 Subject: [PATCH 2/6] t7510: use consistent &&-chains in loop 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 Signed-off-by: Junio C Hamano --- 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 37c3778b29..cdffcbdc33 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 ) ' From 4baf839fe0e71dca3f26198b7a4faab439fa2ef6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 16 Jun 2014 20:05:54 -0400 Subject: [PATCH 3/6] t7510: test a commit signed by an unknown key 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 Signed-off-by: Junio C Hamano --- 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 cdffcbdc33..04fc2c5c2e 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 ) ' From 06ca0f45a063f7186cce96a58a5fb6bef16ec204 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 16 Jun 2014 20:06:24 -0400 Subject: [PATCH 4/6] t7510: check %G* pretty-format output 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 Signed-off-by: Junio C Hamano --- 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 04fc2c5c2e..e97477a3b9 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 + 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 + 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 + 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 From aa4b78d483a918ebee810993e420b4697b0de4d3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 16 Jun 2014 20:07:07 -0400 Subject: [PATCH 5/6] pretty: avoid reading past end-of-string with "%G" 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 Signed-off-by: Junio C Hamano --- pretty.c | 2 ++ t/t7510-signed-commit.sh | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/pretty.c b/pretty.c index 3c43db558a..fe249f8820 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 e97477a3b9..9810242435 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 From 958b2eb26c82f9d99d1fb4d3270601d0a18eaf38 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 25 Jun 2014 17:42:17 -0400 Subject: [PATCH 6/6] move "%G" format test from t7510 to t6006 The final test in t7510 checks that "--format" placeholders that look similar to GPG placeholders (but that we don't actually understand) are passed through. That test was placed in t7510, since the other GPG placeholder tests are there. However, it does not have a GPG prerequisite, because it is not actually checking any signed commits. This causes the test to erroneously fail when gpg is not installed on a system, however. Not because we need signed commits, but because we need _any_ commit to run "git log". If we don't have gpg installed, t7510 doesn't create any commits at all. We can fix this by moving the test into t6006. This is arguably a better place anyway, because it is where we test most of the other placeholders (we do not test GPG placeholders there because of the infrastructure needed to make signed commits). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t6006-rev-list-format.sh | 6 ++++++ t/t7510-signed-commit.sh | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh index 9d9d9de08e..9ab20ee09d 100755 --- a/t/t6006-rev-list-format.sh +++ b/t/t6006-rev-list-format.sh @@ -394,4 +394,10 @@ test_expect_success 'single-character name is parsed correctly' ' 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 diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 9810242435..e97477a3b9 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -147,10 +147,4 @@ 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