[PATCH] Use meson's detected python installation

Peter Foley posted 1 patch 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250904-python-v1-1-c43b3209a0cd@google.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>
block/meson.build   |  6 ++----
meson.build         | 10 +++++-----
plugins/meson.build |  2 +-
3 files changed, 8 insertions(+), 10 deletions(-)
[PATCH] Use meson's detected python installation
Posted by Peter Foley 5 months ago
Relying on `python3` to be avilable in $PATH doesn't work in some build
environments. Update the build files to use the found python binary
explicitly.

Signed-off-by: Peter Foley <pefoley@google.com>
---
 block/meson.build   |  6 ++----
 meson.build         | 10 +++++-----
 plugins/meson.build |  2 +-
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/block/meson.build b/block/meson.build
index 34b1b2a30630214959630d5543181bc82a54d2b3..67e9bee1210307ff15ca87ba0f5e7f785df15042 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -139,14 +139,12 @@ if get_option('dmg').allowed()
   endforeach
 endif
 
-module_block_py = find_program('../scripts/modules/module_block.py')
 module_block_h = custom_target('module_block.h',
                                output: 'module_block.h',
                                input: modsrc,
-                               command: [module_block_py, '@OUTPUT0@', modsrc])
+                               command: [python, files('../scripts/modules/module_block.py'), '@OUTPUT0@', modsrc])
 block_ss.add(module_block_h)
 
-wrapper_py = find_program('../scripts/block-coroutine-wrapper.py')
 block_gen_c = custom_target('block-gen.c',
                             output: 'block-gen.c',
                             input: files(
@@ -158,7 +156,7 @@ block_gen_c = custom_target('block-gen.c',
                                       '../include/system/block-backend-io.h',
                                       'coroutines.h'
                                       ),
-                            command: [wrapper_py, '@OUTPUT@', '@INPUT@'])
+                            command: [python, files('../scripts/block-coroutine-wrapper.py'), '@OUTPUT@', '@INPUT@'])
 block_ss.add(block_gen_c)
 
 block_ss.add(files('stream.c'))
diff --git a/meson.build b/meson.build
index fa6186db33435c26d06dce2971a9f536250607e0..6e8baf3deb13c172eecd371ea302b1c2539048d0 100644
--- a/meson.build
+++ b/meson.build
@@ -12,8 +12,6 @@ add_test_setup('slow', exclude_suites: ['thorough'],
 add_test_setup('thorough',
                env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'])
 
-meson.add_postconf_script(find_program('scripts/symlink-install-tree.py'))
-
 ####################
 # Global variables #
 ####################
@@ -76,6 +74,8 @@ have_user = have_linux_user or have_bsd_user
 sh = find_program('sh')
 python = import('python').find_installation()
 
+meson.add_postconf_script([python, 'scripts/symlink-install-tree.py'])
+
 cc = meson.get_compiler('c')
 all_languages = ['c']
 if host_os == 'windows' and add_languages('cpp', required: false, native: false)
@@ -3474,7 +3474,7 @@ foreach target : target_dirs
       output: config_devices_mak,
       depfile: config_devices_mak + '.d',
       capture: true,
-      command: [minikconf,
+      command: [python, minikconf,
                 get_option('default_devices') ? '--defconfig' : '--allnoconfig',
                 config_devices_mak, '@DEPFILE@', '@INPUT@',
                 host_kconfig, target_kconfig])
@@ -3545,8 +3545,8 @@ config_host_h = configure_file(output: 'config-host.h', configuration: config_ho
 genh += config_host_h
 
 hxtool = find_program('scripts/hxtool')
-shaderinclude = find_program('scripts/shaderinclude.py')
-qapi_gen = find_program('scripts/qapi-gen.py')
+shaderinclude = [python, 'scripts/shaderinclude.py']
+qapi_gen = [python, 'scripts/qapi-gen.py']
 qapi_gen_depends = [ meson.current_source_dir() / 'scripts/qapi/__init__.py',
                      meson.current_source_dir() / 'scripts/qapi/commands.py',
                      meson.current_source_dir() / 'scripts/qapi/common.py',
diff --git a/plugins/meson.build b/plugins/meson.build
index 62c991d87fcdd8bcde8edddcc73909c6133f5460..6bf72a69060414ba1b7c1857515eeceb5a2c7b7c 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -6,7 +6,7 @@ qemu_plugin_symbols = configure_file(
   input: files('../include/qemu/qemu-plugin.h'),
   output: 'qemu-plugin.symbols',
   capture: true,
-  command: [files('../scripts/qemu-plugin-symbols.py'), '@INPUT@'])
+  command: [python, files('../scripts/qemu-plugin-symbols.py'), '@INPUT@'])
 
 # Modules need more symbols than just those in plugins/qemu-plugins.symbols
 if not enable_modules

---
base-commit: baa79455fa92984ff0f4b9ae94bed66823177a27
change-id: 20250904-python-78ccebd0fded

Best regards,
-- 
Peter Foley <pefoley@google.com>
Re: [PATCH] Use meson's detected python installation
Posted by Paolo Bonzini 5 months ago
On 9/4/25 17:11, Peter Foley wrote:
> Relying on `python3` to be avilable in $PATH doesn't work in some build
> environments. Update the build files to use the found python binary
> explicitly.

Meson already does this, if the file is not executable.  See 
docs/devel/build-system.rst:

   Meson has a special convention for invoking Python scripts: if their
   first line is ``#! /usr/bin/env python3`` and the file is *not*
   executable, find_program() arranges to invoke the script under the
   same Python interpreter that was used to invoke Meson.  This is the
   most common and preferred way to invoke support scripts from Meson
   build files, because it automatically uses the value of configure's
   --python= option.

Using "[python, 'foo']" is only needed for scripts "where it is 
desirable to make the script executable (for example for test scripts 
that developers may want to invoke from the command line, such as 
tests/qapi-schema/test-qapi.py)".

I think the only file you touched that is executable is 
scripts/qemu-plugin-symbols.py; so your issue can be fixed without code 
changes, and with a patch that only does "chmod -x 
scripts/qemu-plugin-symbols.py".  Please correct me if I'm wrong though!

Thanks,

Paolo

> Signed-off-by: Peter Foley <pefoley@google.com>
> ---
>   block/meson.build   |  6 ++----
>   meson.build         | 10 +++++-----
>   plugins/meson.build |  2 +-
>   3 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/block/meson.build b/block/meson.build
> index 34b1b2a30630214959630d5543181bc82a54d2b3..67e9bee1210307ff15ca87ba0f5e7f785df15042 100644
> --- a/block/meson.build
> +++ b/block/meson.build
> @@ -139,14 +139,12 @@ if get_option('dmg').allowed()
>     endforeach
>   endif
>   
> -module_block_py = find_program('../scripts/modules/module_block.py')
>   module_block_h = custom_target('module_block.h',
>                                  output: 'module_block.h',
>                                  input: modsrc,
> -                               command: [module_block_py, '@OUTPUT0@', modsrc])
> +                               command: [python, files('../scripts/modules/module_block.py'), '@OUTPUT0@', modsrc])
>   block_ss.add(module_block_h)
>   
> -wrapper_py = find_program('../scripts/block-coroutine-wrapper.py')
>   block_gen_c = custom_target('block-gen.c',
>                               output: 'block-gen.c',
>                               input: files(
> @@ -158,7 +156,7 @@ block_gen_c = custom_target('block-gen.c',
>                                         '../include/system/block-backend-io.h',
>                                         'coroutines.h'
>                                         ),
> -                            command: [wrapper_py, '@OUTPUT@', '@INPUT@'])
> +                            command: [python, files('../scripts/block-coroutine-wrapper.py'), '@OUTPUT@', '@INPUT@'])
>   block_ss.add(block_gen_c)
>   
>   block_ss.add(files('stream.c'))
> diff --git a/meson.build b/meson.build
> index fa6186db33435c26d06dce2971a9f536250607e0..6e8baf3deb13c172eecd371ea302b1c2539048d0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -12,8 +12,6 @@ add_test_setup('slow', exclude_suites: ['thorough'],
>   add_test_setup('thorough',
>                  env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'])
>   
> -meson.add_postconf_script(find_program('scripts/symlink-install-tree.py'))
> -
>   ####################
>   # Global variables #
>   ####################
> @@ -76,6 +74,8 @@ have_user = have_linux_user or have_bsd_user
>   sh = find_program('sh')
>   python = import('python').find_installation()
>   
> +meson.add_postconf_script([python, 'scripts/symlink-install-tree.py'])
> +
>   cc = meson.get_compiler('c')
>   all_languages = ['c']
>   if host_os == 'windows' and add_languages('cpp', required: false, native: false)
> @@ -3474,7 +3474,7 @@ foreach target : target_dirs
>         output: config_devices_mak,
>         depfile: config_devices_mak + '.d',
>         capture: true,
> -      command: [minikconf,
> +      command: [python, minikconf,
>                   get_option('default_devices') ? '--defconfig' : '--allnoconfig',
>                   config_devices_mak, '@DEPFILE@', '@INPUT@',
>                   host_kconfig, target_kconfig])
> @@ -3545,8 +3545,8 @@ config_host_h = configure_file(output: 'config-host.h', configuration: config_ho
>   genh += config_host_h
>   
>   hxtool = find_program('scripts/hxtool')
> -shaderinclude = find_program('scripts/shaderinclude.py')
> -qapi_gen = find_program('scripts/qapi-gen.py')
> +shaderinclude = [python, 'scripts/shaderinclude.py']
> +qapi_gen = [python, 'scripts/qapi-gen.py']
>   qapi_gen_depends = [ meson.current_source_dir() / 'scripts/qapi/__init__.py',
>                        meson.current_source_dir() / 'scripts/qapi/commands.py',
>                        meson.current_source_dir() / 'scripts/qapi/common.py',
> diff --git a/plugins/meson.build b/plugins/meson.build
> index 62c991d87fcdd8bcde8edddcc73909c6133f5460..6bf72a69060414ba1b7c1857515eeceb5a2c7b7c 100644
> --- a/plugins/meson.build
> +++ b/plugins/meson.build
> @@ -6,7 +6,7 @@ qemu_plugin_symbols = configure_file(
>     input: files('../include/qemu/qemu-plugin.h'),
>     output: 'qemu-plugin.symbols',
>     capture: true,
> -  command: [files('../scripts/qemu-plugin-symbols.py'), '@INPUT@'])
> +  command: [python, files('../scripts/qemu-plugin-symbols.py'), '@INPUT@'])
>   
>   # Modules need more symbols than just those in plugins/qemu-plugins.symbols
>   if not enable_modules
> 
> ---
> base-commit: baa79455fa92984ff0f4b9ae94bed66823177a27
> change-id: 20250904-python-78ccebd0fded
> 
> Best regards,
Re: [PATCH] Use meson's detected python installation
Posted by Peter Maydell 5 months ago
On Fri, 5 Sept 2025 at 08:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 9/4/25 17:11, Peter Foley wrote:
> > Relying on `python3` to be avilable in $PATH doesn't work in some build
> > environments. Update the build files to use the found python binary
> > explicitly.
>
> Meson already does this, if the file is not executable.  See
> docs/devel/build-system.rst:
>
>    Meson has a special convention for invoking Python scripts: if their
>    first line is ``#! /usr/bin/env python3`` and the file is *not*
>    executable, find_program() arranges to invoke the script under the
>    same Python interpreter that was used to invoke Meson.  This is the
>    most common and preferred way to invoke support scripts from Meson
>    build files, because it automatically uses the value of configure's
>    --python= option.
>
> Using "[python, 'foo']" is only needed for scripts "where it is
> desirable to make the script executable (for example for test scripts
> that developers may want to invoke from the command line, such as
> tests/qapi-schema/test-qapi.py)".

Wow, this seems like a super fragile way to do things. My
natural expectation would be that scripts generally would
be executable. If we accidentally make a script executable
that meson is invoking without explicit [python, ...]
then everything continues to work for almost everybody,
except in this corner case where the 'python3' on the path
is sufficiently wrong to not work: so we're likely to not
notice the mistake for ages.

-- PMM
Re: [PATCH] Use meson's detected python installation
Posted by Peter Foley 5 months ago
On Fri, Sep 5, 2025 at 3:25 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 9/4/25 17:11, Peter Foley wrote:
> > Relying on `python3` to be avilable in $PATH doesn't work in some build
> > environments. Update the build files to use the found python binary
> > explicitly.
>
> Meson already does this, if the file is not executable.  See
> docs/devel/build-system.rst:
>
>    Meson has a special convention for invoking Python scripts: if their
>    first line is ``#! /usr/bin/env python3`` and the file is *not*
>    executable, find_program() arranges to invoke the script under the
>    same Python interpreter that was used to invoke Meson.  This is the
>    most common and preferred way to invoke support scripts from Meson
>    build files, because it automatically uses the value of configure's
>    --python= option.
>
> Using "[python, 'foo']" is only needed for scripts "where it is
> desirable to make the script executable (for example for test scripts
> that developers may want to invoke from the command line, such as
> tests/qapi-schema/test-qapi.py)".
>

Interesting, that's not what I'm seeing in practice.
For example, locally reverting the change to block/meson.build results in:
FAILED: block/module_block.h
/build/work/046b6fd7014012220d3de53b1bd62f6eb1e9/google3/third_party/qemu/block/../scripts/modules/module_block.py
block/module_block.h
/usr/bin/env: 'python3': No such file or directory

Where module_block.py is *not* executable:
-rw-rw-r-- 1 pefoley primarygroup 2751 Feb 10  2021
third_party/qemu/scripts/modules/module_block.py


>
> I think the only file you touched that is executable is
> scripts/qemu-plugin-symbols.py; so your issue can be fixed without code
> changes, and with a patch that only does "chmod -x
> scripts/qemu-plugin-symbols.py".  Please correct me if I'm wrong though!
>
> Thanks,
>
> Paolo
>
> > Signed-off-by: Peter Foley <pefoley@google.com>
> > ---
> >   block/meson.build   |  6 ++----
> >   meson.build         | 10 +++++-----
> >   plugins/meson.build |  2 +-
> >   3 files changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/block/meson.build b/block/meson.build
> > index
> 34b1b2a30630214959630d5543181bc82a54d2b3..67e9bee1210307ff15ca87ba0f5e7f785df15042
> 100644
> > --- a/block/meson.build
> > +++ b/block/meson.build
> > @@ -139,14 +139,12 @@ if get_option('dmg').allowed()
> >     endforeach
> >   endif
> >
> > -module_block_py = find_program('../scripts/modules/module_block.py')
> >   module_block_h = custom_target('module_block.h',
> >                                  output: 'module_block.h',
> >                                  input: modsrc,
> > -                               command: [module_block_py, '@OUTPUT0@',
> modsrc])
> > +                               command: [python,
> files('../scripts/modules/module_block.py'), '@OUTPUT0@', modsrc])
> >   block_ss.add(module_block_h)
> >
> > -wrapper_py = find_program('../scripts/block-coroutine-wrapper.py')
> >   block_gen_c = custom_target('block-gen.c',
> >                               output: 'block-gen.c',
> >                               input: files(
> > @@ -158,7 +156,7 @@ block_gen_c = custom_target('block-gen.c',
> >
>  '../include/system/block-backend-io.h',
> >                                         'coroutines.h'
> >                                         ),
> > -                            command: [wrapper_py, '@OUTPUT@', '@INPUT@
> '])
> > +                            command: [python,
> files('../scripts/block-coroutine-wrapper.py'), '@OUTPUT@', '@INPUT@'])
> >   block_ss.add(block_gen_c)
> >
> >   block_ss.add(files('stream.c'))
> > diff --git a/meson.build b/meson.build
> > index
> fa6186db33435c26d06dce2971a9f536250607e0..6e8baf3deb13c172eecd371ea302b1c2539048d0
> 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -12,8 +12,6 @@ add_test_setup('slow', exclude_suites: ['thorough'],
> >   add_test_setup('thorough',
> >                  env: ['G_TEST_SLOW=1', 'SPEED=thorough',
> 'RUST_BACKTRACE=1'])
> >
> >
> -meson.add_postconf_script(find_program('scripts/symlink-install-tree.py'))
> > -
> >   ####################
> >   # Global variables #
> >   ####################
> > @@ -76,6 +74,8 @@ have_user = have_linux_user or have_bsd_user
> >   sh = find_program('sh')
> >   python = import('python').find_installation()
> >
> > +meson.add_postconf_script([python, 'scripts/symlink-install-tree.py'])
> > +
> >   cc = meson.get_compiler('c')
> >   all_languages = ['c']
> >   if host_os == 'windows' and add_languages('cpp', required: false,
> native: false)
> > @@ -3474,7 +3474,7 @@ foreach target : target_dirs
> >         output: config_devices_mak,
> >         depfile: config_devices_mak + '.d',
> >         capture: true,
> > -      command: [minikconf,
> > +      command: [python, minikconf,
> >                   get_option('default_devices') ? '--defconfig' :
> '--allnoconfig',
> >                   config_devices_mak, '@DEPFILE@', '@INPUT@',
> >                   host_kconfig, target_kconfig])
> > @@ -3545,8 +3545,8 @@ config_host_h = configure_file(output:
> 'config-host.h', configuration: config_ho
> >   genh += config_host_h
> >
> >   hxtool = find_program('scripts/hxtool')
> > -shaderinclude = find_program('scripts/shaderinclude.py')
> > -qapi_gen = find_program('scripts/qapi-gen.py')
> > +shaderinclude = [python, 'scripts/shaderinclude.py']
> > +qapi_gen = [python, 'scripts/qapi-gen.py']
> >   qapi_gen_depends = [ meson.current_source_dir() /
> 'scripts/qapi/__init__.py',
> >                        meson.current_source_dir() /
> 'scripts/qapi/commands.py',
> >                        meson.current_source_dir() /
> 'scripts/qapi/common.py',
> > diff --git a/plugins/meson.build b/plugins/meson.build
> > index
> 62c991d87fcdd8bcde8edddcc73909c6133f5460..6bf72a69060414ba1b7c1857515eeceb5a2c7b7c
> 100644
> > --- a/plugins/meson.build
> > +++ b/plugins/meson.build
> > @@ -6,7 +6,7 @@ qemu_plugin_symbols = configure_file(
> >     input: files('../include/qemu/qemu-plugin.h'),
> >     output: 'qemu-plugin.symbols',
> >     capture: true,
> > -  command: [files('../scripts/qemu-plugin-symbols.py'), '@INPUT@'])
> > +  command: [python, files('../scripts/qemu-plugin-symbols.py'), '@INPUT@
> '])
> >
> >   # Modules need more symbols than just those in
> plugins/qemu-plugins.symbols
> >   if not enable_modules
> >
> > ---
> > base-commit: baa79455fa92984ff0f4b9ae94bed66823177a27
> > change-id: 20250904-python-78ccebd0fded
> >
> > Best regards,
>
>
Re: [PATCH] Use meson's detected python installation
Posted by Paolo Bonzini 5 months ago
On Fri, Sep 5, 2025 at 5:01 PM Peter Foley <pefoley@google.com> wrote:
> Interesting, that's not what I'm seeing in practice.
> For example, locally reverting the change to block/meson.build results in:
> FAILED: block/module_block.h
> /build/work/046b6fd7014012220d3de53b1bd62f6eb1e9/google3/third_party/qemu/block/../scripts/modules/module_block.py block/module_block.h
> /usr/bin/env: 'python3': No such file or directory
>
> Where module_block.py is *not* executable:
> -rw-rw-r-- 1 pefoley primarygroup 2751 Feb 10  2021 third_party/qemu/scripts/modules/module_block.py

What is the version of meson, and the actual command line? In my case
it's  "/home/.../+build/pyvenv/bin/python3
/home/pbonzini/work/upstream/qemu/block/../scripts/modules/module_block.py
block/module_block.h"..

In case you would like to debug it, here are some pointers. The Meson
code that handles it is, starting from the constructor:

    if search_dirs is None:
        # For compat with old behaviour
        search_dirs = [None]
    self.command = self._search(name, search_dirs, exclude_paths)

The search_dirs list is simply
[os.path.join(self.environment.get_source_dir(), self.subdir)]; see
program_for_siystem in mesonbuild/interpreter/interpreter.py. _search
simply walks the list:

    for search_dir in search_dirs:
        commands = self._search_dir(name, search_dir)
        if commands:
            return commands

and here is when the non-executable case is handled:

    def _search_dir(self, name: str, search_dir: T.Optional[str]) ->
T.Optional[list]:
        if os.path.exists(trial):
            if self._is_executable(trial):
                return [trial]
            # Now getting desperate. Maybe it is a script file that is
            # a) not chmodded executable, or
            # b) we are on windows so they can't be directly executed.
            return self._shebang_to_cmd(trial)

from which you go to

   # Replace python3 with the actual python3 that we are using
   if commands[0] == '/usr/bin/env' and commands[1] == 'python3':
       commands = mesonlib.python_command + commands[2:]
   elif commands[0].split('/')[-1] == 'python3':
       commands = mesonlib.python_command + commands[1:]

and mesonlib.python_command should be the pyvenv Python interpreter.

Paolo