Re: [PATCH] test-lib: check Bash version for '-x' without using shell arrays

From: Jeff King <peff@peff.net>
Date: Jan 3, 2019, 04:52 UTC
Message-ID: <20190103045200.GB20047@sigill.intra.peff.net>
In-Reply-To: <a82251fa-38e1-233e-50d3-3ed4850b4e11@kdbg.org>

On Wed, Jan 02, 2019 at 01:20:47AM +0100, Johannes Sixt wrote:

Show 31 quoted lines

> > 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
> >   		       }
> >   		     }
> >   		   }
> > 
> 
> Would it perhaps be simpler to just hide the syntax behind eval? Like
> 
>  		if test -z "$test_untraceable" || {
>  		     test -n "$BASH_VERSION" && eval '
> 		       test ${BASH_VERSINFO[0]} -gt 4 || {
> 			 test ${BASH_VERSINFO[0]} -eq 4 &&
> 			 test ${BASH_VERSINFO[1]} -ge 1
> 		       }
>  		     '

That was my first thought, too. :)

The parsing here is simple enough that I'd be fine either with the original patch, or an eval-based version (and otherwise, the goal and description seem quite good to me).

-Peff