qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: John Snow
Subject: Re: [RFC PATCH 1/3] python: add mkvenv.py
Date: Thu, 30 Mar 2023 10:00:32 -0400



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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]