Skip to content

feat(pip): provide pypi -> whl target mapping in requirements.bzl#1532

Merged
rickeylev merged 4 commits intomainfrom
allow_wheel_reqs
Nov 2, 2023
Merged

feat(pip): provide pypi -> whl target mapping in requirements.bzl#1532
rickeylev merged 4 commits intomainfrom
allow_wheel_reqs

Conversation

@alexeagle
Copy link
Copy Markdown
Contributor

@alexeagle alexeagle commented Nov 1, 2023 •
edited
Loading

Currently a BUILD file can load all_whl_requirements but then can't determine which one is associated with a given package installed by the user.
This makes it impossible to build rules where the user can choose a given package and then override the wheel used, for example. @mattem and I are working at a client where we need this ability.

This PR makes a small, non-breaking refactoring to the generated requirements.bzl file so that this information is available in a new all_whl_requirements_by_package symbol.

Users can then do something like this:

load("@pip//:requirements.bzl", "all_whl_requirements_by_package")

some_rule(
  wheels = dict(all_whl_requirements_by_package, **{
    "charset-normalizer": "@charset_1_2_3//:file"
  }),
)

refactor: requirements.bzl allows wheel files to be traced from the p…
8a23cbc 
…ackage that installed them

This is needed in custom rules that want to locate the wheel file for a given package name.

Demo:

```
$ head -15 $(bazel info output_base)/external/pypi/requirements.bzl
"""Starlark representation of locked requirements.

@generated by rules_python pip_parse repository rule
from @//:requirements_lock.txt
"""

load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library")

all_requirements = ["@pypi//certifi:pkg", "@pypi//chardet:pkg", "@pypi//idna:pkg", "@pypi//pathspec:pkg", "@pypi//python_dateutil:pkg", "@pypi//python_magic:pkg", "@pypi//pyyaml:pkg", "@pypi//requests:pkg", "@pypi//s3cmd:pkg", "@pypi//setuptools:pkg", "@pypi//six:pkg", "@pypi//urllib3:pkg", "@pypi//yamllint:pkg"]

all_whl_requirements_by_package = {"certifi": "@pypi//certifi:whl", "chardet": "@pypi//chardet:whl", "idna": "@pypi//idna:whl", "pathspec": "@pypi//pathspec:whl", "python_dateutil": "@pypi//python_dateutil:whl", "python_magic": "@pypi//python_magic:whl", "pyyaml": "@pypi//pyyaml:whl", "requests": "@pypi//requests:whl", "s3cmd": "@pypi//s3cmd:whl", "setuptools": "@pypi//setuptools:whl", "six": "@pypi//six:whl", "urllib3": "@pypi//urllib3:whl", "yamllint": "@pypi//yamllint:whl"}

all_whl_requirements = all_whl_requirements_by_package.values()

all_data_requirements = ["@pypi//certifi:data", "@pypi//chardet:data", "@pypi//idna:data", "@pypi//pathspec:data", "@pypi//python_dateutil:data", "@pypi//python_magic:data", "@pypi//pyyaml:data", "@pypi//requests:data", "@pypi//s3cmd:data", "@pypi//setuptools:data", "@pypi//six:data", "@pypi//urllib3:data", "@pypi//yamllint:data"]
```
@alexeagle alexeagle requested a review from aignas November 1, 2023 20:05
@alexeagle alexeagle changed the title refactor: requirements.bzl allows wheel files to be traced from the p… refactor: allow wheel targets to be traced from the package Nov 1, 2023
@alexeagle alexeagle requested a review from mattem November 1, 2023 20:16
@alexeagle alexeagle marked this pull request as ready for review November 1, 2023 20:31
@alexeagle alexeagle requested a review from rickeylev as a code owner November 1, 2023 20:31
rickeylev
rickeylev requested changes Nov 2, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update the bzlmod implementation:

  • python/private/bzlmod/pip_repository.bzl
  • python/private/bzlmod/requirements.bzl.tmpl

@rickeylev rickeylev changed the title refactor: allow wheel targets to be traced from the package feat(pip): provide pypi -> whl target mapping in requirements.bzl Nov 2, 2023
@rickeylev
Copy link
Copy Markdown
Collaborator

rickeylev commented Nov 2, 2023

Can you also update the changelog, please?

@alexeagle
Copy link
Copy Markdown
Contributor Author

alexeagle commented Nov 2, 2023

@rickeylev attempting to update the bzlmod implementation, but I'm not seeing the code used. Even in examples/pip_parse with --enable_bzlmod, I still get the requirements.bzl produced by the implementation I modified. Is the new bzlmod implementation not wired up yet?

@rickeylev
Copy link
Copy Markdown
Collaborator

rickeylev commented Nov 2, 2023

Yes, it's wired up.

The dependency goes:

python/extension/pip.bzl -> python/private/bzlmod/pip.bzl -> python/private/bzlmod/pip_repository.bzl -> python/private/bzlmod/requirements.bzl.tmpl

@alexeagle
Copy link
Copy Markdown
Contributor Author

alexeagle commented Nov 2, 2023

Thanks, updated.

I found one issue that I don't think I want to try to solve here: in this code
https://github.com/bazelbuild/rules_python/blob/main/python/private/bzlmod/pip.bzl#L124-L128
we overwrite whl_name with the normalized name, but only after a comment about how users shouldn't need to guess our sanitization scheme. I agree with that comment - but FWICT we then use the normalized whl_name in places that ARE user-affordances like the whl_overrides.

As a consequence, under the bzlmod implementation, my feature has the normalized names appearing as the keys in all_whl_requirements_by_package. I think that's not desirable, but it seems like a substantial refactoring to instead take more care about the normalized whl_name vs. the original distribution name.

@rickeylev rickeylev added this pull request to the merge queue Nov 2, 2023
Merged via the queue into main with commit 78fc4de Nov 2, 2023
@aignas aignas deleted the allow_wheel_reqs branch October 17, 2024 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aignas aignas aignas approved these changes

@rickeylev rickeylev rickeylev approved these changes

@mattem mattem Awaiting requested review from mattem

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexeagle @rickeylev @aignas