From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Lars Schneider <larsxschneider@gmail.com>,
Ann T Ropea <bedhanger@gmx.de>
Subject: Re: [PATCH 1/2] tests: fix non-portable "${var:-"str"}" construct
Date: Tue, 28 Aug 2018 12:48:19 -0700 [thread overview]
Message-ID: <xmqqlg8qff24.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180828193827.8648-2-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Tue, 28 Aug 2018 19:38:26 +0000")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On both AIX 7200-00-01-1543 and FreeBSD 11.2-RELEASE-p2 the
> "${var:-"str"}" syntax means something different than what it does
> under the bash or dash shells.
>
> Both will consider the start of the new unescaped quotes to be a new
> argument to test_expect_success, resulting in the following error:
>
> error: bug in the test script: 'git diff-tree initial # magic
> is (not' does not look like a prereq
>
> Fix this by removing the redundant quotes. There's no need for them,
> and the resulting code works under all the aforementioned shells.
Yup, there is no need for that inner dq pair in this particular case.
I was more worried about scripted Porcelains, like
: "${GIT_OBJECT_DIRECTORY="$(git rev-parse --git-path objects)"}"
which I think can safely lose the outer dq pair. In t/ directory,
there is this one in test-lib-functions.sh which I do not offhand
know how these problematic shells would handle.
echo "#!${2-"$SHELL_PATH"}" &&
Other than the presence of these two that are not covered by this
patch, the patch itself looks good. Thanks.
> fixes a regression in c2f1d3989 ("t4013: test new output from diff
> --abbrev --raw", 2017-12-03) first released with Git v2.16.0.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t4013-diff-various.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index f8d853595b..73f7038253 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -140,7 +140,7 @@ do
> expect="$TEST_DIRECTORY/t4013/diff.$test"
> actual="$pfx-diff.$test"
>
> - test_expect_success "git $cmd # magic is ${magic:-"(not used)"}" '
> + test_expect_success "git $cmd # magic is ${magic:-(not used)}" '
> {
> echo "$ git $cmd"
> case "$magic" in
next prev parent reply other threads:[~2018-08-28 19:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-28 19:38 [PATCH 0/2] FreeBSD & AIX test portability fixes Ævar Arnfjörð Bjarmason
2018-08-28 19:38 ` [PATCH 1/2] tests: fix non-portable "${var:-"str"}" construct Ævar Arnfjörð Bjarmason
2018-08-28 19:48 ` Junio C Hamano [this message]
2018-08-29 20:09 ` Ann T Ropea
2018-08-28 19:38 ` [PATCH 2/2] tests: fix non-portable iconv invocation Ævar Arnfjörð Bjarmason
2018-08-28 22:55 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqlg8qff24.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=bedhanger@gmx.de \
--cc=git@vger.kernel.org \
--cc=larsxschneider@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.