automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/3] {testsuite-work} tests defs: better requirements for XSI


From: Stefano Lattarini
Subject: Re: [PATCH 1/3] {testsuite-work} tests defs: better requirements for XSI shells
Date: Tue, 7 Jun 2011 12:07:54 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

Hi Peter, thanks for the review.

On Tuesday 07 June 2011, Peter Rosin wrote:
>
> [SNIP]
>
> > diff --git a/tests/defs b/tests/defs
> > index ea036f8..37b5baa 100644
> > --- a/tests/defs
> > +++ b/tests/defs
> > @@ -283,6 +283,22 @@ unindent ()
> >  }
> >  sed_unindent_prog="" # Avoid interferences from the environment.
> 
> s/interferences/interference/
>
This isn't introduced by my patch (well, not this one ;-), so I think
it should be fixed by an indipentent patch.  You're obviously free to
do so yourself if you want.

> >  
> > +# require_xsi SHELL
> > +# -----------------
> > +# Skip the test is the given shell fails to support common XSI constructs.
> 
> s/ is / if /
>
Fixed.

> > +require_xsi ()
> > +{
> > +  test $# -eq 1 || framework_failure_ "require_xsi needs exactly one arg"
> > +  echo "$me: trying some XSI constructs with $1"
> > +  $1 -c "$xsi_shell_code" || skip_ "$1 lacks XSI features"
> > +}
> > +# Shell code supposed to work only with XSI shells.  Inspired to
> 
> s/Inspired to/Inspired by/
>
Damned L1 interference :-(  I hope I won't make this mistake again at least.

> > +# libtool.m4:_LT_CHECK_SHELL_FEATURES.
> > +xsi_shell_code='
> > +    var="a/b/c" \
> > +      && test "${var##*/},${var%/*},${var#??}"${var%"$var"}, = c,a/b,b/c, \
> > +      && test $(( 1 + 1 )) -eq 2 && test "${#var}" -eq 5'
> > +
> 
> Changing the variable names makes it harder to keep it in sync with Libtool.
>
Well, it was already out-of-sync with libtool anyway, as the git version
of libtool.m4 contains this:

  m4_defun([_LT_CHECK_SHELL_FEATURES],
  [AC_MSG_CHECKING([whether the shell understands some XSI constructs])
  # Try some XSI features
  xsi_shell=no
  ( _lt_dummy="a/b/c"
    test 
"${_lt_dummy##*/},${_lt_dummy%/*},${_lt_dummy#??}"${_lt_dummy%"$_lt_dummy"}, \
        = c,a/b,b/c, \
      && eval 'test $(( 1 + 1 )) -eq 2 \
      && test "${#_lt_dummy}" -eq 5' ) >/dev/null 2>&1 \
    && xsi_shell=yes
  AC_MSG_RESULT([$xsi_shell])
  _LT_CONFIG_LIBTOOL_INIT([xsi_shell='$xsi_shell'])

> I think this was mentioned the last time you tried to change it?
> Also, I notice that the code is not completely equal (one extra && occurrence,
> and one eval is removed), are you sure the code is equivalent? I'm not 
> qualified
> to tell.
>
Right, better avoid risks.  The fact that the previous code has already gone
out-of-sync with libtool.m4:_LT_CHECK_SHELL_FEATURES is not a good reason to
worsen the situation.  I'll thus go for this unless there further objections:

  # Shell code supposed to work only with XSI shells.  Keep this in sync
  # with libtool.m4:_LT_CHECK_SHELL_FEATURES.
  xsi_shell_code='
    _lt_dummy="a/b/c"
    test 
"${_lt_dummy##*/},${_lt_dummy%/*},${_lt_dummy#??}"${_lt_dummy%"$_lt_dummy"}, \
        = c,a/b,b/c, \
      && eval '\''test $(( 1 + 1 )) -eq 2 \
      && test "${#_lt_dummy}" -eq 5'\'

Note that this isn't *really* unchanged unfortunately, because the code in
_LT_CHECK_SHELL_FEATURES uses single quotes, and since we are single-quoting
it, we need to introduce some extra escapes.


Regards,
  Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]