Re: [PATCH] run: introduce a script for running devel commands

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Dec 10, 2025 at 04:22:18PM +0000, Peter Maydell wrote:
> On Wed, 10 Dec 2025 at 16:06, Daniel P. Berrangé <berrange@redhat.com> 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é <berrange@redhat.com>
> > ---
> >
> > 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/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.)

Opps, since this is just an optional convenience script, the quick
fix is to just hide this within

   if host_os != 'windows'
      ...
   endif

> > 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 ?

I don't know, but we should probably believe what the
comment says and use /bin/bash to be sure.

> > +# 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.

Opps, yes, I tested this on top of John Snow's series
which moves the stuff into the venv. Until then, we do
need to add $SRC_DIR/python too

> 
> > +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?

Yes, I should quote it

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

Yes.


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 :|