[RFC PATCH 0/3] configure: create a python venv and install meson

[RFC PATCH 0/3] configure: create a python venv and install meson

[John Snow]

This patch series creates a mandatory venv during configure time and
uses it to install meson.

The eventual point of this is to ensure that the Python used to run
meson is the same Python used to run Sphinx, tests, and any build-time
scripting we have. As it stands, meson and sphinx (and their extensions)
may run in a different Python environment than the one configured and
chosen by the user at configure/build time.

My goals for this series and future work are:

- Unconditionally create a venv to be used for both building and testing
  at configure time
- The python interpreter used by this venv is unconditionally the one
  specified by configure
- *all* python scripts in qemu.git executed as part of the build system,
   tests, or CI are *always* executed in the context of this venv.
- It is possible to build and test fully offline with sufficient
  preparation by installing appropriate system packages.
- Missing depdencies, when possible, are fetched and installed
  automatically to make developer environments "just work".
- Minimize cost during configure time whenever possible.

This series is in RFC state; some notable things:
- So far, only meson is included in this setup.
- There's a ton of debugging prints everywhere. It's extremely chatty right now.
- Sphinx and qemu.qmp are not yet included in this venv.
  (qemu.qmp isn't able to be removed from the tree yet.)
- Testing isn't yet *fully* switched over.
- There is no online functionality yet, this series is *100% offline* -
  it's the harder option, so I tackled it first.

Some known bugs as of now:
- venv-in-venv setups are not yet handled.
- python3.7 setups without setuptools/pip in the host environment may be
  unable to generate script file shims; a workaround is in development
  but wasn't ready today. I decided to exclude it.

The good news:
- Works for Python 3.7 and up, on Fedora, OpenSuSE, Red Hat, CentOS, Alpine,
  Debian, Ubuntu, NetBSD, OpenBSD, and hopefully everywhere
- Startup cost is only milliseconds on the fast path
- No new dependencies (...for most platforms; the asterisk is Debian.)
- No new configure flags (...yet.)

John Snow (3):
  python: add mkvenv.py
  tests: add python3-venv dependency
  configure: install meson to a python virtual environment

 configure                                     | 155 ++++--
 .gitlab-ci.d/buildtest-template.yml           |   2 +-
 python/scripts/mkvenv.py                      | 445 ++++++++++++++++++
 .../dockerfiles/debian-all-test-cross.docker  |   3 +-
 .../dockerfiles/debian-hexagon-cross.docker   |   3 +-
 .../dockerfiles/debian-riscv64-cross.docker   |   3 +-
 .../dockerfiles/debian-tricore-cross.docker   |   3 +-
 7 files changed, 561 insertions(+), 53 deletions(-)
 create mode 100644 python/scripts/mkvenv.py

-- 
2.39.2

[RFC PATCH 1/3] python: add mkvenv.py

[John Snow]

This script will be responsible for building a Python virtual
environment at configure time. As such, it exists outside of the
installable python packages and *must* be runnable with minimal
dependencies.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/scripts/mkvenv.py | 445 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 445 insertions(+)
 create mode 100644 python/scripts/mkvenv.py

diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
new file mode 100644
index 00000000000..d48880c4205
--- /dev/null
+++ b/python/scripts/mkvenv.py
@@ -0,0 +1,445 @@
+"""
+mkvenv - QEMU venv bootstrapping utility
+
+usage: TODO/FIXME
+"""
+
+# Copyright (C) 2022-2023 Red Hat, Inc.
+#
+# Authors:
+#  John Snow <jsnow@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+import argparse
+from importlib.util import find_spec
+import logging
+import os
+from os import PathLike
+from pathlib import Path
+import re
+import stat
+import subprocess
+import sys
+import traceback
+from types import SimpleNamespace
+from typing import Generator, Dict, Sequence, Optional, Union, Iterator
+import venv
+
+
+# Do not add any mandatory dependencies from outside the stdlib:
+# This script *must* be usable standalone!
+
+DirType = Union[str, bytes, 'PathLike[str]', 'PathLike[bytes]']
+logger = logging.getLogger('mkvenv')
+
+
+class Ouch(RuntimeError):
+    """An Exception class we can't confuse with a builtin."""
+
+
+class QemuEnvBuilder(venv.EnvBuilder):
+    """
+    An extension of venv.EnvBuilder for building QEMU's configure-time venv.
+
+    The only functional change is that it adds the ability to regenerate
+    console_script shims for packages available via system_site
+    packages.
+
+    (And a metric ton of debugging printfs)
+
+    Parameters for base class init:
+      - system_site_packages: bool = False
+      - clear: bool = False
+      - symlinks: bool = False
+      - upgrade: bool = False
+      - with_pip: bool = False
+      - prompt: Optional[str] = None
+      - upgrade_deps: bool = False             (Since 3.9)
+    """
+    def __init__(self, *args, **kwargs) -> None:
+        self.script_packages = kwargs.pop('script_packages', ())
+        super().__init__(*args, **kwargs)
+        # venv class is cute and toggles this off before post_setup,
+        # but we need it to decide if we want to generate shims or not.
+        self._system_site_packages = self.system_site_packages
+        # Make the context available post-creation:
+        self._context: Optional[SimpleNamespace] = None
+
+    def ensure_directories(self, env_dir: DirType) -> SimpleNamespace:
+        logger.debug("ensure_directories(env_dir=%s)", env_dir)
+        context = super().ensure_directories(env_dir)
+        # Here's what's in the context blob:
+        logger.debug("env_dir      %s", context.env_dir)
+        logger.debug("env_name     %s", context.env_name)
+        logger.debug("prompt       %s", context.prompt)
+        logger.debug("executable   %s", context.executable)
+        logger.debug("inc_path     %s", context.inc_path)
+        if 'lib_path' in context.__dict__:
+            # 3.12+
+            logger.debug("lib_path     %s", context.lib_path)
+        logger.debug("bin_path     %s", context.bin_path)
+        logger.debug("bin_name     %s", context.bin_name)
+        logger.debug("env_exe      %s", context.env_exe)
+        if 'env_exec_cmd' in context.__dict__:
+            # 3.9+
+            logger.debug("env_exec_cmd %s", context.env_exec_cmd)
+        self._context = context
+        return context
+
+    def create(self, env_dir: DirType) -> None:
+        logger.debug("create(env_dir=%s)", env_dir)
+        super().create(env_dir)
+        self.post_post_setup(self._context)
+
+    def create_configuration(self, context: SimpleNamespace) -> None:
+        logger.debug("create_configuration(...)")
+        super().create_configuration(context)
+
+    def setup_python(self, context: SimpleNamespace) -> None:
+        logger.debug("setup_python(...)")
+        super().setup_python(context)
+
+    def setup_scripts(self, context: SimpleNamespace) -> None:
+        logger.debug("setup_scripts(...)")
+        super().setup_scripts(context)
+
+    # def upgrade_dependencies(self, context): ...  # only Since 3.9
+
+    def post_setup(self, context: SimpleNamespace) -> None:
+        # Generate console_script entry points for system packages
+        # e.g. meson, sphinx, pip, etc.
+        logger.debug("post_setup(...)")
+        if self._system_site_packages:
+            generate_console_scripts(
+                context.env_exe, context.bin_path, self.script_packages)
+        #
+        # print the python executable to stdout for configure.
+        print(context.env_exe)
+
+    def post_post_setup(self, context: SimpleNamespace) -> None:
+        # This is the very final hook that occurs *after* enabling
+        # system-site-packages.
+        subprocess.run((context.env_exe, __file__, 'checkpip'), check=True)
+
+
+def need_ensurepip() -> bool:
+    """
+    Tests for the presence of setuptools and pip.
+
+    :return: `True` if we do not detect both packages.
+    """
+    # Don't try to actually import them, it's fraught with danger:
+    # https://github.com/pypa/setuptools/issues/2993
+    if find_spec("setuptools") and find_spec("pip"):
+        return False
+    return True
+
+
+def make_venv(  # pylint: disable=too-many-arguments
+        venv_path: Union[str, Path],
+        system_site_packages: bool = False,
+        clear: bool = True,
+        symlinks: Optional[bool] = None,
+        upgrade: bool = False,
+        with_pip: Optional[bool] = None,
+        script_packages: Sequence[str]=(),
+) -> None:
+    """
+    Create a venv using the QemuEnvBuilder class.
+
+    TODO: write docs O:-)
+    """
+    logging.debug("%s: make_venv(venv_path=%s, system_site_packages=%s, "
+                  "clear=%s, upgrade=%s, with_pip=%s, script_packages=%s)",
+                  __file__, str(venv_path), system_site_packages,
+                  clear, upgrade, with_pip, script_packages)
+
+    print(f"MKVENV {str(venv_path)}", file=sys.stderr)
+
+    # ensurepip is slow: venv creation can be very fast for cases where
+    # we allow the use of system_site_packages. Toggle ensure_pip on only
+    # in the cases where we really need it.
+    if with_pip is None:
+        with_pip = True if not system_site_packages else need_ensurepip()
+        logger.debug("with_pip unset, choosing %s", with_pip)
+
+    if symlinks is None:
+        # Default behavior of standard venv CLI
+        symlinks = os.name != "nt"
+
+    if with_pip and not find_spec("ensurepip"):
+        msg = ("Python's ensurepip module is not found.\n"
+
+               "It's normally part of the Python standard library, "
+               "maybe your distribution packages it separately?\n"
+
+               "Either install ensurepip, or alleviate the need for it in the "
+               "first place by installing pip and setuptools for "
+               f"'{sys.executable}'.\n"
+
+               "(Hint: Debian puts ensurepip in its python3-venv package.)")
+        raise Ouch(msg)
+
+    builder = QemuEnvBuilder(
+        system_site_packages=system_site_packages,
+        clear=clear,
+        symlinks=symlinks,
+        upgrade=upgrade,
+        with_pip=with_pip,
+        script_packages=script_packages,
+    )
+    try:
+        logger.debug("Invoking builder.create()")
+        try:
+            builder.create(str(venv_path))
+        except SystemExit as exc:
+            # Some versions of the venv module raise SystemExit; *nasty*!
+            # We want the exception that prompted it. It might be a subprocess
+            # error that has output we *really* want to see.
+            logger.debug("Intercepted SystemExit from EnvBuilder.create()")
+            raise exc.__cause__ or exc.__context__ or exc
+        logger.debug("builder.create() finished")
+    except subprocess.CalledProcessError as exc:
+        print(f"cmd: {exc.cmd}", file=sys.stderr)
+        print(f"returncode: {exc.returncode}", file=sys.stderr)
+        if exc.stdout:
+            print("========== stdout ==========", file=sys.stderr)
+            print(exc.stdout, file=sys.stderr)
+            print("============================", file=sys.stderr)
+        if exc.stderr:
+            print("========== stderr ==========", file=sys.stderr)
+            print(exc.stderr, file=sys.stderr)
+            print("============================", file=sys.stderr)
+        raise Ouch("VENV creation subprocess failed.") from exc
+
+
+def _gen_importlib(packages: Sequence[str]) -> Iterator[Dict[str, str]]:
+    # pylint: disable=import-outside-toplevel
+    try:
+        # First preference: Python 3.8+ stdlib
+        from importlib.metadata import (
+            PackageNotFoundError,
+            distribution,
+        )
+    except ImportError as exc:
+        logger.debug("%s", str(exc))
+        # Second preference: Commonly available PyPI backport
+        from importlib_metadata import (
+            PackageNotFoundError,
+            distribution,
+        )
+
+    # Borrowed from CPython (Lib/importlib/metadata/__init__.py)
+    pattern = re.compile(
+        r'(?P<module>[\w.]+)\s*'
+        r'(:\s*(?P<attr>[\w.]+)\s*)?'
+        r'((?P<extras>\[.*\])\s*)?$'
+    )
+
+    def _generator() -> Iterator[Dict[str, str]]:
+        for package in packages:
+            try:
+                entry_points = distribution(package).entry_points
+            except PackageNotFoundError:
+                continue
+
+            # The EntryPoints type is only available in 3.10+,
+            # treat this as a vanilla list and filter it ourselves.
+            entry_points = filter(
+                lambda ep: ep.group == 'console_scripts', entry_points)
+
+            for entry_point in entry_points:
+                # Python 3.8 doesn't have 'module' or 'attr' attributes
+                if not (hasattr(entry_point, 'module') and
+                        hasattr(entry_point, 'attr')):
+                    match = pattern.match(entry_point.value)
+                    assert match is not None
+                    module = match.group('module')
+                    attr = match.group('attr')
+                else:
+                    module = entry_point.module
+                    attr = entry_point.attr
+                yield {
+                    'name': entry_point.name,
+                    'module': module,
+                    'import_name': attr,
+                    'func': attr,
+                }
+
+    return _generator()
+
+
+def _gen_pkg_resources(packages: Sequence[str]) -> Iterator[Dict[str, str]]:
+    # pylint: disable=import-outside-toplevel
+    # Bundled with setuptools; has a good chance of being available.
+    import pkg_resources
+
+    def _generator() -> Iterator[Dict[str, str]]:
+        for package in packages:
+            try:
+                eps = pkg_resources.get_entry_map(package, 'console_scripts')
+            except pkg_resources.DistributionNotFound:
+                continue
+
+            for entry_point in eps.values():
+                yield {
+                    'name': entry_point.name,
+                    'module': entry_point.module_name,
+                    'import_name': ".".join(entry_point.attrs),
+                    'func': ".".join(entry_point.attrs),
+                }
+
+    return _generator()
+
+
+# Borrowed/adapted from pip's vendored version of distutils:
+SCRIPT_TEMPLATE = r'''#!{python_path:s}
+# -*- coding: utf-8 -*-
+import re
+import sys
+from {module:s} import {import_name:s}
+if __name__ == '__main__':
+    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
+    sys.exit({func:s}())
+'''
+
+
+def generate_console_scripts(python_path: str, bin_path: str,
+                             packages: Sequence[str]) -> None:
+    """
+    Generate script shims for console_script entry points in @packages.
+    """
+    if not packages:
+        return
+
+    def _get_entry_points() -> Iterator[Dict[str, str]]:
+        try:
+            return _gen_importlib(packages)
+        except ImportError as exc:
+            logger.debug("%s", str(exc))
+        return _gen_pkg_resources(packages)
+
+    try:
+        entry_points = _get_entry_points()
+    except ImportError as exc:
+        logger.debug("%s", str(exc))
+        raise Ouch(
+            "Neither importlib.metadata nor pkg_resources found, "
+            "can't generate console script shims.\n"
+            "Use Python 3.8+, or install importlib-metadata, or setuptools."
+        ) from exc
+
+    for entry_point in entry_points:
+        script_path = os.path.join(bin_path, entry_point['name'])
+        script = SCRIPT_TEMPLATE.format(python_path=python_path, **entry_point)
+        with open(script_path, "w", encoding='UTF-8') as file:
+            file.write(script)
+            fd = file.fileno()
+            os.chmod(fd, os.stat(fd).st_mode | stat.S_IEXEC)
+        logger.debug("wrote '%s'", script_path)
+
+
+def checkpip():
+    """
+    Debian10 has a pip that's broken when used inside of a virtual environment.
+
+    We try to detect and correct that case here.
+    """
+    try:
+        import pip._internal
+        logger.debug("pip appears to be working correctly.")
+        return
+    except ModuleNotFoundError as exc:
+        if exc.name == 'pip._internal':
+            # Uh, fair enough. They did say "internal".
+            # Let's just assume it's fine.
+            return
+        logger.warning("pip appears to be malfunctioning: %s", str(exc))
+
+    # Test for ensurepip:
+    try:
+        import ensurepip
+    except ImportError as exc:
+        raise Ouch(
+            "pip appears to be non-functional, "
+            "and Python's ensurepip module is not found.\n"
+            "It's normally part of the Python standard library, "
+            "maybe your distribution packages it separately?\n"
+            "(Hint: Debian puts ensurepip in its python3-venv package.)"
+        ) from exc
+
+    logging.debug("Attempting to repair pip ...")
+    subprocess.run((sys.executable, '-m', 'ensurepip'),
+                   stdout=subprocess.DEVNULL, check=True)
+    logging.debug("Pip is now (hopefully) repaired!")
+
+
+def main() -> int:
+    """CLI interface to make_qemu_venv. See module docstring."""
+    if os.environ.get('DEBUG') or os.environ.get('GITLAB_CI'):
+        # You're welcome.
+        logging.basicConfig(level=logging.DEBUG)
+
+    parser = argparse.ArgumentParser(description="Bootstrap QEMU venv.")
+    subparsers = parser.add_subparsers(
+        title="Commands",
+        description="Various actions this utility can perform",
+        prog="prog",
+        dest="command",
+        required=True,
+        metavar="command",
+        help='Description')
+
+    #
+    subparser = subparsers.add_parser('create', help='create a venv')
+    subparser.add_argument(
+        '--gen',
+        type=str,
+        action='append',
+        help="Regenerate console_scripts for given packages, if found.",
+    )
+    subparser.add_argument(
+        'target',
+        type=str,
+        action='store',
+        help="Target directory to install virtual environment into.",
+    )
+
+    #
+    subparser = subparsers.add_parser(
+        'checkpip', help='test pip and fix if necessary')
+
+    args = parser.parse_args()
+
+    try:
+        if args.command == 'create':
+            script_packages = []
+            for element in args.gen or ():
+                script_packages.extend(element.split(","))
+            make_venv(
+                args.target,
+                system_site_packages=True,
+                clear=True,
+                upgrade=False,
+                with_pip=None,  # Autodetermine
+                script_packages=script_packages,
+            )
+            logger.debug("mkvenv.py create - exiting")
+        if args.command == 'checkpip':
+            checkpip()
+            logger.debug("mkvenv.py checkpip - exiting")
+    except Ouch as exc:
+        print("\n*** Ouch! ***\n", file=sys.stderr)
+        print(str(exc), "\n\n", file=sys.stderr)
+        return 1
+    except:  # pylint: disable=bare-except
+        print("mkvenv did not complete successfully:", file=sys.stderr)
+        traceback.print_exc()
+        return 2
+    return 0
+
+
+if __name__ == '__main__':
+    sys.exit(main())
-- 
2.39.2

Re: [RFC PATCH 1/3] python: add mkvenv.py

[Paolo Bonzini]

On 3/28/23 23:11, John Snow wrote:
> +        # venv class is cute and toggles this off before post_setup,
> +        # but we need it to decide if we want to generate shims or not.

Ha, yeah that's a bug in the venv package.  post_setup() can already run 
with system_site_packages reverted to True.

> +            for entry_point in entry_points:
> +                # Python 3.8 doesn't have 'module' or 'attr' attributes
> +                if not (hasattr(entry_point, 'module') and
> +                        hasattr(entry_point, 'attr')):
> +                    match = pattern.match(entry_point.value)
> +                    assert match is not None
> +                    module = match.group('module')
> +                    attr = match.group('attr')
> +                else:
> +                    module = entry_point.module
> +                    attr = entry_point.attr
> +                yield {
> +                    'name': entry_point.name,
> +                    'module': module,
> +                    'import_name': attr,
> +                    'func': attr,

What about using a dataclass or namedtuple instead of a dictionary?

> 
> +
> +    try:
> +        entry_points = _get_entry_points()
> +    except ImportError as exc:
> +        logger.debug("%s", str(exc))
> +        raise Ouch(
> +            "Neither importlib.metadata nor pkg_resources found, "
> +            "can't generate console script shims.\n"
> +            "Use Python 3.8+, or install importlib-metadata, or setuptools."
> +        ) from exc

Why not put this extra try/except inside _get_entry_points()?

> +
> +    # Test for ensurepip:
> +    try:
> +        import ensurepip

Use find_spec()?

BTW, another way to repair Debian 10's pip is to create a symbolic link 
to sys.base_prefix + '/share/python-wheels' in sys.prefix + 
'/share/python-wheels'.  Since this is much faster, perhaps it can be 
done unconditionally and checkpip mode can go away together with 
self._context?

Paolo

Re: [RFC PATCH 1/3] python: add mkvenv.py

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2961 bytes --]

On Wed, Mar 29, 2023, 8:56 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 3/28/23 23:11, John Snow wrote:
> > +        # venv class is cute and toggles this off before post_setup,
> > +        # but we need it to decide if we want to generate shims or not.
>
> Ha, yeah that's a bug in the venv package.  post_setup() can already run
> with system_site_packages reverted to True.
>
> > +            for entry_point in entry_points:
> > +                # Python 3.8 doesn't have 'module' or 'attr' attributes
> > +                if not (hasattr(entry_point, 'module') and
> > +                        hasattr(entry_point, 'attr')):
> > +                    match = pattern.match(entry_point.value)
> > +                    assert match is not None
> > +                    module = match.group('module')
> > +                    attr = match.group('attr')
> > +                else:
> > +                    module = entry_point.module
> > +                    attr = entry_point.attr
> > +                yield {
> > +                    'name': entry_point.name,
> > +                    'module': module,
> > +                    'import_name': attr,
> > +                    'func': attr,
>
> What about using a dataclass or namedtuple instead of a dictionary?
>

Sure. Once 3.8 is our minimum there's no point, though.


> >
> > +
> > +    try:
> > +        entry_points = _get_entry_points()
> > +    except ImportError as exc:
> > +        logger.debug("%s", str(exc))
> > +        raise Ouch(
> > +            "Neither importlib.metadata nor pkg_resources found, "
> > +            "can't generate console script shims.\n"
> > +            "Use Python 3.8+, or install importlib-metadata, or
> setuptools."
> > +        ) from exc
>
> Why not put this extra try/except inside _get_entry_points()?
>

I don't remember. I'll look! I know it looks goofy. The ultimate answer is
"So I can log all import failures without nesting eight layers deep".


> > +
> > +    # Test for ensurepip:
> > +    try:
> > +        import ensurepip
>
> Use find_spec()?
>

That might be better. Originally I tried to use ensurepip directly, but
found it didn't work right if you had already imported pip. This survived
from the earlier draft.


> BTW, another way to repair Debian 10's pip is to create a symbolic link
> to sys.base_prefix + '/share/python-wheels' in sys.prefix +
> '/share/python-wheels'.  Since this is much faster, perhaps it can be
> done unconditionally and checkpip mode can go away together with
> self._context?
>

I guess I like it less because it's way more Debian-specific at that point.
I think I'd sooner say "Sorry, Debian 10 isn't supported!"

(Or encourage users to upgrade their pip/setuptools/ensurepip to something
that doesn't trigger the bug.)

Or, IOW, I feel like it's normal to expect ensurepip to work but mussing
around with symlinks to special directories created by a distribution just
feels way more fiddly.


> Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 5053 bytes --]

Re: [RFC PATCH 1/3] python: add mkvenv.py

[Paolo Bonzini]

On 3/30/23 16:00, John Snow wrote:
>      > +                yield {
>      > +                    'name': entry_point.name
>     <http://entry_point.name>,
>      > +                    'module': module,
>      > +                    'import_name': attr,
>      > +                    'func': attr,
> 
>     What about using a dataclass or namedtuple instead of a dictionary?
> 
> 
> Sure. Once 3.8 is our minimum there's no point, though.

Well, that's why I also mentioned namedtuples.  But no big deal.

>     BTW, another way to repair Debian 10's pip is to create a symbolic link
>     to sys.base_prefix + '/share/python-wheels' in sys.prefix +
>     '/share/python-wheels'.  Since this is much faster, perhaps it can be
>     done unconditionally and checkpip mode can go away together with
>     self._context?
> 
> 
> I guess I like it less because it's way more Debian-specific at that 
> point. I think I'd sooner say "Sorry, Debian 10 isn't supported!"
> 
> (Or encourage users to upgrade their pip/setuptools/ensurepip to 
> something that doesn't trigger the bug.)
> 
> Or, IOW, I feel like it's normal to expect ensurepip to work but mussing 
> around with symlinks to special directories created by a distribution 
> just feels way more fiddly.

No doubt about that.  It's just the balance between simple fiddly code 
and more robust code that is also longer.

Anyhow later on we will split mkvenv.py in multiple patches so it will 
be easy to revert checkpip when time comes.  For example, when Python 
3.7 is dropped for good rather than being just "untested but should 
work", this Debian 10 hack and the importlib_metadata/pkg_resources 
fallbacks go away at the same time.

Paolo

Re: [RFC PATCH 1/3] python: add mkvenv.py

[Paolo Bonzini]

On 3/31/23 10:44, Paolo Bonzini wrote:
>>
>>     What about using a dataclass or namedtuple instead of a dictionary?
>>
>>
>> Sure. Once 3.8 is our minimum there's no point, though.
> 
> Well, that's why I also mentioned namedtuples.  But no big deal.

Sorry, I misunderstood this (I read "until 3.8 is our minimum" and 
interpreted that as "dataclasses are not in 3.6").

I agree, not much need to future-proof the <=3.7 parts of the code.

Paolo

Re: [RFC PATCH 1/3] python: add mkvenv.py

[John Snow]

On Wed, Mar 29, 2023 at 8:56 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 3/28/23 23:11, John Snow wrote:

> > +            for entry_point in entry_points:
> > +                # Python 3.8 doesn't have 'module' or 'attr' attributes
> > +                if not (hasattr(entry_point, 'module') and
> > +                        hasattr(entry_point, 'attr')):
> > +                    match = pattern.match(entry_point.value)
> > +                    assert match is not None
> > +                    module = match.group('module')
> > +                    attr = match.group('attr')
> > +                else:
> > +                    module = entry_point.module
> > +                    attr = entry_point.attr
> > +                yield {
> > +                    'name': entry_point.name,
> > +                    'module': module,
> > +                    'import_name': attr,
> > +                    'func': attr,
>
> What about using a dataclass or namedtuple instead of a dictionary?
>

I suppose what I meant was: Once 3.8 is our minimum, we can delete
most of this compat code anyway, so there may not be a point in
creating a new type-safe structure to house it. I can definitely add
that in if you'd like, but I suppose I felt like a dict was "good
enough" for now, since 3.7 will also get dropped off the face of the
earth soon, too.

Before I send a non-RFC patch I'll get everything scrubbed down with
the usual pylint/mypy/isort/flake8 combo, and if I wind up needing to
for type safety I will add something.

Or if you are requesting it specifically. :~)

> >
> > +
> > +    try:
> > +        entry_points = _get_entry_points()
> > +    except ImportError as exc:
> > +        logger.debug("%s", str(exc))
> > +        raise Ouch(
> > +            "Neither importlib.metadata nor pkg_resources found, "
> > +            "can't generate console script shims.\n"
> > +            "Use Python 3.8+, or install importlib-metadata, or setuptools."
> > +        ) from exc
>
> Why not put this extra try/except inside _get_entry_points()?

Hm, no good reason, apparently. O:-) I've fixed this one up.


Unrelated question I'm going to tuck in here:

For the script generation, I am making another call to mkvenv.py using
the venv'ified python to do final initializations. As part of that, I
pass the binpath to the script again because I wasn't sure it was safe
to compute it again myself. CPython seems to assume it's always going
to be env_path/Scripts/ or env_path/bin/, but I wasn't 1000% sure that
this wasn't patched by e.g. Debian or had some complications with the
adjustments to site configuration in recent times. I'll circle back
around to investigating this, but for now I've left it with the dumber
approach of always passing the bindir.

Re: [RFC PATCH 1/3] python: add mkvenv.py

[John Snow]

On Wed, Mar 29, 2023 at 8:56 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> BTW, another way to repair Debian 10's pip is to create a symbolic link
> to sys.base_prefix + '/share/python-wheels' in sys.prefix +
> '/share/python-wheels'.  Since this is much faster, perhaps it can be
> done unconditionally and checkpip mode can go away together with
> self._context?
>
> Paolo
>

I'm coming around on this one a bit; it's definitely going to be a lot
faster. As you say, my version is more robust, but more complex and
with more lines. We may decide to drop any workarounds for Debian 10
entirely and we can live without either fix. I'll mention this in the
commit message for the Debian 10 workaround.

I do not know right now if other distros suffer from the same problem;
we could attempt to omit the fix and just see if anyone barks. Not
very nice, but impossible to enumerate all of the bugs that exist in
various downstream distros...

--js

[RFC PATCH 2/3] tests: add python3-venv dependency

[John Snow]

Several debian-based tests need the python3-venv dependency as a
consequence of Debian debundling the "ensurepip" module normally
included with Python.

As this series stands right now, in most cases, Debian requires EITHER:

(A) setuptools and pip, or
(B) ensurepip

It's quite likely most developer workstations running Debian or a
Debian-based distro already have all of these packages. venv creation
will actually be quite a bit faster by installing pip and setuptools,
but for the purposes of this patch I am only adding the strict minimum
necessary.

(For that reason, I chose ensurepip to make sure that as we continue
tweaking and developing the mkvenv script, we can count on Debian having
a fully functional Python installation; but for the sake of this series
it is an arbitrary choice here.)

In one case, even though we already have pip and setuptools, we still
need ensurepip because the version of these tools that shipped with
Debian are known to be broken and do not function correctly inside of a
venv. Using ensurepip bypasses the bug. (This is only for Debian 10,
which we technically no longer support. Even so, I wanted to prove it's
possible to support our oldest platforms.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/docker/dockerfiles/debian-all-test-cross.docker | 3 ++-
 tests/docker/dockerfiles/debian-hexagon-cross.docker  | 3 ++-
 tests/docker/dockerfiles/debian-riscv64-cross.docker  | 3 ++-
 tests/docker/dockerfiles/debian-tricore-cross.docker  | 3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
index 981e9bdc7b0..f9f401544a0 100644
--- a/tests/docker/dockerfiles/debian-all-test-cross.docker
+++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
@@ -57,7 +57,8 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
         gcc-sh4-linux-gnu \
         libc6-dev-sh4-cross \
         gcc-sparc64-linux-gnu \
-        libc6-dev-sparc64-cross
+        libc6-dev-sparc64-cross \
+        python3-venv
 
 ENV QEMU_CONFIGURE_OPTS --disable-system --disable-docs --disable-tools
 ENV DEF_TARGET_LIST aarch64-linux-user,alpha-linux-user,arm-linux-user,hppa-linux-user,i386-linux-user,m68k-linux-user,mips-linux-user,mips64-linux-user,mips64el-linux-user,mipsel-linux-user,ppc-linux-user,ppc64-linux-user,ppc64le-linux-user,riscv64-linux-user,s390x-linux-user,sh4-linux-user,sparc64-linux-user
diff --git a/tests/docker/dockerfiles/debian-hexagon-cross.docker b/tests/docker/dockerfiles/debian-hexagon-cross.docker
index 5308ccb8fe9..344eec900b0 100644
--- a/tests/docker/dockerfiles/debian-hexagon-cross.docker
+++ b/tests/docker/dockerfiles/debian-hexagon-cross.docker
@@ -20,7 +20,8 @@ RUN apt-get update && \
         bison \
         flex \
         git \
-        ninja-build && \
+        ninja-build \
+        python3-venv && \
 # Install QEMU build deps for use in CI
     DEBIAN_FRONTEND=noninteractive eatmydata \
     apt build-dep -yy --arch-only qemu
diff --git a/tests/docker/dockerfiles/debian-riscv64-cross.docker b/tests/docker/dockerfiles/debian-riscv64-cross.docker
index 803afb95734..081404e0141 100644
--- a/tests/docker/dockerfiles/debian-riscv64-cross.docker
+++ b/tests/docker/dockerfiles/debian-riscv64-cross.docker
@@ -28,7 +28,8 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata apt install -yy \
     libglib2.0-dev \
     ninja-build \
     pkg-config \
-    python3
+    python3 \
+    python3-venv
 
 # Add ports and riscv64 architecture
 RUN echo "deb http://ftp.ports.debian.org/debian-ports/ sid main" >> /etc/apt/sources.list
diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker b/tests/docker/dockerfiles/debian-tricore-cross.docker
index cfd2faf9a80..269bfa8d423 100644
--- a/tests/docker/dockerfiles/debian-tricore-cross.docker
+++ b/tests/docker/dockerfiles/debian-tricore-cross.docker
@@ -33,7 +33,8 @@ RUN apt update && \
        pkgconf \
        python3-pip \
        python3-setuptools \
-       python3-wheel
+       python3-wheel \
+       python3-venv
 
 RUN curl -#SL https://github.com/bkoppelmann/package_940/releases/download/tricore-toolchain-9.40/tricore-toolchain-9.4.0.tar.gz \
     | tar -xzC /usr/local/
-- 
2.39.2

[RFC PATCH 3/3] configure: install meson to a python virtual environment

[John Snow]

This patch changes how we detect and install meson.

This patch creates a lightweight Python virtual environment using the
user's configured $python that inherits system packages. If Meson is
installed there and meets our minimum version requirements, we will use
that Meson.

In the event that Meson is installed but *not for the chosen Python
interpreter*, not found, or of insufficient version, we will attempt to
install Meson from source into the newly created Python virtual
environment.

At present, the source is obtained in the same manner as it has been;
preferring git submodules first and relying on vendored source as a
backup.

This patch does *not* connect to PyPI and will work offline for all
supported build platforms. Future improvements to this patch may add
optional PyPI support as a replacement for git submodules.

As a result of this patch, the Python interpreter we use for both our
own build scripts *and* Meson extensions are always known to be the
exact same Python. As a further benefit, there will also be a symlink
available in the build directory that points to the correct, configured
python and can be used by e.g. manual tests to invoke the correct,
configured Python unambiguously.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure                           | 155 +++++++++++++++++++---------
 .gitlab-ci.d/buildtest-template.yml |   2 +-
 2 files changed, 108 insertions(+), 49 deletions(-)

diff --git a/configure b/configure
index 05bed4f4a11..1ec054775f6 100755
--- a/configure
+++ b/configure
@@ -607,7 +607,6 @@ check_py_version() {
 python=
 first_python=
 if test -z "${PYTHON}"; then
-    explicit_python=no
     # A bare 'python' is traditionally python 2.x, but some distros
     # have it as python 3.x, so check in both places.
     for binary in python3 python python3.11 python3.10 python3.9 python3.8 python3.7 python3.6; do
@@ -626,7 +625,6 @@ else
     # Same as above, but only check the environment variable.
     has "${PYTHON}" || error_exit "The PYTHON environment variable does not point to an executable"
     python=$(command -v "$PYTHON")
-    explicit_python=yes
     if check_py_version "$python"; then
         # This one is good.
         first_python=
@@ -718,7 +716,7 @@ for opt do
   ;;
   --install=*)
   ;;
-  --python=*) python="$optarg" ; explicit_python=yes
+  --python=*) python="$optarg"
   ;;
   --skip-meson) skip_meson=yes
   ;;
@@ -1078,61 +1076,122 @@ if ! check_py_version "$python"; then
       "Use --python=/path/to/python to specify a supported Python."
 fi
 
-# Resolve PATH + suppress writing compiled files
-python="$(command -v "$python") -B"
-
-has_meson() {
-  local python_dir=$(dirname "$python")
-  # PEP405: pyvenv.cfg is either adjacent to the Python executable
-  # or one directory above
-  if test -f $python_dir/pyvenv.cfg || test -f $python_dir/../pyvenv.cfg; then
-    # Ensure that Meson and Python come from the same virtual environment
-    test -x "$python_dir/meson" &&
-      test "$(command -v meson)" -ef "$python_dir/meson"
-  else
-    has meson
-  fi
+# Resolve PATH
+python="$(command -v "$python")"
+# This variable is intended to be used only for error messages:
+target_python=$python
+
+# Create a Python virtual environment using our configured python.
+# The stdout of this script will be the location of a symlink that
+# points to the configured Python.
+# Entry point scripts for pip, meson, and sphinx are generated if those
+# packages are present.
+
+# Defaults assumed for now:
+# - venv is cleared if it exists already;
+# - venv is allowed to use system packages;
+# - all setup is performed **offline**;
+# - No packages are installed by default;
+# - pip is not installed into the venv when possible,
+#   but ensurepip is called as a fallback when necessary.
+
+echo "python determined to be '$python'"
+echo "python version: $($python --version)"
+
+echo " === pip list (host) === "
+$python -m pip list --disable-pip-version-check
+echo " ======================= "
+echo ""
+
+python="$($python -B "${source_path}/python/scripts/mkvenv.py" create --gen pip,meson,sphinx pyvenv)"
+if test "$?" -ne 0 ; then
+    error_exit "python venv creation failed"
+fi
+
+# Suppress writing compiled files
+python="$python -B"
+echo "Configured python as '$python'"
+
+echo " === pip list (venv) === "
+$python -m pip list --disable-pip-version-check
+echo " ======================= "
+echo ""
+
+pip_install() {
+    $python -m pip install -v \
+            --disable-pip-version-check \
+            --no-index \
+            "$@"
 }
 
-if test -z "$meson"; 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
-    elif test -e "${source_path}/meson/meson.py" ; then
-        meson=internal
+# OK, let's have some fun!
+
+# This install command is intended to either fail or be a NOP;
+# because we're offline, it's just a convenient version check.
+if ! pip_install 'meson>=0.61.5'; then
+    # Either we don't have Meson, or our Meson is too old.
+    # (Future revisions of this patchset can be less chatty.)
+    if test -e pyvenv/bin/meson; then
+        echo "Meson in pyvenv is too old: $(pyvenv/bin/meson --version)"
+    elif has meson ; then
+        echo "Meson was found installed on your system," \
+             "but not for the configured Python interpreter ($target_python)."
+        echo "(Hint: check '$(which meson)' to see which interpreter its shebang uses.)"
     else
-        if test "$explicit_python" = yes; then
-            error_exit "--python requires using QEMU's embedded Meson distribution, but it was not found."
-        else
-            error_exit "Meson not found.  Use --meson=/path/to/meson"
+        echo "Meson was not found."
+    fi
+
+    # OK, but can we fix it, though? :~)
+    if test "$git_submodules_action" != 'ignore' ; then
+        git_submodules="${git_submodules} meson"
+        echo "Attempting to install meson from git submodule ..."
+        # Stolen from later in the configure file.
+        # Is it a problem if we front-load this now and run it again later?
+        if ! (GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
+            exit 1
         fi
+    elif test -e "${source_path}/meson/setup.cfg" ; then
+        echo "Attempting to install meson from vendored source ..."
+    else
+        # In the future, we could use PyPI as a source if the user allows it.
+        # For now, you're outta luck!
+        error_exit "A suitable version of Meson was not found."
     fi
-else
-    # Meson uses its own Python interpreter to invoke other Python scripts,
-    # but the user wants to use the one they specified with --python.
+
+    # If we're here, we have the meson source and we can attempt to
+    # install it into our venv.
+
+    # We want to install meson with --no-use-pep517 if possible,
+    # because it avoids needing a 'wheel' dependency. Old versions
+    # of pip do this by default, so test for the behavior.
     #
-    # We do not want to override the distro Python interpreter (and sometimes
-    # cannot: for example in Homebrew /usr/bin/meson is a bash script), so
-    # just require --meson=git|internal together with --python.
-    if test "$explicit_python" = yes; then
-        case "$meson" in
-            git | internal) ;;
-            *) error_exit "--python requires using QEMU's embedded Meson distribution." ;;
-        esac
+    # --no-build-isolation was added to pip 10.0.
+    # --no-use-pep517 was added ... sometime after 18.1?
+    pip_flags='--no-build-isolation'
+    if $python -m pip install --help | grep 'no-use-pep517' > /dev/null 2>&1 ; then
+        pip_flags="${pip_flags} --no-use-pep517"
+    fi
+    if ! pip_install $pip_flags "${source_path}/meson" ; then
+        exit 1
     fi
 fi
 
-if test "$meson" = git; then
-    git_submodules="${git_submodules} meson"
-fi
+# At this point, we expect Meson to be installed and available.
+# We expect mkvenv or pip to have created pyvenv/bin/meson for us.
+# We ignore PATH completely here: we want to use the venv's Meson
+# *exclusively*.
+meson="$(cd pyvenv/bin; pwd)/meson"
 
-case "$meson" in
-    git | internal)
-        meson="$python ${source_path}/meson/meson.py"
-        ;;
-    *) meson=$(command -v "$meson") ;;
-esac
+# Smoke tests: these shouldn't fail, but I have little faith in esoteric
+# platforms I've never used to not do something Deep Strange when I am
+# not looking.
+if ! test -e "$meson" ; then
+    error_exit "??? pyvenv/bin/meson not found, somehow ???"
+fi
+echo "Meson $($meson --version) found: $meson"
+if ! version_ge "$($meson --version)" 0.61.5; then
+    error_exit "??? Meson version is too old ???"
+fi
 
 # Probe for ninja
 
diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index a6cfe9be970..6cfe644d8fe 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -17,7 +17,7 @@
       { cat config.log meson-logs/meson-log.txt && exit 1; }
     - if test -n "$LD_JOBS";
       then
-        ../meson/meson.py configure . -Dbackend_max_links="$LD_JOBS" ;
+        pyvenv/bin/meson configure . -Dbackend_max_links="$LD_JOBS" ;
       fi || exit 1;
     - make -j"$JOBS"
     - if test -n "$MAKE_CHECK_ARGS";
-- 
2.39.2

Re: [RFC PATCH 3/3] configure: install meson to a python virtual environment

[Paolo Bonzini]

On 3/28/23 23:11, John Snow wrote:
> +        # Is it a problem if we front-load this now and run it again later?

I would just move all the "pip install" bits after configure has called 
out to git-submodule.sh.

> +    # --no-build-isolation was added to pip 10.0.
> +    # --no-use-pep517 was added ... sometime after 18.1?
> +    pip_flags='--no-build-isolation'
> +    if $python -m pip install --help | grep 'no-use-pep517' > /dev/null 2>&1 ; then
> +        pip_flags="${pip_flags} --no-use-pep517"
> +    fi
> +    if ! pip_install $pip_flags "${source_path}/meson" ; then
> +        exit 1
>       fi
>   fi

Hmm, I'm not a huge fan of using this flag to cover (essentially) an 
implementation detail of pip.  I would really keep on using either 
meson.py here, though I appreciate that this patch is effective in 
showing off the venv concept, without getting into downloading packages 
and adding flags to the configure script.

> 
> +# Smoke tests: these shouldn't fail, but I have little faith in esoteric
> +# platforms I've never used to not do something Deep Strange when I am
> +# not looking.
> +if ! test -e "$meson" ; then
> +    error_exit "??? pyvenv/bin/meson not found, somehow ???"
> +fi
> +echo "Meson $($meson --version) found: $meson"
> +if ! version_ge "$($meson --version)" 0.61.5; then
> +    error_exit "??? Meson version is too old ???"
> +fi

I think this can go away in the final version, since the version is 
checked against the project() invocation of meson.build.

Paolo

Re: [RFC PATCH 3/3] configure: install meson to a python virtual environment

[Daniel P. Berrangé]

On Tue, Mar 28, 2023 at 05:11:19PM -0400, John Snow wrote:
> This patch changes how we detect and install meson.
> 
> This patch creates a lightweight Python virtual environment using the
> user's configured $python that inherits system packages. If Meson is
> installed there and meets our minimum version requirements, we will use
> that Meson.
> 
> In the event that Meson is installed but *not for the chosen Python
> interpreter*, not found, or of insufficient version, we will attempt to
> install Meson from source into the newly created Python virtual
> environment.
> 
> At present, the source is obtained in the same manner as it has been;
> preferring git submodules first and relying on vendored source as a
> backup.
> 
> This patch does *not* connect to PyPI and will work offline for all
> supported build platforms. Future improvements to this patch may add
> optional PyPI support as a replacement for git submodules.
> 
> As a result of this patch, the Python interpreter we use for both our
> own build scripts *and* Meson extensions are always known to be the
> exact same Python. As a further benefit, there will also be a symlink
> available in the build directory that points to the correct, configured
> python and can be used by e.g. manual tests to invoke the correct,
> configured Python unambiguously.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  configure                           | 155 +++++++++++++++++++---------

I would love to see a day where configure isn't involved in the
build process. Obviously we can't put this new logic into
meson as its a chicken & egg problem. Could we potentially
have your new  python/scripts/mkvenv.py script be responsible
for setting up meson in the venv though, so we can avoid
adding more shell code to configure ?

>  .gitlab-ci.d/buildtest-template.yml |   2 +-
>  2 files changed, 108 insertions(+), 49 deletions(-)
> 
> diff --git a/configure b/configure
> index 05bed4f4a11..1ec054775f6 100755
> --- a/configure
> +++ b/configure
> @@ -607,7 +607,6 @@ check_py_version() {
>  python=
>  first_python=
>  if test -z "${PYTHON}"; then
> -    explicit_python=no
>      # A bare 'python' is traditionally python 2.x, but some distros
>      # have it as python 3.x, so check in both places.
>      for binary in python3 python python3.11 python3.10 python3.9 python3.8 python3.7 python3.6; do
> @@ -626,7 +625,6 @@ else
>      # Same as above, but only check the environment variable.
>      has "${PYTHON}" || error_exit "The PYTHON environment variable does not point to an executable"
>      python=$(command -v "$PYTHON")
> -    explicit_python=yes
>      if check_py_version "$python"; then
>          # This one is good.
>          first_python=
> @@ -718,7 +716,7 @@ for opt do
>    ;;
>    --install=*)
>    ;;
> -  --python=*) python="$optarg" ; explicit_python=yes
> +  --python=*) python="$optarg"
>    ;;
>    --skip-meson) skip_meson=yes
>    ;;
> @@ -1078,61 +1076,122 @@ if ! check_py_version "$python"; then
>        "Use --python=/path/to/python to specify a supported Python."
>  fi
>  
> -# Resolve PATH + suppress writing compiled files
> -python="$(command -v "$python") -B"
> -
> -has_meson() {
> -  local python_dir=$(dirname "$python")
> -  # PEP405: pyvenv.cfg is either adjacent to the Python executable
> -  # or one directory above
> -  if test -f $python_dir/pyvenv.cfg || test -f $python_dir/../pyvenv.cfg; then
> -    # Ensure that Meson and Python come from the same virtual environment
> -    test -x "$python_dir/meson" &&
> -      test "$(command -v meson)" -ef "$python_dir/meson"
> -  else
> -    has meson
> -  fi
> +# Resolve PATH
> +python="$(command -v "$python")"
> +# This variable is intended to be used only for error messages:
> +target_python=$python
> +
> +# Create a Python virtual environment using our configured python.
> +# The stdout of this script will be the location of a symlink that
> +# points to the configured Python.
> +# Entry point scripts for pip, meson, and sphinx are generated if those
> +# packages are present.
> +
> +# Defaults assumed for now:
> +# - venv is cleared if it exists already;
> +# - venv is allowed to use system packages;
> +# - all setup is performed **offline**;
> +# - No packages are installed by default;
> +# - pip is not installed into the venv when possible,
> +#   but ensurepip is called as a fallback when necessary.
> +
> +echo "python determined to be '$python'"
> +echo "python version: $($python --version)"
> +
> +echo " === pip list (host) === "
> +$python -m pip list --disable-pip-version-check
> +echo " ======================= "
> +echo ""
> +
> +python="$($python -B "${source_path}/python/scripts/mkvenv.py" create --gen pip,meson,sphinx pyvenv)"
> +if test "$?" -ne 0 ; then
> +    error_exit "python venv creation failed"
> +fi
> +
> +# Suppress writing compiled files
> +python="$python -B"
> +echo "Configured python as '$python'"
> +
> +echo " === pip list (venv) === "
> +$python -m pip list --disable-pip-version-check
> +echo " ======================= "
> +echo ""
> +
> +pip_install() {
> +    $python -m pip install -v \
> +            --disable-pip-version-check \
> +            --no-index \
> +            "$@"
>  }
>  
> -if test -z "$meson"; 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
> -    elif test -e "${source_path}/meson/meson.py" ; then
> -        meson=internal
> +# OK, let's have some fun!
> +
> +# This install command is intended to either fail or be a NOP;
> +# because we're offline, it's just a convenient version check.
> +if ! pip_install 'meson>=0.61.5'; then
> +    # Either we don't have Meson, or our Meson is too old.
> +    # (Future revisions of this patchset can be less chatty.)
> +    if test -e pyvenv/bin/meson; then
> +        echo "Meson in pyvenv is too old: $(pyvenv/bin/meson --version)"
> +    elif has meson ; then
> +        echo "Meson was found installed on your system," \
> +             "but not for the configured Python interpreter ($target_python)."
> +        echo "(Hint: check '$(which meson)' to see which interpreter its shebang uses.)"
>      else
> -        if test "$explicit_python" = yes; then
> -            error_exit "--python requires using QEMU's embedded Meson distribution, but it was not found."
> -        else
> -            error_exit "Meson not found.  Use --meson=/path/to/meson"
> +        echo "Meson was not found."
> +    fi
> +
> +    # OK, but can we fix it, though? :~)
> +    if test "$git_submodules_action" != 'ignore' ; then
> +        git_submodules="${git_submodules} meson"
> +        echo "Attempting to install meson from git submodule ..."
> +        # Stolen from later in the configure file.
> +        # Is it a problem if we front-load this now and run it again later?
> +        if ! (GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
> +            exit 1
>          fi
> +    elif test -e "${source_path}/meson/setup.cfg" ; then
> +        echo "Attempting to install meson from vendored source ..."
> +    else
> +        # In the future, we could use PyPI as a source if the user allows it.
> +        # For now, you're outta luck!
> +        error_exit "A suitable version of Meson was not found."
>      fi
> -else
> -    # Meson uses its own Python interpreter to invoke other Python scripts,
> -    # but the user wants to use the one they specified with --python.
> +
> +    # If we're here, we have the meson source and we can attempt to
> +    # install it into our venv.
> +
> +    # We want to install meson with --no-use-pep517 if possible,
> +    # because it avoids needing a 'wheel' dependency. Old versions
> +    # of pip do this by default, so test for the behavior.
>      #
> -    # We do not want to override the distro Python interpreter (and sometimes
> -    # cannot: for example in Homebrew /usr/bin/meson is a bash script), so
> -    # just require --meson=git|internal together with --python.
> -    if test "$explicit_python" = yes; then
> -        case "$meson" in
> -            git | internal) ;;
> -            *) error_exit "--python requires using QEMU's embedded Meson distribution." ;;
> -        esac
> +    # --no-build-isolation was added to pip 10.0.
> +    # --no-use-pep517 was added ... sometime after 18.1?
> +    pip_flags='--no-build-isolation'
> +    if $python -m pip install --help | grep 'no-use-pep517' > /dev/null 2>&1 ; then
> +        pip_flags="${pip_flags} --no-use-pep517"
> +    fi
> +    if ! pip_install $pip_flags "${source_path}/meson" ; then
> +        exit 1
>      fi
>  fi
>  
> -if test "$meson" = git; then
> -    git_submodules="${git_submodules} meson"
> -fi
> +# At this point, we expect Meson to be installed and available.
> +# We expect mkvenv or pip to have created pyvenv/bin/meson for us.
> +# We ignore PATH completely here: we want to use the venv's Meson
> +# *exclusively*.
> +meson="$(cd pyvenv/bin; pwd)/meson"
>  
> -case "$meson" in
> -    git | internal)
> -        meson="$python ${source_path}/meson/meson.py"
> -        ;;
> -    *) meson=$(command -v "$meson") ;;
> -esac
> +# Smoke tests: these shouldn't fail, but I have little faith in esoteric
> +# platforms I've never used to not do something Deep Strange when I am
> +# not looking.
> +if ! test -e "$meson" ; then
> +    error_exit "??? pyvenv/bin/meson not found, somehow ???"
> +fi
> +echo "Meson $($meson --version) found: $meson"
> +if ! version_ge "$($meson --version)" 0.61.5; then
> +    error_exit "??? Meson version is too old ???"
> +fi
>  
>  # Probe for ninja
>  
> diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
> index a6cfe9be970..6cfe644d8fe 100644
> --- a/.gitlab-ci.d/buildtest-template.yml
> +++ b/.gitlab-ci.d/buildtest-template.yml
> @@ -17,7 +17,7 @@
>        { cat config.log meson-logs/meson-log.txt && exit 1; }
>      - if test -n "$LD_JOBS";
>        then
> -        ../meson/meson.py configure . -Dbackend_max_links="$LD_JOBS" ;
> +        pyvenv/bin/meson configure . -Dbackend_max_links="$LD_JOBS" ;
>        fi || exit 1;
>      - make -j"$JOBS"
>      - if test -n "$MAKE_CHECK_ARGS";
> -- 
> 2.39.2
> 
> 

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

Re: [RFC PATCH 3/3] configure: install meson to a python virtual environment

[Paolo Bonzini]

On 3/29/23 14:53, Daniel P. Berrangé wrote:
> I would love to see a day where configure isn't involved in the
> build process. Obviously we can't put this new logic into
> meson as its a chicken & egg problem. Could we potentially
> have your new  python/scripts/mkvenv.py script be responsible
> for setting up meson in the venv though, so we can avoid
> adding more shell code to configure ?

Not sure this is the same thing a what you want, but I do have a 
pipedream of rewriting configure in Python.  At this point it has a lot 
more logic than it has command invocations.

John, do you think it makes sense to do the "pip install" steps already 
in mkvenv's post_post_setup phase?  Maybe by passing a version spec like 
meson>=0.61.5 in place of the simpler "--gen pip,meson,sphinx" that you 
have now?

Not for this series; just dumping some thoughts prompted by Daniel's 
question.

Paolo

Re: [RFC PATCH 3/3] configure: install meson to a python virtual environment

[Daniel P. Berrangé]

On Wed, Mar 29, 2023 at 03:27:53PM +0200, Paolo Bonzini wrote:
> On 3/29/23 14:53, Daniel P. Berrangé wrote:
> > I would love to see a day where configure isn't involved in the
> > build process. Obviously we can't put this new logic into
> > meson as its a chicken & egg problem. Could we potentially
> > have your new  python/scripts/mkvenv.py script be responsible
> > for setting up meson in the venv though, so we can avoid
> > adding more shell code to configure ?
> 
> Not sure this is the same thing a what you want, but I do have a pipedream
> of rewriting configure in Python.  At this point it has a lot more logic
> than it has command invocations.

In a choice between 'shell configure' and 'python configure', I'll
take the python version, as it is a fundamentally better language to
be writing anything non-trivial in. My desire is to see the elimination
of as much shell code as possible. As a broad goal, python should be the
only scripting language we aim to use, in preference to shell or perl
or any equivalent.

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

Re: [RFC PATCH 3/3] configure: install meson to a python virtual environment

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

On Wed, Mar 29, 2023, 9:32 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Mar 29, 2023 at 03:27:53PM +0200, Paolo Bonzini wrote:
> > On 3/29/23 14:53, Daniel P. Berrangé wrote:
> > > I would love to see a day where configure isn't involved in the
> > > build process. Obviously we can't put this new logic into
> > > meson as its a chicken & egg problem. Could we potentially
> > > have your new  python/scripts/mkvenv.py script be responsible
> > > for setting up meson in the venv though, so we can avoid
> > > adding more shell code to configure ?
> >
> > Not sure this is the same thing a what you want, but I do have a
> pipedream
> > of rewriting configure in Python.  At this point it has a lot more logic
> > than it has command invocations.
>
> In a choice between 'shell configure' and 'python configure', I'll
> take the python version, as it is a fundamentally better language to
> be writing anything non-trivial in. My desire is to see the elimination
> of as much shell code as possible. As a broad goal, python should be the
> only scripting language we aim to use, in preference to shell or perl
> or any equivalent.
>

I certainly prefer python to shell; I can try to move as much logic as I
can into mkvenv.

For this RFC I tried to "disrupt" as little as I could so it was easier to
see the through-line and gain a sense for what exactly was changing. If
there's an appetite for being bolder and more aggressive, we can certainly
do that. I'm going to work out the kinks for the baseline functionality
first, then we can apply the lipstick.

Thanks,
(And sorry for the hiatus again. I haven't been feeling well, please
forgive my spotty attention.)

--js


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

[-- Attachment #2: Type: text/html, Size: 3376 bytes --]

Re: [RFC PATCH 0/3] configure: create a python venv and install meson

[Paolo Bonzini]

Thanks for posting this!

On 3/28/23 23:11, John Snow wrote:
> This series is in RFC state; some notable things:
> - So far, only meson is included in this setup.
> - There's a ton of debugging prints everywhere. It's extremely chatty right now.
> - Sphinx and qemu.qmp are not yet included in this venv.
>    (qemu.qmp isn't able to be removed from the tree yet.)
> - Testing isn't yet*fully*  switched over.
> - There is no online functionality yet, this series is *100% offline* -
>    it's the harder option, so I tackled it first.
> 
> Some known bugs as of now:
> - venv-in-venv setups are not yet handled.
> - python3.7 setups without setuptools/pip in the host environment may be
>    unable to generate script file shims; a workaround is in development
>    but wasn't ready today. I decided to exclude it.

I don't think that's needed at all---requiring setuptools on those older 
Python versions is good enough and can be mentioned in either an error 
message or the release notes.  But out of curiosity what was the workaround?

I think the steps for the first working version should be:

* fix venv-in-venv using the .pth trick

* undo the meson parts from PATCH 3; make patch 3 create the venv + 
subsume the MKVENV parts of the Makefiles + always set 
explicit_python=yes (so that at this point the in-tree meson is always 
used).

* add a patch that starts rejecting --meson=/path/to/meson and drops 
explicit_python (instead using pyvenv/bin/meson to check whether a 
system meson is usable)

* make Meson use a sphinx-build binary from the virtual environment 
(i.e. pass -Dsphinx_build=$PWD/pyvenv/bin/sphinx-build)

These are the bare minimum needed to drop Python 3.6 support in QEMU 8.1.

Paolo

Re: [RFC PATCH 0/3] configure: create a python venv and install meson

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]

On Wed, Mar 29, 2023, 9:02 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> Thanks for posting this!
>
> On 3/28/23 23:11, John Snow wrote:
> > This series is in RFC state; some notable things:
> > - So far, only meson is included in this setup.
> > - There's a ton of debugging prints everywhere. It's extremely chatty
> right now.
> > - Sphinx and qemu.qmp are not yet included in this venv.
> >    (qemu.qmp isn't able to be removed from the tree yet.)
> > - Testing isn't yet*fully*  switched over.
> > - There is no online functionality yet, this series is *100% offline* -
> >    it's the harder option, so I tackled it first.
> >
> > Some known bugs as of now:
> > - venv-in-venv setups are not yet handled.
> > - python3.7 setups without setuptools/pip in the host environment may be
> >    unable to generate script file shims; a workaround is in development
> >    but wasn't ready today. I decided to exclude it.
>
> I don't think that's needed at all---requiring setuptools on those older
> Python versions is good enough and can be mentioned in either an error
> message or the release notes.  But out of curiosity what was the
> workaround?
>

Run the shim generation inside the venv, alongside checkpip.

That way we know we have setuptools.

(It broke for some other reason I didn't diagnose in time to send.)


> I think the steps for the first working version should be:
>
> * fix venv-in-venv using the .pth trick
>

Yep.


> * undo the meson parts from PATCH 3; make patch 3 create the venv +
> subsume the MKVENV parts of the Makefiles + always set
> explicit_python=yes (so that at this point the in-tree meson is always
> used).
>
> * add a patch that starts rejecting --meson=/path/to/meson and drops
> explicit_python (instead using pyvenv/bin/meson to check whether a
> system meson is usable)
>
> * make Meson use a sphinx-build binary from the virtual environment
> (i.e. pass -Dsphinx_build=$PWD/pyvenv/bin/sphinx-build)
>

Yep, let's talk about this part in particular.


> These are the bare minimum needed to drop Python 3.6 support in QEMU 8.1.
>
> Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 3407 bytes --]

Re: [RFC PATCH 0/3] configure: create a python venv and install meson

[Paolo Bonzini]

On 3/30/23 16:11, John Snow wrote:
>     * undo the meson parts from PATCH 3; make patch 3 create the venv +
>     subsume the MKVENV parts of the Makefiles + always set
>     explicit_python=yes (so that at this point the in-tree meson is always
>     used).
> 
>     * add a patch that starts rejecting --meson=/path/to/meson and drops
>     explicit_python (instead using pyvenv/bin/meson to check whether a
>     system meson is usable)
> 
>     * make Meson use a sphinx-build binary from the virtual environment
>     (i.e. pass -Dsphinx_build=$PWD/pyvenv/bin/sphinx-build)
> 
> 
> Yep, let's talk about this part in particular.

Oh, wait, for this one I already have a patch from my experiment that
used importlib.metadata to look up the entry point dynamically[1] (and
that's where the shim idea developed from).  All I need to do is change
the path passed to find_program() and rewrite the commit message.

Paolo

[1] https://lore.kernel.org/qemu-devel/2c63f79d-b46d-841b-bed3-0dca33eab2c0@redhat.com/

--------------------------- 8< --------------------------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] meson: pick sphinx-build from virtual environment

configure is now creating a virtual environment and populating it
with shim binaries that always refer to the correct Python runtime.
docs/meson.build can rely on this, and stop using a sphinx_build
option that may or may not refer to the same version of Python that
is used for the rest of the build.

In the long term, it may actually make sense for Meson's Python
module to include the logic to build such shims, so that other
programs can do the same without needing a full-blown virtual
environment.  However, in the context of QEMU there is no need to
wait for that; QEMU's meson.build already relies on config-host.mak
and on the target list that configure prepares, i.e. it is not
standalone.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


diff --git a/docs/conf.py b/docs/conf.py
index 7e215aa9a5c6..c687ff266301 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -32,15 +32,6 @@
  from distutils.version import LooseVersion
  from sphinx.errors import ConfigError
  
-# Make Sphinx fail cleanly if using an old Python, rather than obscurely
-# failing because some code in one of our extensions doesn't work there.
-# In newer versions of Sphinx this will display nicely; in older versions
-# Sphinx will also produce a Python backtrace but at least the information
-# gets printed...
-if sys.version_info < (3,7):
-    raise ConfigError(
-        "QEMU requires a Sphinx that uses Python 3.7 or better\n")
-
  # The per-manual conf.py will set qemu_docdir for a single-manual build;
  # otherwise set it here if this is an entire-manual-set build.
  # This is always the absolute path of the docs/ directory in the source tree.
diff --git a/docs/meson.build b/docs/meson.build
index f220800e3e59..1c5fd66bfa7f 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -1,5 +1,6 @@
-sphinx_build = find_program(get_option('sphinx_build'),
# This assumes that Python is inside the venv that configure prepares
+sphinx_build = find_program(fs.parent(python.full_path()) / 'sphinx-build',
                              required: get_option('docs'))
  
  # Check if tools are available to build documentation.
diff --git a/meson_options.txt b/meson_options.txt
index b541ab2851dd..8dedec0cf91a 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -12,8 +12,6 @@ option('pkgversion', type : 'string', value : '',
         description: 'use specified string as sub-version of the package')
  option('smbd', type : 'string', value : '',
         description: 'Path to smbd for slirp networking')
-option('sphinx_build', type : 'string', value : 'sphinx-build',
-       description: 'Use specified sphinx-build for building document')
  option('iasl', type : 'string', value : '',
         description: 'Path to ACPI disassembler')
  option('tls_priority', type : 'string', value : 'NORMAL',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index bf852f4b957e..6a71c3bad296 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -58,8 +58,6 @@ meson_options_help() {
    printf "%s\n" '  --localedir=VALUE        Locale data directory [share/locale]'
    printf "%s\n" '  --localstatedir=VALUE    Localstate data directory [/var/local]'
    printf "%s\n" '  --mandir=VALUE           Manual page directory [share/man]'
-  printf "%s\n" '  --sphinx-build=VALUE     Use specified sphinx-build for building document'
-  printf "%s\n" '                           [sphinx-build]'
    printf "%s\n" '  --sysconfdir=VALUE       Sysconf data directory [etc]'
    printf "%s\n" '  --tls-priority=VALUE     Default TLS protocol/cipher priority string'
    printf "%s\n" '                           [NORMAL]'
@@ -429,7 +427,6 @@ _meson_option_parse() {
      --disable-sndio) printf "%s" -Dsndio=disabled ;;
      --enable-sparse) printf "%s" -Dsparse=enabled ;;
      --disable-sparse) printf "%s" -Dsparse=disabled ;;
-    --sphinx-build=*) quote_sh "-Dsphinx_build=$2" ;;
      --enable-spice) printf "%s" -Dspice=enabled ;;
      --disable-spice) printf "%s" -Dspice=disabled ;;
      --enable-spice-protocol) printf "%s" -Dspice_protocol=enabled ;;
-- 
2.39.2