[v2,1/2] iotests: Require Python 3.6 or later
diff mbox series

Message ID 20190919162905.21830-2-kwolf@redhat.com
State New
Headers show
Series
  • iotests: Require Python 3.6 or later
Related show

Commit Message

Kevin Wolf Sept. 19, 2019, 4:29 p.m. UTC
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(-)

Comments

Eduardo Habkost Sept. 19, 2019, 4:38 p.m. UTC | #1
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>
Thomas Huth Sept. 19, 2019, 4:41 p.m. UTC | #2
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>
Vladimir Sementsov-Ogievskiy Sept. 20, 2019, 8:40 a.m. UTC | #3
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
>
Kevin Wolf Sept. 20, 2019, 9:27 a.m. UTC | #4
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
Vladimir Sementsov-Ogievskiy Sept. 20, 2019, 9:44 a.m. UTC | #5
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>

Patch
diff mbox series

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