git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors

From
Santiago Torres <santiago@nyu.edu>
Date
Mar 22, 2017, 22:51 UTC
Message-ID
<20170322225108.wub4bmr63hk2sp33@LykOS.localdomain>
In-Reply-To
<20170322224124.u3eax4ui3y4saxks@sigill.intra.peff.net>
On Wed, Mar 22, 2017 at 06:41:24PM -0400, Jeff King wrote:
Show 11 quoted lines
> > In that case, something like this would be closer to the desired
> > behavior?
> 
> Yes, though you can spell:
> 
>   cat >expect <<-\EOF
>   EOF
> 
> as just:
> 
>   >expect
Ah, that sounds like a better way to fix this with a smaller diff.
Show 35 quoted lines
> 
> > I'm also unsure on what would be the right thing to put on the commit
> > message.
> 
> I think the argument is:
> 
>   1. It's safer not to expound on tags that have failed verification (so
>      that the caller cannot accidentally use them). Especially since the
>      --format cannot tell anything about the GPG status.
> 
>      That means that
> 
>        tag=$(git verify-tag --format='%(tag)' foo)
> 
>      can use a non-blank $tag without having to wonder whether it is
>      valid or not.
> 
> and
> 
>   2. That's what we've done since the feature was released.
> 
> The only thing that would give me pause is if were to later add
> %G-like formatters, and then:
> 
>   xargs git verify-tag --format='%(gpg:status) %(tag)' |
>   while read status tag
>   do
>      ...
>   done
> 
> would become useful, but we'd be tied to the behavior that we omit the
> tag when the gpg verification failed (for backwards compatibility).
> OTOH, we could perhaps make the rule "ignored unless %(gpg) formatters
> are used". Which would be backwards-compatible and safe for old formats,
> and work correctly for new ones.

This sounds like a helpful addition to implement. We could update/add tests for compliance on this once the feature is addded and fix the ambiguous behavior in the tests now.

Thanks, -Santiago.

---
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f..0581053a0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -896,17 +896,15 @@ test_expect_success GPG 'verifying a forged tag should fail' '
 '
 
 test_expect_success 'verifying a proper tag with --format pass and format accordingly' '
-	cat >expect <<-\EOF
+	cat >expect <<-\EOF &&
 	tagname : signed-tag
-	EOF &&
+	EOF
 	git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
-	cat >expect <<-\EOF
-	tagname : forged-tag
-	EOF &&
+test_expect_success 'verifying a forged tag with --format should fail silently' '
+	>expect &&
 	test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index d62ccbb98..173a88e89 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -126,17 +126,15 @@ test_expect_success GPG 'verify multiple tags' '
 '
 
 test_expect_success 'verifying tag with --format' '
-	cat >expect <<-\EOF
+	cat >expect <<-\EOF &&
 	tagname : fourth-signed
-	EOF &&
+	EOF
 	git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
-	cat >expect <<-\EOF
-	tagname : 7th forged-signed
-	EOF &&
+test_expect_success 'verifying a forged tag with --format should fail silently' '
+	>expect &&
 	test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
 	test_cmp expect actual-forged
 '
Previous: Junio C HamanoNext: Junio C Hamano
Message 31 of 46 in “EOF test fixes (t5615/t7004)”
  1. Jan PalusMar 22, 2017
  2. Jan PalusMar 22, 2017
  3. Stefan BellerMar 22, 2017
  4. Junio C HamanoMar 22, 2017
  5. 0/3 fix "here-doc" syntax errorsJunio C Hamano, Mar 22, 2017
  6. 1/3 t5615: fix a here-doc syntax errorJunio C Hamano, Mar 22, 2017
  7. Jeff KingMar 22, 2017
  8. 2/3 t7406: fix here-doc syntax errorsJunio C Hamano, Mar 22, 2017
  9. Jeff KingMar 22, 2017
  10. Stefan BellerMar 22, 2017
  11. Jeff KingMar 22, 2017
  12. t7406: correct test case for submodule-update initial populationStefan Beller, Mar 22, 2017
  13. Jeff KingMar 22, 2017
  14. Stefan BellerMar 22, 2017
  15. Jeff KingMar 22, 2017
  16. Jonathan NiederMar 22, 2017
  17. Junio C HamanoMar 22, 2017
  18. Jeff KingMar 22, 2017
  19. Stefan BellerMar 22, 2017
  20. Junio C HamanoMar 22, 2017
  21. 3/3 t7004, t7030: fix here-doc syntax errorsJunio C Hamano, Mar 22, 2017
  22. Jeff KingMar 22, 2017
  23. Santiago TorresMar 22, 2017
  24. Jeff KingMar 22, 2017
  25. Junio C HamanoMar 22, 2017
  26. Santiago TorresMar 22, 2017
  27. Jeff KingMar 22, 2017
  28. Santiago TorresMar 22, 2017
  29. Jeff KingMar 22, 2017
  30. Junio C HamanoMar 22, 2017
  31. Santiago TorresMar 22, 2017
  32. Junio C HamanoMar 23, 2017
  33. Santiago TorresMar 23, 2017
  34. Jeff KingMar 23, 2017
  35. Junio C HamanoMar 24, 2017
  36. Jeff KingMar 24, 2017
  37. Junio C HamanoMar 24, 2017
  38. Jeff KingMar 24, 2017
  39. Jeff KingMar 24, 2017
  40. Junio C HamanoMar 22, 2017
  41. Jan PalusMar 22, 2017
  42. tests: lint for run-away here-docJunio C Hamano, Mar 23, 2017
  43. Junio C HamanoMar 23, 2017
  44. Jonathan NiederMar 24, 2017
  45. Junio C HamanoMar 24, 2017
  46. Jeff KingMar 24, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.