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

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 <redacted>
---

On Wed, Nov 28 2018, Junio C Hamano wrote:

Eric Sunshine [off-list ref] writes:

quoted

On Tue, Nov 27, 2018 at 11:43 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

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