On Wed, 10 Dec 2025 at 16:06, Daniel P. Berrangé <[email protected]> wrote:
>
> Various aspects of the development workflow are complicated by the need
> to set env variables ahead of time, or use specific paths. Introduce a
> $BUILD_DIR/run script that will do a number of things
>
>  * Set $PATH to point to $BUILD_DIR/qemu-bundle/$PREFIX/$BIN_DIR
>  * Set $PYTHONPATH to point to $SRC_DIR/tests/functional
>
>  * Source $BUILD_DIR/pyvenv/bin/activate
>
> To see the benefits of this consider this command:
>
>   $ source ./build/pyvenv/bin/activate
>   $ ./scripts/qmp/qmp-shell-wrap ./build/qemu-system-x86_64
>
> which is now simplified to
>
>   $ ./build/run ./scripts/qmp/qmp-shell-wrap qemu-system-x86_64 [args..]
>
> This avoids the need repeat './build' several times and avoids polluting
> the current terminal's environment and/or avoids errors from forgetting
> to source the venv settings.
>
> As another example running functional tests
>
>   $ export PYTHONPATH=./python:./tests/functional
>   $ export QEMU_TEST_QEMU_BINARY=./build/qemu-system-x86_64
>   $ build/pyvenv/bin/python3 ./tests/functional/x86_64/test_virtio_version.py
>
> which is now simplified to
>
>   $ export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
>   $ ./build/run ./tests/functional/x86_64/test_virtio_version.py
>
> This usefulness of this will be further enhanced with the pending
> removal of the QEMU python APIs from git, as that will require the use
> of the python venv in even more scenarios that today.

Mmm, I think this will be helpful. My review comments
below are all nitpicks.

> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
>
> Historical context: this 'run' script concept is something introduced
> by libguestfs a decade & a half ago, and copied by libvirt shortly
> after that. It has been very helpful in simplifying life for developers
> and should do likewise for QEMU.
>
>  docs/devel/build-system.rst       | 12 ++++++++++++
>  docs/devel/testing/functional.rst | 17 ++++++++---------
>  meson.build                       | 11 +++++++++++
>  run.in                            | 15 +++++++++++++++
>  4 files changed, 46 insertions(+), 9 deletions(-)
>  create mode 100644 run.in
>
> diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
> index 6204aa6a72..8ec8d20175 100644
> --- a/docs/devel/build-system.rst
> +++ b/docs/devel/build-system.rst
> @@ -515,6 +515,18 @@ generates ``Makefile`` from ``Makefile.in``.
>
>  Built by configure:
>
> +``run``
> +  Used to run commands / scripts from the git checkout. Sets ``$PATH``
> +  to point to locally built binaries & activates the python venv before

", and"

> +  running the requested command. Pass the command to run as args, for
> +  example::
> +
> +    $ ./build/run ./script/qmp/qmp-shell-wrap qemu-system-x86_64
> +
> +  will use the ``python3`` binary and site-packages from the local
> +  venv to run ``qmp-shell-wrap`` and spawn the QEMU emulator from
> +  the build directory.
> +
>  ``config-host.mak``
>    When configure has determined the characteristics of the build host it
>    will write the paths to various tools to this file, for use in ``Makefile``
> diff --git a/docs/devel/testing/functional.rst 
> b/docs/devel/testing/functional.rst
> index fdeaebaadc..1978f96eba 100644
> --- a/docs/devel/testing/functional.rst
> +++ b/docs/devel/testing/functional.rst
> @@ -53,15 +53,14 @@ the following line will only run the tests for the x86_64 
> target:
>    make check-functional-x86_64
>
>  To run a single test file without the meson test runner, you can also
> -execute the file directly by specifying two environment variables first,
> -the PYTHONPATH that has to include the python folder and the tests/functional
> -folder of the source tree, and QEMU_TEST_QEMU_BINARY that has to point
> -to the QEMU binary that should be used for the test. The current working
> -directory should be your build folder. For example::
> -
> -  $ export PYTHONPATH=../python:../tests/functional
> -  $ export QEMU_TEST_QEMU_BINARY=$PWD/qemu-system-x86_64
> -  $ pyvenv/bin/python3 ../tests/functional/test_file.py
> +execute the file directly by specifying the name of the emulator target
> +binary as an env variable.
> +
> +Assuming the current working directory is the top level source checkout
> +and the build directory is './build'::
> +
> +  $ export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
> +  $ ./build/run tests/functional/x86_64/test_virtio_version.py
>
>  The test framework will automatically purge any scratch files created during
>  the tests. If needing to debug a failed test, it is possible to keep these
> diff --git a/meson.build b/meson.build
> index d9293294d8..8f2320d362 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3507,6 +3507,17 @@ endif
>  config_host_h = configure_file(output: 'config-host.h', configuration: 
> config_host_data)
>  genh += config_host_h
>
> +run_config = configuration_data(
> +    {'build_dir': meson.current_build_dir(),
> +     'src_dir': meson.current_source_dir(),
> +     'bin_dir': get_option('prefix') / get_option('bindir')},
> +)
> +
> +run = configure_file(input: 'run.in',
> +                     output: 'run',
> +                     configuration: run_config)
> +run_command('chmod', 'a+x', meson.current_build_dir() / 'run', check: true)

Does this work on Windows hosts ? (Obviously the run script
isn't going to be any use if you don't have a Posix-ish shell,
but we should at least not fall over.)

> +
>  hxtool = find_program('scripts/hxtool')
>  shaderinclude = find_program('scripts/shaderinclude.py')
>  qapi_gen = find_program('scripts/qapi-gen.py')
> diff --git a/run.in b/run.in
> new file mode 100644
> index 0000000000..124f0daed2
> --- /dev/null
> +++ b/run.in
> @@ -0,0 +1,15 @@
> +#!/bin/sh

In my build tree the 'activate' script says it must
be used "from bash"; is it wrong, or should this be
#!/bin/bash instead ?

> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +# Ensure that we find our local builds first
> +PATH=@build_dir@/qemu-bundle/@bin_dir@:$PATH
> +export PATH
> +
> +# Ensure that functional tests find their lib
> +PYTHONPATH=@src_dir@/tests/functional${PYTHONPATH:+:${PYTHONPATH}}

docs/devel/testing/functional.rst says PYTHONPATH also
needs to have $SRC_DIR/python on it.

> +export PYTHONPATH
> +
> +# Ensure that everything uses the venv python & site packages
> +source @build_dir@/pyvenv/bin/activate

Aren't there quoting issues here if e.g. @build_dir@ has
a space in it, or some existing PYTHONPATH or PATH
entry has a space in it?

> +exec $@

I thought this needed to be quoted, i.e. "$@" rather
than $@.

thanks
-- PMM

Reply via email to