[Mesa-dev] [PATCH] glx/tests: Fix bash-specific code in dispatch-index-check

[Aaron Watry]

On Sun, Feb 26, 2017 at 7:51 AM, Eric Engestrom <eric at engestrom.ch> wrote:

> On Friday, 2017-02-24 22:03:36 -0600, Aaron Watry wrote:
> > Using <<< for variable redirection is bash-specific behavior.
> > Ubuntu redirects sh -> dash, so this was erroring out.
> >
> > Also, the initial error that led me to this was that srcdir is null when
> running make check
> > so I just copied something similar to what the optimization-test script
> does.
> > ---
> >  src/glx/tests/dispatch-index-check | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/glx/tests/dispatch-index-check
> b/src/glx/tests/dispatch-index-check
> > index 78464b8..ee1b9ee 100755
> > --- a/src/glx/tests/dispatch-index-check
> > +++ b/src/glx/tests/dispatch-index-check
> > @@ -1,24 +1,31 @@
> >  #!/bin/sh
> >
> > +if [ -z "$srcdir" ]; then
> > +   scriptdir=`dirname "$0"`
> > +else
> > +   scriptdir=$srcdir
> > +fi
> > +
> > +
> >  # extract enum definition
> >  dispatch_list=$(sed '/__GLXdispatchIndex/,/__GLXdispatchIndex/!d' \
> > -  "$srcdir"/../g_glxglvnddispatchindices.h)
> > +  "$scriptdir"/../g_glxglvnddispatchindices.h)
>
> No need to create a new var that just copies the old one :)
>
>
Fair enough.  I just didn't want this script to screw up anything in the
future that sourced this script in by modifying externally declared
variables that we knew about.


> >
> >  # extract values inside of enum
> > -dispatch_list=$(sed '1d;$d' <<< "$dispatch_list")
> > +dispatch_list=$(printf "$dispatch_list" | sed '1d;$d')
>
> Never use a variable you have no control over as the format string for
> printf! Use `printf '%s' "$var"` instead.
>

Yeah, that's my bad. I basically did a SO search for ways to de-bashify the
'<<<' operator, and one of the first results used printf $var.  In
retrospect, your version is much more appropriate.

I'm generally a bash guy, so just declaring the scripts as actually being
bash scripts works for me as well.  Make check works again for me, and I
agree with Vinson that it'd be nice to translate this to pure sh, but for
now, this at least works.


>
> I just pushed a1e5e55989 ("check: mark two tests are requiring bash")
> which fixes this by simply asking for bash in the shebang, which was
> what my original patch did, and was changed just before pushing because
> of a review comment that turned out to be wrong :)
>
> Cheers,
>   Eric
>
> >
> >  # remove indentation
> > -dispatch_list=$(sed 's/^\s\+//' <<< "$dispatch_list")
> > +dispatch_list=$(printf "$dispatch_list" | sed 's/^\s\+//')
> >
> >  # extract function names
> > -dispatch_list=$(sed 's/DI_//;s/,//' <<< "$dispatch_list")
> > +dispatch_list=$(printf "$dispatch_list" | sed 's/DI_//;s/,//')
> >
> >  # same for commented functions, we want to keep them sorted too
> > -dispatch_list=$(sed 's#// ##;s/ implemented by [a-z]\+//' <<<
> "$dispatch_list")
> > +dispatch_list=$(printf "$dispatch_list" | sed 's#// ##;s/ implemented
> by [a-z]\+//')
> >
> >  # remove LAST_INDEX, as it will not be in alphabetical order
> > -dispatch_list=$(sed '/LAST_INDEX/d' <<< "$dispatch_list")
> > +dispatch_list=$(printf "$dispatch_list" | sed '/LAST_INDEX/d')
> >
> > -sorted=$(LC_ALL=C sort <<< "$dispatch_list")
> > +sorted=$(LC_ALL=C printf "$dispatch_list" | sort)
> >
> >  test "$dispatch_list" = "$sorted"
> > --
> > 2.9.3
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170227/7c71f396/attachment.html>