Repository navigation
Conversation
…hon_internal The `patch_whl.bzl` and `whl_extract.bzl` files loaded the `@rules_python_internal` repository only to get the `supports_whl_extraction` and `extract_needs_chmod` values, which only depend on the Bazel version. Compute them from `native.bazel_version` in a new `extract_features` function, which `internal_config_repo` also uses. This allows loading `whl_library` from the files that are loaded before `py_repositories` defines `@rules_python_internal` in WORKSPACE, e.g. `python/repositories.bzl`.
|
This PR was drafted by an AI coding agent (Claude Code) working for @aignas as part of the plan in #3902. It stays a draft until @aignas has reviewed it. Review comments will be handled by @aignas, possibly with the help of the agent; any agent-written replies will be marked as such. Agent-generated comment, posted automatically on behalf of the user. |
The `pypi_repo_utils.bzl` and `util.bzl` files only used `types.is_string` and `types.is_tuple` from `@bazel_skylib`, which are simple `type()` checks. Without them, `whl_library` can be loaded from the files that are loaded before the `WORKSPACE` defines `@bazel_skylib`, e.g. `python/repositories.bzl`.
aignas
changed the title
Replace the private `extract_features` function with a public `bazel_features(bazel_version)` function in `//python:features.bzl`, which returns the features of a Bazel version, e.g. `bazel_features(native.bazel_version).supports_whl_extraction`. The Bazel version is only available when a repository rule or a module extension runs, so this is a function and not fields of the `features` struct. The values are no longer written to the `@rules_python_internal` config, as nothing reads them from there anymore.
aignas
changed the title
Now that `whl_library` can be loaded from `python/repositories.bzl` (bazel-contrib#4458), use it for the coverage wheels instead of a hand-crafted `http_archive`, so the wheels are extracted the same way as for the pip hubs.
| """ | ||
| major, minor = _major_minor(bazel_version) | ||
| return struct( | ||
| supports_whl_extraction = major > 8 or (major == 8 and minor >= 3), |
There was a problem hiding this comment.
Lets just inline this logic into the whl_extract.bzl / patch_whl.bzl files instead.
features.bzl is intended for user-facing functionality. e.g
- to know if a target is available so macro-phase code can set deps
- if certain load() files are available so userspace repo-phase code can generate stubs
The "supports whl extraction" and "extract needs chmod" behavior is an internal detail that users wouldn't vary behavior base don.
I think this conflicts with another directive in the agent configs. I'd prefer to use the helpers, but if its posing bootstrapping issues in workspace, then i'm fine with it. also: re PR format: The AIs love putting things in bullet points. I think the configs are suppose to dissuade it from doing that and use more natural and useful prose |
|
SGTM, thank you for the review, I'll inline the logic. |
Remove the public `bazel_features` function from `//python:features.bzl` and compute the `supports_whl_extraction` and `extract_needs_chmod` values from `native.bazel_version` directly in `whl_extract.bzl` and `patch_whl.bzl`. These are internal details that users would not vary their behaviour on, so they do not belong in the user-facing features.
|
Re the skylib I also rewrote the PR description in prose to match the inlined approach. This reply was generated by an AI agent on behalf of @aignas and posted automatically. |
In WORKSPACE,
python/repositories.bzlis loaded beforepy_repositories()defines
@rules_python_internaland@bazel_skylib, so nothing loaded from itcan depend on those repositories. The WORKSPACE toolchains in #4457 need to
use
whl_libraryfor the coverage wheels, which means thewhl_libraryloadchain must only load
@bazel_tools.The
whl_extract.bzlandpatch_whl.bzlfiles loaded@rules_python_internalonly to read
supports_whl_extractionandextract_needs_chmod, which dependon nothing but the Bazel version. They now compute these from
native.bazel_versionthemselves, and the values are no longer written to the@rules_python_internalconfig. Thepypi_repo_utils.bzlandutil.bzlfilesloaded
@bazel_skylibonly fortypes.is_stringandtypes.is_tuple, sothose are replaced with plain
type()checks.Work towards #3902