Preload script wrappers at import time

[pfmoore]

Pip encountered an issue (pypa/pip#12666) where we can't find the script wrappers because we've uninstalled the running version of pip. Pre-loading the wrapper code into memory when importing distlib.scripts avoids this problem.

We can patch this locally, but would it be acceptable as a change here instead?

[notatallshaw]

macos-latest 3.7 is failing because macos-latest has moved from macos-12 to macos-14, and macos-14 does not have Python 3.7 on it.

Given macos-12 is x86 and macos-14 is ARM and distlib tests run so quickly, I would reccomend running against both and excluding Python 3.7 for macos-14/latest, a little like in https://github.com/pypa/pip/pull/12593/files

[vsajip]

WRAPPERS is only used when os.name == 'nt' or (os.name == 'java' and os._name == 'nt'), so this computation suite could also be done only when the above condition is true, right?

[pfmoore]

Good catch - fixed.

[vsajip]

@pfmoore do you need an imminent release with this change? Otherwise I would normally do a release mid-year-ish (unless an urgent issue needs fixing).

[pfmoore]

I don't think it's needed. The pip issue isn't urgent, and we can patch locally in the interim. It's not worth making you do the extra work IMO.

[gaborbernat]

This broke virtualenv Windows zipapps #235

[pfmoore]

Hmm, sorry about that. I thought that registered finders needed to be compatible with the ResourceFinder class. The docs should probably be clearer on what's required - PEP 302 is extremely out of date these days, but unfortunately the importlib.resources API, which is the stdlib analogue of the distlib resources API, is still pretty unstable and not something I think we should reference 🙁

Maybe we should say that all registered finders must implement the ResourceFinder interface? @vsajip what do you think? I can update the docs if that seems reasonable to you.

[vsajip]

what do you think

That seems reasonable to me, but what fix do you propose to the changes in this PR? I mean, it looks like Gabor's worked around it, but isn't it better to sort it at source?

[pfmoore]

TBH, I'd assumed it wasn't (easily) fixable at source. Isn't the problem here that virtualenv loads distlib from a location that's registered with a finder that doesn't support iterating over resources?

In this code, virtualenv registers a custom finder, but it's not clear that the registration satisfies the necessary contract, which says that the finder maker "must return a finder for that package". What constitutes a "finder" isn't clear (and the reference to PEP 302 in that section of the docs further muddies the water) but I'd assume it means a ResourceFinder (or at least something that provides the interface that ResourceFinder does).

My proposal was simply to make that clearer. In theory, virtualenv is still incorrect, as it doesn't implement methods like is_container, or get_stream, but I would consider that a practical decision on virtualenv's part, to only implement as much of the interface as it needs to. We could go further and make ResourceFinder subclassable, and make register_finder require that the finder_maker returns a ResourceFinder subclass, but that's a bigger change than I feel comfortable proposing (or making!)

If, on the other hand, you have a suggestion of how we can find the wrappers using a finder that only implements find, I'm happy to have a go at implementing that. But I can't think of a way myself that doesn't require hard coding the names of the wrapper executables.

[gaborbernat]

Yes, my primary complaint I made here is about the documentation not being clear what it actually required And I had to reverse engineer the interface I needed to implement.

[pfmoore]

@gaborbernat @vsajip is #236 sufficient?

[vsajip]

Thank you, Paul, I've merged it. @gaborbernat let us know if you think more is needed.

[gaborbernat]

yeah, that sounds better