[Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes

[Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes

[Cleber Rosa]

A recent experience with Travis-CI[1] showed that some tests were not
running with the intended Python version.  Let's add the Python
version the configure output, which serves as a general debugging
aid that the intended Python version was used[2][3].

Additionally, the recently introduced "check-venv" target, used by the
"check-acceptance" target, verifies if the configured Python interpreter
is Python 3, and does so on the Makefile itself.  Since the Python version
is being captured on configure, let's avoid rerunning Python on every
make invocation.

Finally, a small cosmetic fix to the "make check-help" output.

[1] https://travis-ci.org/clebergnu/qemu/jobs/452033247#L983
[2] https://travis-ci.org/clebergnu/qemu/jobs/452663112#L960
[3] https://travis-ci.org/clebergnu/qemu/jobs/452663113#L956

Cleber Rosa (4):
  configure: keep track of Python version
  check-venv: use recorded Python version
  Travis CI: make specified Python versions usable on jobs
  check-help: visual and content improvements

 .travis.yml            |  4 +++-
 configure              |  6 +++++-
 tests/Makefile.include | 11 ++++++-----
 3 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.19.1

[Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

Some functionality is dependent on the Python version
detected/configured on configure.  While it's possible to run the
Python version later and check for the version, doing it once is
preferable.  Also, it's a relevant information to keep in build logs,
as the overall behavior of the build can be affected by it.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 configure | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 74e313a810..67fff0290d 100755
--- a/configure
+++ b/configure
@@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
       "Use --python=/path/to/python to specify a supported Python."
 fi
 
+# Preserve python version since some functionality is dependent on it
+python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
+
 # Suppress writing compiled files
 python="$python -B"
 
@@ -5918,7 +5921,7 @@ echo "LDFLAGS           $LDFLAGS"
 echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
 echo "make              $make"
 echo "install           $install"
-echo "python            $python"
+echo "python            $python ($python_version)"
 if test "$slirp" = "yes" ; then
     echo "smbd              $smbd"
 fi
@@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
 echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
 echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
 echo "PYTHON=$python" >> $config_host_mak
+echo "PYTHON_VERSION=$python_version" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
 if $iasl -h > /dev/null 2>&1; then
   echo "IASL=$iasl" >> $config_host_mak
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 10:07:07AM -0500, Cleber Rosa wrote:
> Some functionality is dependent on the Python version
> detected/configured on configure.  While it's possible to run the
> Python version later and check for the version, doing it once is
> preferable.  Also, it's a relevant information to keep in build logs,
> as the overall behavior of the build can be affected by it.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  configure | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 74e313a810..67fff0290d 100755
> --- a/configure
> +++ b/configure
> @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
>        "Use --python=/path/to/python to specify a supported Python."
>  fi
>  
> +# Preserve python version since some functionality is dependent on it
> +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')

What about:
  $($python -c 'import sys;print(sys.version)')
?

It is very verbose, but I think that's a good thing.

> +
>  # Suppress writing compiled files
>  python="$python -B"
>  
> @@ -5918,7 +5921,7 @@ echo "LDFLAGS           $LDFLAGS"
>  echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
>  echo "make              $make"
>  echo "install           $install"
> -echo "python            $python"
> +echo "python            $python ($python_version)"
>  if test "$slirp" = "yes" ; then
>      echo "smbd              $smbd"
>  fi
> @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
>  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
>  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
>  echo "PYTHON=$python" >> $config_host_mak
> +echo "PYTHON_VERSION=$python_version" >> $config_host_mak

The output of "python -V" and "sys.version" seems to be meant for
humans, not software.  If we really want something to be used in
conditional makefile rules, I'd prefer to use sys.version_info.
e.g.:

  python_major_version="$($python -c 'import sys;print(sys.version_info[0])')"
  echo "PYTHON_MAJOR_VERSION=$python_major_version"


>  echo "CC=$cc" >> $config_host_mak
>  if $iasl -h > /dev/null 2>&1; then
>    echo "IASL=$iasl" >> $config_host_mak
> -- 
> 2.19.1
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On 11/9/18 10:49 AM, Eduardo Habkost wrote:
> On Fri, Nov 09, 2018 at 10:07:07AM -0500, Cleber Rosa wrote:
>> Some functionality is dependent on the Python version
>> detected/configured on configure.  While it's possible to run the
>> Python version later and check for the version, doing it once is
>> preferable.  Also, it's a relevant information to keep in build logs,
>> as the overall behavior of the build can be affected by it.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  configure | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 74e313a810..67fff0290d 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
>>        "Use --python=/path/to/python to specify a supported Python."
>>  fi
>>  
>> +# Preserve python version since some functionality is dependent on it
>> +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> 
> What about:
>   $($python -c 'import sys;print(sys.version)')
> ?
> 
> It is very verbose, but I think that's a good thing.
> 

Well, something like:

'3.7.1 (default, Oct 23 2018, 18:19:07) \n[GCC 8.2.1 20180801 (Red Hat
8.2.1-2)]'

Doesn't qualify as simply the Python *version*.  The documentation[1]
puts it clearly: "A string containing the version number of the Python
interpreter plus additional information on the build number and compiler
used. This string is displayed when the interactive interpreter is started."

I can't see how the compiler used on the Python build, or the build
date, can be significant to someone debugging the type of Python code
that will be on QEMU.

>> +
>>  # Suppress writing compiled files
>>  python="$python -B"
>>  
>> @@ -5918,7 +5921,7 @@ echo "LDFLAGS           $LDFLAGS"
>>  echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
>>  echo "make              $make"
>>  echo "install           $install"
>> -echo "python            $python"
>> +echo "python            $python ($python_version)"
>>  if test "$slirp" = "yes" ; then
>>      echo "smbd              $smbd"
>>  fi
>> @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
>>  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
>>  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
>>  echo "PYTHON=$python" >> $config_host_mak
>> +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> 
> The output of "python -V" and "sys.version" seems to be meant for
> humans, not software.  If we really want something to be used in
> conditional makefile rules, I'd prefer to use sys.version_info.
> e.g.:
> 

"Python -V" is quite different from "sys.version".  Indeed it includes
the "Python " prefix, but that's all, everything else is the version number.

>   python_major_version="$($python -c 'import sys;print(sys.version_info[0])')"
>   echo "PYTHON_MAJOR_VERSION=$python_major_version"
> 
> 

No, I'm actually interested in the other version components, not just
the major version.  As I tried to explain in another thread, differences
from Python 3.0 to 3.4 are many, and from 3.4 to 3.6 and so on.

Then, we can either introduce "PYTHON_MAJOR_VERSION",
"PYTHON_MINOR_VERSION", "PYTHON_RELEASE" of sorts, or just have a single
dot separated version string.

- Cleber

[1] - https://docs.python.org/3/library/sys.html#sys.version

>>  echo "CC=$cc" >> $config_host_mak
>>  if $iasl -h > /dev/null 2>&1; then
>>    echo "IASL=$iasl" >> $config_host_mak
>> -- 
>> 2.19.1
>>
> 

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 11:39:32AM -0500, Cleber Rosa wrote:
> 
> 
> On 11/9/18 10:49 AM, Eduardo Habkost wrote:
> > On Fri, Nov 09, 2018 at 10:07:07AM -0500, Cleber Rosa wrote:
> >> Some functionality is dependent on the Python version
> >> detected/configured on configure.  While it's possible to run the
> >> Python version later and check for the version, doing it once is
> >> preferable.  Also, it's a relevant information to keep in build logs,
> >> as the overall behavior of the build can be affected by it.
> >>
> >> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> >> ---
> >>  configure | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/configure b/configure
> >> index 74e313a810..67fff0290d 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> >>        "Use --python=/path/to/python to specify a supported Python."
> >>  fi
> >>  
> >> +# Preserve python version since some functionality is dependent on it
> >> +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > 
> > What about:
> >   $($python -c 'import sys;print(sys.version)')
> > ?
> > 
> > It is very verbose, but I think that's a good thing.
> > 
> 
> Well, something like:
> 
> '3.7.1 (default, Oct 23 2018, 18:19:07) \n[GCC 8.2.1 20180801 (Red Hat
> 8.2.1-2)]'
> 
> Doesn't qualify as simply the Python *version*.  The documentation[1]
> puts it clearly: "A string containing the version number of the Python
> interpreter plus additional information on the build number and compiler
> used. This string is displayed when the interactive interpreter is started."
> 
> I can't see how the compiler used on the Python build, or the build
> date, can be significant to someone debugging the type of Python code
> that will be on QEMU.

No problem to me.

> 
> >> +
> >>  # Suppress writing compiled files
> >>  python="$python -B"
> >>  
> >> @@ -5918,7 +5921,7 @@ echo "LDFLAGS           $LDFLAGS"
> >>  echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
> >>  echo "make              $make"
> >>  echo "install           $install"
> >> -echo "python            $python"
> >> +echo "python            $python ($python_version)"
> >>  if test "$slirp" = "yes" ; then
> >>      echo "smbd              $smbd"
> >>  fi
> >> @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
> >>  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> >>  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> >>  echo "PYTHON=$python" >> $config_host_mak
> >> +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> > 
> > The output of "python -V" and "sys.version" seems to be meant for
> > humans, not software.  If we really want something to be used in
> > conditional makefile rules, I'd prefer to use sys.version_info.
> > e.g.:
> > 
> 
> "Python -V" is quite different from "sys.version".  Indeed it includes
> the "Python " prefix, but that's all, everything else is the version number.

Is this a guarantee documented somewhere?

> 
> >   python_major_version="$($python -c 'import sys;print(sys.version_info[0])')"
> >   echo "PYTHON_MAJOR_VERSION=$python_major_version"
> > 
> > 
> 
> No, I'm actually interested in the other version components, not just
> the major version.  As I tried to explain in another thread, differences
> from Python 3.0 to 3.4 are many, and from 3.4 to 3.6 and so on.

Do you have any examples in mind?  I really doubt we will need to
look at the Python minor version number inside shell scripts or
makefiles.

> 
> Then, we can either introduce "PYTHON_MAJOR_VERSION",
> "PYTHON_MINOR_VERSION", "PYTHON_RELEASE" of sorts, or just have a single
> dot separated version string.

A dot separated version string would work for me, too.  I don't
mind the format that much, because I expect the new variable to
become unnecessary in the next year.  :)

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On 11/9/18 1:25 PM, Eduardo Habkost wrote:
> On Fri, Nov 09, 2018 at 11:39:32AM -0500, Cleber Rosa wrote:
>>
>>
>> On 11/9/18 10:49 AM, Eduardo Habkost wrote:
>>> On Fri, Nov 09, 2018 at 10:07:07AM -0500, Cleber Rosa wrote:
>>>> Some functionality is dependent on the Python version
>>>> detected/configured on configure.  While it's possible to run the
>>>> Python version later and check for the version, doing it once is
>>>> preferable.  Also, it's a relevant information to keep in build logs,
>>>> as the overall behavior of the build can be affected by it.
>>>>
>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>> ---
>>>>  configure | 6 +++++-
>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 74e313a810..67fff0290d 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
>>>>        "Use --python=/path/to/python to specify a supported Python."
>>>>  fi
>>>>  
>>>> +# Preserve python version since some functionality is dependent on it
>>>> +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
>>>
>>> What about:
>>>   $($python -c 'import sys;print(sys.version)')
>>> ?
>>>
>>> It is very verbose, but I think that's a good thing.
>>>
>>
>> Well, something like:
>>
>> '3.7.1 (default, Oct 23 2018, 18:19:07) \n[GCC 8.2.1 20180801 (Red Hat
>> 8.2.1-2)]'
>>
>> Doesn't qualify as simply the Python *version*.  The documentation[1]
>> puts it clearly: "A string containing the version number of the Python
>> interpreter plus additional information on the build number and compiler
>> used. This string is displayed when the interactive interpreter is started."
>>
>> I can't see how the compiler used on the Python build, or the build
>> date, can be significant to someone debugging the type of Python code
>> that will be on QEMU.
> 
> No problem to me.
> 
>>
>>>> +
>>>>  # Suppress writing compiled files
>>>>  python="$python -B"
>>>>  
>>>> @@ -5918,7 +5921,7 @@ echo "LDFLAGS           $LDFLAGS"
>>>>  echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
>>>>  echo "make              $make"
>>>>  echo "install           $install"
>>>> -echo "python            $python"
>>>> +echo "python            $python ($python_version)"
>>>>  if test "$slirp" = "yes" ; then
>>>>      echo "smbd              $smbd"
>>>>  fi
>>>> @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
>>>>  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
>>>>  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
>>>>  echo "PYTHON=$python" >> $config_host_mak
>>>> +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
>>>
>>> The output of "python -V" and "sys.version" seems to be meant for
>>> humans, not software.  If we really want something to be used in
>>> conditional makefile rules, I'd prefer to use sys.version_info.
>>> e.g.:
>>>
>>
>> "Python -V" is quite different from "sys.version".  Indeed it includes
>> the "Python " prefix, but that's all, everything else is the version number.
> 
> Is this a guarantee documented somewhere?
> 
>>
>>>   python_major_version="$($python -c 'import sys;print(sys.version_info[0])')"
>>>   echo "PYTHON_MAJOR_VERSION=$python_major_version"
>>>
>>>
>>
>> No, I'm actually interested in the other version components, not just
>> the major version.  As I tried to explain in another thread, differences
>> from Python 3.0 to 3.4 are many, and from 3.4 to 3.6 and so on.
> 
> Do you have any examples in mind?  I really doubt we will need to
> look at the Python minor version number inside shell scripts or
> makefiles.
> 

I guess I failed to make myself clear, because it looks like you're
missing my point here.  The situation that I envision is a developer
writes a Python based test, runs it locally, gets a PASS, puts in a
patch, sends it upstream.  It gets executed on a number of different CI
environments.  It fails on one (or some).  What's causing the failure?
The Python *minor* version may be a significant difference.

Like I said before, printing that is the most important functionality at
this point.  It is indeed debatable whether we'll need to keep it
available for make/shell usage.

>>
>> Then, we can either introduce "PYTHON_MAJOR_VERSION",
>> "PYTHON_MINOR_VERSION", "PYTHON_RELEASE" of sorts, or just have a single
>> dot separated version string.
> 
> A dot separated version string would work for me, too.  I don't
> mind the format that much, because I expect the new variable to
> become unnecessary in the next year.  :)
> 

Cool.  Thanks for the feedback.  Just to get this right, what do I need
to change here?

- Cleber.

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On 11/9/18 1:25 PM, Eduardo Habkost wrote:
>>
>> "Python -V" is quite different from "sys.version".  Indeed it includes
>> the "Python " prefix, but that's all, everything else is the version number.
> 
> Is this a guarantee documented somewhere?
> 

Oops, looks like I missed that comment, and failed to address it.  Now,
I must say I don't expect a documented guarantee about this will exist,
but it looks like this is an acknowledged use case:

https://bugs.python.org/issue18338

- Cleber.

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 02:51:11PM -0500, Cleber Rosa wrote:
> 
> 
> On 11/9/18 1:25 PM, Eduardo Habkost wrote:
> >>
> >> "Python -V" is quite different from "sys.version".  Indeed it includes
> >> the "Python " prefix, but that's all, everything else is the version number.
> > 
> > Is this a guarantee documented somewhere?
> > 
> 
> Oops, looks like I missed that comment, and failed to address it.  Now,
> I must say I don't expect a documented guarantee about this will exist,
> but it looks like this is an acknowledged use case:
> 
> https://bugs.python.org/issue18338

Well, considering that it even has a unit test checking for a
specific format[1], I think the usage in this patch can be
considered acceptable.

[1] https://hg.python.org/cpython/rev/e6384b8b2325

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 10:07:07AM -0500, Cleber Rosa wrote:
> Some functionality is dependent on the Python version
> detected/configured on configure.  While it's possible to run the
> Python version later and check for the version, doing it once is
> preferable.  Also, it's a relevant information to keep in build logs,
> as the overall behavior of the build can be affected by it.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Peter Maydell]

On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> wrote:
>
> Some functionality is dependent on the Python version
> detected/configured on configure.  While it's possible to run the
> Python version later and check for the version, doing it once is
> preferable.  Also, it's a relevant information to keep in build logs,
> as the overall behavior of the build can be affected by it.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  configure | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 74e313a810..67fff0290d 100755
> --- a/configure
> +++ b/configure
> @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
>        "Use --python=/path/to/python to specify a supported Python."
>  fi
>
> +# Preserve python version since some functionality is dependent on it
> +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> +

Hi. Somebody on IRC has just fallen over a problem where
their python's "-V" output prints multiple lines, which
means that "$python_version" here is multiple lines, which
means that the eventual config-host.mak has invalid syntax
because we assume here:

> @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
>  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
>  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
>  echo "PYTHON=$python" >> $config_host_mak
> +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
>  echo "CC=$cc" >> $config_host_mak
>  if $iasl -h > /dev/null 2>&1; then
>    echo "IASL=$iasl" >> $config_host_mak

that it's only one line, and will generate bogus makefile
syntax if it's got an embedded newline. (Problem system
seems to be Fedora 29.)

I've reread this thread, where there seems to have been
some discussion about just running Python itself to
get the sys.version value (which is how we check for
"is this python too old" earlier in the configure script).
But I'm not really clear why trying to parse -V output is better:
it's definitely less reliable, as demonstrated by this bug.

Given that the only thing as far as I can tell that we
do with PYTHON_VERSION is use it in tests/Makefile.inc
to suppress a bit of test functionality if we don't have
Python 3, could we stop trying to parse -V output and run
python to print sys.version_info instead, and/or just
have the makefile variable track "is this python 2",
since that's what we really care about and would mean we
don't have to then search the string for "v2"  ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> wrote:
> >
> > Some functionality is dependent on the Python version
> > detected/configured on configure.  While it's possible to run the
> > Python version later and check for the version, doing it once is
> > preferable.  Also, it's a relevant information to keep in build logs,
> > as the overall behavior of the build can be affected by it.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  configure | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure b/configure
> > index 74e313a810..67fff0290d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> >        "Use --python=/path/to/python to specify a supported Python."
> >  fi
> >
> > +# Preserve python version since some functionality is dependent on it
> > +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > +
> 
> Hi. Somebody on IRC has just fallen over a problem where
> their python's "-V" output prints multiple lines, which
> means that "$python_version" here is multiple lines, which
> means that the eventual config-host.mak has invalid syntax
> because we assume here:
>

We've tried a number of things, and just when I thought we wouldn't be
able to make any sense out of it, I arrived at a still senseless but
precise reproducer.  TL;DR: it has to do with interactive shells and
that exact Python build.

Reproducer (docker may also do the trick here):

  $ podman run --rm -ti fedora:29 /bin/bash -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
  Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
  [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)]

With an interactive shell instead:

  $ podman run --rm -ti fedora:29 /bin/bash -i -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
  Python 3.7.0

How this behavior came to be, baffles me.  But, it seems to be fixed
on newer versions.

> > @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
> >  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> >  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> >  echo "PYTHON=$python" >> $config_host_mak
> > +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> >  echo "CC=$cc" >> $config_host_mak
> >  if $iasl -h > /dev/null 2>&1; then
> >    echo "IASL=$iasl" >> $config_host_mak
> 
> that it's only one line, and will generate bogus makefile
> syntax if it's got an embedded newline. (Problem system
> seems to be Fedora 29.)
>

The assumption could be guaranteed by a "head -1", and while
it's not a failproof solution, it would at least not corrupt
the makefile and the whole build system.

> I've reread this thread, where there seems to have been
> some discussion about just running Python itself to
> get the sys.version value (which is how we check for
> "is this python too old" earlier in the configure script).
> But I'm not really clear why trying to parse -V output is better:
> it's definitely less reliable, as demonstrated by this bug.
>
> Given that the only thing as far as I can tell that we
> do with PYTHON_VERSION is use it in tests/Makefile.inc
> to suppress a bit of test functionality if we don't have
> Python 3, could we stop trying to parse -V output and run
> python to print sys.version_info instead, and/or just
> have the makefile variable track "is this python 2",
> since that's what we really care about and would mean we
> don't have to then search the string for "v2"  ?

Because I've been bitten way too many times with differences in Python
minor versions, I see a lot of value in keeping the version
information in the build system.  But, the same information can
certainly be obtained in a more resilient way.  Would you object something
like:

  python_version=$($python -c 'import sys; print(sys.version().split()[0])')

Or an even more paranoid version?  On my side, I understand the
fragility of the current approach, but I also appreciate the
information it stores.

> 
> thanks
> -- PMM

Thanks!
- Cleber.

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Eduardo Habkost]

On Thu, Aug 22, 2019 at 05:19:26PM -0400, Cleber Rosa wrote:
> On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> > On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> wrote:
> > >
> > > Some functionality is dependent on the Python version
> > > detected/configured on configure.  While it's possible to run the
> > > Python version later and check for the version, doing it once is
> > > preferable.  Also, it's a relevant information to keep in build logs,
> > > as the overall behavior of the build can be affected by it.
> > >
> > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > ---
> > >  configure | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/configure b/configure
> > > index 74e313a810..67fff0290d 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> > >        "Use --python=/path/to/python to specify a supported Python."
> > >  fi
> > >
> > > +# Preserve python version since some functionality is dependent on it
> > > +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > > +
> > 
> > Hi. Somebody on IRC has just fallen over a problem where
> > their python's "-V" output prints multiple lines, which
> > means that "$python_version" here is multiple lines, which
> > means that the eventual config-host.mak has invalid syntax
> > because we assume here:
> >
> 
> We've tried a number of things, and just when I thought we wouldn't be
> able to make any sense out of it, I arrived at a still senseless but
> precise reproducer.  TL;DR: it has to do with interactive shells and
> that exact Python build.
> 
> Reproducer (docker may also do the trick here):
> 
>   $ podman run --rm -ti fedora:29 /bin/bash -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
>   Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
>   [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)]
> 
> With an interactive shell instead:
> 
>   $ podman run --rm -ti fedora:29 /bin/bash -i -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
>   Python 3.7.0
> 
> How this behavior came to be, baffles me.  But, it seems to be fixed
> on newer versions.
> 
> > > @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
> > >  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> > >  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> > >  echo "PYTHON=$python" >> $config_host_mak
> > > +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> > >  echo "CC=$cc" >> $config_host_mak
> > >  if $iasl -h > /dev/null 2>&1; then
> > >    echo "IASL=$iasl" >> $config_host_mak
> > 
> > that it's only one line, and will generate bogus makefile
> > syntax if it's got an embedded newline. (Problem system
> > seems to be Fedora 29.)
> >
> 
> The assumption could be guaranteed by a "head -1", and while
> it's not a failproof solution, it would at least not corrupt
> the makefile and the whole build system.
> 
> > I've reread this thread, where there seems to have been
> > some discussion about just running Python itself to
> > get the sys.version value (which is how we check for
> > "is this python too old" earlier in the configure script).
> > But I'm not really clear why trying to parse -V output is better:
> > it's definitely less reliable, as demonstrated by this bug.

Agreed.

> >
> > Given that the only thing as far as I can tell that we
> > do with PYTHON_VERSION is use it in tests/Makefile.inc
> > to suppress a bit of test functionality if we don't have
> > Python 3, could we stop trying to parse -V output and run
> > python to print sys.version_info instead, and/or just
> > have the makefile variable track "is this python 2",
> > since that's what we really care about and would mean we
> > don't have to then search the string for "v2"  ?
> 
> Because I've been bitten way too many times with differences in Python
> minor versions, I see a lot of value in keeping the version
> information in the build system.  But, the same information can
> certainly be obtained in a more resilient way.  Would you object something
> like:
> 
>   python_version=$($python -c 'import sys; print(sys.version().split()[0])')

Sounds much better, but why sys.version().split() instead of
sys.version_info?

  python_version=$($python -c 'import sys; print(sys.version_info[0])')

> 
> Or an even more paranoid version?  On my side, I understand the
> fragility of the current approach, but I also appreciate the
> information it stores.

We have only one place where $(PYTHON_VERSION) is used, and that
code will be removed once we stop supporting Python 2.  I don't
see the point of trying to store extra information that is not
used anywhere in our makefiles.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On Thu, Aug 22, 2019 at 06:54:20PM -0300, Eduardo Habkost wrote:
> On Thu, Aug 22, 2019 at 05:19:26PM -0400, Cleber Rosa wrote:
> > On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> > > On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> wrote:
> > > >
> > > > Some functionality is dependent on the Python version
> > > > detected/configured on configure.  While it's possible to run the
> > > > Python version later and check for the version, doing it once is
> > > > preferable.  Also, it's a relevant information to keep in build logs,
> > > > as the overall behavior of the build can be affected by it.
> > > >
> > > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > > ---
> > > >  configure | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/configure b/configure
> > > > index 74e313a810..67fff0290d 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> > > >        "Use --python=/path/to/python to specify a supported Python."
> > > >  fi
> > > >
> > > > +# Preserve python version since some functionality is dependent on it
> > > > +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > > > +
> > > 
> > > Hi. Somebody on IRC has just fallen over a problem where
> > > their python's "-V" output prints multiple lines, which
> > > means that "$python_version" here is multiple lines, which
> > > means that the eventual config-host.mak has invalid syntax
> > > because we assume here:
> > >
> > 
> > We've tried a number of things, and just when I thought we wouldn't be
> > able to make any sense out of it, I arrived at a still senseless but
> > precise reproducer.  TL;DR: it has to do with interactive shells and
> > that exact Python build.
> > 
> > Reproducer (docker may also do the trick here):
> > 
> >   $ podman run --rm -ti fedora:29 /bin/bash -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
> >   Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
> >   [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)]
> > 
> > With an interactive shell instead:
> > 
> >   $ podman run --rm -ti fedora:29 /bin/bash -i -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
> >   Python 3.7.0
> > 
> > How this behavior came to be, baffles me.  But, it seems to be fixed
> > on newer versions.
> > 
> > > > @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
> > > >  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> > > >  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> > > >  echo "PYTHON=$python" >> $config_host_mak
> > > > +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> > > >  echo "CC=$cc" >> $config_host_mak
> > > >  if $iasl -h > /dev/null 2>&1; then
> > > >    echo "IASL=$iasl" >> $config_host_mak
> > > 
> > > that it's only one line, and will generate bogus makefile
> > > syntax if it's got an embedded newline. (Problem system
> > > seems to be Fedora 29.)
> > >
> > 
> > The assumption could be guaranteed by a "head -1", and while
> > it's not a failproof solution, it would at least not corrupt
> > the makefile and the whole build system.
> > 
> > > I've reread this thread, where there seems to have been
> > > some discussion about just running Python itself to
> > > get the sys.version value (which is how we check for
> > > "is this python too old" earlier in the configure script).
> > > But I'm not really clear why trying to parse -V output is better:
> > > it's definitely less reliable, as demonstrated by this bug.
> 
> Agreed.
> 
> > >
> > > Given that the only thing as far as I can tell that we
> > > do with PYTHON_VERSION is use it in tests/Makefile.inc
> > > to suppress a bit of test functionality if we don't have
> > > Python 3, could we stop trying to parse -V output and run
> > > python to print sys.version_info instead, and/or just
> > > have the makefile variable track "is this python 2",
> > > since that's what we really care about and would mean we
> > > don't have to then search the string for "v2"  ?
> > 
> > Because I've been bitten way too many times with differences in Python
> > minor versions, I see a lot of value in keeping the version
> > information in the build system.  But, the same information can
> > certainly be obtained in a more resilient way.  Would you object something
> > like:
> > 
> >   python_version=$($python -c 'import sys; print(sys.version().split()[0])')
> 
> Sounds much better, but why sys.version().split() instead of
> sys.version_info?
> 
>   python_version=$($python -c 'import sys; print(sys.version_info[0])')
>

I meant to capture not only the major version, but the minor and release
as well.  My reasoning (may not appeal more people):

 "Because I've been bitten way too many times with differences in Python
  minor versions, I see a lot of value in keeping the version
  information in the build system."

> > 
> > Or an even more paranoid version?  On my side, I understand the
> > fragility of the current approach, but I also appreciate the
> > information it stores.
> 
> We have only one place where $(PYTHON_VERSION) is used, and that
> code will be removed once we stop supporting Python 2.  I don't
> see the point of trying to store extra information that is not
> used anywhere in our makefiles.
>
> -- 
> Eduardo
> 

I see it being used by humans, so that brings a lot of subjetivity
into the matter.  IMO this is not out of place within the build
system, given that a lot of requirements detected by configure will
print out their versions (GTK, nettle, spice, etc).

But I'm certainly OK with dropping it if no value is perceived by
anyone else.

Cheers,
- Cleber.

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Peter Maydell]

On Fri, 23 Aug 2019 at 14:41, Cleber Rosa <crosa@redhat.com> wrote:
> I see it being used by humans, so that brings a lot of subjetivity
> into the matter.  IMO this is not out of place within the build
> system, given that a lot of requirements detected by configure will
> print out their versions (GTK, nettle, spice, etc).
>
> But I'm certainly OK with dropping it if no value is perceived by
> anyone else.

I'd be happy with keeping it in the human-readable output
that configure emits: if it's the wrong format there that's
pretty harmless. But we shouldn't feed it into the makefiles
unless we really need it, and we shouldn't let the format
of whatever we do feed into the makefiles be driven by
the desire to print something human-readable in configure's
output -- there's no need for the two things to be the
exact same string.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On Fri, Aug 23, 2019 at 02:44:15PM +0100, Peter Maydell wrote:
> On Fri, 23 Aug 2019 at 14:41, Cleber Rosa <crosa@redhat.com> wrote:
> > I see it being used by humans, so that brings a lot of subjetivity
> > into the matter.  IMO this is not out of place within the build
> > system, given that a lot of requirements detected by configure will
> > print out their versions (GTK, nettle, spice, etc).
> >
> > But I'm certainly OK with dropping it if no value is perceived by
> > anyone else.
> 
> I'd be happy with keeping it in the human-readable output
> that configure emits: if it's the wrong format there that's
> pretty harmless. But we shouldn't feed it into the makefiles
> unless we really need it, and we shouldn't let the format
> of whatever we do feed into the makefiles be driven by
> the desire to print something human-readable in configure's
> output -- there's no need for the two things to be the
> exact same string.
> 
> thanks
> -- PMM

I couldn't agree more.  The shortcut taken to print both the human
readable version and use that to check the version in the makefile was
unfortunate.

I'll send a fix proposal in a few.

Best,
- Cleber.

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Eduardo Habkost]

On Fri, Aug 23, 2019 at 09:40:54AM -0400, Cleber Rosa wrote:
> On Thu, Aug 22, 2019 at 06:54:20PM -0300, Eduardo Habkost wrote:
> > On Thu, Aug 22, 2019 at 05:19:26PM -0400, Cleber Rosa wrote:
> > > On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> > > > On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> wrote:
> > > > >
> > > > > Some functionality is dependent on the Python version
> > > > > detected/configured on configure.  While it's possible to run the
> > > > > Python version later and check for the version, doing it once is
> > > > > preferable.  Also, it's a relevant information to keep in build logs,
> > > > > as the overall behavior of the build can be affected by it.
> > > > >
> > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > > > ---
> > > > >  configure | 6 +++++-
> > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/configure b/configure
> > > > > index 74e313a810..67fff0290d 100755
> > > > > --- a/configure
> > > > > +++ b/configure
> > > > > @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> > > > >        "Use --python=/path/to/python to specify a supported Python."
> > > > >  fi
> > > > >
> > > > > +# Preserve python version since some functionality is dependent on it
> > > > > +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > > > > +
> > > > 
> > > > Hi. Somebody on IRC has just fallen over a problem where
> > > > their python's "-V" output prints multiple lines, which
> > > > means that "$python_version" here is multiple lines, which
> > > > means that the eventual config-host.mak has invalid syntax
> > > > because we assume here:
> > > >
> > > 
> > > We've tried a number of things, and just when I thought we wouldn't be
> > > able to make any sense out of it, I arrived at a still senseless but
> > > precise reproducer.  TL;DR: it has to do with interactive shells and
> > > that exact Python build.
> > > 
> > > Reproducer (docker may also do the trick here):
> > > 
> > >   $ podman run --rm -ti fedora:29 /bin/bash -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
> > >   Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
> > >   [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)]
> > > 
> > > With an interactive shell instead:
> > > 
> > >   $ podman run --rm -ti fedora:29 /bin/bash -i -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
> > >   Python 3.7.0
> > > 
> > > How this behavior came to be, baffles me.  But, it seems to be fixed
> > > on newer versions.
> > > 
> > > > > @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
> > > > >  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> > > > >  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> > > > >  echo "PYTHON=$python" >> $config_host_mak
> > > > > +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> > > > >  echo "CC=$cc" >> $config_host_mak
> > > > >  if $iasl -h > /dev/null 2>&1; then
> > > > >    echo "IASL=$iasl" >> $config_host_mak
> > > > 
> > > > that it's only one line, and will generate bogus makefile
> > > > syntax if it's got an embedded newline. (Problem system
> > > > seems to be Fedora 29.)
> > > >
> > > 
> > > The assumption could be guaranteed by a "head -1", and while
> > > it's not a failproof solution, it would at least not corrupt
> > > the makefile and the whole build system.
> > > 
> > > > I've reread this thread, where there seems to have been
> > > > some discussion about just running Python itself to
> > > > get the sys.version value (which is how we check for
> > > > "is this python too old" earlier in the configure script).
> > > > But I'm not really clear why trying to parse -V output is better:
> > > > it's definitely less reliable, as demonstrated by this bug.
> > 
> > Agreed.
> > 
> > > >
> > > > Given that the only thing as far as I can tell that we
> > > > do with PYTHON_VERSION is use it in tests/Makefile.inc
> > > > to suppress a bit of test functionality if we don't have
> > > > Python 3, could we stop trying to parse -V output and run
> > > > python to print sys.version_info instead, and/or just
> > > > have the makefile variable track "is this python 2",
> > > > since that's what we really care about and would mean we
> > > > don't have to then search the string for "v2"  ?
> > > 
> > > Because I've been bitten way too many times with differences in Python
> > > minor versions, I see a lot of value in keeping the version
> > > information in the build system.  But, the same information can
> > > certainly be obtained in a more resilient way.  Would you object something
> > > like:
> > > 
> > >   python_version=$($python -c 'import sys; print(sys.version().split()[0])')
> > 
> > Sounds much better, but why sys.version().split() instead of
> > sys.version_info?
> > 
> >   python_version=$($python -c 'import sys; print(sys.version_info[0])')
> >
> 
> I meant to capture not only the major version, but the minor and release
> as well.  My reasoning (may not appeal more people):
> 
>  "Because I've been bitten way too many times with differences in Python
>   minor versions, I see a lot of value in keeping the version
>   information in the build system."
> 
> > > 
> > > Or an even more paranoid version?  On my side, I understand the
> > > fragility of the current approach, but I also appreciate the
> > > information it stores.
> > 
> > We have only one place where $(PYTHON_VERSION) is used, and that
> > code will be removed once we stop supporting Python 2.  I don't
> > see the point of trying to store extra information that is not
> > used anywhere in our makefiles.
[...]
> 
> I see it being used by humans, so that brings a lot of subjetivity
> into the matter.  IMO this is not out of place within the build
> system, given that a lot of requirements detected by configure will
> print out their versions (GTK, nettle, spice, etc).

Absolutely, but are we talking about the output printed by
./configure, or about variables in config-host.mak?

config-host.mak is not for humans, it's just input for our
makefile code.  The output printed by ./configure on stdout is
for humans, and I'd agree completely if ./configure keeps
printing full Python version information on stdout.

> [...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On Fri, Aug 23, 2019 at 12:04:46PM -0300, Eduardo Habkost wrote:
> On Fri, Aug 23, 2019 at 09:40:54AM -0400, Cleber Rosa wrote:
> > On Thu, Aug 22, 2019 at 06:54:20PM -0300, Eduardo Habkost wrote:
> > > On Thu, Aug 22, 2019 at 05:19:26PM -0400, Cleber Rosa wrote:
> > > > On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> > > > > On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> wrote:
> > > > > >
> > > > > > Some functionality is dependent on the Python version
> > > > > > detected/configured on configure.  While it's possible to run the
> > > > > > Python version later and check for the version, doing it once is
> > > > > > preferable.  Also, it's a relevant information to keep in build logs,
> > > > > > as the overall behavior of the build can be affected by it.
> > > > > >
> > > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > > > > ---
> > > > > >  configure | 6 +++++-
> > > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/configure b/configure
> > > > > > index 74e313a810..67fff0290d 100755
> > > > > > --- a/configure
> > > > > > +++ b/configure
> > > > > > @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> > > > > >        "Use --python=/path/to/python to specify a supported Python."
> > > > > >  fi
> > > > > >
> > > > > > +# Preserve python version since some functionality is dependent on it
> > > > > > +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > > > > > +
> > > > > 
> > > > > Hi. Somebody on IRC has just fallen over a problem where
> > > > > their python's "-V" output prints multiple lines, which
> > > > > means that "$python_version" here is multiple lines, which
> > > > > means that the eventual config-host.mak has invalid syntax
> > > > > because we assume here:
> > > > >
> > > > 
> > > > We've tried a number of things, and just when I thought we wouldn't be
> > > > able to make any sense out of it, I arrived at a still senseless but
> > > > precise reproducer.  TL;DR: it has to do with interactive shells and
> > > > that exact Python build.
> > > > 
> > > > Reproducer (docker may also do the trick here):
> > > > 
> > > >   $ podman run --rm -ti fedora:29 /bin/bash -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
> > > >   Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
> > > >   [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)]
> > > > 
> > > > With an interactive shell instead:
> > > > 
> > > >   $ podman run --rm -ti fedora:29 /bin/bash -i -c 'dnf -y install http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm; python3 -V'
> > > >   Python 3.7.0
> > > > 
> > > > How this behavior came to be, baffles me.  But, it seems to be fixed
> > > > on newer versions.
> > > > 
> > > > > > @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
> > > > > >  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> > > > > >  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> > > > > >  echo "PYTHON=$python" >> $config_host_mak
> > > > > > +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> > > > > >  echo "CC=$cc" >> $config_host_mak
> > > > > >  if $iasl -h > /dev/null 2>&1; then
> > > > > >    echo "IASL=$iasl" >> $config_host_mak
> > > > > 
> > > > > that it's only one line, and will generate bogus makefile
> > > > > syntax if it's got an embedded newline. (Problem system
> > > > > seems to be Fedora 29.)
> > > > >
> > > > 
> > > > The assumption could be guaranteed by a "head -1", and while
> > > > it's not a failproof solution, it would at least not corrupt
> > > > the makefile and the whole build system.
> > > > 
> > > > > I've reread this thread, where there seems to have been
> > > > > some discussion about just running Python itself to
> > > > > get the sys.version value (which is how we check for
> > > > > "is this python too old" earlier in the configure script).
> > > > > But I'm not really clear why trying to parse -V output is better:
> > > > > it's definitely less reliable, as demonstrated by this bug.
> > > 
> > > Agreed.
> > > 
> > > > >
> > > > > Given that the only thing as far as I can tell that we
> > > > > do with PYTHON_VERSION is use it in tests/Makefile.inc
> > > > > to suppress a bit of test functionality if we don't have
> > > > > Python 3, could we stop trying to parse -V output and run
> > > > > python to print sys.version_info instead, and/or just
> > > > > have the makefile variable track "is this python 2",
> > > > > since that's what we really care about and would mean we
> > > > > don't have to then search the string for "v2"  ?
> > > > 
> > > > Because I've been bitten way too many times with differences in Python
> > > > minor versions, I see a lot of value in keeping the version
> > > > information in the build system.  But, the same information can
> > > > certainly be obtained in a more resilient way.  Would you object something
> > > > like:
> > > > 
> > > >   python_version=$($python -c 'import sys; print(sys.version().split()[0])')
> > > 
> > > Sounds much better, but why sys.version().split() instead of
> > > sys.version_info?
> > > 
> > >   python_version=$($python -c 'import sys; print(sys.version_info[0])')
> > >
> > 
> > I meant to capture not only the major version, but the minor and release
> > as well.  My reasoning (may not appeal more people):
> > 
> >  "Because I've been bitten way too many times with differences in Python
> >   minor versions, I see a lot of value in keeping the version
> >   information in the build system."
> > 
> > > > 
> > > > Or an even more paranoid version?  On my side, I understand the
> > > > fragility of the current approach, but I also appreciate the
> > > > information it stores.
> > > 
> > > We have only one place where $(PYTHON_VERSION) is used, and that
> > > code will be removed once we stop supporting Python 2.  I don't
> > > see the point of trying to store extra information that is not
> > > used anywhere in our makefiles.
> [...]
> > 
> > I see it being used by humans, so that brings a lot of subjetivity
> > into the matter.  IMO this is not out of place within the build
> > system, given that a lot of requirements detected by configure will
> > print out their versions (GTK, nettle, spice, etc).
> 
> Absolutely, but are we talking about the output printed by
> ./configure, or about variables in config-host.mak?
>

Sure, that was a major oversight (or a ill planned shortcut).

> config-host.mak is not for humans, it's just input for our
> makefile code.  The output printed by ./configure on stdout is
> for humans, and I'd agree completely if ./configure keeps
> printing full Python version information on stdout.
> 
> > [...]
> 
> -- 
> Eduardo
> 

Yes, absolutely agree.  I'll send a fix proposal in a few.

Regards!
- Cleber.

[Qemu-devel] [PATCH 2/4] check-venv: use recorded Python version

[Cleber Rosa]

The current approach works fine, but it runs Python on every make
command (even if it's not related to the venv usage).

This is just an optimization, and not a change of behavior.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/Makefile.include | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 074eece558..c0a341c923 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -913,8 +913,7 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
 # information please refer to "avocado --help".
 AVOCADO_SHOW=none
 
-PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= (3, 0) else 0)')
-ifeq ($(PYTHON3), 1)
+ifneq ($(findstring v2,"v$(PYTHON_VERSION)"),v2)
 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
 	$(call quiet-command, \
             $(PYTHON) -m venv --system-site-packages $@, \
-- 
2.19.1

Re: [Qemu-devel] [PATCH 2/4] check-venv: use recorded Python version

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 10:07:08AM -0500, Cleber Rosa wrote:
> The current approach works fine, but it runs Python on every make
> command (even if it's not related to the venv usage).
> 
> This is just an optimization, and not a change of behavior.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

[Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Cleber Rosa]

For the two Python jobs, which seem to have the goal of making sure
QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
the specified version is only applicable if a Python virtual
environment is used.  To do that, it's necessary to define the
(primary?) language of the job to be Python.

Also, Travis doesn't have a 3.0 Python installation available for the
chosen distro, 3.2 being the lower version available.

Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index aa49c7b114..5c18d3e268 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -112,9 +112,11 @@ matrix:
       compiler: clang
     # Python builds
     - env: CONFIG="--target-list=x86_64-softmmu"
+      language: python
       python:
-        - "3.0"
+        - "3.2"
     - env: CONFIG="--target-list=x86_64-softmmu"
+      language: python
       python:
         - "3.6"
     # Acceptance (Functional) tests
-- 
2.19.1

Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Alex Bennée]

Cleber Rosa <crosa@redhat.com> writes:

> For the two Python jobs, which seem to have the goal of making sure
> QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
> the specified version is only applicable if a Python virtual
> environment is used.  To do that, it's necessary to define the
> (primary?) language of the job to be Python.
>
> Also, Travis doesn't have a 3.0 Python installation available for the
> chosen distro, 3.2 being the lower version available.
>
> Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Do you want me to take this via my tree or keep it with the other patches?

> ---
>  .travis.yml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index aa49c7b114..5c18d3e268 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -112,9 +112,11 @@ matrix:
>        compiler: clang
>      # Python builds
>      - env: CONFIG="--target-list=x86_64-softmmu"
> +      language: python
>        python:
> -        - "3.0"
> +        - "3.2"
>      - env: CONFIG="--target-list=x86_64-softmmu"
> +      language: python
>        python:
>          - "3.6"
>      # Acceptance (Functional) tests


--
Alex Bennée

Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 03:52:49PM +0000, Alex Bennée wrote:
> 
> Cleber Rosa <crosa@redhat.com> writes:
> 
> > For the two Python jobs, which seem to have the goal of making sure
> > QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
> > the specified version is only applicable if a Python virtual
> > environment is used.  To do that, it's necessary to define the
> > (primary?) language of the job to be Python.
> >
> > Also, Travis doesn't have a 3.0 Python installation available for the
> > chosen distro, 3.2 being the lower version available.
> >
> > Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 
> Do you want me to take this via my tree or keep it with the other patches?

Considering that the rest of the series will be included only
after 3.1 and I don't have other Python patches queued for 3.1,
can you merge this one through your tree?  Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Philippe Mathieu-Daudé]

On 9/11/18 16:07, Cleber Rosa wrote:
> For the two Python jobs, which seem to have the goal of making sure
> QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
> the specified version is only applicable if a Python virtual
> environment is used.  To do that, it's necessary to define the
> (primary?) language of the job to be Python.
> 
> Also, Travis doesn't have a 3.0 Python installation available for the
> chosen distro, 3.2 being the lower version available.
> 
> Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions

On this link the lower version available is 3.3, isnt' it?

> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   .travis.yml | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index aa49c7b114..5c18d3e268 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -112,9 +112,11 @@ matrix:
>         compiler: clang
>       # Python builds
>       - env: CONFIG="--target-list=x86_64-softmmu"
> +      language: python
>         python:
> -        - "3.0"
> +        - "3.2"
>       - env: CONFIG="--target-list=x86_64-softmmu"
> +      language: python
>         python:
>           - "3.6"
>       # Acceptance (Functional) tests
> 

Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Cleber Rosa]

On 11/9/18 2:34 PM, Philippe Mathieu-Daudé wrote:
> On 9/11/18 16:07, Cleber Rosa wrote:
>> For the two Python jobs, which seem to have the goal of making sure
>> QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
>> the specified version is only applicable if a Python virtual
>> environment is used.  To do that, it's necessary to define the
>> (primary?) language of the job to be Python.
>>
>> Also, Travis doesn't have a 3.0 Python installation available for the
>> chosen distro, 3.2 being the lower version available.
>>
>> Reference:
>> https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
>>
> 
> On this link the lower version available is 3.3, isnt' it?

I found out that this list is not comprehensive.  It looks like it's
just an example.

- Cleber.

> 
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>   .travis.yml | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/.travis.yml b/.travis.yml
>> index aa49c7b114..5c18d3e268 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -112,9 +112,11 @@ matrix:
>>         compiler: clang
>>       # Python builds
>>       - env: CONFIG="--target-list=x86_64-softmmu"
>> +      language: python
>>         python:
>> -        - "3.0"
>> +        - "3.2"
>>       - env: CONFIG="--target-list=x86_64-softmmu"
>> +      language: python
>>         python:
>>           - "3.6"
>>       # Acceptance (Functional) tests
>>

Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 10:07:09AM -0500, Cleber Rosa wrote:
> For the two Python jobs, which seem to have the goal of making sure
> QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
> the specified version is only applicable if a Python virtual
> environment is used.  To do that, it's necessary to define the
> (primary?) language of the job to be Python.
> 
> Also, Travis doesn't have a 3.0 Python installation available for the
> chosen distro, 3.2 being the lower version available.
> 
> Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

This doesn't depend on patches 1 and 2, right?

The rest of the series will wait for the next release, but this
looks like something we want to merge before QEMU 3.1.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

[Cleber Rosa]

On 11/12/18 11:23 AM, Eduardo Habkost wrote:
> On Fri, Nov 09, 2018 at 10:07:09AM -0500, Cleber Rosa wrote:
>> For the two Python jobs, which seem to have the goal of making sure
>> QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
>> the specified version is only applicable if a Python virtual
>> environment is used.  To do that, it's necessary to define the
>> (primary?) language of the job to be Python.
>>
>> Also, Travis doesn't have a 3.0 Python installation available for the
>> chosen distro, 3.2 being the lower version available.
>>
>> Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> 
> This doesn't depend on patches 1 and 2, right?
> 

No, it doesn't.

> The rest of the series will wait for the next release, but this
> looks like something we want to merge before QEMU 3.1.
> 

Sounds good!

- Cleber.

[Qemu-devel] [PATCH 4/4] check-help: visual and content improvements

[Cleber Rosa]

The "check" target is not a target that will run all other tests
listed, so in order to be accurate it's necessary to list those that
will run.  The same is true for "check-clean".

Then, to give a better visual impression of the differences in the
various targets, let's add empty lines.

Finally, a small (and hopeful) grammar fix from a non-native speaker.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/Makefile.include | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index c0a341c923..552faf9bbe 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -3,7 +3,8 @@
 check-help:
 	@echo "Regression testing targets:"
 	@echo
-	@echo " $(MAKE) check                Run all tests"
+	@echo " $(MAKE) check                Run unit, qapi-schema, qtest and decodetree"
+	@echo
 	@echo " $(MAKE) check-qtest-TARGET   Run qtest tests for given target"
 	@echo " $(MAKE) check-qtest          Run qtest tests"
 	@echo " $(MAKE) check-unit           Run qobject tests"
@@ -12,12 +13,13 @@ check-help:
 	@echo " $(MAKE) check-block          Run block tests"
 	@echo " $(MAKE) check-tcg            Run TCG tests"
 	@echo " $(MAKE) check-acceptance     Run all acceptance (functional) tests"
+	@echo
 	@echo " $(MAKE) check-report.html    Generates an HTML test report"
 	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
-	@echo " $(MAKE) check-clean          Clean the tests"
+	@echo " $(MAKE) check-clean          Clean the tests and related data"
 	@echo
 	@echo "Please note that HTML reports do not regenerate if the unit tests"
-	@echo "has not changed."
+	@echo "have not changed."
 	@echo
 	@echo "The variable SPEED can be set to control the gtester speed setting."
 	@echo "Default options are -k and (for $(MAKE) V=1) --verbose; they can be"
-- 
2.19.1

Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements

[Eric Blake]

On 11/9/18 9:07 AM, Cleber Rosa wrote:
> The "check" target is not a target that will run all other tests
> listed, so in order to be accurate it's necessary to list those that
> will run.  The same is true for "check-clean".
> 
> Then, to give a better visual impression of the differences in the
> various targets, let's add empty lines.
> 
> Finally, a small (and hopeful) grammar fix from a non-native speaker.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/Makefile.include | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements

[Philippe Mathieu-Daudé]

On 9/11/18 16:07, Cleber Rosa wrote:
> The "check" target is not a target that will run all other tests
> listed, so in order to be accurate it's necessary to list those that
> will run.  The same is true for "check-clean".
> 
> Then, to give a better visual impression of the differences in the
> various targets, let's add empty lines.
> 
> Finally, a small (and hopeful) grammar fix from a non-native speaker.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   tests/Makefile.include | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index c0a341c923..552faf9bbe 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -3,7 +3,8 @@
>   check-help:
>   	@echo "Regression testing targets:"
>   	@echo
> -	@echo " $(MAKE) check                Run all tests"
> +	@echo " $(MAKE) check                Run unit, qapi-schema, qtest and decodetree"
> +	@echo
>   	@echo " $(MAKE) check-qtest-TARGET   Run qtest tests for given target"
>   	@echo " $(MAKE) check-qtest          Run qtest tests"
>   	@echo " $(MAKE) check-unit           Run qobject tests"
> @@ -12,12 +13,13 @@ check-help:
>   	@echo " $(MAKE) check-block          Run block tests"
>   	@echo " $(MAKE) check-tcg            Run TCG tests"
>   	@echo " $(MAKE) check-acceptance     Run all acceptance (functional) tests"
> +	@echo
>   	@echo " $(MAKE) check-report.html    Generates an HTML test report"
>   	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
> -	@echo " $(MAKE) check-clean          Clean the tests"
> +	@echo " $(MAKE) check-clean          Clean the tests and related data"
>   	@echo
>   	@echo "Please note that HTML reports do not regenerate if the unit tests"
> -	@echo "has not changed."
> +	@echo "have not changed."
>   	@echo
>   	@echo "The variable SPEED can be set to control the gtester speed setting."
>   	@echo "Default options are -k and (for $(MAKE) V=1) --verbose; they can be"
> 

Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements

[Eduardo Habkost]

On Fri, Nov 09, 2018 at 10:07:10AM -0500, Cleber Rosa wrote:
> The "check" target is not a target that will run all other tests
> listed, so in order to be accurate it's necessary to list those that
> will run.  The same is true for "check-clean".
> 
> Then, to give a better visual impression of the differences in the
> various targets, let's add empty lines.
> 
> Finally, a small (and hopeful) grammar fix from a non-native speaker.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements

[Wainer dos Santos Moschetta]

On 11/09/2018 01:07 PM, Cleber Rosa wrote:
> The "check" target is not a target that will run all other tests
> listed, so in order to be accurate it's necessary to list those that
> will run.  The same is true for "check-clean".
>
> Then, to give a better visual impression of the differences in the
> various targets, let's add empty lines.
>
> Finally, a small (and hopeful) grammar fix from a non-native speaker.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/Makefile.include | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index c0a341c923..552faf9bbe 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -3,7 +3,8 @@
>   check-help:
>   	@echo "Regression testing targets:"
>   	@echo
> -	@echo " $(MAKE) check                Run all tests"
> +	@echo " $(MAKE) check                Run unit, qapi-schema, qtest and decodetree"

Hi Cleber!

I would leave "tests" to the description, then it becomes:
"Run unit, qapi-schema, qtest and decodetree tests"

Note: there isn't an entry for check-decodetree on the help. You may 
want to address it in this patch (or I can send in a separate patch).

Overall, this patch series looks good for me. I tested patches 1, 2 and 
4 on Fedora 29. So:

Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

- Wainer

> +	@echo
>   	@echo " $(MAKE) check-qtest-TARGET   Run qtest tests for given target"
>   	@echo " $(MAKE) check-qtest          Run qtest tests"
>   	@echo " $(MAKE) check-unit           Run qobject tests"
> @@ -12,12 +13,13 @@ check-help:
>   	@echo " $(MAKE) check-block          Run block tests"
>   	@echo " $(MAKE) check-tcg            Run TCG tests"
>   	@echo " $(MAKE) check-acceptance     Run all acceptance (functional) tests"
> +	@echo
>   	@echo " $(MAKE) check-report.html    Generates an HTML test report"
>   	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
> -	@echo " $(MAKE) check-clean          Clean the tests"
> +	@echo " $(MAKE) check-clean          Clean the tests and related data"
>   	@echo
>   	@echo "Please note that HTML reports do not regenerate if the unit tests"
> -	@echo "has not changed."
> +	@echo "have not changed."
>   	@echo
>   	@echo "The variable SPEED can be set to control the gtester speed setting."
>   	@echo "Default options are -k and (for $(MAKE) V=1) --verbose; they can be"