[RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[John Snow]

RFC: This is a very early, crude attempt at switching over to an
external Python package dependency for QMP. This series does not
actually make the switch in and of itself, but instead just switches to
the paradigm of using a venv in general to install the QEMU python
packages instead of using PYTHONPATH to load them from the source tree.

(By installing the package, we can process dependencies.)

I'm sending it to the list so I can show you some of what's ugly so far
and my notes on how I might make it less ugly.

(1) This doesn't trigger venv creation *from* iotests, it merely prints
a friendly error message if "make check-venv" has not been run
first. Not the greatest.

(2) This isn't acceptable for SRPM builds, because it uses PyPI to fetch
packages just-in-time. My thought is to use an environment variable like
QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
process. We can use "--system-site-packages" as an argument to venv
creation and "--no-index" as an argument to pip installation to achieve
good behavior in SRPM building scenarios. It'd be up to the spec-writer
to opt into that behavior.

(3) Using one venv for *all* tests means that avocado comes as a pre-req
for iotests -- which adds avocado as a BuildRequires for the Fedora
SRPM. That's probably not ideal. It may be better to model the test venv
as something that can be created in stages: the "core" venv first, and
the avocado packages only when needed.

You can see in these patches that I wasn't really sure how to tie the
check-venv step as a dependency of 'check' or 'check-block', and it
winds up feeling kind of hacky and fragile as a result.

(Patches 6 and 7 feel particularly fishy.)

What I think I would like to do is replace the makefile logic with a
Python bootstrapping script. This will allow me to add in environment
variable logic to accommodate #2 pretty easily. It will also allow
iotests to call into the bootstrap script whenever it detects the venv
isn't set up, which it needed to do anyway in order to print a
user-friendly error message. Lastly, it will make it easier to create a
"tiered" venv that layers in the avocado dependencies only as-needed,
which avoids us having to bloat the SRPM build dependencies.

In the end, I think that approach will:

- Allow us to run iotests without having to run a manual prep step
- Keep additional SRPM deps to a minimum
- Keep makefile hacks to a minimum

The only downside I am really frowning at is that I will have to
replicate some "update the venv if it's outdated" logic that is usually
handled by the Make system in the venv bootstrapper. Still, I think it's
probably the only way to hit all of the requirements here without trying
to concoct a fairly complex Makefile.

any thoughts? If not, I'll just move on to trying to hack up that
version next instead.

--js

John Snow (9):
  python: update for mypy 0.950
  tests: add "TESTS_PYTHON" variable to Makefile
  tests: install "qemu" namespace package into venv
  tests: silence pip upgrade warnings during venv creation
  tests: use tests/venv to run basevm.py-based scripts
  tests: add check-venv as a dependency of check and check-block
  tests: add check-venv to build-tcg-disabled CI recipe
  iotests: fix source directory location
  iotests: use tests/venv for running tests

 .gitlab-ci.d/buildtest.yml             |  1 +
 python/qemu/qmp/util.py                |  4 +++-
 python/setup.cfg                       |  1 +
 tests/Makefile.include                 | 23 +++++++++++++++------
 tests/avocado/avocado_qemu/__init__.py | 11 +++++-----
 tests/avocado/virtio_check_params.py   |  1 -
 tests/avocado/virtio_version.py        |  1 -
 tests/qemu-iotests/testenv.py          | 28 +++++++++++++++++---------
 tests/requirements.txt                 |  1 +
 tests/vm/Makefile.include              | 13 ++++++------
 tests/vm/basevm.py                     |  6 +++---
 11 files changed, 57 insertions(+), 33 deletions(-)

-- 
2.34.1

[RFC PATCH 1/9] python: update for mypy 0.950

[John Snow]

typeshed (included in mypy) recently updated to improve the typing for
WriteTransport objects. I was working around this, but now there's a
version where I shouldn't work around it.

Unfortunately this creates some minor ugliness if I want to support both
pre- and post-0.950 versions. For now, for my sanity, just disable the
unused-ignores warning.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/util.py | 4 +++-
 python/setup.cfg        | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
index eaa5fc7d5f9..ca6225e9cda 100644
--- a/python/qemu/qmp/util.py
+++ b/python/qemu/qmp/util.py
@@ -40,7 +40,9 @@ async def flush(writer: asyncio.StreamWriter) -> None:
     drain. The flow control limits are restored after the call is
     completed.
     """
-    transport = cast(asyncio.WriteTransport, writer.transport)
+    transport = cast(  # type: ignore[redundant-cast]
+        asyncio.WriteTransport, writer.transport
+    )
 
     # https://github.com/python/typeshed/issues/5779
     low, high = transport.get_write_buffer_limits()  # type: ignore
diff --git a/python/setup.cfg b/python/setup.cfg
index e877ea56475..c2c61c75190 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -79,6 +79,7 @@ strict = True
 python_version = 3.6
 warn_unused_configs = True
 namespace_packages = True
+warn_unused_ignores = False
 
 [mypy-qemu.utils.qom_fuse]
 # fusepy has no type stubs:
-- 
2.34.1

[RFC PATCH 2/9] tests: add "TESTS_PYTHON" variable to Makefile

[John Snow]

This is a convenience feature: $(PYTHON) points to the Python executable
we were instructed to use by the configure script. We use that Python to
create a virtual environment with the "check-venv" target in
tests/Makefile.include.

$(TESTS_PYTHON) points to the Python executable belonging to the virtual
environment tied to the build. This Python executable is a symlink to
the binary used to create the venv, which will be the version provided
at configure time.

Using $(TESTS_PYTHON) therefore uses the $(PYTHON) executable, but with
paths modified to use packages installed to the venv.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile.include | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index ec84b2ebc04..146aaa96a00 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -89,6 +89,7 @@ TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
 TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
 TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
 TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
+TESTS_PYTHON=$(TESTS_VENV_DIR)/bin/python
 ifndef AVOCADO_TESTS
 	AVOCADO_TESTS=tests/avocado
 endif
@@ -108,7 +109,7 @@ $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
             $(PYTHON) -m venv $@, \
             VENV, $@)
 	$(call quiet-command, \
-            $(TESTS_VENV_DIR)/bin/python -m pip -q install -r $(TESTS_VENV_REQ), \
+            $(TESTS_PYTHON) -m pip -q install -r $(TESTS_VENV_REQ), \
             PIP, $(TESTS_VENV_REQ))
 	$(call quiet-command, touch $@)
 
@@ -126,7 +127,7 @@ FEDORA_31_DOWNLOAD=$(filter $(FEDORA_31_ARCHES),$(FEDORA_31_ARCHES_CANDIDATES))
 # download one specific Fedora 31 image
 get-vm-image-fedora-31-%: check-venv
 	$(call quiet-command, \
-             $(TESTS_VENV_DIR)/bin/python -m avocado vmimage get \
+             $(TESTS_PYTHON) -m avocado vmimage get \
              --distro=fedora --distro-version=31 --arch=$*, \
 	"AVOCADO", "Downloading avocado tests VM image for $*")
 
@@ -135,7 +136,7 @@ get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOW
 
 check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
 	$(call quiet-command, \
-            $(TESTS_VENV_DIR)/bin/python -m avocado \
+            $(TESTS_PYTHON) -m avocado \
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
             $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
 			--filter-by-tags-include-empty-key) \
-- 
2.34.1

[RFC PATCH 3/9] tests: install "qemu" namespace package into venv

[John Snow]

This patch adds the "qemu" namespace package to the $build/tests/venv
directory. It does so in "editable" mode, which means that changes to
the source python directory will actively be reflected by the venv.

This patch also then removes any sys.path hacking from the avocado test
scripts directly. By doing this, the environment of where to find these
packages is managed entirely by the virtual environment and not by the
scripts themselves.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile.include                 |  5 ++++-
 tests/avocado/avocado_qemu/__init__.py | 11 +++++------
 tests/avocado/virtio_check_params.py   |  1 -
 tests/avocado/virtio_version.py        |  1 -
 tests/requirements.txt                 |  1 +
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 146aaa96a00..dbbf1ba535b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -104,10 +104,13 @@ else
 	AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
 endif
 
-$(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
+$(TESTS_VENV_DIR): $(TESTS_VENV_REQ) $(SRC_PATH)/python/setup.cfg
 	$(call quiet-command, \
             $(PYTHON) -m venv $@, \
             VENV, $@)
+	$(call quiet-command, \
+            $(TESTS_PYTHON) -m pip -q install \
+            -e "$(SRC_PATH)/python/", PIP, "$(SRC_PATH)/python/")
 	$(call quiet-command, \
             $(TESTS_PYTHON) -m pip -q install -r $(TESTS_VENV_REQ), \
             PIP, $(TESTS_VENV_REQ))
diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index 39f15c1d518..b656a70c55b 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -21,6 +21,11 @@
 from avocado.utils import cloudinit, datadrainer, process, ssh, vmimage
 from avocado.utils.path import find_command
 
+from qemu.machine import QEMUMachine
+from qemu.utils import (get_info_usernet_hostfwd_port, kvm_available,
+                        tcg_available)
+
+
 #: The QEMU build root directory.  It may also be the source directory
 #: if building from the source dir, but it's safer to use BUILD_DIR for
 #: that purpose.  Be aware that if this code is moved outside of a source
@@ -35,12 +40,6 @@
 else:
     SOURCE_DIR = BUILD_DIR
 
-sys.path.append(os.path.join(SOURCE_DIR, 'python'))
-
-from qemu.machine import QEMUMachine
-from qemu.utils import (get_info_usernet_hostfwd_port, kvm_available,
-                        tcg_available)
-
 
 def has_cmd(name, args=None):
     """
diff --git a/tests/avocado/virtio_check_params.py b/tests/avocado/virtio_check_params.py
index e869690473a..4093da8a674 100644
--- a/tests/avocado/virtio_check_params.py
+++ b/tests/avocado/virtio_check_params.py
@@ -22,7 +22,6 @@
 import re
 import logging
 
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import QEMUMachine
 from avocado_qemu import QemuSystemTest
 from avocado import skip
diff --git a/tests/avocado/virtio_version.py b/tests/avocado/virtio_version.py
index 208910bb844..c84e48813a1 100644
--- a/tests/avocado/virtio_version.py
+++ b/tests/avocado/virtio_version.py
@@ -11,7 +11,6 @@
 import sys
 import os
 
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import QEMUMachine
 from avocado_qemu import QemuSystemTest
 
diff --git a/tests/requirements.txt b/tests/requirements.txt
index a21b59b4439..0ba561b6bdf 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -1,5 +1,6 @@
 # Add Python module requirements, one per line, to be installed
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
+# Note that qemu.git/python/ is always implicitly installed.
 avocado-framework==88.1
 pycdlib==1.11.0
-- 
2.34.1

[RFC PATCH 4/9] tests: silence pip upgrade warnings during venv creation

[John Snow]

Turn off the nag warning coaxing us to upgrade pip. It's not really that
interesting to see in CI logs, and as long as nothing is broken --
nothing is broken.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile.include | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index dbbf1ba535b..dfb678d379f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -109,11 +109,11 @@ $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) $(SRC_PATH)/python/setup.cfg
             $(PYTHON) -m venv $@, \
             VENV, $@)
 	$(call quiet-command, \
-            $(TESTS_PYTHON) -m pip -q install \
+            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
             -e "$(SRC_PATH)/python/", PIP, "$(SRC_PATH)/python/")
 	$(call quiet-command, \
-            $(TESTS_PYTHON) -m pip -q install -r $(TESTS_VENV_REQ), \
-            PIP, $(TESTS_VENV_REQ))
+            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
+            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
 	$(call quiet-command, touch $@)
 
 $(TESTS_RESULTS_DIR):
-- 
2.34.1

[RFC PATCH 5/9] tests: use tests/venv to run basevm.py-based scripts

[John Snow]

This patch co-opts the virtual environment being used by avocado tests
to also run the basevm.py tests. This is being done in preparation for
for the qemu.qmp package being removed from qemu.git.

As part of the change, remove any sys.path() hacks and treat "qemu" as a
normal third-party import.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/vm/Makefile.include | 13 +++++++------
 tests/vm/basevm.py        |  6 +++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index ae91f5043e5..588bc999cc9 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -84,10 +84,11 @@ vm-clean-all:
 
 $(IMAGES_DIR)/%.img:	$(SRC_PATH)/tests/vm/% \
 			$(SRC_PATH)/tests/vm/basevm.py \
-			$(SRC_PATH)/tests/vm/Makefile.include
+			$(SRC_PATH)/tests/vm/Makefile.include \
+			check-venv
 	@mkdir -p $(IMAGES_DIR)
 	$(call quiet-command, \
-		$(PYTHON) $< \
+		$(TESTS_PYTHON) $< \
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(GENISOIMAGE),--genisoimage $(GENISOIMAGE)) \
 		$(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
@@ -101,9 +102,9 @@ $(IMAGES_DIR)/%.img:	$(SRC_PATH)/tests/vm/% \
 
 
 # Build in VM $(IMAGE)
-vm-build-%: $(IMAGES_DIR)/%.img
+vm-build-%: $(IMAGES_DIR)/%.img check-venv
 	$(call quiet-command, \
-		$(PYTHON) $(SRC_PATH)/tests/vm/$* \
+		$(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(DEBUG), --interactive) \
 		$(if $(J),--jobs $(J)) \
@@ -127,9 +128,9 @@ vm-boot-serial-%: $(IMAGES_DIR)/%.img
 		-device virtio-net-pci,netdev=vnet \
 	|| true
 
-vm-boot-ssh-%: $(IMAGES_DIR)/%.img
+vm-boot-ssh-%: $(IMAGES_DIR)/%.img check-venv
 	$(call quiet-command, \
-		$(PYTHON) $(SRC_PATH)/tests/vm/$* \
+		$(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
 		$(if $(J),--jobs $(J)) \
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 254e11c932b..d7d0413df35 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -18,9 +18,6 @@
 import logging
 import time
 import datetime
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.machine import QEMUMachine
-from qemu.utils import get_info_usernet_hostfwd_port, kvm_available
 import subprocess
 import hashlib
 import argparse
@@ -31,6 +28,9 @@
 import traceback
 import shlex
 
+from qemu.machine import QEMUMachine
+from qemu.utils import get_info_usernet_hostfwd_port, kvm_available
+
 SSH_KEY_FILE = os.path.join(os.path.dirname(__file__),
                "..", "keys", "id_rsa")
 SSH_PUB_KEY_FILE = os.path.join(os.path.dirname(__file__),
-- 
2.34.1

[RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block

[John Snow]

This patch is being front-loaded before iotests actually relies on the
tests/venv being created in order to preserve bisectability.

Problems I am aware of here (There are a lot, sorry):

- I am not sure the right place to express this dependency, so I did it
  in tests/Makefile.include. It seems to work. I wasn't sure how to
  express it in tests/qemu-iotests/meson.build, but I am not sure if
  that would make it work for both "check" and "check-block" anyway.

- I don't really understand why I need empty rules for Make to process
  the pre-requisite. Without the "do-nothing" recipes, the venv building
  doesn't seem to trigger at all. What I have seems to work, but I'm
  worried I broke something unseen.

- This adds avocado as a dependency of "check"/"check-block", which is
  not technically true. It was just a sin of convenience to create one
  shared "testing venv". Maybe I'll figure out a scheme to have avocado's
  dependencies be "extras" that get added in to a standard "core set".

- This patch ignore the requisite that RPM builds be able to run without
  internet access, meaning that a PyPI fetch is not permissable. I plan
  to solve this by using an environment variable (QEMU_CHECK_NO_PYPI)
  that changes the behavior of the venv setup slightly, and qemu
  specfiles can opt-in to this behavior.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile.include | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index dfb678d379f..fa7af711fe5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -154,10 +154,17 @@ check-acceptance-deprecated-warning:
 
 check-acceptance: check-acceptance-deprecated-warning | check-avocado
 
+# Before we delegate to meson, create the python venv for block tests.
+.PHONY: check-block
+check-block: check-venv
+	@echo  # Without some rule, this doesn't run at all. Why?
+
+
 # Consolidated targets
 
 .PHONY: check check-clean get-vm-images
-check:
+check: check-venv
+	@echo # ???
 
 check-build: run-ninja
 
-- 
2.34.1

[RFC PATCH 7/9] tests: add check-venv to build-tcg-disabled CI recipe

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0aea7ab84c2..5c6201847f1 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -245,6 +245,7 @@ build-tcg-disabled:
     - make -j"$JOBS"
     - make check-unit
     - make check-qapi-schema
+    - make check-venv
     - cd tests/qemu-iotests/
     - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
             052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163
-- 
2.34.1

[RFC PATCH 8/9] iotests: fix source directory location

[John Snow]

If you invoke the check script from outside of the tests/qemu-iotests
directory, the directories initialized as source_iotests and
build_iotests will be incorrect.

We can use the location of the source file itself to be more accurate.

(I don't know if this is actually *used*, but what was there was wrong,
I think.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/testenv.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index a864c74b123..0007da3f06c 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
             self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
         else:
             # called from the source tree
-            self.source_iotests = os.getcwd()
+            self.source_iotests = str(Path(__file__, '../').resolve())
             self.build_iotests = self.source_iotests
 
-        self.build_root = os.path.join(self.build_iotests, '..', '..')
+        self.build_root = str(Path(self.build_iotests, '../..').resolve())
 
         self.init_directories()
         self.init_binaries()
-- 
2.34.1

[RFC PATCH 9/9] iotests: use tests/venv for running tests

[John Snow]

Essentially, this:

(A) adjusts the python binary to be the one found in the venv (which is
a symlink to the python binary chosen at configure time)

(B) adds a new VIRTUAL_ENV export variable

(C) changes PATH to front-load the venv binary directory.

If the venv directory isn't found, raise a friendly exception that tries
to give the human operator a friendly clue as to what's gone wrong. In
the very near future, I'd like to teach iotests how to fix this problem
entirely of its own volition, but that's a trick for a little later.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/testenv.py | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 0007da3f06c..fd3720ed7e7 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
     # lot of them. Silence pylint:
     # pylint: disable=too-many-instance-attributes
 
-    env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
-                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+    env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON',
+                     'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+                     'QEMU_PROG', 'QEMU_IMG_PROG',
                      'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
                      'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
                      'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
@@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
             if val is not None:
                 env[v] = val
 
+        env['PATH'] = os.pathsep.join((
+            os.path.join(self.virtual_env, 'bin'),
+            os.environ['PATH']
+        ))
         return env
 
     def init_directories(self) -> None:
@@ -107,13 +112,17 @@ def init_directories(self) -> None:
              SOCK_DIR
              SAMPLE_IMG_DIR
         """
-
-        # Path where qemu goodies live in this source tree.
-        qemu_srctree_path = Path(__file__, '../../../python').resolve()
+        venv_path = Path(self.build_root, 'tests/venv/')
+        if not venv_path.exists():
+            raise FileNotFoundError(
+                f"Virtual environment \"{venv_path!s}\" isn't found."
+                " (Maybe you need to run 'make check-venv'"
+                " from the build dir?)"
+            )
+        self.virtual_env: str = str(venv_path)
 
         self.pythonpath = os.pathsep.join(filter(None, (
             self.source_iotests,
-            str(qemu_srctree_path),
             os.getenv('PYTHONPATH'),
         )))
 
@@ -138,7 +147,7 @@ def init_binaries(self) -> None:
              PYTHON (for bash tests)
              QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
         """
-        self.python = sys.executable
+        self.python: str = os.path.join(self.virtual_env, 'bin', 'python3')
 
         def root(*names: str) -> str:
             return os.path.join(self.build_root, *names)
@@ -300,6 +309,7 @@ def print_env(self, prefix: str = '') -> None:
 {prefix}GDB_OPTIONS   -- {GDB_OPTIONS}
 {prefix}VALGRIND_QEMU -- {VALGRIND_QEMU}
 {prefix}PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
+{prefix}VIRTUAL_ENV   -- {VIRTUAL_ENV}
 {prefix}"""
 
         args = collections.defaultdict(str, self.get_env())
-- 
2.34.1

Re: [RFC PATCH 2/9] tests: add "TESTS_PYTHON" variable to Makefile

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> This is a convenience feature: $(PYTHON) points to the Python executable
> we were instructed to use by the configure script. We use that Python to
> create a virtual environment with the "check-venv" target in
> tests/Makefile.include.
> 
> $(TESTS_PYTHON) points to the Python executable belonging to the virtual
> environment tied to the build. This Python executable is a symlink to
> the binary used to create the venv, which will be the version provided
> at configure time.
> 
> Using $(TESTS_PYTHON) therefore uses the $(PYTHON) executable, but with
> paths modified to use packages installed to the venv.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/Makefile.include | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index ec84b2ebc04..146aaa96a00 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -89,6 +89,7 @@ TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
>   TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
>   TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
>   TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
> +TESTS_PYTHON=$(TESTS_VENV_DIR)/bin/python
>   ifndef AVOCADO_TESTS
>   	AVOCADO_TESTS=tests/avocado
>   endif
> @@ -108,7 +109,7 @@ $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
>               $(PYTHON) -m venv $@, \
>               VENV, $@)
>   	$(call quiet-command, \
> -            $(TESTS_VENV_DIR)/bin/python -m pip -q install -r $(TESTS_VENV_REQ), \
> +            $(TESTS_PYTHON) -m pip -q install -r $(TESTS_VENV_REQ), \
>               PIP, $(TESTS_VENV_REQ))
>   	$(call quiet-command, touch $@)
>   
> @@ -126,7 +127,7 @@ FEDORA_31_DOWNLOAD=$(filter $(FEDORA_31_ARCHES),$(FEDORA_31_ARCHES_CANDIDATES))
>   # download one specific Fedora 31 image
>   get-vm-image-fedora-31-%: check-venv
>   	$(call quiet-command, \
> -             $(TESTS_VENV_DIR)/bin/python -m avocado vmimage get \
> +             $(TESTS_PYTHON) -m avocado vmimage get \
>                --distro=fedora --distro-version=31 --arch=$*, \
>   	"AVOCADO", "Downloading avocado tests VM image for $*")
>   
> @@ -135,7 +136,7 @@ get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOW
>   
>   check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
>   	$(call quiet-command, \
> -            $(TESTS_VENV_DIR)/bin/python -m avocado \
> +            $(TESTS_PYTHON) -m avocado \
>               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
>               $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
>   			--filter-by-tags-include-empty-key) \

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [RFC PATCH 3/9] tests: install "qemu" namespace package into venv

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index a21b59b4439..0ba561b6bdf 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -1,5 +1,6 @@
>   # Add Python module requirements, one per line, to be installed
>   # in the tests/venv Python virtual environment. For more info,
>   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> +# Note that qemu.git/python/ is always implicitly installed.
>   avocado-framework==88.1
>   pycdlib==1.11.0

Any reason not to put ./python here?  But anyway,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [RFC PATCH 4/9] tests: silence pip upgrade warnings during venv creation

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index dbbf1ba535b..dfb678d379f 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -109,11 +109,11 @@ $(TESTS_VENV_DIR): $(TESTS_VENV_REQ) $(SRC_PATH)/python/setup.cfg
>              $(PYTHON) -m venv $@, \
>              VENV, $@)
>  	$(call quiet-command, \
> -            $(TESTS_PYTHON) -m pip -q install \
> +            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
>              -e "$(SRC_PATH)/python/", PIP, "$(SRC_PATH)/python/")
>  	$(call quiet-command, \
> -            $(TESTS_PYTHON) -m pip -q install -r $(TESTS_VENV_REQ), \
> -            PIP, $(TESTS_VENV_REQ))
> +            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
> +            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
>  	$(call quiet-command, touch $@)

Really nitpicking but I would have placed this change before adding the 
second invocation of pip. :)

Paolo

Re: [RFC PATCH 5/9] tests: use tests/venv to run basevm.py-based scripts

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> This patch co-opts the virtual environment being used by avocado tests
> to also run the basevm.py tests. This is being done in preparation for
> for the qemu.qmp package being removed from qemu.git.
> 
> As part of the change, remove any sys.path() hacks and treat "qemu" as a
> normal third-party import.

That's already a good reason to do it, independent of qemu.qmp being 
removed from qemu.git.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/vm/Makefile.include | 13 +++++++------
>   tests/vm/basevm.py        |  6 +++---
>   2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index ae91f5043e5..588bc999cc9 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -84,10 +84,11 @@ vm-clean-all:
>   
>   $(IMAGES_DIR)/%.img:	$(SRC_PATH)/tests/vm/% \
>   			$(SRC_PATH)/tests/vm/basevm.py \
> -			$(SRC_PATH)/tests/vm/Makefile.include
> +			$(SRC_PATH)/tests/vm/Makefile.include \
> +			check-venv
>   	@mkdir -p $(IMAGES_DIR)
>   	$(call quiet-command, \
> -		$(PYTHON) $< \
> +		$(TESTS_PYTHON) $< \
>   		$(if $(V)$(DEBUG), --debug) \
>   		$(if $(GENISOIMAGE),--genisoimage $(GENISOIMAGE)) \
>   		$(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
> @@ -101,9 +102,9 @@ $(IMAGES_DIR)/%.img:	$(SRC_PATH)/tests/vm/% \
>   
>   
>   # Build in VM $(IMAGE)
> -vm-build-%: $(IMAGES_DIR)/%.img
> +vm-build-%: $(IMAGES_DIR)/%.img check-venv
>   	$(call quiet-command, \
> -		$(PYTHON) $(SRC_PATH)/tests/vm/$* \
> +		$(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
>   		$(if $(V)$(DEBUG), --debug) \
>   		$(if $(DEBUG), --interactive) \
>   		$(if $(J),--jobs $(J)) \
> @@ -127,9 +128,9 @@ vm-boot-serial-%: $(IMAGES_DIR)/%.img
>   		-device virtio-net-pci,netdev=vnet \
>   	|| true
>   
> -vm-boot-ssh-%: $(IMAGES_DIR)/%.img
> +vm-boot-ssh-%: $(IMAGES_DIR)/%.img check-venv
>   	$(call quiet-command, \
> -		$(PYTHON) $(SRC_PATH)/tests/vm/$* \
> +		$(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
>   		$(if $(J),--jobs $(J)) \
>   		$(if $(V)$(DEBUG), --debug) \
>   		$(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 254e11c932b..d7d0413df35 100644
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -18,9 +18,6 @@
>   import logging
>   import time
>   import datetime
> -sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> -from qemu.machine import QEMUMachine
> -from qemu.utils import get_info_usernet_hostfwd_port, kvm_available
>   import subprocess
>   import hashlib
>   import argparse
> @@ -31,6 +28,9 @@
>   import traceback
>   import shlex
>   
> +from qemu.machine import QEMUMachine
> +from qemu.utils import get_info_usernet_hostfwd_port, kvm_available
> +
>   SSH_KEY_FILE = os.path.join(os.path.dirname(__file__),
>                  "..", "keys", "id_rsa")
>   SSH_PUB_KEY_FILE = os.path.join(os.path.dirname(__file__),

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Daniel P. Berrangé]

On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> RFC: This is a very early, crude attempt at switching over to an
> external Python package dependency for QMP. This series does not
> actually make the switch in and of itself, but instead just switches to
> the paradigm of using a venv in general to install the QEMU python
> packages instead of using PYTHONPATH to load them from the source tree.
> 
> (By installing the package, we can process dependencies.)
> 
> I'm sending it to the list so I can show you some of what's ugly so far
> and my notes on how I might make it less ugly.
> 
> (1) This doesn't trigger venv creation *from* iotests, it merely prints
> a friendly error message if "make check-venv" has not been run
> first. Not the greatest.

So if we run the sequence

  mkdir build
  cd build
  ../configure
  make
  ./tests/qemu-iotests/check 001

It won't work anymore, until we 'make check-venv' (or simply
'make check') ?

I'm somewhat inclined to say that venv should be created
unconditionally by default. ie a plain 'make' should always
everything needed to be able to invoke the tests directly.

> (2) This isn't acceptable for SRPM builds, because it uses PyPI to fetch
> packages just-in-time. My thought is to use an environment variable like
> QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
> process. We can use "--system-site-packages" as an argument to venv
> creation and "--no-index" as an argument to pip installation to achieve
> good behavior in SRPM building scenarios. It'd be up to the spec-writer
> to opt into that behavior.

I think I'd expect --system-site-packages to be the default behaviour.
We expect QEMU to be compatible with the packages available in the
distros that we're targetting. So if the dev has the python packages
installed from their distro, we should be using them preferentially.

This is similar to how we bundle slirp/capstone/etc, but will
preferentially use the distro version if it is available.

> (3) Using one venv for *all* tests means that avocado comes as a pre-req
> for iotests -- which adds avocado as a BuildRequires for the Fedora
> SRPM. That's probably not ideal. It may be better to model the test venv
> as something that can be created in stages: the "core" venv first, and
> the avocado packages only when needed.
> 
> You can see in these patches that I wasn't really sure how to tie the
> check-venv step as a dependency of 'check' or 'check-block', and it
> winds up feeling kind of hacky and fragile as a result.

See above, I'm inclined to say the venv should be created unconditionally

> (Patches 6 and 7 feel particularly fishy.)
> 
> What I think I would like to do is replace the makefile logic with a
> Python bootstrapping script. This will allow me to add in environment
> variable logic to accommodate #2 pretty easily. It will also allow
> iotests to call into the bootstrap script whenever it detects the venv
> isn't set up, which it needed to do anyway in order to print a
> user-friendly error message. Lastly, it will make it easier to create a
> "tiered" venv that layers in the avocado dependencies only as-needed,
> which avoids us having to bloat the SRPM build dependencies.

The tests is an area where we still have too much taking place in
Makefiles, as opposed to meson. Can we put a rule in
tests/meson.build to trigger the ven creation ? Gets us closer to
being able to run ninja without using make as a wrapper.

> In the end, I think that approach will:
> 
> - Allow us to run iotests without having to run a manual prep step
> - Keep additional SRPM deps to a minimum
> - Keep makefile hacks to a minimum
> 
> The only downside I am really frowning at is that I will have to
> replicate some "update the venv if it's outdated" logic that is usually
> handled by the Make system in the venv bootstrapper. Still, I think it's
> probably the only way to hit all of the requirements here without trying
> to concoct a fairly complex Makefile.

The only reason we need to update the venv is if a python dependancy
changes right ? If we're using system packages by default that's
a non-issue. If we're using the python-qemu.qmp as a git submodule,
we presumably only need to re-create the venv if we see that the
git submodule hash has changed. IOW, we don't need to worry about
tracking whether individual python deps are outdated.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH 8/9] iotests: fix source directory location

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
>               # called from the source tree
> -            self.source_iotests = os.getcwd()
> +            self.source_iotests = str(Path(__file__, '../').resolve())

'../' could be just '..', otherwise

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> Essentially, this:
> 
> (A) adjusts the python binary to be the one found in the venv (which is
> a symlink to the python binary chosen at configure time)
> 
> (B) adds a new VIRTUAL_ENV export variable
> 
> (C) changes PATH to front-load the venv binary directory.
> 
> If the venv directory isn't found, raise a friendly exception that tries
> to give the human operator a friendly clue as to what's gone wrong. In
> the very near future, I'd like to teach iotests how to fix this problem
> entirely of its own volition, but that's a trick for a little later.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/testenv.py | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 0007da3f06c..fd3720ed7e7 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
>       # lot of them. Silence pylint:
>       # pylint: disable=too-many-instance-attributes
>   
> -    env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> -                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> +    env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON',
> +                     'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> +                     'QEMU_PROG', 'QEMU_IMG_PROG',
>                        'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
>                        'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
>                        'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
> @@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
>               if val is not None:
>                   env[v] = val
>   
> +        env['PATH'] = os.pathsep.join((
> +            os.path.join(self.virtual_env, 'bin'),
> +            os.environ['PATH']
> +        ))
>           return env
>   
>       def init_directories(self) -> None:
> @@ -107,13 +112,17 @@ def init_directories(self) -> None:
>                SOCK_DIR
>                SAMPLE_IMG_DIR
>           """
> -
> -        # Path where qemu goodies live in this source tree.
> -        qemu_srctree_path = Path(__file__, '../../../python').resolve()
> +        venv_path = Path(self.build_root, 'tests/venv/')
> +        if not venv_path.exists():
> +            raise FileNotFoundError(
> +                f"Virtual environment \"{venv_path!s}\" isn't found."
> +                " (Maybe you need to run 'make check-venv'"
> +                " from the build dir?)"
> +            )
> +        self.virtual_env: str = str(venv_path)
>   
>           self.pythonpath = os.pathsep.join(filter(None, (
>               self.source_iotests,
> -            str(qemu_srctree_path),
>               os.getenv('PYTHONPATH'),
>           )))
>   
> @@ -138,7 +147,7 @@ def init_binaries(self) -> None:
>                PYTHON (for bash tests)
>                QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
>           """
> -        self.python = sys.executable
> +        self.python: str = os.path.join(self.virtual_env, 'bin', 'python3')

Is this guaranteed even if, say, only a /usr/bin/python3.9 exists? 
os.path.basename(sys.executable) might be more weirdness-proof than 
'python3'.

Paolo

Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
>   meson, create the python venv for block tests.
> +.PHONY: check-block
> +check-block: check-venv
> +	@echo  # Without some rule, this doesn't run at all. Why?
> +
> +
>   # Consolidated targets
>   
>   .PHONY: check check-clean get-vm-images
> -check:
> +check: check-venv
> +	@echo # ???
>   

I think you need instead:

# The do-meson-check and do-meson-bench targets are defined in Makefile.mtest
do-meson-check do-meson-bench: check-venv

and I would even add "all" to the targets that create the virtual environment.

Paolo

Re: [RFC PATCH 7/9] tests: add check-venv to build-tcg-disabled CI recipe

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   .gitlab-ci.d/buildtest.yml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 0aea7ab84c2..5c6201847f1 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -245,6 +245,7 @@ build-tcg-disabled:
>       - make -j"$JOBS"
>       - make check-unit
>       - make check-qapi-schema
> +    - make check-venv
>       - cd tests/qemu-iotests/
>       - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
>               052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

(unless you add it to "all").

Paolo

Re: [RFC PATCH 1/9] python: update for mypy 0.950

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> typeshed (included in mypy) recently updated to improve the typing for
> WriteTransport objects. I was working around this, but now there's a
> version where I shouldn't work around it.
> 
> Unfortunately this creates some minor ugliness if I want to support both
> pre- and post-0.950 versions. For now, for my sanity, just disable the
> unused-ignores warning.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Whatever floats your boat :)

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Paolo Bonzini]

On 5/13/22 10:35, Daniel P. Berrangé wrote:
> The tests is an area where we still have too much taking place in
> Makefiles, as opposed to meson. Can we put a rule in
> tests/meson.build to trigger the ven creation ? Gets us closer to
> being able to run ninja without using make as a wrapper.

I don't think this is or even should be a goal, because we have multiple 
projects under the QEMU tree:

- the QEMU binaries proper (emulators, tools, etc.)

- the firmware (pc-bios/{vof,s390-ccw,optionrom} with sources, the rest 
as submodules)

- tests/tcg


Each of these has its own build system and it's not possible to unify 
them under a single meson-based build:

- tests/tcg supports cross compilation for a different target, and 
pc-bios/ firmware will soon follow suit (which is why these directories 
haven't been converted to Meson, even though patches exist)

- firmware outside pc-bios/ consists of many external projects each with 
its own build system; right now it is not even buildable except by magic 
invocations that no one really knows

On top of this, there's support for building Docker images for 
cross-compilation which obviously doesn't fit the Meson usecases either.

In other words, Meson is the build system for QEMU *executables* (and 
that's why tests for the QEMU executables are being moved from configure 
to meson), but not for QEMU as a whole.


So I don't expect configure and Make to disappear.  Meson is great at 
building a C program as big as QEMU; but QEMU is not just a C program, 
and isolating the C parts into Meson lets Make handle the rest of the 
complexity better than before, for example with cross compiled firmware 
support.

Likewise, we're now using "meson test" for "make check" and a custom 
runner for "make check-block"; but perhaps one day Avocado could replace 
both runners via custom Avocado resolvers (basically JSON emitters 
similar to Meson introspection data).  That of course depends on whether 
Avocado has feature parity with "meson test".

Paolo

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Paolo Bonzini]

On 5/13/22 02:06, John Snow wrote:
> The only downside I am really frowning at is that I will have to
> replicate some "update the venv if it's outdated" logic that is usually
> handled by the Make system in the venv bootstrapper. Still, I think it's
> probably the only way to hit all of the requirements here without trying
> to concoct a fairly complex Makefile.
> 
> any thoughts? If not, I'll just move on to trying to hack up that
> version next instead.

I would not even bother with keeping the venv up to date.  Just initialize
it in configure, this is exactly what configure remains useful for in the
Meson-based world:

- add configure options --enable-python-qemu={enabled,system,internal,pip,
auto}/--disable-python-qemu (auto means system>internal>pip>disabled; enabled means
system>internal>pip>error) and matching CONFIG_PYTHON_QEMU=y to
config-host.mak

- use CONFIG_PYTHON_QEMU to enable/disable iotests in tests/qemu-iotests/meson.build

- add a configure option --enable-avocado=
{system,pip,auto,enabled}/--disable-avocado and matching
CONFIG_AVOCADO=y to config-host.mak

- use it to enable/disable acceptance tests in tests/Makefile.include

- build the venv in configure and use the options to pick the right pip install
commands, like

has_python_module() {
   $python -c "import $1" > /dev/null 2>&1
}

# do_pip VENV-PATH VAR PACKAGE [PATH] -- PIP-OPTIONS
do_pip() {
     local num_args source
     num_args=5
     test $4 = '--' && num_args=4
     eval source=\$$2
     # Try to resolve the package using a system install
     case $source in
       enabled|auto|system)
         if has_python_module $3; then
           source=system
         elif test $source = system; then
           error_exit "Python package $3 not installed"
         fi
     esac
     # See if a bundled copy is present
     case $source in
       enabled|auto|internal)
         if test $num_args = 5 && test -f $4/setup.cfg; then
           source=internal
         elif test $source = internal; then
           error_exit "Sources for Python package $3 not found in the QEMU source tree"
         fi
     esac
     # Install the bundled copy or let pip download the package
     case $source in
       internal)
         # The Pip error message should be clear enough
         (cd $1 && . bin/activate && pip install "$@") || exit 1
       ;;
       enabled|auto|pip)
         shift $num_args
         if (cd $1 && . bin/activate && pip install "$@"); then
           source=pip
         elif test $source = auto; then
           source=disabled
         else
           # The Pip error message should be clear enough
           exit 1
         fi
       ;;
     esac
     eval $2=\$source
}

rm -rf venv/
$python -m venv venv/
do_pip venv/ enable_python_qemu qemu.qmp python/qemu -- qemu.qmp
do_pip venv/ enable_avocado avocado -- -r tests/requirements.txt

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Daniel P. Berrangé]

On Fri, May 13, 2022 at 09:35:23AM +0100, Daniel P. Berrangé wrote:
> On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> > RFC: This is a very early, crude attempt at switching over to an
> > external Python package dependency for QMP. This series does not
> > actually make the switch in and of itself, but instead just switches to
> > the paradigm of using a venv in general to install the QEMU python
> > packages instead of using PYTHONPATH to load them from the source tree.
> > 
> > (By installing the package, we can process dependencies.)
> > 
> > I'm sending it to the list so I can show you some of what's ugly so far
> > and my notes on how I might make it less ugly.
> > 
> > (1) This doesn't trigger venv creation *from* iotests, it merely prints
> > a friendly error message if "make check-venv" has not been run
> > first. Not the greatest.
> 
> So if we run the sequence
> 
>   mkdir build
>   cd build
>   ../configure
>   make
>   ./tests/qemu-iotests/check 001
> 
> It won't work anymore, until we 'make check-venv' (or simply
> 'make check') ?
> 
> I'm somewhat inclined to say that venv should be created
> unconditionally by default. ie a plain 'make' should always
> everything needed to be able to invoke the tests directly.
> 
> > (2) This isn't acceptable for SRPM builds, because it uses PyPI to fetch
> > packages just-in-time. My thought is to use an environment variable like
> > QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
> > process. We can use "--system-site-packages" as an argument to venv
> > creation and "--no-index" as an argument to pip installation to achieve
> > good behavior in SRPM building scenarios. It'd be up to the spec-writer
> > to opt into that behavior.
> 
> I think I'd expect --system-site-packages to be the default behaviour.
> We expect QEMU to be compatible with the packages available in the
> distros that we're targetting. So if the dev has the python packages
> installed from their distro, we should be using them preferentially.
> 
> This is similar to how we bundle slirp/capstone/etc, but will
> preferentially use the distro version if it is available.

AFAICT from testing it, when '--system-site-packages' is set
for the venv, then 'pip install' appears to end up being a
no-op if the package is already present in the host, but
installs it if missing.

IOW, if we default to --system-site-packages, but still
also run 'pip install', we should "do the right thing".
It'll use any  distro packages that are available, and
augment with stuff from pip. In the no-op case, pip will
still try to consult the pipy servers to fetch version
info IIUC, so we need to be able to stop that. So I'd
suggest a  --python-env arg taking three values

 * "auto" (the default) - add --system-site-packages, but
   also run 'pip install'. Good for developers day-to-day

 * "system" - add --system-site-packages but never run
   'pip install'.  Good for formal package builds.

 * "pip"  - don't add --system-site-packages, always run
   'pip install'. Good for testing compatibility with
   bleeding edge python versions, or if explicit full
   independance from host python install is desired.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Kevin Wolf]

Am 13.05.2022 um 10:35 hat Daniel P. Berrangé geschrieben:
> On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> > RFC: This is a very early, crude attempt at switching over to an
> > external Python package dependency for QMP. This series does not
> > actually make the switch in and of itself, but instead just switches to
> > the paradigm of using a venv in general to install the QEMU python
> > packages instead of using PYTHONPATH to load them from the source tree.
> > 
> > (By installing the package, we can process dependencies.)
> > 
> > I'm sending it to the list so I can show you some of what's ugly so far
> > and my notes on how I might make it less ugly.
> > 
> > (1) This doesn't trigger venv creation *from* iotests, it merely prints
> > a friendly error message if "make check-venv" has not been run
> > first. Not the greatest.
> 
> So if we run the sequence
> 
>   mkdir build
>   cd build
>   ../configure
>   make
>   ./tests/qemu-iotests/check 001
> 
> It won't work anymore, until we 'make check-venv' (or simply
> 'make check') ?
> 
> I'm somewhat inclined to say that venv should be created
> unconditionally by default. ie a plain 'make' should always
> everything needed to be able to invoke the tests directly.

To be honest, I'm inclined to say that we already do too much of this.
When I change a source file, I usually just want qemu/qemu-img/qemu-io
updated to run my manual tests. If I want to run a test case, I have no
problem specifying this explicitly. This is what we had before the meson
switch. But what it does now is linking lots of test binaries (that I
won't run before I think that my change is finished and looking good)
and taking much longer than it should take.

Kevin

Re: [RFC PATCH 3/9] tests: install "qemu" namespace package into venv

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

On Fri, May 13, 2022, 4:26 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> > diff --git a/tests/requirements.txt b/tests/requirements.txt
> > index a21b59b4439..0ba561b6bdf 100644
> > --- a/tests/requirements.txt
> > +++ b/tests/requirements.txt
> > @@ -1,5 +1,6 @@
> >   # Add Python module requirements, one per line, to be installed
> >   # in the tests/venv Python virtual environment. For more info,
> >   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> > +# Note that qemu.git/python/ is always implicitly installed.
> >   avocado-framework==88.1
> >   pycdlib==1.11.0
>
> Any reason not to put ./python here?  But anyway,
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Paolo
>

The path didn't work under all circumstances - I got some bad path errors
for some permutations of CWD/build-type.

And I was not able to combine -e (for qemu) and -r (for this file) in a
single command, so I kept the qemu install separate/special.

Not ideal, I do admit.

(I wanted -e for the in-tree install to not create a potential future
landmine for someone changing python code and then getting confused as to
why nothing changed when running e.g. iotests.)

[-- Attachment #2: Type: text/html, Size: 2154 bytes --]

Re: [RFC PATCH 4/9] tests: silence pip upgrade warnings during venv creation

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

On Fri, May 13, 2022, 4:27 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index dbbf1ba535b..dfb678d379f 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -109,11 +109,11 @@ $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
> $(SRC_PATH)/python/setup.cfg
> >              $(PYTHON) -m venv $@, \
> >              VENV, $@)
> >       $(call quiet-command, \
> > -            $(TESTS_PYTHON) -m pip -q install \
> > +            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check
> install \
> >              -e "$(SRC_PATH)/python/", PIP, "$(SRC_PATH)/python/")
> >       $(call quiet-command, \
> > -            $(TESTS_PYTHON) -m pip -q install -r $(TESTS_VENV_REQ), \
> > -            PIP, $(TESTS_VENV_REQ))
> > +            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check
> install \
> > +            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
> >       $(call quiet-command, touch $@)
>
> Really nitpicking but I would have placed this change before adding the
> second invocation of pip. :)
>
> Paolo
>

You're right. This RFC was a little disorganized, I wasn't sure I was going
to keep any of this code just yet, so it missed a cleanup pass.

(Forgive me, please!)

[-- Attachment #2: Type: text/html, Size: 2084 bytes --]

Re: [RFC PATCH 1/9] python: update for mypy 0.950

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Fri, May 13, 2022, 4:42 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> > typeshed (included in mypy) recently updated to improve the typing for
> > WriteTransport objects. I was working around this, but now there's a
> > version where I shouldn't work around it.
> >
> > Unfortunately this creates some minor ugliness if I want to support both
> > pre- and post-0.950 versions. For now, for my sanity, just disable the
> > unused-ignores warning.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> Whatever floats your boat :)
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Paolo
>

Maybe I'll move towards pinning specific versions of analysis tools once we
move to always using a venv, and I won't have to try so hard to target a
wide spread of versions for mypy, pylint, etc.

I've tried pretty hard to "just have it work", but with the prevailing
idioms in the Python world being what they are, I am playing whackamole
virtually every release.

But, yeah, for now... meh. This keeps the boat afloat.

[-- Attachment #2: Type: text/html, Size: 1936 bytes --]

Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Fri, May 13, 2022, 4:41 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> >   meson, create the python venv for block tests.
> > +.PHONY: check-block
> > +check-block: check-venv
> > +     @echo  # Without some rule, this doesn't run at all. Why?
> > +
> > +
> >   # Consolidated targets
> >
> >   .PHONY: check check-clean get-vm-images
> > -check:
> > +check: check-venv
> > +     @echo # ???
> >
>
> I think you need instead:
>
> # The do-meson-check and do-meson-bench targets are defined in
> Makefile.mtest
> do-meson-check do-meson-bench: check-venv
>
> and I would even add "all" to the targets that create the virtual
> environment.
>
> Paolo
>

Great, thanks! I'll try that out today.

[-- Attachment #2: Type: text/html, Size: 1425 bytes --]

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 4517 bytes --]

On Fri, May 13, 2022, 4:38 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> > Essentially, this:
> >
> > (A) adjusts the python binary to be the one found in the venv (which is
> > a symlink to the python binary chosen at configure time)
> >
> > (B) adds a new VIRTUAL_ENV export variable
> >
> > (C) changes PATH to front-load the venv binary directory.
> >


(amending my commit message/rfc notes while I'm here:)

I'll add that this way of entering a venv is more complex than the method
used for VM tests and Avocado tests because it allows the possibility of
shell tests (et al) invoking python utilities and having those be "in the
venv" as well.

i.e. it's more rigorous and it matches the behavior of the "activate" shell
script bundled with every venv.


> > If the venv directory isn't found, raise a friendly exception that tries
> > to give the human operator a friendly clue as to what's gone wrong. In
> > the very near future, I'd like to teach iotests how to fix this problem
> > entirely of its own volition, but that's a trick for a little later.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   tests/qemu-iotests/testenv.py | 24 +++++++++++++++++-------
> >   1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index 0007da3f06c..fd3720ed7e7 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
> >       # lot of them. Silence pylint:
> >       # pylint: disable=too-many-instance-attributes
> >
> > -    env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR',
> 'SAMPLE_IMG_DIR',
> > -                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> > +    env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON',
> > +                     'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> > +                     'QEMU_PROG', 'QEMU_IMG_PROG',
> >                        'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
> >                        'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
> >                        'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
> > @@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
> >               if val is not None:
> >                   env[v] = val
> >
> > +        env['PATH'] = os.pathsep.join((
> > +            os.path.join(self.virtual_env, 'bin'),
> > +            os.environ['PATH']
> > +        ))
> >           return env
> >
> >       def init_directories(self) -> None:
> > @@ -107,13 +112,17 @@ def init_directories(self) -> None:
> >                SOCK_DIR
> >                SAMPLE_IMG_DIR
> >           """
> > -
> > -        # Path where qemu goodies live in this source tree.
> > -        qemu_srctree_path = Path(__file__, '../../../python').resolve()
> > +        venv_path = Path(self.build_root, 'tests/venv/')
> > +        if not venv_path.exists():
> > +            raise FileNotFoundError(
> > +                f"Virtual environment \"{venv_path!s}\" isn't found."
> > +                " (Maybe you need to run 'make check-venv'"
> > +                " from the build dir?)"
> > +            )
> > +        self.virtual_env: str = str(venv_path)
> >
> >           self.pythonpath = os.pathsep.join(filter(None, (
> >               self.source_iotests,
> > -            str(qemu_srctree_path),
> >               os.getenv('PYTHONPATH'),
> >           )))
> >
> > @@ -138,7 +147,7 @@ def init_binaries(self) -> None:
> >                PYTHON (for bash tests)
> >                QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG,
> QSD_PROG
> >           """
> > -        self.python = sys.executable
> > +        self.python: str = os.path.join(self.virtual_env, 'bin',
> 'python3')
>
> Is this guaranteed even if, say, only a /usr/bin/python3.9 exists?
> os.path.basename(sys.executable) might be more weirdness-proof than
> 'python3'.
>
> Paolo
>

It *should*, because "#!/usr/bin/env python3" is the preferred shebang for
Python scripts.

https://peps.python.org/pep-0394/

'python3' "should" be available. 'python' may not be.

Probably the "python" name in Makefile for TESTS_PYTHON should actually be
"python3" as well. In practice, all permutations (python, python3,
python3.9, etc.) are symlinks* to the binary used to create the venv. Which
links are present may be site configurable, but pep394 should guarantee
that python3 is always available.

(* not on Windows, where it'll be a copy.)

>

[-- Attachment #2: Type: text/html, Size: 6872 bytes --]

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 8039 bytes --]

On Fri, May 13, 2022, 4:35 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> > RFC: This is a very early, crude attempt at switching over to an
> > external Python package dependency for QMP. This series does not
> > actually make the switch in and of itself, but instead just switches to
> > the paradigm of using a venv in general to install the QEMU python
> > packages instead of using PYTHONPATH to load them from the source tree.
> >
> > (By installing the package, we can process dependencies.)
> >
> > I'm sending it to the list so I can show you some of what's ugly so far
> > and my notes on how I might make it less ugly.
> >
> > (1) This doesn't trigger venv creation *from* iotests, it merely prints
> > a friendly error message if "make check-venv" has not been run
> > first. Not the greatest.
>
> So if we run the sequence
>
>   mkdir build
>   cd build
>   ../configure
>   make
>   ./tests/qemu-iotests/check 001
>
> It won't work anymore, until we 'make check-venv' (or simply
> 'make check') ?
>

In this RFC as-is, that's correct. I want to fix that, because I dislike it
too.

Several ways to go about that.

I'm somewhat inclined to say that venv should be created
> unconditionally by default. ie a plain 'make' should always
> everything needed to be able to invoke the tests directly.
>

I'm leaning to agree with you, but I see Kevin has some doubts. My #1 goal
for Python refactoring is usually minimizing interruption to the block
maintainers. I do like the idea of just having it always available and
always taken care of, though.

(This would be useful for making sure that any python scripts or utilities
that need access to qmp/machine can be made to work, too. We can discuss
this problem a little later - the scripts/qmp/ folder needs some work. It
will come up in the full series to make the switch.)

OTOH, A concern about unconditionally building the test venv is that it
might introduce new dependencies for lots of downstreams that don't even
run the tests yet. I think I am partial to having it install on-demand,
because then the dependencies are opt-in. mjt told me that Debian does not
run make check as part of its build yet, for example.

I guess I can see it working either way. I think in the very immediate term
I'm motivated to have it be on-demand, but long term I think "as part of
make" is the eventual goal.


> > (2) This isn't acceptable for SRPM builds, because it uses PyPI to fetch
> > packages just-in-time. My thought is to use an environment variable like
> > QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
> > process. We can use "--system-site-packages" as an argument to venv
> > creation and "--no-index" as an argument to pip installation to achieve
> > good behavior in SRPM building scenarios. It'd be up to the spec-writer
> > to opt into that behavior.
>
> I think I'd expect --system-site-packages to be the default behaviour.
> We expect QEMU to be compatible with the packages available in the
> distros that we're targetting. So if the dev has the python packages
> installed from their distro, we should be using them preferentially.
>
> This is similar to how we bundle slirp/capstone/etc, but will
> preferentially use the distro version if it is available.
>

If you think that behavior should apply to tests as well, then OK. I shied
away from having it as the default because it's somewhat unusual to "cede
control" in a venv like this - the mere presence of certain packages in the
system environment may change behavior of certain python libraries. It is a
less well defined environment inherently.

I'll do some testing and I can try having it always do this. I'm curious
about cases where I might require "exactly mypy 0.780" and the user has
mypy 0.770 installed, or maybe even the other way around.

It may be surprising as to when the system packages get used and when they
don't - instinctively I like things that are less dynamic, but I see the
argument for wanting to prefer system packages when possible. At least for
the sake of downstream.

(I kind of feel like upstream should likewise prefer the upstream python
packages too, but ... You've got a lot more packaging experience than me,
so I'm willing to trust you on this point, but I'm personally a little
uncertain.)


> > (3) Using one venv for *all* tests means that avocado comes as a pre-req
> > for iotests -- which adds avocado as a BuildRequires for the Fedora
> > SRPM. That's probably not ideal. It may be better to model the test venv
> > as something that can be created in stages: the "core" venv first, and
> > the avocado packages only when needed.
> >
> > You can see in these patches that I wasn't really sure how to tie the
> > check-venv step as a dependency of 'check' or 'check-block', and it
> > winds up feeling kind of hacky and fragile as a result.
>
> See above, I'm inclined to say the venv should be created unconditionally
>
> > (Patches 6 and 7 feel particularly fishy.)
> >
> > What I think I would like to do is replace the makefile logic with a
> > Python bootstrapping script. This will allow me to add in environment
> > variable logic to accommodate #2 pretty easily. It will also allow
> > iotests to call into the bootstrap script whenever it detects the venv
> > isn't set up, which it needed to do anyway in order to print a
> > user-friendly error message. Lastly, it will make it easier to create a
> > "tiered" venv that layers in the avocado dependencies only as-needed,
> > which avoids us having to bloat the SRPM build dependencies.
>
> The tests is an area where we still have too much taking place in
> Makefiles, as opposed to meson. Can we put a rule in
> tests/meson.build to trigger the ven creation ? Gets us closer to
> being able to run ninja without using make as a wrapper.
>

Paolo has written a lot about this now, and he had some suggestions on
patches 6-8. I'll experiment with that and see if it feels less fragile.


> > In the end, I think that approach will:
> >
> > - Allow us to run iotests without having to run a manual prep step
> > - Keep additional SRPM deps to a minimum
> > - Keep makefile hacks to a minimum
> >
> > The only downside I am really frowning at is that I will have to
> > replicate some "update the venv if it's outdated" logic that is usually
> > handled by the Make system in the venv bootstrapper. Still, I think it's
> > probably the only way to hit all of the requirements here without trying
> > to concoct a fairly complex Makefile.
>
> The only reason we need to update the venv is if a python dependancy
> changes right ? If we're using system packages by default that's
> a non-issue. If we're using the python-qemu.qmp as a git submodule,
> we presumably only need to re-create the venv if we see that the
> git submodule hash has changed. IOW, we don't need to worry about
> tracking whether individual python deps are outdated.
>

The venv should probably not need to be updated very often, but it may
happen occasionally.

If tests/requirements.txt changes it should be updated, and if
python/setup.cfg|py changes it *might* need to be updated. (e.g. new or
removed subpackages, dependency updates, etc. An obvious one coming up is
the removal of qemu.qmp from in-tree and having that dependency be added to
setup.cfg.)

Using the editable installation mode, we won't need to reinstall the venv
if you edit any of the in-tree python modules (e.g. you add some debugging
prints to machine.py)

Even if we use system packages, we need to check that the version
requirements are fulfilled which involves at least re-running pip (not
necessarily recreating the whole venv) and allowing it the chance to fetch
new deps.

I have no plans to use git submodules.

With regards,
> Daniel
>

Thanks! I appreciate the feedback.

--js

[-- Attachment #2: Type: text/html, Size: 10636 bytes --]

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[Paolo Bonzini]

On 5/13/22 16:38, John Snow wrote:
> It *should*, because "#!/usr/bin/env python3" is the preferred shebang 
> for Python scripts.
> 
> https://peps.python.org/pep-0394/ <https://peps.python.org/pep-0394/>
> 
> 'python3' "should" be available. 'python' may not be.
> 
> Probably the "python" name in Makefile for TESTS_PYTHON should actually 
> be "python3" as well. In practice, all permutations (python, python3, 
> python3.9, etc.) are symlinks* to the binary used to create the venv. 
> Which links are present may be site configurable, but pep394 should 
> guarantee that python3 is always available.

IIRC we have some cases (FreeBSD?) where only the python3.x executable 
is available.  This is why we 1) default to Meson's Python 3 if neither 
--meson nor --python are passed, and 2) use the shebang you mention but 
with *non-executable* files, which Meson treats magically as "invoke 
with the Python interpreter that was used to launch me".

Paolo

Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block

[Paolo Bonzini]

On 5/13/22 16:12, John Snow wrote:
> 
>     I think you need instead:
> 
>     # The do-meson-check and do-meson-bench targets are defined in
>     Makefile.mtest
>     do-meson-check do-meson-bench: check-venv
> 
>     and I would even add "all" to the targets that create the virtual
>     environment.
> 
>     Paolo
> 
> 
> Great, thanks! I'll try that out today.

Well, check out the other suggestion of creating the venv at configure 
time, because that would remove all these complications/annoyances.

Paolo

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 4301 bytes --]

On Fri, May 13, 2022, 6:21 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> > The only downside I am really frowning at is that I will have to
> > replicate some "update the venv if it's outdated" logic that is usually
> > handled by the Make system in the venv bootstrapper. Still, I think it's
> > probably the only way to hit all of the requirements here without trying
> > to concoct a fairly complex Makefile.
> >
> > any thoughts? If not, I'll just move on to trying to hack up that
> > version next instead.
>
> I would not even bother with keeping the venv up to date.  Just initialize
>

I'm worried about this idea being very inconvenient for iterative
development of the python code.

it in configure, this is exactly what configure remains useful for in the
> Meson-based world:
>
> - add configure options --enable-python-qemu={enabled,system,internal,pip,
> auto}/--disable-python-qemu (auto means system>internal>pip>disabled;
> enabled means
> system>internal>pip>error) and matching CONFIG_PYTHON_QEMU=y to
> config-host.mak
>

I'm not sure this makes sense. python/qemu will continue to exist in-tree
and will only ever be "internal" in that sense. It won't be something you
can wholesale install from pip.

i.e. I plan to continue to break off pieces and upstream them, but I intend
to leave several modules as internal only.

So I'm not sure "internal" vs "pip" makes sense config-wise, it's intended
to be a mixture of both, really.

But, I suppose this is how you'd like to address different venv setup
behaviors to accommodate spec builds vs dev builds - with a configure flag
of some kind.

(I suppose you'd then like to see configure error out if it doesn't have
the necessary requisites given the venv-style chosen?)

- use CONFIG_PYTHON_QEMU to enable/disable iotests in
> tests/qemu-iotests/meson.build
>

So it's just skipped if you don't have the reqs to make the venv? (Not an
error?)


> - add a configure option --enable-avocado=
> {system,pip,auto,enabled}/--disable-avocado and matching
> CONFIG_AVOCADO=y to config-host.mak
>
> - use it to enable/disable acceptance tests in tests/Makefile.include
>

And this is how you propose eliminating the need for an always-present
avocado builddep.


> - build the venv in configure and use the options to pick the right pip
> install
> commands, like
>
> has_python_module() {
>    $python -c "import $1" > /dev/null 2>&1
> }
>
> # do_pip VENV-PATH VAR PACKAGE [PATH] -- PIP-OPTIONS
> do_pip() {
>      local num_args source
>      num_args=5
>      test $4 = '--' && num_args=4
>      eval source=\$$2
>      # Try to resolve the package using a system install
>      case $source in
>        enabled|auto|system)
>          if has_python_module $3; then
>            source=system
>          elif test $source = system; then
>            error_exit "Python package $3 not installed"
>          fi
>      esac
>      # See if a bundled copy is present
>      case $source in
>        enabled|auto|internal)
>          if test $num_args = 5 && test -f $4/setup.cfg; then
>            source=internal
>          elif test $source = internal; then
>            error_exit "Sources for Python package $3 not found in the QEMU
> source tree"
>          fi
>      esac
>      # Install the bundled copy or let pip download the package
>      case $source in
>        internal)
>          # The Pip error message should be clear enough
>          (cd $1 && . bin/activate && pip install "$@") || exit 1
>        ;;
>        enabled|auto|pip)
>          shift $num_args
>          if (cd $1 && . bin/activate && pip install "$@"); then
>            source=pip
>          elif test $source = auto; then
>            source=disabled
>          else
>            # The Pip error message should be clear enough
>            exit 1
>          fi
>        ;;
>      esac
>      eval $2=\$source
> }
>
> rm -rf venv/
> $python -m venv venv/
> do_pip venv/ enable_python_qemu qemu.qmp python/qemu -- qemu.qmp
> do_pip venv/ enable_avocado avocado -- -r tests/requirements.txt
>

Won't this rebuild the venv like *all of the time*, basically whenever you
see the "configuration is stale" message?

That both seems like way too often, *and* it wouldn't cover cases when it
really ought to be refreshed.

[-- Attachment #2: Type: text/html, Size: 6528 bytes --]

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]

On Fri, May 13, 2022, 6:29 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Fri, May 13, 2022 at 09:35:23AM +0100, Daniel P. Berrangé wrote:
> > On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> > > RFC: This is a very early, crude attempt at switching over to an
> > > external Python package dependency for QMP. This series does not
> > > actually make the switch in and of itself, but instead just switches to
> > > the paradigm of using a venv in general to install the QEMU python
> > > packages instead of using PYTHONPATH to load them from the source tree.
> > >
> > > (By installing the package, we can process dependencies.)
> > >
> > > I'm sending it to the list so I can show you some of what's ugly so far
> > > and my notes on how I might make it less ugly.
> > >
> > > (1) This doesn't trigger venv creation *from* iotests, it merely prints
> > > a friendly error message if "make check-venv" has not been run
> > > first. Not the greatest.
> >
> > So if we run the sequence
> >
> >   mkdir build
> >   cd build
> >   ../configure
> >   make
> >   ./tests/qemu-iotests/check 001
> >
> > It won't work anymore, until we 'make check-venv' (or simply
> > 'make check') ?
> >
> > I'm somewhat inclined to say that venv should be created
> > unconditionally by default. ie a plain 'make' should always
> > everything needed to be able to invoke the tests directly.
> >
> > > (2) This isn't acceptable for SRPM builds, because it uses PyPI to
> fetch
> > > packages just-in-time. My thought is to use an environment variable
> like
> > > QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
> > > process. We can use "--system-site-packages" as an argument to venv
> > > creation and "--no-index" as an argument to pip installation to achieve
> > > good behavior in SRPM building scenarios. It'd be up to the spec-writer
> > > to opt into that behavior.
> >
> > I think I'd expect --system-site-packages to be the default behaviour.
> > We expect QEMU to be compatible with the packages available in the
> > distros that we're targetting. So if the dev has the python packages
> > installed from their distro, we should be using them preferentially.
> >
> > This is similar to how we bundle slirp/capstone/etc, but will
> > preferentially use the distro version if it is available.
>
> AFAICT from testing it, when '--system-site-packages' is set
> for the venv, then 'pip install' appears to end up being a
> no-op if the package is already present in the host, but
> installs it if missing.
>
> IOW, if we default to --system-site-packages, but still
> also run 'pip install', we should "do the right thing".
> It'll use any  distro packages that are available, and
> augment with stuff from pip. In the no-op case, pip will
> still try to consult the pipy servers to fetch version
> info IIUC, so we need to be able to stop that. So I'd
> suggest a  --python-env arg taking three values
>
>  * "auto" (the default) - add --system-site-packages, but
>    also run 'pip install'. Good for developers day-to-day
>

Sounds like a decent balance...

...My only concern is that the system packages might be very old and it's
possible that the qemu packages will be "too new" or have conflicts with
the system deps.

I'll just have to test this.

e.g. what if I want to require mypy >= 0.900 for testing, but you have a
system package that requires mypy < 0.700?

I don't *know* that this is a problem, but my python-sense is tingling.

The python dep compatibility matrix I try to enforce and support for
testing is already feeling overly wide. This might force me to support an
even wider matrix, which I think is the precisely wrong direction for venvs
where we want tighter control as a rule.


>  * "system" - add --system-site-packages but never run
>    'pip install'.  Good for formal package builds.
>

We still have to install the in-tree qemu ns package, but we can use
--no-index to do it. It'll fail if the deps aren't met.


>  * "pip"  - don't add --system-site-packages, always run
>    'pip install'. Good for testing compatibility with
>    bleeding edge python versions, or if explicit full
>    independance from host python install is desired.
>

as arguments to configure, this spread of options makes sense to me than
paolo's version, but I've still got some doubt on mixing system and venv
packages.

I am also as of yet not sold on building the venv *from* configure, see my
response to Paolo on that topic.

I'll keep plugging away for now, but the big picture is still a tad murky
in my head.

--js

[-- Attachment #2: Type: text/html, Size: 6421 bytes --]

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]

On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 16:38, John Snow wrote:
> > It *should*, because "#!/usr/bin/env python3" is the preferred shebang
> > for Python scripts.
> >
> > https://peps.python.org/pep-0394/ <https://peps.python.org/pep-0394/>
> >
> > 'python3' "should" be available. 'python' may not be.
> >
> > Probably the "python" name in Makefile for TESTS_PYTHON should actually
> > be "python3" as well. In practice, all permutations (python, python3,
> > python3.9, etc.) are symlinks* to the binary used to create the venv.
> > Which links are present may be site configurable, but pep394 should
> > guarantee that python3 is always available.
>
> IIRC we have some cases (FreeBSD?) where only the python3.x executable
> is available.  This is why we 1) default to Meson's Python 3 if neither
> --meson nor --python are passed, and 2) use the shebang you mention but
> with *non-executable* files, which Meson treats magically as "invoke
> with the Python interpreter that was used to launch me".
>

This tidbit is particularly 😥


> Paolo
>

FreeBSD, why do you insist on hurting me?

(I'm surprised - python3 is *supposed* to be defined. Isn't this supremely
annoying for FreeBSD users to have every last Python shebang script not
work?)

OK. I'll test, and possibly update avocado's existing makefile magic if
necessary. It may be the case that the venv just works anyway. No way to
know but to test, I guess.

[-- Attachment #2: Type: text/html, Size: 2708 bytes --]

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Paolo Bonzini]

On 5/13/22 17:39, John Snow wrote:
> On Fri, May 13, 2022, 6:21 AM Paolo Bonzini <pbonzini@redhat.com 
> <mailto:pbonzini@redhat.com>> wrote:
> 
>     On 5/13/22 02:06, John Snow wrote:
>      > The only downside I am really frowning at is that I will have to
>      > replicate some "update the venv if it's outdated" logic that is
>     usually
>      > handled by the Make system in the venv bootstrapper. Still, I
>     think it's
>      > probably the only way to hit all of the requirements here without
>     trying
>      > to concoct a fairly complex Makefile.
>      >
>      > any thoughts? If not, I'll just move on to trying to hack up that
>      > version next instead.
> 
>     I would not even bother with keeping the venv up to date.  Just
>     initialize
> 
> I'm worried about this idea being very inconvenient for iterative 
> development of the python code.

Wouldn't "-e" mostly avoid the inconvenience?

> I'm not sure this makes sense. python/qemu will continue to exist 
> in-tree and will only ever be "internal" in that sense. It won't be 
> something you can wholesale install from pip.
>
> i.e. I plan to continue to break off pieces and upstream them, but I 
> intend to leave several modules as internal only.

Oh, that's what I was missing.  I thought long term all of it would come 
from pip.  But anyway...

> So I'm not sure "internal" vs "pip" makes sense config-wise, it's 
> intended to be a mixture of both, really.

... then neither "system" nor "pip" would not cover the parts of 
"python-qemu" that are always internal, i.e. currently only 
"python-qemu-qmp".  And after

  $python -m venv venv/

you would have

  $python -m pip install -e python/

(That's probably a better way to invoke a pip that's related to $python, 
at least until the venv exists).

By the way, where would you put the python-qemu-qmp submodule?

> But, I suppose this is how you'd like to address different venv setup 
> behaviors to accommodate spec builds vs dev builds - with a configure 
> flag of some kind.

Yes.

> (I suppose you'd then like to see configure error out if it doesn't have 
> the necessary requisites given the venv-style chosen?)
> 
>     - use CONFIG_PYTHON_QEMU to enable/disable iotests in
>     tests/qemu-iotests/meson.build
> 
> So it's just skipped if you don't have the reqs to make the venv? (Not 
> an error?)

That's usually what we do with missing bits yes; you can use 
--enable-python-qemu to force an error in that case.

>     - add a configure option --enable-avocado=
>     {system,pip,auto,enabled}/--disable-avocado and matching
>     CONFIG_AVOCADO=y to config-host.mak
> 
>     - use it to enable/disable acceptance tests in tests/Makefile.include
> 
> And this is how you propose eliminating the need for an always-present 
> avocado builddep.

Yep.

>     rm -rf venv/
>     $python -m venv venv/
>     do_pip venv/ enable_python_qemu qemu.qmp python/qemu -- qemu.qmp
>     do_pip venv/ enable_avocado avocado -- -r tests/requirements.txt
> 
> Won't this rebuild the venv like *all of the time*, basically whenever 
> you see the "configuration is stale" message?

Yes, it would.  I think that's going to happen less and less since 
configure is on a serious diet; but it might still be annoying.  OTOH 
installing system packages (or also "pip install --user") will speed up 
creating the virtual env, because then pip will not be run in the venv.

> That both seems like way too often, *and* it wouldn't cover cases when 
> it really ought to be refreshed.

Which cases are those?

Paolo

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Daniel P. Berrangé]

On Fri, May 13, 2022 at 11:55:18AM -0400, John Snow wrote:
> On Fri, May 13, 2022, 6:29 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Fri, May 13, 2022 at 09:35:23AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> > > > RFC: This is a very early, crude attempt at switching over to an
> > > > external Python package dependency for QMP. This series does not
> > > > actually make the switch in and of itself, but instead just switches to
> > > > the paradigm of using a venv in general to install the QEMU python
> > > > packages instead of using PYTHONPATH to load them from the source tree.
> > > >
> > > > (By installing the package, we can process dependencies.)
> > > >
> > > > I'm sending it to the list so I can show you some of what's ugly so far
> > > > and my notes on how I might make it less ugly.
> > > >
> > > > (1) This doesn't trigger venv creation *from* iotests, it merely prints
> > > > a friendly error message if "make check-venv" has not been run
> > > > first. Not the greatest.
> > >
> > > So if we run the sequence
> > >
> > >   mkdir build
> > >   cd build
> > >   ../configure
> > >   make
> > >   ./tests/qemu-iotests/check 001
> > >
> > > It won't work anymore, until we 'make check-venv' (or simply
> > > 'make check') ?
> > >
> > > I'm somewhat inclined to say that venv should be created
> > > unconditionally by default. ie a plain 'make' should always
> > > everything needed to be able to invoke the tests directly.
> > >
> > > > (2) This isn't acceptable for SRPM builds, because it uses PyPI to
> > fetch
> > > > packages just-in-time. My thought is to use an environment variable
> > like
> > > > QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
> > > > process. We can use "--system-site-packages" as an argument to venv
> > > > creation and "--no-index" as an argument to pip installation to achieve
> > > > good behavior in SRPM building scenarios. It'd be up to the spec-writer
> > > > to opt into that behavior.
> > >
> > > I think I'd expect --system-site-packages to be the default behaviour.
> > > We expect QEMU to be compatible with the packages available in the
> > > distros that we're targetting. So if the dev has the python packages
> > > installed from their distro, we should be using them preferentially.
> > >
> > > This is similar to how we bundle slirp/capstone/etc, but will
> > > preferentially use the distro version if it is available.
> >
> > AFAICT from testing it, when '--system-site-packages' is set
> > for the venv, then 'pip install' appears to end up being a
> > no-op if the package is already present in the host, but
> > installs it if missing.
> >
> > IOW, if we default to --system-site-packages, but still
> > also run 'pip install', we should "do the right thing".
> > It'll use any  distro packages that are available, and
> > augment with stuff from pip. In the no-op case, pip will
> > still try to consult the pipy servers to fetch version
> > info IIUC, so we need to be able to stop that. So I'd
> > suggest a  --python-env arg taking three values
> >
> >  * "auto" (the default) - add --system-site-packages, but
> >    also run 'pip install'. Good for developers day-to-day
> >
> 
> Sounds like a decent balance...
> 
> ...My only concern is that the system packages might be very old and it's
> possible that the qemu packages will be "too new" or have conflicts with
> the system deps.
> 
> I'll just have to test this.
> 
> e.g. what if I want to require mypy >= 0.900 for testing, but you have a
> system package that requires mypy < 0.700?

I would expect us to not require packages that are not present in
the distros implied by 

  https://www.qemu.org/docs/master/about/build-platforms.html

if that was absolutely a must have, then gracefully skip tests
if the system version wasn't new enough. The user could always
pass --python-env=pip if they want to force new enough

> The python dep compatibility matrix I try to enforce and support for
> testing is already feeling overly wide. This might force me to support an
> even wider matrix, which I think is the precisely wrong direction for venvs
> where we want tighter control as a rule.

As above, from a QEMU POV we have clearly defined our targetted
platform matrix. Splitting off python packages isn't a reason
to change QEMU's platform matrix IMHO.

> >  * "system" - add --system-site-packages but never run
> >    'pip install'.  Good for formal package builds.
> >
> 
> We still have to install the in-tree qemu ns package, but we can use
> --no-index to do it. It'll fail if the deps aren't met.

> >  * "pip"  - don't add --system-site-packages, always run
> >    'pip install'. Good for testing compatibility with
> >    bleeding edge python versions, or if explicit full
> >    independance from host python install is desired.
> >
> 
> as arguments to configure, this spread of options makes sense to me than
> paolo's version, but I've still got some doubt on mixing system and venv
> packages.
> 
> I am also as of yet not sold on building the venv *from* configure, see my
> response to Paolo on that topic.
> 
> I'll keep plugging away for now, but the big picture is still a tad murky
> in my head.
> 
> --js

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On Fri, May 13, 2022, 11:34 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 16:12, John Snow wrote:
> >
> >     I think you need instead:
> >
> >     # The do-meson-check and do-meson-bench targets are defined in
> >     Makefile.mtest
> >     do-meson-check do-meson-bench: check-venv
> >
> >     and I would even add "all" to the targets that create the virtual
> >     environment.
> >
> >     Paolo
> >
> >
> > Great, thanks! I'll try that out today.
>
> Well, check out the other suggestion of creating the venv at configure
> time, because that would remove all these complications/annoyances.
>
> Paolo
>

They also raise new annoyances and questions for me, so it might be worth
updating this "branch" of the patchset to have a basis of comparison for
what's the least annoying in the end.

(Or maybe even to serve as a basis while transitioning to the "better"
solution. It's quick to try, at least.)

Config script ideas are gonna take me a bit longer to work through.

(I'm not against developing a minimum viable patchset and having you tweak
it to your desire afterwards, if you have the time/interest. We can chat on
irc if you'd like. Otherwise, I'll just push forward.)

--js

>

[-- Attachment #2: Type: text/html, Size: 2064 bytes --]

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[Paolo Bonzini]

On 5/13/22 18:07, Daniel P. Berrangé wrote:
>> e.g. what if I want to require mypy >= 0.900 for testing, but you have a
>> system package that requires mypy < 0.700?
> I would expect us to not require packages that are not present in
> the distros implied by
> 
>    https://www.qemu.org/docs/master/about/build-platforms.html
> 
> if that was absolutely a must have, then gracefully skip tests
> if the system version wasn't new enough. The user could always
> pass --python-env=pip if they want to force new enough
> 

Consider that e.g. RHEL RPMs do not do mypy or pylint in %check, because 
the version of the linters in RHEL is usually older than what the 
upstream packages expect.

I don't think it's a good idea for QEMU to support what Red Hat 
packagers decided was a bad idea to support.

Paolo

Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2807 bytes --]

On Fri, May 13, 2022, 12:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 18:07, Daniel P. Berrangé wrote:
> >> e.g. what if I want to require mypy >= 0.900 for testing, but you have a
> >> system package that requires mypy < 0.700?
> > I would expect us to not require packages that are not present in
> > the distros implied by
> >
> >    https://www.qemu.org/docs/master/about/build-platforms.html
> >
> > if that was absolutely a must have, then gracefully skip tests
> > if the system version wasn't new enough. The user could always
> > pass --python-env=pip if they want to force new enough
> >
>
> Consider that e.g. RHEL RPMs do not do mypy or pylint in %check, because
> the version of the linters in RHEL is usually older than what the
> upstream packages expect.
>
> I don't think it's a good idea for QEMU to support what Red Hat
> packagers decided was a bad idea to support.
>
> Paolo
>

Yeah, I have to insist that due to the way these packages are developed
upstream that it is simply not reasonable to expect that the local package
version will work. pylint changes behavior virtually every single release.
This series itself even has a patch that is playing whackamole to support a
mypy that's brand new while supporting older mypy versions.

It's a huge overhead for little gain.

Far preferable to just say "Oh, your linter version is too old, we can't
run this test locally."

the qemu (and qemu.qmp) packages do not express a runtime/install
dependency on mypy/pylint/isort/flake8/avocado/tox. These packages only get
pulled in for the [devel] option group, and for good reason.

What is really the outlier here is iotest 297, which brings a kind of
meta-test into the same category as functional/regression tests. Supporting
this on default system packages is not on my personal todo list. Moving
this test off to a "make check-python" and deleting iotest 297 might be an
option. This is a test that simply might need to be skipped by an SRPM
build. (it already isn't run, so there's no additional harm by continuing
to not run it.)

If we are running a test suite and we allow pypi via the config, then I
believe we ought to allow the pypi versions to supersede the system ones -
*iff* the system ones are insufficient. I will continue to endeavor to test
a very wide matrix of dependencies, I just have to be honest that I don't
think I will reasonably achieve the full breadth you are asking for here.

For no-internet configs, we may have to accept that some platforms simply
don't have new enough dependencies to run some tests. I don't see this as
violating our build platform promise. I don't believe the build platform
promise ever reasonably extended to a "development platform promise".

--js

>

[-- Attachment #2: Type: text/html, Size: 3869 bytes --]

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]

On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 16:38, John Snow wrote:
> > It *should*, because "#!/usr/bin/env python3" is the preferred shebang
> > for Python scripts.
> >
> > https://peps.python.org/pep-0394/ <https://peps.python.org/pep-0394/>
> >
> > 'python3' "should" be available. 'python' may not be.
> >
> > Probably the "python" name in Makefile for TESTS_PYTHON should actually
> > be "python3" as well. In practice, all permutations (python, python3,
> > python3.9, etc.) are symlinks* to the binary used to create the venv.
> > Which links are present may be site configurable, but pep394 should
> > guarantee that python3 is always available.
>
> IIRC we have some cases (FreeBSD?) where only the python3.x executable
> is available.  This is why we 1) default to Meson's Python 3 if neither
> --meson nor --python are passed, and 2) use the shebang you mention but
> with *non-executable* files, which Meson treats magically as "invoke
> with the Python interpreter that was used to launch me".
>
> Paolo
>

pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do you
know in what circumstances you get only a point release binary?

Creating a venv on fbsd with "python3 -m venv testvenv" created a python3
binary link, but not a python3.8 link, also.

Still leaning towards the idea that "python3" is safest, but maybe it
depends on how you install from ports etc. I'd still say that it's
reasonable to expect that a system with python pays heed to PEP0394, I
think you've got a broken python install if you don't.

(But, what's the use case that forced your hand otherwise?)

--js

>

[-- Attachment #2: Type: text/html, Size: 2748 bytes --]

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[Paolo Bonzini]

On 5/14/22 17:55, John Snow wrote:
> On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com 
> <mailto:pbonzini@redhat.com>> wrote:
>     IIRC we have some cases (FreeBSD?) where only the python3.x executable
>     is available.  This is why we 1) default to Meson's Python 3 if neither
>     --meson nor --python are passed, and 2) use the shebang you mention but
>     with *non-executable* files, which Meson treats magically as "invoke
>     with the Python interpreter that was used to launch me".
> 
> pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do 
> you know in what circumstances you get only a point release binary?

Aha, tests/vm/freebsd installs python37, not python3.  But I guess it's 
still a plausible configuration for this packaging setup.

Paolo

> Creating a venv on fbsd with "python3 -m venv testvenv" created a 
> python3 binary link, but not a python3.8 link, also.
> 
> Still leaning towards the idea that "python3" is safest, but maybe it 
> depends on how you install from ports etc. I'd still say that it's 
> reasonable to expect that a system with python pays heed to PEP0394, I 
> think you've got a broken python install if you don't.
> 
> (But, what's the use case that forced your hand otherwise?)
> 
> --js
> 

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[John Snow]

On Mon, May 16, 2022 at 3:41 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 5/14/22 17:55, John Snow wrote:
> > On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com
> > <mailto:pbonzini@redhat.com>> wrote:
> >     IIRC we have some cases (FreeBSD?) where only the python3.x executable
> >     is available.  This is why we 1) default to Meson's Python 3 if neither
> >     --meson nor --python are passed, and 2) use the shebang you mention but
> >     with *non-executable* files, which Meson treats magically as "invoke
> >     with the Python interpreter that was used to launch me".
> >
> > pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do
> > you know in what circumstances you get only a point release binary?
>
> Aha, tests/vm/freebsd installs python37, not python3.  But I guess it's
> still a plausible configuration for this packaging setup.
>

Just confirming here that if you do 'pkg install python37' and you
have no 'python3' link, the venv package will still make 'python' and
'python3' links. I think it's likely best to use the 'python3' one.

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests

[Paolo Bonzini]

On 5/18/22 01:51, John Snow wrote:
>>> pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do
>>> you know in what circumstances you get only a point release binary?
>> Aha, tests/vm/freebsd installs python37, not python3.  But I guess it's
>> still a plausible configuration for this packaging setup.
>>
> Just confirming here that if you do 'pkg install python37' and you
> have no 'python3' link, the venv package will still make 'python' and
> 'python3' links. I think it's likely best to use the 'python3' one.

Ok, another good reason to always go through a venv.

Paolo