| Message ID | 20190919162905.21830-2-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Require Python 3.6 or later | expand |
On Thu, Sep 19, 2019 at 06:29:04PM +0200, Kevin Wolf wrote: > Running iotests is not required to build QEMU, so we can have stricter > version requirements for Python here and can make use of new features > and drop compatibility code earlier. > > This makes qemu-iotests skip all Python tests if a Python version before > 3.6 is used for the build. > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
On 19/09/2019 18.29, Kevin Wolf wrote: > Running iotests is not required to build QEMU, so we can have stricter > version requirements for Python here and can make use of new features > and drop compatibility code earlier. > > This makes qemu-iotests skip all Python tests if a Python version before > 3.6 is used for the build. > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/check | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 875399d79f..588c453a94 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -633,6 +633,12 @@ then > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > fi > > +python_usable=false > +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' > +then > + python_usable=true > +fi > + > default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p') > default_alias_machine=$($QEMU_PROG -machine help | \ > sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }") > @@ -809,7 +815,12 @@ do > start=$(_wallclock) > > if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then > - run_command="$PYTHON $seq" > + if $python_usable; then > + run_command="$PYTHON $seq" > + else > + run_command="false" > + echo "Unsupported Python version" > $seq.notrun > + fi > else > run_command="./$seq" > fi > Reviewed-by: Thomas Huth <thuth@redhat.com>
19.09.2019 19:29, Kevin Wolf wrote: > Running iotests is not required to build QEMU, so we can have stricter > version requirements for Python here and can make use of new features > and drop compatibility code earlier. > > This makes qemu-iotests skip all Python tests if a Python version before > 3.6 is used for the build. > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/check | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 875399d79f..588c453a94 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -633,6 +633,12 @@ then > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > fi > > +python_usable=false > +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' > +then > + python_usable=true > +fi > + > default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p') > default_alias_machine=$($QEMU_PROG -machine help | \ > sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }") > @@ -809,7 +815,12 @@ do > start=$(_wallclock) > > if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then > - run_command="$PYTHON $seq" > + if $python_usable; then hmm I wanted to say that this should not work, as python_usable is a string. But I checked and see - it's work. Wow. Googled. And now I understand that here "false" or "true" command is called, to obtain it's return value.. I don't like bash and don't know its best practice, but I'd prefer python_usable to be just return value of your python command, like above: $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' python_usable=$? and here: if [ $python_usable -eq 0 ]; then > + run_command="$PYTHON $seq" > + else > + run_command="false" > + echo "Unsupported Python version" > $seq.notrun > + fi > else > run_command="./$seq" > fi >
Am 20.09.2019 um 10:40 hat Vladimir Sementsov-Ogievskiy geschrieben: > 19.09.2019 19:29, Kevin Wolf wrote: > > Running iotests is not required to build QEMU, so we can have stricter > > version requirements for Python here and can make use of new features > > and drop compatibility code earlier. > > > > This makes qemu-iotests skip all Python tests if a Python version before > > 3.6 is used for the build. > > > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/qemu-iotests/check | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > > index 875399d79f..588c453a94 100755 > > --- a/tests/qemu-iotests/check > > +++ b/tests/qemu-iotests/check > > @@ -633,6 +633,12 @@ then > > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > > fi > > > > +python_usable=false > > +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' > > +then > > + python_usable=true > > +fi > > + > > default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p') > > default_alias_machine=$($QEMU_PROG -machine help | \ > > sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }") > > @@ -809,7 +815,12 @@ do > > start=$(_wallclock) > > > > if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then > > - run_command="$PYTHON $seq" > > + if $python_usable; then > > hmm I wanted to say that this should not work, as python_usable is a > string. But I checked and see - it's work. Wow. Googled. And now I > understand that here "false" or "true" command is called, to obtain > it's return value.. I don't like bash and don't know its best > practice, but I'd prefer python_usable to be just return value of your > python command, like > > above: > > $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' > python_usable=$? > > and here: > > if [ $python_usable -eq 0 ]; then I'm just trying to be consistent with other variables used in the 'check' script. It has many boolean variables and they all end up calling the true/false commands. And actually I think the version used is easier to read, even if maybe not as easy to understand why exactly it works. Kevin
20.09.2019 12:27, Kevin Wolf wrote: > Am 20.09.2019 um 10:40 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 19.09.2019 19:29, Kevin Wolf wrote: >>> Running iotests is not required to build QEMU, so we can have stricter >>> version requirements for Python here and can make use of new features >>> and drop compatibility code earlier. >>> >>> This makes qemu-iotests skip all Python tests if a Python version before >>> 3.6 is used for the build. >>> >>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> tests/qemu-iotests/check | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >>> index 875399d79f..588c453a94 100755 >>> --- a/tests/qemu-iotests/check >>> +++ b/tests/qemu-iotests/check >>> @@ -633,6 +633,12 @@ then >>> export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" >>> fi >>> >>> +python_usable=false >>> +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' >>> +then >>> + python_usable=true >>> +fi don't we want else PYTHON="false" fi to prevent some occasional unchecked call below? >>> + >>> default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p') >>> default_alias_machine=$($QEMU_PROG -machine help | \ >>> sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }") >>> @@ -809,7 +815,12 @@ do >>> start=$(_wallclock) >>> >>> if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then >>> - run_command="$PYTHON $seq" >>> + if $python_usable; then >> >> hmm I wanted to say that this should not work, as python_usable is a >> string. But I checked and see - it's work. Wow. Googled. And now I >> understand that here "false" or "true" command is called, to obtain >> it's return value.. I don't like bash and don't know its best >> practice, but I'd prefer python_usable to be just return value of your >> python command, like >> >> above: >> >> $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' >> python_usable=$? >> >> and here: >> >> if [ $python_usable -eq 0 ]; then > > I'm just trying to be consistent with other variables used in the > 'check' script. It has many boolean variables and they all end up > calling the true/false commands. Missed this, sorry. Then OK, of course.. still, echo "Unsupported Python version (must be >= 3.6)" > $seq.notrun may be a bit more useful, if someone see it. With or without any of suggestions: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 875399d79f..588c453a94 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -633,6 +633,12 @@ then export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" fi +python_usable=false +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)' +then + python_usable=true +fi + default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p') default_alias_machine=$($QEMU_PROG -machine help | \ sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }") @@ -809,7 +815,12 @@ do start=$(_wallclock) if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then - run_command="$PYTHON $seq" + if $python_usable; then + run_command="$PYTHON $seq" + else + run_command="false" + echo "Unsupported Python version" > $seq.notrun + fi else run_command="./$seq" fi
Running iotests is not required to build QEMU, so we can have stricter version requirements for Python here and can make use of new features and drop compatibility code earlier. This makes qemu-iotests skip all Python tests if a Python version before 3.6 is used for the build. Suggested-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/check | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)