-
Notifications
You must be signed in to change notification settings - Fork 546
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
feat(pip): support specifying requirements per (os, arch) #1885
Conversation
20d2045
to
a1a90d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. I think I see how this is working. Just making sure I got the gist of it:
- Input is basically a list of (filename, csv-string-platforms)
- It expands that into (filename, platform-pattern)
- It reads each file and populates a distribution->list-of-platforms map
- That map eventually gets to whl_library
There's a bunch of surrounding code to handle some edge cases and error checking, but I think that's the gist of it.
Yeah, this makes sense. Looking at the requirements parser, it's pretty simple and doesn't have any support for env markers, which is...yeah, we'd only be able to do limited support given what env markers can do compared to bazel constraints.
docs/sphinx/pip.md
Outdated
(per-os-arch-requirements)= | ||
## Requirements for a specific OS/Architecture | ||
|
||
IN some cases you may need to use different requirements files for different OS, Arch combinations. This is enabled via the `requirements_by_platform` attribute in `pip.parse` extension and the `pip_parse` repository rule. The keys of the dictionary are labels to the file and the values are a list of comma separated target (os, arch) tuples. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IN some cases you may need to use different requirements files for different OS, Arch combinations. This is enabled via the `requirements_by_platform` attribute in `pip.parse` extension and the `pip_parse` repository rule. The keys of the dictionary are labels to the file and the values are a list of comma separated target (os, arch) tuples. | |
In some cases you may need to use different requirements files for different OS, Arch combinations. This is enabled via the `requirements_by_platform` attribute in `pip.parse` extension and the `pip_parse` repository rule. The keys of the dictionary are labels to the file and the values are a list of comma separated target (os, arch) tuples. |
doc = """\ | ||
The requirements files and the comma delimited list of target platforms as values. | ||
|
||
The keys are the requirement files and the values are coma-separated platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keys are the requirement files and the values are coma-separated platform | |
The keys are the requirement files and the values are comma-separated platform |
|
||
"""Requirements parsing for whl_library creation. | ||
|
||
Usecases that the code needs to cover: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usecases that the code needs to cover: | |
Use cases that the code needs to cover: |
Yeah, you got the gist correctly. The map of the platforms gets to
|
This change implements the necessary selection of the requirement files based on the (os, arch) for both - bzlmod and legacy workspace code paths. As part of this addition I have moved some tests from integration tests to unit tests as now all of the logic on selecting the right requirement file, ensuring that there are no duplicate requirement lines and that the user knows what to provide is done in a single place. This should be a non-breaking change for most users unless they have been passing `requirements_linux` together with `extra_pip_args = ["--platform=manylinux_2_4_x86_64"]`, in which case they would have to change their code to use `requirements_lock` attribute which itself is a trivial change.
dfabf80
to
6e31814
Compare
This PR implements a better way of specifying the requirements files for
different (os, cpu) tuples. It allows for more granular specification than what
is available today and allows for future extension to have all of the sources
in the select statements in the hub repository.
This is replacing the previous selection of the requirements and there are a
few differences in behaviour that should not be visible to the external user.
Instead of selecting the right file which we should then use to create
whl_library
instances we parse all of the provided requirements files andmerge them based on the contents. The merging is done based on the blocks
within the requirement file and this allows the starlark code to understand if
we are working with different versions of the same package on different target
platforms.
Fixes #1868
Work towards #1643, #735