Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a19e1e4

Browse filesBrowse files
aignasaignas
authored
fix: load target_platforms through the hub (#2781)
This PR moves the parsing of `Requires-Dist` to the loading phase within the `whl_library_targets_from_requires` macro. The original `whl_library_targets` macro has been left unchanged so that I don't have to reinvent the unit tests - it is well covered under tests. Before this PR we had to wire the `target_platforms` via the `experimental_target_platforms` attr in the `whl_library`, which means that whenever this would change (e.g. the minor Python version changes), the wheel would be re-extracted even though the final result may be the same. This refactor uncovered that the dependency graph creation was incorrect if we had multiple target Python versions due to various heuristics that this had. In hindsight I had them to make the generated `BUILD.bazel` files more readable when the unit test coverage was not great. Now this is unnecessary and since everything is happening in Starlark I thought that having a simpler algorithm that does the right thing always is the best way. This also cleans up the code by removing left over TODO notes or code that no longer make sense. Work towards #260, #2319
1 parent cc46fb2 commit a19e1e4
Copy full SHA for a19e1e4

17 files changed

+451
-466
lines changed

‎CHANGELOG.md

Copy file name to clipboardExpand all lines: CHANGELOG.md
+7
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ END_UNRELEASED_TEMPLATE
105105
[PR #2746](https://github.com/bazel-contrib/rules_python/pull/2746).
106106
* (rules) {attr}`py_binary.srcs` and {attr}`py_test.srcs` is no longer mandatory when
107107
`main_module` is specified (for `--bootstrap_impl=script`)
108+
* (pypi) From now on the `Requires-Dist` from the wheel metadata is analysed in
109+
the loading phase instead of repository rule phase giving better caching
110+
performance when the target platforms are changed (e.g. target python
111+
versions). This is preparatory work for stabilizing the cross-platform wheel
112+
support. From now on the usage of `experimental_target_platforms` should be
113+
avoided and the `requirements_by_platform` values should be instead used to
114+
specify the target platforms for the given dependencies.
108115

109116
[20250317]: https://github.com/astral-sh/python-build-standalone/releases/tag/20250317
110117

‎config.bzl.tmpl.bzlmod

Copy file name to clipboardExpand all lines: config.bzl.tmpl.bzlmod
Whitespace-only changes.

‎python/private/pypi/BUILD.bazel

Copy file name to clipboardExpand all lines: python/private/pypi/BUILD.bazel
+2-12
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,6 @@ bzl_library(
212212
],
213213
)
214214

215-
bzl_library(
216-
name = "pep508_bzl",
217-
srcs = ["pep508.bzl"],
218-
deps = [
219-
":pep508_env_bzl",
220-
":pep508_evaluate_bzl",
221-
],
222-
)
223-
224215
bzl_library(
225216
name = "pep508_deps_bzl",
226217
srcs = ["pep508_deps.bzl"],
@@ -378,13 +369,12 @@ bzl_library(
378369
":attrs_bzl",
379370
":deps_bzl",
380371
":generate_whl_library_build_bazel_bzl",
381-
":parse_whl_name_bzl",
382372
":patch_whl_bzl",
383-
":pep508_deps_bzl",
373+
":pep508_requirement_bzl",
384374
":pypi_repo_utils_bzl",
385375
":whl_metadata_bzl",
386-
":whl_target_platforms_bzl",
387376
"//python/private:auth_bzl",
377+
"//python/private:bzlmod_enabled_bzl",
388378
"//python/private:envsubst_bzl",
389379
"//python/private:is_standalone_interpreter_bzl",
390380
"//python/private:repo_utils_bzl",

‎python/private/pypi/attrs.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/attrs.bzl
+3
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ Warning:
123123
"experimental_target_platforms": attr.string_list(
124124
default = [],
125125
doc = """\
126+
*NOTE*: This will be removed in the next major version, so please consider migrating
127+
to `bzlmod` and rely on {attr}`pip.parse.requirements_by_platform` for this feature.
128+
126129
A list of platforms that we will generate the conditional dependency graph for
127130
cross platform wheels by parsing the wheel metadata. This will generate the
128131
correct dependencies for packages like `sphinx` or `pylint`, which include

‎python/private/pypi/config.bzl.tmpl.bzlmod

Copy file name to clipboard
+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
"""Extra configuration values that are exposed from the hub repository for spoke repositories to access.
2+
3+
NOTE: This is internal `rules_python` API and if you would like to depend on it, please raise an issue
4+
with your usecase. This may change in between rules_python versions without any notice.
5+
6+
@generated by rules_python pip.parse bzlmod extension.
7+
"""
8+
9+
target_platforms = %%TARGET_PLATFORMS%%

‎python/private/pypi/extension.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/extension.bzl
+18-23
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ load(":simpleapi_download.bzl", "simpleapi_download")
3232
load(":whl_config_setting.bzl", "whl_config_setting")
3333
load(":whl_library.bzl", "whl_library")
3434
load(":whl_repo_name.bzl", "pypi_repo_name", "whl_repo_name")
35-
load(":whl_target_platforms.bzl", "whl_target_platforms")
3635

3736
def _major_minor_version(version):
3837
version = semver(version)
@@ -68,7 +67,6 @@ def _create_whl_repos(
6867
*,
6968
pip_attr,
7069
whl_overrides,
71-
evaluate_markers = evaluate_markers,
7270
available_interpreters = INTERPRETER_LABELS,
7371
get_index_urls = None):
7472
"""create all of the whl repositories
@@ -77,7 +75,6 @@ def _create_whl_repos(
7775
module_ctx: {type}`module_ctx`.
7876
pip_attr: {type}`struct` - the struct that comes from the tag class iteration.
7977
whl_overrides: {type}`dict[str, struct]` - per-wheel overrides.
80-
evaluate_markers: the function to use to evaluate markers.
8178
get_index_urls: A function used to get the index URLs
8279
available_interpreters: {type}`dict[str, Label]` The dictionary of available
8380
interpreters that have been registered using the `python` bzlmod extension.
@@ -162,14 +159,12 @@ def _create_whl_repos(
162159
requirements_osx = pip_attr.requirements_darwin,
163160
requirements_windows = pip_attr.requirements_windows,
164161
extra_pip_args = pip_attr.extra_pip_args,
162+
# TODO @aignas 2025-04-15: pass the full version into here
165163
python_version = major_minor,
166164
logger = logger,
167165
),
168166
extra_pip_args = pip_attr.extra_pip_args,
169167
get_index_urls = get_index_urls,
170-
# NOTE @aignas 2025-02-24: we will use the "cp3xx_os_arch" platform labels
171-
# for converting to the PEP508 environment and will evaluate them in starlark
172-
# without involving the interpreter at all.
173168
evaluate_markers = evaluate_markers,
174169
logger = logger,
175170
)
@@ -191,7 +186,6 @@ def _create_whl_repos(
191186
enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs,
192187
environment = pip_attr.environment,
193188
envsubst = pip_attr.envsubst,
194-
experimental_target_platforms = pip_attr.experimental_target_platforms,
195189
group_deps = group_deps,
196190
group_name = group_name,
197191
pip_data_exclude = pip_attr.pip_data_exclude,
@@ -244,6 +238,12 @@ def _create_whl_repos(
244238
},
245239
extra_aliases = extra_aliases,
246240
whl_libraries = whl_libraries,
241+
target_platforms = {
242+
plat: None
243+
for reqs in requirements_by_platform.values()
244+
for req in reqs
245+
for plat in req.target_platforms
246+
},
247247
)
248248

249249
def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patterns, multiple_requirements_for_whl = False, python_version):
@@ -274,20 +274,11 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt
274274
args["urls"] = [distribution.url]
275275
args["sha256"] = distribution.sha256
276276
args["filename"] = distribution.filename
277-
args["experimental_target_platforms"] = requirement.target_platforms
278277

279278
# Pure python wheels or sdists may need to have a platform here
280279
target_platforms = None
281280
if distribution.filename.endswith(".whl") and not distribution.filename.endswith("-any.whl"):
282-
parsed_whl = parse_whl_name(distribution.filename)
283-
whl_platforms = whl_target_platforms(
284-
platform_tag = parsed_whl.platform_tag,
285-
)
286-
args["experimental_target_platforms"] = [
287-
p
288-
for p in requirement.target_platforms
289-
if [None for wp in whl_platforms if p.endswith(wp.target_platform)]
290-
]
281+
pass
291282
elif multiple_requirements_for_whl:
292283
target_platforms = requirement.target_platforms
293284

@@ -416,6 +407,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
416407
hub_group_map = {}
417408
exposed_packages = {}
418409
extra_aliases = {}
410+
target_platforms = {}
419411
whl_libraries = {}
420412

421413
for mod in module_ctx.modules:
@@ -498,6 +490,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
498490
for whl_name, aliases in out.extra_aliases.items():
499491
extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
500492
exposed_packages.setdefault(hub_name, {}).update(out.exposed_packages)
493+
target_platforms.setdefault(hub_name, {}).update(out.target_platforms)
501494
whl_libraries.update(out.whl_libraries)
502495

503496
# TODO @aignas 2024-04-05: how do we support different requirement
@@ -535,6 +528,10 @@ You cannot use both the additive_build_content and additive_build_content_file a
535528
}
536529
for hub_name, extra_whl_aliases in extra_aliases.items()
537530
},
531+
target_platforms = {
532+
hub_name: sorted(p)
533+
for hub_name, p in target_platforms.items()
534+
},
538535
whl_libraries = {
539536
k: dict(sorted(args.items()))
540537
for k, args in sorted(whl_libraries.items())
@@ -626,15 +623,13 @@ def _pip_impl(module_ctx):
626623
},
627624
packages = mods.exposed_packages.get(hub_name, []),
628625
groups = mods.hub_group_map.get(hub_name),
626+
target_platforms = mods.target_platforms.get(hub_name, []),
629627
)
630628

631629
if bazel_features.external_deps.extension_metadata_has_reproducible:
632-
# If we are not using the `experimental_index_url feature, the extension is fully
633-
# deterministic and we don't need to create a lock entry for it.
634-
#
635-
# In order to be able to dogfood the `experimental_index_url` feature before it gets
636-
# stabilized, we have created the `_pip_non_reproducible` function, that will result
637-
# in extra entries in the lock file.
630+
# NOTE @aignas 2025-04-15: this is set to be reproducible, because the
631+
# results after calling the PyPI index should be reproducible on each
632+
# machine.
638633
return module_ctx.extension_metadata(reproducible = True)
639634
else:
640635
return None

‎python/private/pypi/generate_whl_library_build_bazel.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/generate_whl_library_build_bazel.bzl
+22-5
Original file line numberDiff line numberDiff line change
@@ -21,42 +21,56 @@ _RENDER = {
2121
"copy_files": render.dict,
2222
"data": render.list,
2323
"data_exclude": render.list,
24-
"dependencies": render.list,
25-
"dependencies_by_platform": lambda x: render.dict(x, value_repr = render.list),
2624
"entry_points": render.dict,
25+
"extras": render.list,
2726
"group_deps": render.list,
27+
"requires_dist": render.list,
2828
"srcs_exclude": render.list,
29-
"tags": render.list,
29+
"target_platforms": lambda x: render.list(x) if x else "target_platforms",
3030
}
3131

3232
# NOTE @aignas 2024-10-25: We have to keep this so that files in
3333
# this repository can be publicly visible without the need for
3434
# export_files
3535
_TEMPLATE = """\
36-
load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets")
36+
{loads}
3737
3838
package(default_visibility = ["//visibility:public"])
3939
40-
whl_library_targets(
40+
whl_library_targets_from_requires(
4141
{kwargs}
4242
)
4343
"""
4444

4545
def generate_whl_library_build_bazel(
4646
*,
4747
annotation = None,
48+
default_python_version = None,
4849
**kwargs):
4950
"""Generate a BUILD file for an unzipped Wheel
5051
5152
Args:
5253
annotation: The annotation for the build file.
54+
default_python_version: The python version to use to parse the METADATA.
5355
**kwargs: Extra args serialized to be passed to the
5456
{obj}`whl_library_targets`.
5557
5658
Returns:
5759
A complete BUILD file as a string
5860
"""
5961

62+
loads = [
63+
"""load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires")""",
64+
]
65+
if not kwargs.setdefault("target_platforms", None):
66+
dep_template = kwargs["dep_template"]
67+
loads.append(
68+
"load(\"{}\", \"{}\")".format(
69+
dep_template.format(name = "", target = "config.bzl"),
70+
"target_platforms",
71+
),
72+
)
73+
6074
additional_content = []
6175
if annotation:
6276
kwargs["data"] = annotation.data
@@ -66,10 +80,13 @@ def generate_whl_library_build_bazel(
6680
kwargs["srcs_exclude"] = annotation.srcs_exclude_glob
6781
if annotation.additive_build_content:
6882
additional_content.append(annotation.additive_build_content)
83+
if default_python_version:
84+
kwargs["default_python_version"] = default_python_version
6985

7086
contents = "\n".join(
7187
[
7288
_TEMPLATE.format(
89+
loads = "\n".join(loads),
7390
kwargs = render.indent("\n".join([
7491
"{} = {},".format(k, _RENDER.get(k, repr)(v))
7592
for k, v in sorted(kwargs.items())

‎python/private/pypi/hub_repository.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/hub_repository.bzl
+16-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,14 @@ def _impl(rctx):
4545
macro_tmpl = "@@{name}//{{}}:{{}}".format(name = rctx.attr.name)
4646

4747
rctx.file("BUILD.bazel", _BUILD_FILE_CONTENTS)
48-
rctx.template("requirements.bzl", rctx.attr._template, substitutions = {
48+
rctx.template(
49+
"config.bzl",
50+
rctx.attr._config_template,
51+
substitutions = {
52+
"%%TARGET_PLATFORMS%%": render.list(rctx.attr.target_platforms),
53+
},
54+
)
55+
rctx.template("requirements.bzl", rctx.attr._requirements_bzl_template, substitutions = {
4956
"%%ALL_DATA_REQUIREMENTS%%": render.list([
5057
macro_tmpl.format(p, "data")
5158
for p in bzl_packages
@@ -80,14 +87,21 @@ The list of packages that will be exposed via all_*requirements macros. Defaults
8087
mandatory = True,
8188
doc = "The apparent name of the repo. This is needed because in bzlmod, the name attribute becomes the canonical name.",
8289
),
90+
"target_platforms": attr.string_list(
91+
mandatory = True,
92+
doc = "All of the target platforms for the hub repo",
93+
),
8394
"whl_map": attr.string_dict(
8495
mandatory = True,
8596
doc = """\
8697
The wheel map where values are json.encoded strings of the whl_map constructed
8798
in the pip.parse tag class.
8899
""",
89100
),
90-
"_template": attr.label(
101+
"_config_template": attr.label(
102+
default = ":config.bzl.tmpl.bzlmod",
103+
),
104+
"_requirements_bzl_template": attr.label(
91105
default = ":requirements.bzl.tmpl.bzlmod",
92106
),
93107
},

‎python/private/pypi/pep508.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/pep508.bzl
-23
This file was deleted.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.