| Message ID | 20190918085519.17290-1-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Require Python 3.5 or later | expand |
On 18/09/2019 10.55, 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.5 is used for the build. > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/check | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 875399d79f..a68f414d6c 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -633,6 +633,13 @@ then > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > fi > > +# Note that if the Python conditional here evaluates True we will exit > +# with status 1 which is a shell 'false' value. > +python_usable=false > +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; 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 +816,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>
On 18.09.19 10:55, 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.5 is used for the build. > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/check | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 875399d79f..a68f414d6c 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -633,6 +633,13 @@ then > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > fi > > +# Note that if the Python conditional here evaluates True we will exit > +# with status 1 which is a shell 'false' value. I’d expect everything to exit with 1 if something does not work. Thus, I find the short script confusing (I think you do, too, or you wouldn’t have written this comment). Why not make it “sys.exit(0 if sys.version_info >= (3, 5) else 1)”? > +python_usable=false > +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; 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 +816,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 But it isn’t wrong, so I suppose: Reviewed-by: Max Reitz <mreitz@redhat.com>
Am 18.09.2019 um 11:20 hat Max Reitz geschrieben: > On 18.09.19 10:55, 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.5 is used for the build. > > > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/qemu-iotests/check | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > > index 875399d79f..a68f414d6c 100755 > > --- a/tests/qemu-iotests/check > > +++ b/tests/qemu-iotests/check > > @@ -633,6 +633,13 @@ then > > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > > fi > > > > +# Note that if the Python conditional here evaluates True we will exit > > +# with status 1 which is a shell 'false' value. > > I’d expect everything to exit with 1 if something does not work. Thus, > I find the short script confusing (I think you do, too, or you wouldn’t > have written this comment). Why not make it “sys.exit(0 if > sys.version_info >= (3, 5) else 1)”? I just copied it from configure, actually. :-) But we can use your way, too. I don't really mind. Kevin
On 9/18/19 11:29 AM, Kevin Wolf wrote: > Am 18.09.2019 um 11:20 hat Max Reitz geschrieben: >> On 18.09.19 10:55, 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.5 is used for the build. Good idea. >>> >>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> tests/qemu-iotests/check | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >>> index 875399d79f..a68f414d6c 100755 >>> --- a/tests/qemu-iotests/check >>> +++ b/tests/qemu-iotests/check >>> @@ -633,6 +633,13 @@ then >>> export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" >>> fi >>> >>> +# Note that if the Python conditional here evaluates True we will exit >>> +# with status 1 which is a shell 'false' value. >> >> I’d expect everything to exit with 1 if something does not work. Thus, >> I find the short script confusing (I think you do, too, or you wouldn’t >> have written this comment). Why not make it “sys.exit(0 if >> sys.version_info >= (3, 5) else 1)”? Good suggestion :) > > I just copied it from configure, actually. :-) > > But we can use your way, too. I don't really mind. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On 9/18/19 4:55 AM, 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.5 is used for the build. > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/check | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 875399d79f..a68f414d6c 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -633,6 +633,13 @@ then > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > fi > > +# Note that if the Python conditional here evaluates True we will exit > +# with status 1 which is a shell 'false' value. > +python_usable=false > +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then > + python_usable=true > +fi > + Do we want this as a temporary fix only until we can stipulate the same version in the configure file? If so, leaving a comment with "python2" in it anywhere will help locate this later. > 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 +816,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 > If you agree with my suggestion: Reviewed-by: John Snow <jsnow@redhat.com> If you don't, that's, like, your opinion, man.
On Wed, Sep 18, 2019 at 02:49:25PM -0400, John Snow wrote: > On 9/18/19 4:55 AM, 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.5 is used for the build. > > > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/qemu-iotests/check | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > > index 875399d79f..a68f414d6c 100755 > > --- a/tests/qemu-iotests/check > > +++ b/tests/qemu-iotests/check > > @@ -633,6 +633,13 @@ then > > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > > fi > > +# Note that if the Python conditional here evaluates True we will exit > > +# with status 1 which is a shell 'false' value. > > +python_usable=false > > +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then > > + python_usable=true > > +fi > > + > > Do we want this as a temporary fix only until we can stipulate the same > version in the configure file? > > If so, leaving a comment with "python2" in it anywhere will help locate this > later. There are other sys.version_info checks in the tree, so we'll surely grep for them once we remove python2 support: scripts/decodetree.py:if sys.version_info >= (3, 4): scripts/qapi/common.py: if sys.version_info[0] >= 3: scripts/qapi/common.py: if sys.version_info[0] >= 3: scripts/qapi/common.py: if sys.version_info[0] >= 3: scripts/qmp/qmp-shell:if sys.version_info[0] == 2: tests/qemu-iotests/044:if sys.version_info.major == 2: tests/qemu-iotests/163:if sys.version_info.major == 2: tests/qemu-iotests/iotests.py: if sys.version_info.major >= 3: tests/qemu-iotests/iotests.py: if sys.version_info.major >= 3: tests/qemu-iotests/nbd-fault-injector.py:if sys.version_info.major >= 3:
Am 18.09.2019 um 20:49 hat John Snow geschrieben: > > > On 9/18/19 4:55 AM, 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.5 is used for the build. > > > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/qemu-iotests/check | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > > index 875399d79f..a68f414d6c 100755 > > --- a/tests/qemu-iotests/check > > +++ b/tests/qemu-iotests/check > > @@ -633,6 +633,13 @@ then > > export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" > > fi > > +# Note that if the Python conditional here evaluates True we will exit > > +# with status 1 which is a shell 'false' value. > > +python_usable=false > > +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then > > + python_usable=true > > +fi > > + > > Do we want this as a temporary fix only until we can stipulate the same > version in the configure file? I thought that maybe we should leave the code around so that at some later point, we could upgrade it to 3.6 (or something else) before QEMU as a whole does so. In fact... Could we already require 3.6 now instead of using 3.5, which I think we only chose because of Debian Stretch (oldstable)? Kevin
On 9/19/19 4:35 AM, Kevin Wolf wrote: > Am 18.09.2019 um 20:49 hat John Snow geschrieben: >> >> >> On 9/18/19 4:55 AM, 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.5 is used for the build. >>> >>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> tests/qemu-iotests/check | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >>> index 875399d79f..a68f414d6c 100755 >>> --- a/tests/qemu-iotests/check >>> +++ b/tests/qemu-iotests/check >>> @@ -633,6 +633,13 @@ then >>> export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" >>> fi >>> +# Note that if the Python conditional here evaluates True we will exit >>> +# with status 1 which is a shell 'false' value. >>> +python_usable=false >>> +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then >>> + python_usable=true >>> +fi >>> + >> >> Do we want this as a temporary fix only until we can stipulate the same >> version in the configure file? > > I thought that maybe we should leave the code around so that at some > later point, we could upgrade it to 3.6 (or something else) before QEMU > as a whole does so. > > In fact... Could we already require 3.6 now instead of using 3.5, which > I think we only chose because of Debian Stretch (oldstable)? > > Kevin > I thought 3.5 was chosen because it's the EPEL version...? No, I guess it's actually 3.6. Well! You won't catch me complaining about that. --js
On 9/19/19 4:35 AM, Kevin Wolf wrote: > Am 18.09.2019 um 20:49 hat John Snow geschrieben: >> >> >> On 9/18/19 4:55 AM, 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.5 is used for the build. >>> >>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> tests/qemu-iotests/check | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >>> index 875399d79f..a68f414d6c 100755 >>> --- a/tests/qemu-iotests/check >>> +++ b/tests/qemu-iotests/check >>> @@ -633,6 +633,13 @@ then >>> export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" >>> fi >>> +# Note that if the Python conditional here evaluates True we will exit >>> +# with status 1 which is a shell 'false' value. >>> +python_usable=false >>> +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then >>> + python_usable=true >>> +fi >>> + >> >> Do we want this as a temporary fix only until we can stipulate the same >> version in the configure file? > > I thought that maybe we should leave the code around so that at some > later point, we could upgrade it to 3.6 (or something else) before QEMU > as a whole does so. > Sorry for the double-post. Do we want the iotest dependencies to formally diverge from QEMU as a whole? It's part of the default `make check` target and it's nice that it only skips instead of failing outright, but ... I would like to somehow centrally express our dependencies instead of hard-coding them in various individual places. Python can be just one example of this. Ideally, in my software utopia; configure expresses these dependencies in a central location where they are easy to verify and maintain. (And easy for downstream maintainers to verify and adjust their package dependencies as-needed, too.) I don't think it's a good idea to make testing run far ahead of the build requirements in case we see more re-use and interdependence of Python code in the future. Or, in general, to allow situations where you can build but not test, especially on older platforms where that testing might be even more important. I think we're being good about that so far, but fracturing the requirements seems like a step we maybe ought not take if we don't have to. But, well, maybe it is a good idea and I am wrong, but I still think it gets confusing to have one-off dependencies expressed in various places. > In fact... Could we already require 3.6 now instead of using 3.5, which > I think we only chose because of Debian Stretch (oldstable)? > > Kevin >
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 875399d79f..a68f414d6c 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -633,6 +633,13 @@ then export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" fi +# Note that if the Python conditional here evaluates True we will exit +# with status 1 which is a shell 'false' value. +python_usable=false +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; 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 +816,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.5 is used for the build. Suggested-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/check | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)