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.