diff mbox series

[04/10] configure: protect against escaping venv when running Meson

Message ID 20230222143752.466090-5-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show
Series improvement to Python detection, preparation for dropping 3.6 | expand

Commit Message

Paolo Bonzini Feb. 22, 2023, 2:37 p.m. UTC
If neither --python nor --meson are specified, Meson's generated
build.ninja will invoke Python script using the interpreter *that Meson
itself is running under*; not the one identified by configure.

This is only an issue if Meson's Python interpreter is not "the first
one in the path", which is the one that is used if --python is not
specified.  A common case where this happen is when the "python3" binary
comes from a virtual environment but Meson is not installed (with pip)
in the virtual environment.  In this case (presumably) whoever set up
the venv wanted to use the venv's Python interpreter to build QEMU,
while Meson might use a different one, for example an enterprise
distro's older runtime.

So, detect whether a virtual environment is setup, and if the virtual
environment does not have Meson, use the meson submodule.  Meson will
then run under the virtual environment's Python interpreter.

Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé Feb. 22, 2023, 3:04 p.m. UTC | #1
On Wed, Feb 22, 2023 at 03:37:46PM +0100, Paolo Bonzini wrote:
> If neither --python nor --meson are specified, Meson's generated
> build.ninja will invoke Python script using the interpreter *that Meson
> itself is running under*; not the one identified by configure.
> 
> This is only an issue if Meson's Python interpreter is not "the first
> one in the path", which is the one that is used if --python is not
> specified.  A common case where this happen is when the "python3" binary
> comes from a virtual environment but Meson is not installed (with pip)
> in the virtual environment.  In this case (presumably) whoever set up
> the venv wanted to use the venv's Python interpreter to build QEMU,
> while Meson might use a different one, for example an enterprise
> distro's older runtime.
> 
> So, detect whether a virtual environment is setup, and if the virtual
> environment does not have Meson, use the meson submodule.  Meson will
> then run under the virtual environment's Python interpreter.

I fear this could be somewhat confusing to contributors. If I have
meson in my $PATH, at a sufficient version, it would be surprising
to find QEMU had been using a different version instead.

I can understand wanting to make it "just work", but should we
perhaps issue a warning from configure when we're intentionally
ignoring an otherwise valid meson installation ?

> 
> Reported-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 00415f0b48d7..4d66a958023e 100755
> --- a/configure
> +++ b/configure
> @@ -1047,8 +1047,18 @@ fi
>  # Suppress writing compiled files
>  python="$python -B"
>  
> +has_meson() {
> +  if test "${VIRTUAL_ENV:+set}" = set; then
> +    # Ensure that Meson and Python come from the same virtual environment
> +    test -x "${VIRTUAL_ENV}/bin/meson" &&
> +      test "$(command -v meson)" -ef "${VIRTUAL_ENV}/bin/meson"
> +  else
> +    has meson
> +  fi
> +}
> +
>  if test -z "$meson"; then
> -    if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.61.5; then
> +    if test "$explicit_python" = no && has_meson && version_ge "$(meson --version)" 0.61.5; then
>          meson=meson
>      elif test "$git_submodules_action" != 'ignore' ; then
>          meson=git
> -- 
> 2.39.1
> 

With regards,
Daniel
Paolo Bonzini Feb. 22, 2023, 3:25 p.m. UTC | #2
On Wed, Feb 22, 2023 at 4:04 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > So, detect whether a virtual environment is setup, and if the virtual
> > environment does not have Meson, use the meson submodule.  Meson will
> > then run under the virtual environment's Python interpreter.
>
> I fear this could be somewhat confusing to contributors. If I have
> meson in my $PATH, at a sufficient version, it would be surprising
> to find QEMU had been using a different version instead.
>
> I can understand wanting to make it "just work", but should we
> perhaps issue a warning from configure when we're intentionally
> ignoring an otherwise valid meson installation ?

I don't think a warning is needed. First, the exact Meson version
should be pretty much neutral to the rest of the build system, and
QEMU should ship a good one (Meson's .0 releases aren't of the best
quality). Second, after all the user has _not_ specified --meson at
all in this scenario.

FWIW I think --sphinx-build and --meson should go away, and the single
way to invoke Python packages should be through $python. This means
that sphinx-build would have to be installed in $python's search path
or, in the future, installed via pip (same for meson once we decide
that it's okay to remove the bundled copy).

Overall, the result will be much more intuitive even if it may seem to
be a "less standard" setup. Both sphinx-build's and meson's
interpreters influence which interpreter is used during the build, and
it's confusing if it is anything but --python.

Paolo
diff mbox series

Patch

diff --git a/configure b/configure
index 00415f0b48d7..4d66a958023e 100755
--- a/configure
+++ b/configure
@@ -1047,8 +1047,18 @@  fi
 # Suppress writing compiled files
 python="$python -B"
 
+has_meson() {
+  if test "${VIRTUAL_ENV:+set}" = set; then
+    # Ensure that Meson and Python come from the same virtual environment
+    test -x "${VIRTUAL_ENV}/bin/meson" &&
+      test "$(command -v meson)" -ef "${VIRTUAL_ENV}/bin/meson"
+  else
+    has meson
+  fi
+}
+
 if test -z "$meson"; then
-    if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.61.5; then
+    if test "$explicit_python" = no && has_meson && version_ge "$(meson --version)" 0.61.5; then
         meson=meson
     elif test "$git_submodules_action" != 'ignore' ; then
         meson=git