feat(bzlmod): support bazel downloader when downloading wheels #1827
Conversation
This is a variant of bazelbuild#1625 and was inspired by bazelbuild#1788. In bazelbuild#1625, we attempt to parse the simple API HTML files in the same `pip.parse` extension and it brings the follownig challenges: * The `pip.parse` cannot be easily use in `isolated` mode and it may be difficult to implement the isolation if bazelbuild/bazel#20186 moves forward. * Splitting the `pypi_index` out of the `pip.parse` allows us to accept the location of the parsed simple API artifacts encoded as a bazel label. * Separation of the logic allows us to very easily implement usage of the downloader for cross-platform wheels. * The `whl` `METADATA` might not be exposed through older versions of Artifactory, so having the complexity hidden in this single extension allows us to not increase the complexity and scope of `pip.parse` too much. * The repository structure can be reused for `pypi_install` extension from bazelbuild#1728. TODO: - [ ] Add unit tests for functions in `pypi_index.bzl` bzlmod extension if the design looks good. - [ ] Changelog. Out of scope of this PR: - Further usage of the downloaded artifacts to implement something similar to bazelbuild#1625 or bazelbuild#1744. This needs bazelbuild#1750 and bazelbuild#1764. - Making the lock file the same on all platforms - We would need to fully parse the requirements file. - Support for different dependency versions in the `pip.parse` hub repos based on each platform - we would need to be able to interpret platform markers in some way, but `pypi_index` should be good already. - Implementing the parsing of METADATA to detect dependency cycles. - Support for `requirements` files that are not created via `pip-compile`. - Support for other lock formats, though that would be reasonably trivial to add. Open questions: - Support for VCS dependencies in requirements files - We should probably handle them as `overrides` in the `pypi_index` extension and treat them in `pip.parse` just as an `sdist`, but I am not sure it would work without any issues.
|
I was normalizing the names in the requirements to use underscores. That works with PyPI (and Artifactory if I remember correctly), but it seems that it does not work with every index. I have changed the code to use exactly the same package name as is in the supplied requirements file. I will be adding various unit tests from now on, so expect a little bit more activity here. Open questions:
|
I'd be more than happy to! I think I'll be able to get something whipped up within a day or three. I'll just submit a PR onto
Thanks! That part works wonderfully 😁. Other Issues/Comments
Here's an example of the error in (3): |
|
After 5643866 it looks like everything's working very well! 404s are still spammy but I can live with that. From what I can tell, both wheels and sdists were correctly installed, it didn't fail out on unknown platforms, and it even fell back to standard pip for one of our more annoying packages klayout1. This is so awesome, thank you very much! Footnotes
|
|
I'll merge this and start looking at supporting multi-platform whl selects which is what #1744 tried to achieve. |
This introduces 3 attributes and the minimal code to be able to download wheels using the bazel downloader for the host platform. This is not yet adding support for targeting a different platform but just allows us to get the wheels for the host platform instead of using `pip`. All of this is achieved by calling the PyPI's SimpleAPI (Artifactory should work as well) and getting the all URLs for packages from there. Then we use the `sha256` information within the requirements files to match the entries found on SimpleAPI and then pass the `url`, `sha256` and the `filename` to `whl_library`, which uses `repository_ctx.download`. If we cannot find any suitable artifact to use, we fallback to legacy `pip` behaviour. Testing notes: * Most of the code has unit tests, but the `pypi_index.bzl` extension could have more. * You can see the lock file for what the output of all of this code would be on your platform. * Thanks to @dougthor42 for testing this using the credentials helper against a private registry that needs authentication to be accessed. Work towards #1357
|
This is the first time I see a non closed PR after merging. I'll just close this one manually. |
…r of python packages (#1834) Add credential helper docs that were requested in #1827 (comment)
| @@ -58,6 +58,7 @@ register_toolchains("@pythons_hub//:all") | |||
|
|
|||
| pip = use_extension("//python/extensions:pip.bzl", "pip") | |||
| pip.parse( | |||
| experimental_index_url = "https://pypi.org/simple", | |||
There was a problem hiding this comment.
Is it intentional not to use PIP_INDEX_URL env here, and why?
There was a problem hiding this comment.
If you want to override this value you can use bazel downloader config. In the general sense it is not a given that people will have their PIP_INDEX_URL configured to contain the index that works for fetching these dependencies.
Hence to avoid unintended breakage, this is hardcoded.
This introduces 3 attributes and the minimal code to be able to download wheels
using the bazel downloader for the host platform. This is not yet adding
support for targeting a different platform but just allows us to get the wheels
for the host platform instead of using
pip.All of this is achieved by calling the PyPI's SimpleAPI (Artifactory should work
as well) and getting the all URLs for packages from there. Then we use the
sha256information within the requirements files to match the entries found on SimpleAPI
and then pass the
url,sha256and thefilenametowhl_library, which usesrepository_ctx.download.If we cannot find any suitable artifact to use, we fallback to legacy
pipbehaviour.Testing notes:
pypi_index.bzlextension could have more.platform.
registry that needs authentication to be accessed.
Work towards #1357