-
Notifications
You must be signed in to change notification settings - Fork 151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide a public API #262
Comments
Just a request to put something in the WheelFile class or other API that will:
Thanks for considering. |
PyPA define a wheel class in at least 6 different places (!) To make matters worse, pip also imports wheel and vendors distlib + setuptools, so it gets 4 different implementations at once. I think there would be great benefit to |
To comment @matthew-brett 's list:
|
Is the idea then, to have two copies of the code, one in |
Summary: Upstream wheel refactored and deleted an API we depend on. We would either have to copy in the specific code we need, and potentially make modifications, or vendor in wheel. I chose to vendor in wheel because it is cleaner, and we can go back to upstream if they provide the API we need again. See pypa/wheel#262. Reviewed By: cooperlees Differential Revision: D14146161 fbshipit-source-id: 5f6a3e49ce5c6d19964a3b067ec79ca6985d0d33
Is the plan to essentially document the WheelFile as it sits today as a supported API? If so, I have one major-ish concern with that - it has a baked-in assumption about the wheel format always being a flat zip. It's likely not going to be possible to transparently add support for newer versions of the wheel format, which don't operate on ".data files as members of same zip". There's been a lot of discussion about nested archives for wheels' .data directory, which allows for smaller overall wheel files on https://discuss.python.org/t/3810/ and we shouldn't lock ourselves out of that enhancement due to this API. Obviously, we can't support that kind of wheel format just yet because those aren't even close to being fully specified (!). However, I think it'd be really nice to have an API that can accommodate for such changes in a wheel 2.0 format, without needing significant user-side code change. It'd be nice to be able to gain support for reading from newer versions of wheel files by upgrading wheel (the package), without needing to update the code that handles the wheel files using the API. In other words, I think we should avoid baking in the assumption in a supported API that wheel files' .data directory will be files directly in the same zip. It's OK if we don't do it now, but we'd be creating more design work, for when a wheel 2.0 format is specified. I'd like it if we clearly from separate them from the supported API (so that pip can "easily" trim those files when vendoring). It'd be even nicer to have a separate library with the reusable code, so that pip doesn't have to do any trimming. In terms of how pip uses wheel (the package) today, pip tries to import it to check if it is even installed, and that's all. :) Beyond that, there's 2 main ways we deal with wheel files:
I don't expect/want In terms of "reading" a wheel, I think having the capability to perform random access on dist_info files, and only having sequential access to data files is all that's needed/can be guaranteed if we wanna be a bit more future-proof. In terms of "writing" a wheel, I don't have a feel for what the API should look like - pip's use case is "dumb". We have a helper function that takes a dictionary of (typed on my phone, with typo happy thumbs) |
I was going to refactor
I suggested back on discuss that setuptools could absorb the |
I can't speak for others, but I'd certainly be a happy pip maintainer if this happened. :) |
I commented on the setuptools issue. I think first wheel needs to publish its API, then setuptools would publish a release that uses the approved wheel API, only then could wheel drop |
Awesome! I'm interested in what this would look like, since it's relevant to pip/any installer as a "reader" of wheel files. I'm happy to provide concrete feedback once there's a (draft?) API for me to provide feedback on, if that's welcome and useful. :) |
Sure, I'll come up with a proposal and post it here. Probably within the next couple days. |
Yes, that's what I was thinking too. |
OK, so thinking about how I'd use a wheel library, I would like to see the following APIs: Wheel filename handlingI'd like to be able to check if a file is a wheel, parse the individual components out of a filename, and build a filename from components. This is purely to abstract away the handling of the PEP-defined name format, to protect client code from possible future changes, and to avoid "home grown" over-simplified code.
Wheel readingMaybe this is the responsibility of @pradyunsg's installer project. But outside of installation per se, I'd imagine needing the following functions:
Open question - do we expect clients to hard code the 'METADATA' filename? If not, then we need an API to get it. Wheel writingI'd want to be able to build wheels in memory or direct to disk. Taking an already-open file handle would work for this.
General questions
|
I've created a sketch of the new API (and its implementation) in the publicapi branch. Some questions came up when I was writing this:
|
@agronholm can you make it a PR so we can comment inline? Would also help to see a few example usages of it, to see how it translates when trying to do some expected usages, the existence of such tests IMHO would help. |
My thoughts:
Upon initialization works well. I don't see why we'd have to change this.
The current implementation (default to
No thoughts on this. global is likely easiest. :)
🤷 |
PR created: #357
I'll convert |
I have now pushed changes that make |
I think extracting the following into
(but in a non-opaque way, e.g. dealing with dictionaries instead of raw-bytes) |
@agronholm Anything I can do to help move this forward? (I'm interested in having |
Sorry for the delayed response. I haven't touched the As for what you can do, making a list of changes we need for a release would be a good start (once I've pushed my changes). We can discuss the next steps once that has been done. UPDATE: more tests are passing than failing :) |
@pradyunsg I've created a draft PR: #472. It's currently failing due to not finding the |
Closing in favor of pypa/packaging#697. |
Several third party libraries broke when the internal API of wheel was changed as the result of a major refactoring effort. Since there is clearly a need for wheel to function as a library as well, a public API should be defined and documented.
Here's what I plan to include in the API:
WheelFile
classThe text was updated successfully, but these errors were encountered: