On 5/26/22 02:09, John Snow wrote:
> Remove the sys.path hacking from device-crash-test, and add in a little
> user-friendly message for anyone who was used to running this script
> directly from the source tree.
>
> Modify the GitLab job recipes to create the tests/venv first, then run
> device-crash-test from that venv.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> .gitlab-ci.d/buildtest.yml | 8 +++++---
> scripts/device-crash-test | 14 +++++++++++---
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index e9620c30748..fde29c35aa3 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -110,7 +110,8 @@ crash-test-debian:
> IMAGE: debian-amd64
> script:
> - cd build
> - - scripts/device-crash-test -q ./qemu-system-i386
> + - make check-venv
> + - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-i386
>
> build-system-fedora:
> extends: .native_build_job_template
> @@ -155,8 +156,9 @@ crash-test-fedora:
> IMAGE: fedora
> script:
> - cd build
> - - scripts/device-crash-test -q ./qemu-system-ppc
> - - scripts/device-crash-test -q ./qemu-system-riscv32
> + - make check-venv
> + - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc
> + - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32
>
> build-system-centos:
> extends: .native_build_job_template
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index a203b3fdea2..73bcb986937 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -33,10 +33,18 @@ import re
> import random
> import argparse
> from itertools import chain
> +from pathlib import Path
>
> -sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
> -from qemu.machine import QEMUMachine
> -from qemu.qmp import ConnectError
> +try:
> + from qemu.machine import QEMUMachine
> + from qemu.qmp import ConnectError
> +except ModuleNotFoundError as exc:
> + path = Path(__file__).resolve()
> + print(f"Module '{exc.name}' not found.")
> + print(" Try 'make check-venv' from your build directory,")
> + print(" and then one way to run this script is like so:")
> + print(f' > $builddir/tests/venv/bin/python3 "{path}"')
> + sys.exit(1)
>
> logger = logging.getLogger('device-crash-test')
> dbg = logger.debug
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Even though I'd still prefer the venv to be setup early (so the
check-venv change in buildtest.yml and the friendly message in the
script will go away), this is a step in the right direction.
Paolo
Agree, figured I'd do baby steps before I wound up with a 40 patch series, and this gives Thomas et al a chance to find out if this ruins their workflow.
(I'll probably keep the friendly message a little while more anyway, though; to catch anyone who runs this script manually for a release or so. I should add a section to our QEMU developer's guide and just link to it in the message and explain the many, many ways you might enter a venv or install the package.)