| Message ID | 1506333262-30011-1-git-send-email-petri.latvala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 25 Sep 2017, Petri Latvala <petri.latvala@intel.com> wrote: > [[ a != b ]] is a bashism. As it's just comparing $1 to an empty > string, use -n with a normal [ ]. > > /bin/sh is dash in CI. There's probably /bin/bash around anyway, but I'm tired of fighting the fight. So never mind about that. You could add shellcheck to your static checks, with a list of exceptions of shellcheck tests you don't care about. It would tell you, In igt_command_line.sh line 115: if [[ "$1" != "" ]] ; then ^-- SC2039: In POSIX sh, [[ ]] is undefined. BR, Jani. > > Fixes: f0243a761f1b ("tests/igt_command_line.sh: Allow testing individual tests") > CC: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Petri Latvala <petri.latvala@intel.com> > --- > tests/igt_command_line.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh > index 57d105e9..a8baaaa1 100755 > --- a/tests/igt_command_line.sh > +++ b/tests/igt_command_line.sh > @@ -112,7 +112,7 @@ if [ $? -ne 0 ]; then > exit 99 > fi > > -if [[ "$1" != "" ]] ; then > +if [ -n "$1" ] ; then > check_test $1 > exit 0 > fi
On Mon, Sep 25, 2017 at 02:48:41PM +0300, Jani Nikula wrote: > On Mon, 25 Sep 2017, Petri Latvala <petri.latvala@intel.com> wrote: > > [[ a != b ]] is a bashism. As it's just comparing $1 to an empty > > string, use -n with a normal [ ]. > > > > /bin/sh is dash in CI. > > There's probably /bin/bash around anyway, but I'm tired of fighting the > fight. So never mind about that. Yeah, let's just switch to /bin/bash and stop bothering with dash. At least I don't see any value in trying to be posix compliant, we're not going to run on anything that doesn't have bash anyway. -Daniel > > You could add shellcheck to your static checks, with a list of > exceptions of shellcheck tests you don't care about. It would tell you, > > In igt_command_line.sh line 115: > if [[ "$1" != "" ]] ; then > ^-- SC2039: In POSIX sh, [[ ]] is undefined. > > > BR, > Jani. > > > > > > Fixes: f0243a761f1b ("tests/igt_command_line.sh: Allow testing individual tests") > > CC: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Petri Latvala <petri.latvala@intel.com> > > --- > > tests/igt_command_line.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh > > index 57d105e9..a8baaaa1 100755 > > --- a/tests/igt_command_line.sh > > +++ b/tests/igt_command_line.sh > > @@ -112,7 +112,7 @@ if [ $? -ne 0 ]; then > > exit 99 > > fi > > > > -if [[ "$1" != "" ]] ; then > > +if [ -n "$1" ] ; then > > check_test $1 > > exit 0 > > fi > > -- > Jani Nikula, Intel Open Source Technology Center
On Tue, Sep 26, 2017 at 02:01:46PM +0200, Daniel Vetter wrote: > On Mon, Sep 25, 2017 at 02:48:41PM +0300, Jani Nikula wrote: > > On Mon, 25 Sep 2017, Petri Latvala <petri.latvala@intel.com> wrote: > > > [[ a != b ]] is a bashism. As it's just comparing $1 to an empty > > > string, use -n with a normal [ ]. > > > > > > /bin/sh is dash in CI. > > > > There's probably /bin/bash around anyway, but I'm tired of fighting the > > fight. So never mind about that. > > Yeah, let's just switch to /bin/bash and stop bothering with dash. At > least I don't see any value in trying to be posix compliant, we're not > going to run on anything that doesn't have bash anyway. This script is used only on build-time anyway so the shebang doesn't really matter much. I made the shebang use /bin/bash in v2 (in addition to the changes in v1) so it doesn't need further handholding and pushed with an IRC ack from Daniel.
On Tue, 2017-09-26 at 14:01 +0200, Daniel Vetter wrote: > On Mon, Sep 25, 2017 at 02:48:41PM +0300, Jani Nikula wrote: > > On Mon, 25 Sep 2017, Petri Latvala <petri.latvala@intel.com> wrote: > > > [[ a != b ]] is a bashism. As it's just comparing $1 to an empty > > > string, use -n with a normal [ ]. > > > > > > /bin/sh is dash in CI. > > > > There's probably /bin/bash around anyway, but I'm tired of fighting the > > fight. So never mind about that. > > Yeah, let's just switch to /bin/bash and stop bothering with dash. At > least I don't see any value in trying to be posix compliant, we're not > going to run on anything that doesn't have bash anyway. I'm actively running I-G-T with busybox sh, so please cut with the /bin/bash stuff, it's not that hard to be POSIX compliant. Original patch is; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
On Fri, Sep 29, 2017 at 12:34:48PM +0300, Joonas Lahtinen wrote: > On Tue, 2017-09-26 at 14:01 +0200, Daniel Vetter wrote: > > On Mon, Sep 25, 2017 at 02:48:41PM +0300, Jani Nikula wrote: > > > On Mon, 25 Sep 2017, Petri Latvala <petri.latvala@intel.com> wrote: > > > > [[ a != b ]] is a bashism. As it's just comparing $1 to an empty > > > > string, use -n with a normal [ ]. > > > > > > > > /bin/sh is dash in CI. > > > > > > There's probably /bin/bash around anyway, but I'm tired of fighting the > > > fight. So never mind about that. > > > > Yeah, let's just switch to /bin/bash and stop bothering with dash. At > > least I don't see any value in trying to be posix compliant, we're not > > going to run on anything that doesn't have bash anyway. > > I'm actively running I-G-T with busybox sh, so please cut with the > /bin/bash stuff, it's not that hard to be POSIX compliant. Is that a machine which doesn't have any bash at all, or is busysbox simply your /bin/sh? And yes it's imo not justified to be posix compliant if there's no use-case for it, afaiui we haven't ever made a decision for igt to be posix compliant when building (running might be a different thing, with android and all that). And this is a build-time script here. -Daniel > > Original patch is; > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation
diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh index 57d105e9..a8baaaa1 100755 --- a/tests/igt_command_line.sh +++ b/tests/igt_command_line.sh @@ -112,7 +112,7 @@ if [ $? -ne 0 ]; then exit 99 fi -if [[ "$1" != "" ]] ; then +if [ -n "$1" ] ; then check_test $1 exit 0 fi
[[ a != b ]] is a bashism. As it's just comparing $1 to an empty string, use -n with a normal [ ]. /bin/sh is dash in CI. Fixes: f0243a761f1b ("tests/igt_command_line.sh: Allow testing individual tests") CC: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Petri Latvala <petri.latvala@intel.com> --- tests/igt_command_line.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)