All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Max Kirillov" <max@max630.net>,
	"Carlo Arenas" <carenas@gmail.com>, "Jeff King" <peff@peff.net>,
	git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH] test-lib: check Bash version for '-x' without using shell arrays
Date: Wed,  2 Jan 2019 00:19:49 +0100	[thread overview]
Message-ID: <20190101231949.8184-1-szeder.dev@gmail.com> (raw)

One of our test scripts, 't1510-repo-setup.sh' [1], still can't be
reliably run with '-x' tracing enabled, unless it's executed with a
Bash version supporting BASH_XTRACEFD (since v4.1).  We have a lengthy
condition to check the version of the shell running the test script,
and disable tracing if it's not executed with a suitable Bash version
[2].

This condition uses non-portable shell array accesses to easily get
Bash's major and minor version number.  This didn't seem to be
problematic, because the simple commands expanding those array
accesses are only executed when the test script is actually run with
Bash.  When run with Dash, the only shell I have at hand that doesn't
support shell arrays, there are no issues, as it apparently skips
right over the non-executed simple commands without noticing the
non-supported constructs.

Alas, it has been reported that NetBSD's /bin/sh does complain about
them:

  ./test-lib.sh: 327: Syntax error: Bad substitution

where line 327 contains the first ${BASH_VERSINFO[0]} array access.

To my understanding both shells are right and conform to POSIX,
because the standard allows both behavior by stating the following
under '2.8.1 Consequences of Shell Errors':

  "An expansion error is one that occurs when the shell expansions
  define in wordexp are carried out (for example, "${x!y}", because
  '!' is not a valid operator); an implementation may treat these as
  syntax errors if it is able to detect them during tokenization,
  rather than during expansion."

Avoid this issue with NetBSD's /bin/sh (and potentially with other,
less common shells) by using a couple of parameter expansions to
extract the major and minor version numbers from $BASH_VERSION.

[1] 5827506928 (t1510-repo-setup: mark as untraceable with '-x',
    2018-02-24)
[2] 5fc98e79fc (t: add means to disable '-x' tracing for individual
    test scripts, 2018-02-24)

Reported-by: Max Kirillov <max@max630.net>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---

This will certainly conflict with patches 3 and 6 in my stress testing
patch series at

  https://public-inbox.org/git/20181230191629.3232-1-szeder.dev@gmail.com/T/#u


 t/test-lib.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0f1faa24b2..f47a191e3b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -324,9 +324,12 @@ do
 		# isn't executed with a suitable Bash version.
 		if test -z "$test_untraceable" || {
 		     test -n "$BASH_VERSION" && {
-		       test ${BASH_VERSINFO[0]} -gt 4 || {
-			 test ${BASH_VERSINFO[0]} -eq 4 &&
-			 test ${BASH_VERSINFO[1]} -ge 1
+		       bash_major=${BASH_VERSION%%.*}
+		       bash_minor=${BASH_VERSION#*.}
+		       bash_minor=${bash_minor%%.*}
+		       test $bash_major -gt 4 || {
+			 test $bash_major -eq 4 &&
+			 test $bash_minor -ge 1
 		       }
 		     }
 		   }
-- 
2.20.1.151.gec613c4b75


             reply	other threads:[~2019-01-01 23:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-01 23:19 SZEDER Gábor [this message]
2019-01-02  0:20 ` [PATCH] test-lib: check Bash version for '-x' without using shell arrays Johannes Sixt
2019-01-02 16:46   ` Junio C Hamano
2019-01-03 11:43     ` [PATCH v2] " SZEDER Gábor
2019-01-03 20:29       ` Junio C Hamano
2019-01-04  9:30         ` SZEDER Gábor
2019-01-04 11:25           ` Junio C Hamano
2019-01-04 12:38             ` SZEDER Gábor
2019-01-04 18:42               ` Carlo Arenas
2019-01-04 20:04                 ` Junio C Hamano
2019-01-03  4:52   ` [PATCH] " Jeff King
2019-01-02 18:05 ` Eric Sunshine
2019-01-02 18:31   ` Carlo Arenas
2019-01-03 11:36 ` SZEDER Gábor

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=20190101231949.8184-1-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=max@max630.net \
    --cc=peff@peff.net \
    /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.