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

[PATCH v2] tests: avoid syntax triggering old dash bug

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 13, 2019, 11:59 UTC
Message-ID
<20190213115951.12096-1-avarab@gmail.com>
In-Reply-To
<xmqqo9a9stbs.fsf@gitster-ct.c.googlers.com>

Avoid a bug in dash that's been fixed ever since its ec2c84d ("[PARSER] Fix clobbering of checkkwd", 2011-03-15)[1] first released with dash v0.5.7 in July 2011. This failing test was introduced in 5f9674243d ("config: add --expiry-date", 2017-11-18).

This fixes 1/2 tests failing on Debian Lenny & Squeeze. The other failure is due to 1b42f45255 ("git-svn: apply "svn.pathnameencoding" before URL encoding", 2016-02-09).

The dash bug is triggered by this test because the heredoc contains a command embedded in "$()" with a "{}" block coming right after it. Refactoring the "$()" to e.g. be a variable that was set earlier will also work around it, but let's instead break up the "EOF" and the "{}".

An earlier version of this patch[2] mitigated the issue by breaking the "$()" out of the "{}" block, that worked, but just because it broke up the "EOF" and "{}" block. Putting e.g. "echo &&" between the two would also work.

1. https://git.kernel.org/pub/scm/utils/dash/dash.git/
2. https://public-inbox.org/git/20181127164253.9832-1-avarab@gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
On Wed, Nov 28 2018, Junio C Hamano wrote:
Show 27 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> On Tue, Nov 27, 2018 at 11:43 AM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>> Avoid a bug in dash that's been fixed ever since its
>>> ec2c84d ("[PARSER] Fix clobbering of checkkwd", 2011-03-15)[1] first
>>> released with dash v0.5.7 in July 2011.
>>
>> Perhaps enhance the commit message to explain the nature of the bug
>> itself. It is not at all obvious from reading the above or from
>> looking at the diff itself what the actual problem is that the patch
>> is fixing. (And it wasn't even immediately obvious by looking at the
>> commit message of ec2c84d in the dash repository.) To help readers of
>> this patch avoid re-introducing this problem or diagnose such a
>> failure, it might be a good idea to give an example of the syntax
>> which trips up old dash (i.e. a here-doc followed immediately by a
>> {...} expression) and the actual error message 'Syntax error: "}"
>> unexpected'.
>
> Indeed.  From the patch text, I would not have even guessed.  I was
> wondering if there were interactions with "" and $() inside it.
>
> If having {...} immediately after a here-doc is a problem, then the
> patch should not touch existing code at all, but instead insert a
> new line, perhaps like
>
> 	: avoid open brace immediately after here-doc for old dash

Late re-roll. Now using a variant of that suggestion, and with an updated commit message explaining what the issue in dash is exactly and why it was triggered.

This isn't a 2.21 regression, but sending it in the rc window anyway in case you'd like to queue an obviously working minor portability fix.

 t/t1300-config.sh | 1 +
 1 file changed, 1 insertion(+)
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9652b241c7..428177c390 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -892,6 +892,7 @@ test_expect_success 'get --expiry-date' '
 	1510348087
 	0
 	EOF
+	: "work around heredoc parsing bug fixed in dash 0.5.7 (in ec2c84d)" &&
 	{
 		echo "$rel_out $(git config --expiry-date date.valid1)"
 		git config --expiry-date date.valid2 &&
-- 
2.20.1.611.gfbb209baf1
Previous: Junio C HamanoNext: Junio C Hamano
Message 5 of 10 in “tests: avoid syntax triggering old dash bug”
  1. tests: avoid syntax triggering old dash bugÆvar Arnfjörð Bjarmason, Nov 27, 2018
  2. Eric SunshineNov 27, 2018
  3. Ævar Arnfjörð BjarmasonNov 27, 2018
  4. Junio C HamanoNov 28, 2018
  5. tests: avoid syntax triggering old dash bugÆvar Arnfjörð Bjarmason, Feb 13, 2019
  6. Junio C HamanoFeb 13, 2019
  7. SZEDER GáborFeb 13, 2019
  8. Ævar Arnfjörð BjarmasonFeb 14, 2019
  9. brian m. carlsonNov 28, 2018
  10. Torsten BögershausenNov 28, 2018

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.