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 <ehabk...@redhat.com> >>> Signed-off-by: Kevin Wolf <kw...@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 <vsement...@virtuozzo.com> -- Best regards, Vladimir