Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On 11/7/18 1:05 AM, Markus Armbruster wrote: > Eduardo Habkost writes: > >> The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis >> seems to provide an older version. Change the existing rules to >> use command output instead of exit code, to make it compatible >> with older GNU make versions. >> >> Signed-off-by: Eduardo Habkost >> --- >> I think that's the cause of the Travis failures. I have >> submitted a test job right now, at: >> https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 >> Let's see if it fixes the issue. >> --- >> tests/Makefile.include | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index d2e577eabb..074eece558 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results >> # information please refer to "avocado --help". >> AVOCADO_SHOW=none >> >> -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' >> >/dev/null 2>&1) >> -ifeq ($(.SHELLSTATUS),0) >> +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= >> (3, 0) else 0)') >> +ifeq ($(PYTHON3), 1) >> $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) >> $(call quiet-command, \ >> $(PYTHON) -m venv --system-site-packages $@, \ > > PEP 394 recommends software distributions install Python 3 into the > default path as python3, and users use that instead of python, except > for programs that are source compatible with both 2 and 3. So, is > finding out whether python is a Python 3 really appropriate? Why can't > we just use python3 and be done with it? > I mentioned that before, when you pointed out the issue you fix here, that configure may be the best place to get the Python version (not only the major version) and make it available elsewhere. Even if not used for other purposes, it is IMO important information to show on the resulting "configure" output. I'm sending patches to do that in a few. > If we can't: isn't this a configure problem? > I believe adhering to PEP394 is a good thing, but even that document recognizes that the real world is not a perfect place: "however, end users should be aware that python refers to python3 on at least Arch Linux". And it only covers *nix systems, so if we hope to care for non-*nix systems, we have to check the Python version manually. So, I guess the safest approach from QEMU's side is to check for the version indeed. - Cleber.
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On Wed, Nov 07, 2018 at 01:45:35PM +, Peter Maydell wrote: > On 7 November 2018 at 12:49, Eduardo Habkost wrote: > > Now, why do we need --with-python, and why do we need to use > > $(PYTHON) when running tests? If somebody wants to use a > > different Python binary when running tests, they can already use > > $PATH for that. > > > > (That's the same argument I used for iotests a while ago: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg566631.html) > > I'm not a great fan of requiring the user to mess with their PATH > to get configure to work. Also, the first python on the path > might be the wrong one, and we don't pass PATH from configure > to make so you end up having to make sure you specify it > right in both places. You're assuming that this will actually require some people to mess with their $PATH because they currently don't have Python on their $PATH. I don't see any evidence that this is expected to happen. Do you? > > Plus we already have --with-python, so if you want to drop > it you need to deprecate it first, and you need a justification > that's strong enough to outweigh breaking users' existing > build/packaging setups and scripts... I would really like to remove the option as soon as we start requiring Python 3. Let's stop reinventing solutions to problems already addressed by PEP 394. -- Eduardo
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On 7 November 2018 at 12:49, Eduardo Habkost wrote: > Now, why do we need --with-python, and why do we need to use > $(PYTHON) when running tests? If somebody wants to use a > different Python binary when running tests, they can already use > $PATH for that. > > (That's the same argument I used for iotests a while ago: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg566631.html) I'm not a great fan of requiring the user to mess with their PATH to get configure to work. Also, the first python on the path might be the wrong one, and we don't pass PATH from configure to make so you end up having to make sure you specify it right in both places. Plus we already have --with-python, so if you want to drop it you need to deprecate it first, and you need a justification that's strong enough to outweigh breaking users' existing build/packaging setups and scripts... thanks -- PMM
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On Wed, Nov 07, 2018 at 07:05:03AM +0100, Markus Armbruster wrote: > Eduardo Habkost writes: > > > The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis > > seems to provide an older version. Change the existing rules to > > use command output instead of exit code, to make it compatible > > with older GNU make versions. > > > > Signed-off-by: Eduardo Habkost > > --- > > I think that's the cause of the Travis failures. I have > > submitted a test job right now, at: > > https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 > > Let's see if it fixes the issue. > > --- > > tests/Makefile.include | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/Makefile.include b/tests/Makefile.include > > index d2e577eabb..074eece558 100644 > > --- a/tests/Makefile.include > > +++ b/tests/Makefile.include > > @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results > > # information please refer to "avocado --help". > > AVOCADO_SHOW=none > > > > -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' > > >/dev/null 2>&1) > > -ifeq ($(.SHELLSTATUS),0) > > +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= > > (3, 0) else 0)') > > +ifeq ($(PYTHON3), 1) > > $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) > > $(call quiet-command, \ > > $(PYTHON) -m venv --system-site-packages $@, \ > > PEP 394 recommends software distributions install Python 3 into the > default path as python3, and users use that instead of python, except > for programs that are source compatible with both 2 and 3. So, is > finding out whether python is a Python 3 really appropriate? Why can't > we just use python3 and be done with it? Because './configure --with-python=...' exists, and I didn't want to break it. Now, why do we need --with-python, and why do we need to use $(PYTHON) when running tests? If somebody wants to use a different Python binary when running tests, they can already use $PATH for that. (That's the same argument I used for iotests a while ago: https://www.mail-archive.com/qemu-devel@nongnu.org/msg566631.html) > > If we can't: isn't this a configure problem? It is, and I think Cleber mentioned that he planned to do it in ./configure, a while ago. But just using the python3 binary from $PATH would be even better. -- Eduardo
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On 7 November 2018 at 06:05, Markus Armbruster wrote: > Eduardo Habkost writes: > >> The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis >> seems to provide an older version. Change the existing rules to >> use command output instead of exit code, to make it compatible >> with older GNU make versions. >> >> Signed-off-by: Eduardo Habkost >> --- >> I think that's the cause of the Travis failures. I have >> submitted a test job right now, at: >> https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 >> Let's see if it fixes the issue. >> --- >> tests/Makefile.include | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index d2e577eabb..074eece558 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results >> # information please refer to "avocado --help". >> AVOCADO_SHOW=none >> >> -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' >> >/dev/null 2>&1) >> -ifeq ($(.SHELLSTATUS),0) >> +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= >> (3, 0) else 0)') >> +ifeq ($(PYTHON3), 1) >> $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) >> $(call quiet-command, \ >> $(PYTHON) -m venv --system-site-packages $@, \ > > PEP 394 recommends software distributions install Python 3 into the > default path as python3, and users use that instead of python, except > for programs that are source compatible with both 2 and 3. So, is > finding out whether python is a Python 3 really appropriate? Why can't > we just use python3 and be done with it? > > If we can't: isn't this a configure problem? You can't just use python3 and be done with it because python3 might not exist, and because (as with python 2) the user might want to tell us the path to it. You could have configure detect whether python3 exists and set a PYTHON3 as well as a PYTHON (plus I guess support for the user to say "my python3 is this binary"), and then have this code in Makefile.include handle "PYTHON3 is not set" to mean "python 3 isn't available". thanks -- PMM
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
Eduardo Habkost writes: > The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis > seems to provide an older version. Change the existing rules to > use command output instead of exit code, to make it compatible > with older GNU make versions. > > Signed-off-by: Eduardo Habkost > --- > I think that's the cause of the Travis failures. I have > submitted a test job right now, at: > https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 > Let's see if it fixes the issue. > --- > tests/Makefile.include | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index d2e577eabb..074eece558 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results > # information please refer to "avocado --help". > AVOCADO_SHOW=none > > -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' > >/dev/null 2>&1) > -ifeq ($(.SHELLSTATUS),0) > +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= > (3, 0) else 0)') > +ifeq ($(PYTHON3), 1) > $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) > $(call quiet-command, \ > $(PYTHON) -m venv --system-site-packages $@, \ PEP 394 recommends software distributions install Python 3 into the default path as python3, and users use that instead of python, except for programs that are source compatible with both 2 and 3. So, is finding out whether python is a Python 3 really appropriate? Why can't we just use python3 and be done with it? If we can't: isn't this a configure problem?
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On 6 November 2018 at 14:13, Eduardo Habkost wrote: > The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis > seems to provide an older version. Change the existing rules to > use command output instead of exit code, to make it compatible > with older GNU make versions. > > Signed-off-by: Eduardo Habkost > --- > I think that's the cause of the Travis failures. I have > submitted a test job right now, at: > https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 > Let's see if it fixes the issue. > --- > tests/Makefile.include | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index d2e577eabb..074eece558 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results > # information please refer to "avocado --help". > AVOCADO_SHOW=none > > -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' > >/dev/null 2>&1) > -ifeq ($(.SHELLSTATUS),0) > +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= > (3, 0) else 0)') > +ifeq ($(PYTHON3), 1) > $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) > $(call quiet-command, \ > $(PYTHON) -m venv --system-site-packages $@, \ > -- Thanks, applied to master as a build fix. -- PMM
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
Hi Peter, Can you apply this patch as a CI bug-fix? Thanks, Phil. On 6/11/18 15:27, Philippe Mathieu-Daudé wrote: On 6/11/18 15:13, Eduardo Habkost wrote: The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis seems to provide an older version. Change the existing rules to use command output instead of exit code, to make it compatible with older GNU make versions. You were quicker, I just found out :) Signed-off-by: Eduardo Habkost --- I think that's the cause of the Travis failures. I have submitted a test job right now, at: https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 Let's see if it fixes the issue. You can run it locally: $ make docker-image-travis ... $ docker run --rm -it -v $(pwd):$(pwd) -w $(pwd) \ -u 0 --entrypoint=/bin/bash qemu:travis root@ede09efe22fd:qemu# cd build/docker_travis/ root@ede09efe22fd:qemu/build/docker_travis# make AVOCADO_SHOW=app check-acceptance VENV qemu/build/docker_travis/tests/venv The virtual environment was not created successfully because ensurepip is not available. On Debian/Ubuntu systems, you need to install the python3-venv package using the following command. apt-get install python3-venv You may need to use sudo with that command. After installing the python3-venv package, recreate your virtual environment. root@ede09efe22fd:qemu/build/docker_travis# apt install python3-pip python3.4-venv root@ede09efe22fd:qemu/build/docker_travis# make AVOCADO_SHOW=app check-acceptance VENV qemu/build/docker_travis/tests/venv PIP qemu/tests/requirements.txt AVOCADO tests/acceptance JOB ID : 5e5529e79456c388e80323acdc71f3887341a498 JOB LOG : qemu/build/docker_travis/tests/results/job-2018-11-06T14.26-5e5529e/job.log (1/6) qemu/tests/acceptance/boot_linux_console.py:BootLinuxConsole.test: CANCEL: No QEMU binary defined or found in the source tree (0.01 s) (2/6) qemu/tests/acceptance/version.py:Version.test_qmp_human_info_version: CANCEL: No QEMU binary defined or found in the source tree (0.01 s) (3/6) qemu/tests/acceptance/vnc.py:Vnc.test_no_vnc: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) (4/6) qemu/tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) (5/6) qemu/tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) (6/6) qemu/tests/acceptance/vnc.py:Vnc.test_vnc_change_password: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) RESULTS : PASS 0 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 6 JOB TIME : 0.45 s Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- tests/Makefile.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index d2e577eabb..074eece558 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results # information please refer to "avocado --help". AVOCADO_SHOW=none -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' >/dev/null 2>&1) -ifeq ($(.SHELLSTATUS),0) +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= (3, 0) else 0)') +ifeq ($(PYTHON3), 1) $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) $(call quiet-command, \ $(PYTHON) -m venv --system-site-packages $@, \
Re: [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
On 6/11/18 15:13, Eduardo Habkost wrote: The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis seems to provide an older version. Change the existing rules to use command output instead of exit code, to make it compatible with older GNU make versions. You were quicker, I just found out :) Signed-off-by: Eduardo Habkost --- I think that's the cause of the Travis failures. I have submitted a test job right now, at: https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 Let's see if it fixes the issue. You can run it locally: $ make docker-image-travis ... $ docker run --rm -it -v $(pwd):$(pwd) -w $(pwd) \ -u 0 --entrypoint=/bin/bash qemu:travis root@ede09efe22fd:qemu# cd build/docker_travis/ root@ede09efe22fd:qemu/build/docker_travis# make AVOCADO_SHOW=app check-acceptance VENVqemu/build/docker_travis/tests/venv The virtual environment was not created successfully because ensurepip is not available. On Debian/Ubuntu systems, you need to install the python3-venv package using the following command. apt-get install python3-venv You may need to use sudo with that command. After installing the python3-venv package, recreate your virtual environment. root@ede09efe22fd:qemu/build/docker_travis# apt install python3-pip python3.4-venv root@ede09efe22fd:qemu/build/docker_travis# make AVOCADO_SHOW=app check-acceptance VENVqemu/build/docker_travis/tests/venv PIP qemu/tests/requirements.txt AVOCADO tests/acceptance JOB ID : 5e5529e79456c388e80323acdc71f3887341a498 JOB LOG: qemu/build/docker_travis/tests/results/job-2018-11-06T14.26-5e5529e/job.log (1/6) qemu/tests/acceptance/boot_linux_console.py:BootLinuxConsole.test: CANCEL: No QEMU binary defined or found in the source tree (0.01 s) (2/6) qemu/tests/acceptance/version.py:Version.test_qmp_human_info_version: CANCEL: No QEMU binary defined or found in the source tree (0.01 s) (3/6) qemu/tests/acceptance/vnc.py:Vnc.test_no_vnc: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) (4/6) qemu/tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) (5/6) qemu/tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) (6/6) qemu/tests/acceptance/vnc.py:Vnc.test_vnc_change_password: CANCEL: No QEMU binary defined or found in the source tree (0.00 s) RESULTS: PASS 0 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 6 JOB TIME : 0.45 s Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- tests/Makefile.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index d2e577eabb..074eece558 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results # information please refer to "avocado --help". AVOCADO_SHOW=none -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' >/dev/null 2>&1) -ifeq ($(.SHELLSTATUS),0) +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= (3, 0) else 0)') +ifeq ($(PYTHON3), 1) $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) $(call quiet-command, \ $(PYTHON) -m venv --system-site-packages $@, \
[Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions
The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis seems to provide an older version. Change the existing rules to use command output instead of exit code, to make it compatible with older GNU make versions. Signed-off-by: Eduardo Habkost --- I think that's the cause of the Travis failures. I have submitted a test job right now, at: https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962 Let's see if it fixes the issue. --- tests/Makefile.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index d2e577eabb..074eece558 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results # information please refer to "avocado --help". AVOCADO_SHOW=none -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' >/dev/null 2>&1) -ifeq ($(.SHELLSTATUS),0) +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= (3, 0) else 0)') +ifeq ($(PYTHON3), 1) $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) $(call quiet-command, \ $(PYTHON) -m venv --system-site-packages $@, \ -- 2.18.0.rc1.1.g3f1ff2140
