[Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Kevin Wolf]

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
-- 
2.20.1

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Thomas Huth]

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>

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 2150 bytes --]

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>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Philippe Mathieu-Daudé]

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>

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[John Snow]

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.

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Eduardo Habkost]

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:

-- 
Eduardo

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[John Snow]

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

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[John Snow]

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
>