Record Python version and misc test/CI fixes
cover Record Python version and misc test/CI fixes
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(-)
Signature · 1 line
2.19.1
PATCH 1/4 configure: keep track of Python version
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.
--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
configure +5 -1
@@ -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
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 16 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1744
Quoted 5 lines
configure +2
-0@@ -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.
Quoted 14 lines
configure +2
-1+ # 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
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6829
Quoted 4 lines
configure +1
-0@@ -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"
Quoted 6 lines
configure +0
-0echo "CC=$cc" >> $config_host_mak if $iasl -h > /dev/null 2>&1; then echo "IASL=$iasl" >> $config_host_makSignature · 1 line
2.19.1
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 20 lines · Eduardo Habkost
Quoted 19 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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
Quoted 9 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1744
Quoted 2 lines
configure +2
-0+# 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.
Quoted 15 lines
Quoted 15 lines
configure +2
-1+ # 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
Quoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6829
Quoted 3 lines
configure +1
-0echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak echo "PYTHON=$python" >> $config_host_mak +echo "PYTHON_VERSION=$python_version" >> $config_host_makThe 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.
Quoted 4 lines
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
Quoted 7 lines
Quoted 6 lines
configure +0
-0echo "CC=$cc" >> $config_host_mak if $iasl -h > /dev/null 2>&1; then echo "IASL=$iasl" >> $config_host_makSignature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 38 lines · Cleber Rosa
Quoted 20 lines · Eduardo Habkost
Quoted 19 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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." fiQuoted 9 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1744
Quoted 2 lines
configure +2
-0+# 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)]'
Quoted 8 lines
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.
Quoted 26 lines
Quoted 16 lines
Quoted 16 lines
configure +2
-1+ # 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_makQuoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6829
Quoted 3 lines
configure +1
-0echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak echo "PYTHON=$python" >> $config_host_mak +echo "PYTHON_VERSION=$python_version" >> $config_host_makThe 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.:
Quoted 2 lines
"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?
Quoted 9 lines
Quoted 5 lines
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.
Quoted 4 lines
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. :)
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 87 lines · Eduardo Habkost
Quoted 38 lines · Cleber Rosa
Quoted 20 lines · Eduardo Habkost
Quoted 19 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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." fiQuoted 9 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1744
Quoted 2 lines
configure +2
-0+# 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)]'
Quoted 8 lines
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.
Quoted 26 lines
Quoted 16 lines
Quoted 16 lines
configure +2
-1+ # 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_makQuoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6829
Quoted 3 lines
configure +1
-0echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak echo "PYTHON=$python" >> $config_host_mak +echo "PYTHON_VERSION=$python_version" >> $config_host_makThe 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.:
Quoted 2 lines
"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?
Quoted 6 lines
Quoted 5 lines
python_major_version="$($python -c 'import sys;print(sys.version_info[0])')" echo "PYTHON_MAJOR_VERSION=$python_major_version"
Quoted 8 lines
Quoted 3 lines
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.
Quoted 9 lines
Quoted 4 lines
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
In reply to Eduardo HabkostQuoted 6 lines · Eduardo Habkost
Quoted 3 lines
"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
Quoted 7 lines · Cleber Rosa
Quoted 4 lines · Eduardo Habkost
Quoted 3 lines
"Python -V" is quite different from "sys.version". Indeed it includes the "Python " prefix, but that's all, everything else is the version number.
Quoted 8 lines
Quoted 2 lines
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:
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
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
In reply to Cleber RosaQuoted 7 lines · 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 <[email protected]>
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
In reply to Cleber RosaQuoted 17 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1745
Quoted 6 lines
configure +3
-0@@ -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:
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6832
Quoted 8 lines
configure +1
-0@@ -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
Quoted 21 lines · Peter Maydell
Quoted 20 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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
Quoted 10 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1745
Quoted 3 lines
configure +3
-0+# 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.
Quoted 4 lines
Quoted 4 lines
configure +0
-0@@ -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
Quoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6832
Quoted 4 lines
configure +1
-0+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_makthat 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.
Quoted 7 lines
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.
Quoted 8 lines
Given that the only thing as far as I can tell that we do with
PYTHON_VERSIONis 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.
Quoted 3 lines
thanks
-- PMM
Thanks!
- Cleber.
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 66 lines · Cleber Rosa
Quoted 21 lines · Peter Maydell
Quoted 20 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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." fiQuoted 10 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1745
Quoted 3 lines
configure +3
-0+# 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.0How this behavior came to be, baffles me. But, it seems to be fixed on newer versions.
Quoted 4 lines
Quoted 4 lines
configure +0
-0@@ -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_makQuoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6832
Quoted 4 lines
configure +1
-0+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_makthat 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.)
Quoted 10 lines
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.
Quoted 6 lines
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.
Quoted 10 lines
Quoted 9 lines
Given that the only thing as far as I can tell that we do with
PYTHON_VERSIONis 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" ?
Quoted 7 lines
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])')
Quoted 4 lines
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.
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 96 lines · Eduardo Habkost
Quoted 66 lines · Cleber Rosa
Quoted 21 lines · Peter Maydell
Quoted 20 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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." fiQuoted 10 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1745
Quoted 3 lines
configure +3
-0+# 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.0How this behavior came to be, baffles me. But, it seems to be fixed on newer versions.
Quoted 4 lines
Quoted 4 lines
configure +0
-0@@ -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_makQuoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6832
Quoted 4 lines
configure +1
-0+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_makthat 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.)
Quoted 10 lines
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.
Quoted 6 lines
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.
Quoted 16 lines
Quoted 9 lines
Given that the only thing as far as I can tell that we do with
PYTHON_VERSIONis 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:
Quoted 7 lines
Quoted 1 line
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."
Quoted 13 lines
Quoted 4 lines
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.
Signature · 1 line
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
Quoted 7 lines · Cleber Rosa
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
Quoted 18 lines · Peter Maydell
Quoted 7 lines · Cleber Rosa
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.
Quoted 2 lines
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
In reply to Cleber RosaQuoted 121 lines · Cleber Rosa
Quoted 96 lines · Eduardo Habkost
Quoted 66 lines · Cleber Rosa
Quoted 21 lines · Peter Maydell
Quoted 20 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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." fiQuoted 10 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1745
Quoted 3 lines
configure +3
-0+# 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.0How this behavior came to be, baffles me. But, it seems to be fixed on newer versions.
Quoted 4 lines
Quoted 4 lines
configure +0
-0@@ -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_makQuoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6832
Quoted 4 lines
configure +1
-0+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_makthat 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.)
Quoted 10 lines
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.
Quoted 6 lines
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.
Quoted 16 lines
Quoted 9 lines
Given that the only thing as far as I can tell that we do with
PYTHON_VERSIONis 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:
Quoted 7 lines
Quoted 1 line
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."Quoted 9 lines
Quoted 4 lines
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.
[...]
Quoted 5 lines
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.
Quoted 1 line
[...]
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Quoted 124 lines · Eduardo Habkost
Quoted 121 lines · Cleber Rosa
Quoted 96 lines · Eduardo Habkost
Quoted 66 lines · Cleber Rosa
Quoted 21 lines · Peter Maydell
Quoted 20 lines · 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 <[email protected]>--- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)configure +0
-0@@ -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." fiQuoted 10 lines
configure if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then · line 1745
Quoted 3 lines
configure +3
-0+# 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.0How this behavior came to be, baffles me. But, it seems to be fixed on newer versions.
Quoted 4 lines
Quoted 4 lines
configure +0
-0@@ -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_makQuoted 9 lines
configure echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak · line 6832
Quoted 4 lines
configure +1
-0+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_makthat 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.)
Quoted 10 lines
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.
Quoted 6 lines
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.
Quoted 16 lines
Quoted 9 lines
Given that the only thing as far as I can tell that we do with
PYTHON_VERSIONis 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:
Quoted 7 lines
Quoted 1 line
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."Quoted 9 lines
Quoted 4 lines
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.
[...]
Quoted 1 line
Quoted 8 lines
Quoted 4 lines
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).
Quoted 10 lines
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.
Quoted 1 line
[...]
Signature · 1 line
Eduardo
Yes, absolutely agree. I'll send a fix proposal in a few.
Regards!
- Cleber.
PATCH 2/4 check-venv: use recorded Python version
In reply to Cleber RosaThe 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.
--- tests/Makefile.include | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
tests/Makefile.include +1 -2
@@ -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 $@, \
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH 2/4] check-venv: use recorded Python version
Quoted 6 lines · 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 <[email protected]>
Signature · 1 line
Eduardo
PATCH 3/4 Travis CI: make specified Python versions usable on jobs
In reply to Cleber RosaFor 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
--- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
.travis.yml +3 -1
@@ -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
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs
Quoted 11 lines · 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 <[email protected]>
Do you want me to take this via my tree or keep it with the other patches?
Quoted 11 lines
--- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-).travis.yml +0
-0@@ -112,9 +112,11 @@ matrix: compiler: clang # Python builds
.travis.yml matrix: · line 122
Quoted 10 lines
.travis.yml +3
-1- 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
Quoted 12 lines · Alex Bennée
Quoted 9 lines · 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.
Quoted 6 lines
Quoted 2 lines
Reference: https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
Signed-off-by Cleber Rosa <[email protected]>Reviewed-by Alex Bennée <[email protected]>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!
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs
In reply to Cleber RosaQuoted 10 lines · 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
On this link the lower version available is 3.3, isnt' it?
Quoted 23 lines
Signed-off-by Cleber Rosa <[email protected]>--- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-).travis.yml +3
-1@@ -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
Quoted 7 lines · Philippe Mathieu-Daudé
Quoted 6 lines · 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.
Quoted 8 lines
Quoted 6 lines
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-versionsOn 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.
Quoted 24 lines
Quoted 24 lines
Signed-off-by Cleber Rosa <[email protected]>---
.travis.yml | 4 +++-1 file changed, 3 insertions(+), 1 deletion(-).travis.yml +0
-0@@ -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
In reply to Cleber RosaQuoted 11 lines · 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 <[email protected]>
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.
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs
Quoted 7 lines · Eduardo Habkost
Quoted 6 lines · 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.
Quoted 8 lines
Quoted 5 lines
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 <[email protected]>This doesn't depend on patches 1 and 2, right?
No, it doesn't.
Quoted 3 lines
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.
PATCH 4/4 check-help: visual and content improvements
In reply to Cleber RosaThe "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.
--- tests/Makefile.include | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
tests/Makefile.include +5 -3
@@ -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"
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements
Quoted 7 lines · 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.
Quoted 6 lines
Finally, a small (and hopeful) grammar fix from a non-native speaker.
Signed-off-by Cleber Rosa <[email protected]>--- tests/Makefile.include | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Signature · 3 lines
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
In reply to Cleber RosaQuoted 10 lines · 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 <[email protected]>
Quoted 35 lines
--- tests/Makefile.include | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)tests/Makefile.include +5
-3@@ -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
In reply to Cleber RosaQuoted 10 lines · 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 <[email protected]>
Signature · 1 line
Eduardo
Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements
In reply to Cleber RosaQuoted 19 lines · 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 <[email protected]>--- tests/Makefile.include | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)tests/Makefile.include +0
-0
tests/Makefile.include line 6
Quoted 5 lines
tests/Makefile.include +1
-1@@ -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:
- Wainer
Quoted 20 lines
tests/Makefile.include +4
-2+ @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"