* [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes @ 2018-11-09 15:07 Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa ` (3 more replies) 0 siblings, 4 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 15:07 UTC (permalink / raw) To: qemu-devel Cc: Caio Carrara, Philippe Mathieu-Daudé, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng, 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 15:07 [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes Cleber Rosa @ 2018-11-09 15:07 ` Cleber Rosa 2018-11-09 15:49 ` Eduardo Habkost ` (2 more replies) 2018-11-09 15:07 ` [Qemu-devel] [PATCH 2/4] check-venv: use recorded " Cleber Rosa ` (2 subsequent siblings) 3 siblings, 3 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 15:07 UTC (permalink / raw) To: qemu-devel Cc: Caio Carrara, Philippe Mathieu-Daudé, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng, 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa @ 2018-11-09 15:49 ` Eduardo Habkost 2018-11-09 16:39 ` Cleber Rosa 2018-11-09 21:26 ` Eduardo Habkost 2019-08-22 16:48 ` Peter Maydell 2 siblings, 1 reply; 31+ messages in thread From: Eduardo Habkost @ 2018-11-09 15:49 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 15:49 ` Eduardo Habkost @ 2018-11-09 16:39 ` Cleber Rosa 2018-11-09 18:25 ` Eduardo Habkost 0 siblings, 1 reply; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 16:39 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 >> > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 16:39 ` Cleber Rosa @ 2018-11-09 18:25 ` Eduardo Habkost 2018-11-09 19:09 ` Cleber Rosa 2018-11-09 19:51 ` Cleber Rosa 0 siblings, 2 replies; 31+ messages in thread From: Eduardo Habkost @ 2018-11-09 18:25 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 18:25 ` Eduardo Habkost @ 2018-11-09 19:09 ` Cleber Rosa 2018-11-09 19:51 ` Cleber Rosa 1 sibling, 0 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 19:09 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 18:25 ` Eduardo Habkost 2018-11-09 19:09 ` Cleber Rosa @ 2018-11-09 19:51 ` Cleber Rosa 2018-11-09 21:25 ` Eduardo Habkost 1 sibling, 1 reply; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 19:51 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 19:51 ` Cleber Rosa @ 2018-11-09 21:25 ` Eduardo Habkost 0 siblings, 0 replies; 31+ messages in thread From: Eduardo Habkost @ 2018-11-09 21:25 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa 2018-11-09 15:49 ` Eduardo Habkost @ 2018-11-09 21:26 ` Eduardo Habkost 2019-08-22 16:48 ` Peter Maydell 2 siblings, 0 replies; 31+ messages in thread From: Eduardo Habkost @ 2018-11-09 21:26 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa 2018-11-09 15:49 ` Eduardo Habkost 2018-11-09 21:26 ` Eduardo Habkost @ 2019-08-22 16:48 ` Peter Maydell 2019-08-22 21:19 ` Cleber Rosa 2 siblings, 1 reply; 31+ messages in thread From: Peter Maydell @ 2019-08-22 16:48 UTC (permalink / raw) To: Cleber Rosa Cc: Fam Zheng, Eduardo Habkost, Alex Bennée, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, QEMU Developers, Caio Carrara, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-22 16:48 ` Peter Maydell @ 2019-08-22 21:19 ` Cleber Rosa 2019-08-22 21:54 ` Eduardo Habkost 0 siblings, 1 reply; 31+ messages in thread From: Cleber Rosa @ 2019-08-22 21:19 UTC (permalink / raw) To: Peter Maydell Cc: Eduardo Habkost, Philippe Mathieu-Daudé, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, QEMU Developers, Alex Bennée 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-22 21:19 ` Cleber Rosa @ 2019-08-22 21:54 ` Eduardo Habkost 2019-08-23 13:40 ` Cleber Rosa 0 siblings, 1 reply; 31+ messages in thread From: Eduardo Habkost @ 2019-08-22 21:54 UTC (permalink / raw) To: Cleber Rosa Cc: Peter Maydell, Philippe Mathieu-Daudé, QEMU Developers, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-22 21:54 ` Eduardo Habkost @ 2019-08-23 13:40 ` Cleber Rosa 2019-08-23 13:44 ` Peter Maydell 2019-08-23 15:04 ` Eduardo Habkost 0 siblings, 2 replies; 31+ messages in thread From: Cleber Rosa @ 2019-08-23 13:40 UTC (permalink / raw) To: Eduardo Habkost Cc: Peter Maydell, Philippe Mathieu-Daudé, QEMU Developers, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-23 13:40 ` Cleber Rosa @ 2019-08-23 13:44 ` Peter Maydell 2019-08-23 17:42 ` Cleber Rosa 2019-08-23 15:04 ` Eduardo Habkost 1 sibling, 1 reply; 31+ messages in thread From: Peter Maydell @ 2019-08-23 13:44 UTC (permalink / raw) To: Cleber Rosa Cc: Eduardo Habkost, Philippe Mathieu-Daudé, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, QEMU Developers, Alex Bennée 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-23 13:44 ` Peter Maydell @ 2019-08-23 17:42 ` Cleber Rosa 0 siblings, 0 replies; 31+ messages in thread From: Cleber Rosa @ 2019-08-23 17:42 UTC (permalink / raw) To: Peter Maydell Cc: Eduardo Habkost, Julio Montes, Philippe Mathieu-Daudé, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, QEMU Developers, Alex Bennée 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-23 13:40 ` Cleber Rosa 2019-08-23 13:44 ` Peter Maydell @ 2019-08-23 15:04 ` Eduardo Habkost 2019-08-23 17:44 ` Cleber Rosa 1 sibling, 1 reply; 31+ messages in thread From: Eduardo Habkost @ 2019-08-23 15:04 UTC (permalink / raw) To: Cleber Rosa Cc: Peter Maydell, Philippe Mathieu-Daudé, QEMU Developers, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version 2019-08-23 15:04 ` Eduardo Habkost @ 2019-08-23 17:44 ` Cleber Rosa 0 siblings, 0 replies; 31+ messages in thread From: Cleber Rosa @ 2019-08-23 17:44 UTC (permalink / raw) To: Eduardo Habkost Cc: Peter Maydell, Julio Montes, Philippe Mathieu-Daudé, QEMU Developers, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 2/4] check-venv: use recorded Python version 2018-11-09 15:07 [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa @ 2018-11-09 15:07 ` Cleber Rosa 2018-11-09 21:27 ` Eduardo Habkost 2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa 3 siblings, 1 reply; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 15:07 UTC (permalink / raw) To: qemu-devel Cc: Caio Carrara, Philippe Mathieu-Daudé, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng, 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] check-venv: use recorded Python version 2018-11-09 15:07 ` [Qemu-devel] [PATCH 2/4] check-venv: use recorded " Cleber Rosa @ 2018-11-09 21:27 ` Eduardo Habkost 0 siblings, 0 replies; 31+ messages in thread From: Eduardo Habkost @ 2018-11-09 21:27 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-09 15:07 [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 2/4] check-venv: use recorded " Cleber Rosa @ 2018-11-09 15:07 ` Cleber Rosa 2018-11-09 15:52 ` Alex Bennée ` (2 more replies) 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa 3 siblings, 3 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 15:07 UTC (permalink / raw) To: qemu-devel Cc: Caio Carrara, Philippe Mathieu-Daudé, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng, 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa @ 2018-11-09 15:52 ` Alex Bennée 2018-11-12 16:25 ` Eduardo Habkost 2018-11-09 19:34 ` Philippe Mathieu-Daudé 2018-11-12 16:23 ` Eduardo Habkost 2 siblings, 1 reply; 31+ messages in thread From: Alex Bennée @ 2018-11-09 15:52 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-09 15:52 ` Alex Bennée @ 2018-11-12 16:25 ` Eduardo Habkost 0 siblings, 0 replies; 31+ messages in thread From: Eduardo Habkost @ 2018-11-12 16:25 UTC (permalink / raw) To: Alex Bennée Cc: Cleber Rosa, qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa 2018-11-09 15:52 ` Alex Bennée @ 2018-11-09 19:34 ` Philippe Mathieu-Daudé 2018-11-09 19:39 ` Cleber Rosa 2018-11-12 16:23 ` Eduardo Habkost 2 siblings, 1 reply; 31+ messages in thread From: Philippe Mathieu-Daudé @ 2018-11-09 19:34 UTC (permalink / raw) To: Cleber Rosa, qemu-devel Cc: Caio Carrara, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-09 19:34 ` Philippe Mathieu-Daudé @ 2018-11-09 19:39 ` Cleber Rosa 0 siblings, 0 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 19:39 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Caio Carrara, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 >> ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa 2018-11-09 15:52 ` Alex Bennée 2018-11-09 19:34 ` Philippe Mathieu-Daudé @ 2018-11-12 16:23 ` Eduardo Habkost 2018-11-12 17:38 ` Cleber Rosa 2 siblings, 1 reply; 31+ messages in thread From: Eduardo Habkost @ 2018-11-12 16:23 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs 2018-11-12 16:23 ` Eduardo Habkost @ 2018-11-12 17:38 ` Cleber Rosa 0 siblings, 0 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-12 17:38 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements 2018-11-09 15:07 [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes Cleber Rosa ` (2 preceding siblings ...) 2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa @ 2018-11-09 15:07 ` Cleber Rosa 2018-11-09 16:43 ` Eric Blake ` (3 more replies) 3 siblings, 4 replies; 31+ messages in thread From: Cleber Rosa @ 2018-11-09 15:07 UTC (permalink / raw) To: qemu-devel Cc: Caio Carrara, Philippe Mathieu-Daudé, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng, 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa @ 2018-11-09 16:43 ` Eric Blake 2018-11-09 19:32 ` Philippe Mathieu-Daudé ` (2 subsequent siblings) 3 siblings, 0 replies; 31+ messages in thread From: Eric Blake @ 2018-11-09 16:43 UTC (permalink / raw) To: Cleber Rosa, qemu-devel Cc: Fam Zheng, Eduardo Habkost, Philippe Mathieu-Daudé, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Caio Carrara, Alex Bennée 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa 2018-11-09 16:43 ` Eric Blake @ 2018-11-09 19:32 ` Philippe Mathieu-Daudé 2018-11-09 21:29 ` Eduardo Habkost 2018-11-12 17:51 ` Wainer dos Santos Moschetta 3 siblings, 0 replies; 31+ messages in thread From: Philippe Mathieu-Daudé @ 2018-11-09 19:32 UTC (permalink / raw) To: Cleber Rosa, qemu-devel Cc: Caio Carrara, Eduardo Habkost, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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" > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa 2018-11-09 16:43 ` Eric Blake 2018-11-09 19:32 ` Philippe Mathieu-Daudé @ 2018-11-09 21:29 ` Eduardo Habkost 2018-11-12 17:51 ` Wainer dos Santos Moschetta 3 siblings, 0 replies; 31+ messages in thread From: Eduardo Habkost @ 2018-11-09 21:29 UTC (permalink / raw) To: Cleber Rosa Cc: qemu-devel, Caio Carrara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Alex Bennée, Fam Zheng 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa ` (2 preceding siblings ...) 2018-11-09 21:29 ` Eduardo Habkost @ 2018-11-12 17:51 ` Wainer dos Santos Moschetta 3 siblings, 0 replies; 31+ messages in thread From: Wainer dos Santos Moschetta @ 2018-11-12 17:51 UTC (permalink / raw) To: Cleber Rosa, qemu-devel Cc: Fam Zheng, Eduardo Habkost, Philippe Mathieu-Daudé, Philippe Mathieu-Daudé, Caio Carrara, Alex Bennée 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" ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2019-08-23 17:47 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-09 15:07 [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa 2018-11-09 15:49 ` Eduardo Habkost 2018-11-09 16:39 ` Cleber Rosa 2018-11-09 18:25 ` Eduardo Habkost 2018-11-09 19:09 ` Cleber Rosa 2018-11-09 19:51 ` Cleber Rosa 2018-11-09 21:25 ` Eduardo Habkost 2018-11-09 21:26 ` Eduardo Habkost 2019-08-22 16:48 ` Peter Maydell 2019-08-22 21:19 ` Cleber Rosa 2019-08-22 21:54 ` Eduardo Habkost 2019-08-23 13:40 ` Cleber Rosa 2019-08-23 13:44 ` Peter Maydell 2019-08-23 17:42 ` Cleber Rosa 2019-08-23 15:04 ` Eduardo Habkost 2019-08-23 17:44 ` Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 2/4] check-venv: use recorded " Cleber Rosa 2018-11-09 21:27 ` Eduardo Habkost 2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa 2018-11-09 15:52 ` Alex Bennée 2018-11-12 16:25 ` Eduardo Habkost 2018-11-09 19:34 ` Philippe Mathieu-Daudé 2018-11-09 19:39 ` Cleber Rosa 2018-11-12 16:23 ` Eduardo Habkost 2018-11-12 17:38 ` Cleber Rosa 2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa 2018-11-09 16:43 ` Eric Blake 2018-11-09 19:32 ` Philippe Mathieu-Daudé 2018-11-09 21:29 ` Eduardo Habkost 2018-11-12 17:51 ` Wainer dos Santos Moschetta
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.