Re: [Mesa-dev] [PATCH] glx/tests: Fix bash-specific code in dispatch-index-check
On Sun, Feb 26, 2017 at 7:51 AM, Eric Engestrom 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
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glx/tests: Fix bash-specific code in dispatch-index-check
On 26 February 2017 at 13:51, Eric Engestrom 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 :)
>
>>
>> # 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.
>
> 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 :)
>
Yes, my bad on that one - guess I shouldn't trust checkbashisms/zsh that much.
I won't object if we can make these tests 'generic' sh - patches welcome ;-)
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glx/tests: Fix bash-specific code in dispatch-index-check
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 :)
>
> # 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.
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
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glx/tests: Fix bash-specific code in dispatch-index-check
On Fri, Feb 24, 2017 at 8:03 PM, 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)
>
> # extract values inside of enum
> -dispatch_list=$(sed '1d;$d' <<< "$dispatch_list")
> +dispatch_list=$(printf "$dispatch_list" | sed '1d;$d')
>
> # 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
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Fixes: 3cc33e764011 ("glx: add GLXdispatchIndex sort check")
Tested-by: Vinson Lee
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glx/tests: Fix bash-specific code in dispatch-index-check
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) # extract values inside of enum -dispatch_list=$(sed '1d;$d' <<< "$dispatch_list") +dispatch_list=$(printf "$dispatch_list" | sed '1d;$d') # 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
