Skip to content

Commit 1d55c50

Browse files
committed
fix(pypi): pull fewer wheels
Before this we would pull all of the wheels that the user target configuration would be compatible with and that meant that it was not customizable. This also meant that there were a lot of footguns in the configuration where the select statements were not really foolproof. With this PR we select only those sources that need to be for the declared configurations. TODO: - [ ] How do we continue supporting the `freethreaded` setups is a little bit unclear to me as the suggested design is "have a single source per target platform" and we need to setup freethreaded platform here. Work towards #2747 Work towards #2759 Work towards #2849 fixup the unit tests spike: use platforms with python versino use pep508 env for selecting the wheels move abi construction to the extension make want_abis configurable
1 parent 2f64607 commit 1d55c50

File tree

9 files changed

+557
-462
lines changed

9 files changed

+557
-462
lines changed

MODULE.bazel

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ pip = use_extension("//python/extensions:pip.bzl", "pip")
6363
pip.parse(
6464
# NOTE @aignas 2024-10-26: We have an integration test that depends on us
6565
# being able to build sdists for this hub, so explicitly set this to False.
66+
#
67+
# how do we test sdists? Maybe just worth adding a single sdist somewhere?
6668
download_only = False,
6769
experimental_index_url = "https://pypi.org/simple",
6870
hub_name = "rules_python_publish_deps",
@@ -155,22 +157,19 @@ dev_pip = use_extension(
155157
dev_dependency = True,
156158
)
157159
dev_pip.parse(
158-
download_only = True,
159160
experimental_index_url = "https://pypi.org/simple",
160161
hub_name = "dev_pip",
161162
parallel_download = False,
162163
python_version = "3.11",
163164
requirements_lock = "//docs:requirements.txt",
164165
)
165166
dev_pip.parse(
166-
download_only = True,
167167
experimental_index_url = "https://pypi.org/simple",
168168
hub_name = "dev_pip",
169169
python_version = "3.13",
170170
requirements_lock = "//docs:requirements.txt",
171171
)
172172
dev_pip.parse(
173-
download_only = True,
174173
experimental_index_url = "https://pypi.org/simple",
175174
hub_name = "pypiserver",
176175
python_version = "3.11",

python/private/pypi/BUILD.bazel

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,5 +422,8 @@ bzl_library(
422422
bzl_library(
423423
name = "whl_target_platforms_bzl",
424424
srcs = ["whl_target_platforms.bzl"],
425-
deps = [":parse_whl_name_bzl"],
425+
deps = [
426+
":parse_whl_name_bzl",
427+
"//python/private:version_bzl",
428+
],
426429
)

python/private/pypi/evaluate_markers.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def evaluate_markers(*, requirements, platforms):
4545
for platform_str in platform_strings:
4646
env = platforms.get(platform_str)
4747
if not env:
48-
fail("Please define platform: '{}'".format(platform_str))
48+
continue
4949

5050
if evaluate(req.marker, env = env):
5151
ret.setdefault(req_string, []).append(platform_str)

python/private/pypi/extension.bzl

Lines changed: 118 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,25 @@ def _platforms(*, python_version, minor_mapping, config):
7272
version = python_version,
7373
minor_mapping = minor_mapping,
7474
)
75-
abi = "cp3{}".format(python_version[2:])
7675

7776
for platform, values in config.platforms.items():
77+
implementation = values.env["implementation_name"][:2].lower()
78+
abi = "{}3{}".format(implementation, python_version[2:])
7879
key = "{}_{}".format(abi, platform)
79-
platforms[key] = env(struct(
80+
81+
env_ = env(struct(
8082
abi = abi,
8183
os = values.os_name,
8284
arch = values.arch_name,
8385
)) | values.env
86+
platforms[key] = struct(
87+
env = env_,
88+
want_abis = [
89+
v.format(*python_version.split("."))
90+
for v in values.want_abis
91+
],
92+
platform_tags = values.platform_tags,
93+
)
8494
return platforms
8595

8696
def _create_whl_repos(
@@ -152,6 +162,8 @@ def _create_whl_repos(
152162
))
153163
python_interpreter_target = available_interpreters[python_name]
154164

165+
# TODO @aignas 2025-06-29: we should not need the version in the pip_name if
166+
# we are using pipstar and we are downloading the wheel using the downloader
155167
pip_name = "{}_{}".format(
156168
hub_name,
157169
version_label(pip_attr.python_version),
@@ -184,11 +196,15 @@ def _create_whl_repos(
184196
elif config.enable_pipstar:
185197
evaluate_markers = lambda _, requirements: evaluate_markers_star(
186198
requirements = requirements,
187-
platforms = _platforms(
188-
python_version = pip_attr.python_version,
189-
minor_mapping = minor_mapping,
190-
config = config,
191-
),
199+
platforms = {
200+
k: p.env
201+
# TODO @aignas 2025-07-05: update evaluate_markers_star
202+
for k, p in _platforms(
203+
python_version = pip_attr.python_version,
204+
minor_mapping = minor_mapping,
205+
config = config,
206+
).items()
207+
},
192208
)
193209
else:
194210
# NOTE @aignas 2024-08-02: , we will execute any interpreter that we find either
@@ -230,6 +246,11 @@ def _create_whl_repos(
230246
),
231247
logger = logger,
232248
),
249+
platforms = _platforms(
250+
python_version = pip_attr.python_version,
251+
minor_mapping = minor_mapping,
252+
config = config,
253+
),
233254
extra_pip_args = pip_attr.extra_pip_args,
234255
get_index_urls = get_index_urls,
235256
evaluate_markers = evaluate_markers,
@@ -359,24 +380,16 @@ def _whl_repo(*, src, whl_library_args, is_multiple_versions, download_only, net
359380
for p in src.target_platforms
360381
]
361382

362-
# Pure python wheels or sdists may need to have a platform here
363-
target_platforms = None
364-
if is_whl and not src.filename.endswith("-any.whl"):
365-
pass
366-
elif is_multiple_versions:
367-
target_platforms = src.target_platforms
368-
369383
return struct(
370384
repo_name = whl_repo_name(src.filename, src.sha256),
371385
args = args,
372386
config_setting = whl_config_setting(
373387
version = python_version,
374-
filename = src.filename,
375-
target_platforms = target_platforms,
388+
target_platforms = src.target_platforms,
376389
),
377390
)
378391

379-
def _configure(config, *, platform, os_name, arch_name, config_settings, env = {}, override = False):
392+
def _configure(config, *, platform, os_name, arch_name, config_settings, env = {}, want_abis, platform_tags, override = False):
380393
"""Set the value in the config if the value is provided"""
381394
config.setdefault("platforms", {})
382395
if platform:
@@ -393,12 +406,25 @@ def _configure(config, *, platform, os_name, arch_name, config_settings, env = {
393406
if not arch_name:
394407
fail("'arch_name' is required")
395408

409+
if platform_tags and "any" not in platform_tags:
410+
# the lowest priority one needs to be the first one
411+
platform_tags = ["any"] + platform_tags
412+
396413
config["platforms"][platform] = struct(
397414
name = platform.replace("-", "_").lower(),
398415
os_name = os_name,
399416
arch_name = arch_name,
400417
config_settings = config_settings,
401-
env = env,
418+
want_abis = want_abis or [
419+
"cp{0}{1}",
420+
"abi3",
421+
"none",
422+
],
423+
platform_tags = platform_tags,
424+
env = {
425+
# default to this
426+
"implementation_name": "cpython",
427+
} | env,
402428
)
403429
else:
404430
config["platforms"].pop(platform)
@@ -424,28 +450,44 @@ def _create_config(defaults):
424450
arch_name = cpu,
425451
os_name = "linux",
426452
platform = "linux_{}".format(cpu),
453+
# TODO @aignas 2025-07-05: handle the freethreaded by having separate
454+
# platforms
455+
want_abis = [],
427456
config_settings = [
428457
"@platforms//os:linux",
429458
"@platforms//cpu:{}".format(cpu),
430459
],
431-
env = {"platform_version": "0"},
460+
platform_tags = [
461+
"linux_*_{}".format(cpu),
462+
"manylinux_*_{}".format(cpu),
463+
],
464+
env = {
465+
"platform_version": "0",
466+
},
432467
)
433-
for cpu in [
434-
"aarch64",
435-
"x86_64",
436-
]:
468+
for cpu, platform_tag_cpus in {
469+
"aarch64": ["universal2", "arm64"],
470+
"x86_64": ["universal2", "x86_64"],
471+
}.items():
437472
_configure(
438473
defaults,
439474
arch_name = cpu,
440-
# We choose the oldest non-EOL version at the time when we release `rules_python`.
441-
# See https://endoflife.date/macos
442475
os_name = "osx",
443476
platform = "osx_{}".format(cpu),
444477
config_settings = [
445478
"@platforms//os:osx",
446479
"@platforms//cpu:{}".format(cpu),
447480
],
448-
env = {"platform_version": "14.0"},
481+
want_abis = [],
482+
platform_tags = [
483+
"macosx_*_{}".format(suffix)
484+
for suffix in platform_tag_cpus
485+
],
486+
# We choose the oldest non-EOL version at the time when we release `rules_python`.
487+
# See https://endoflife.date/macos
488+
env = {
489+
"platform_version": "14.0",
490+
},
449491
)
450492

451493
_configure(
@@ -457,7 +499,11 @@ def _create_config(defaults):
457499
"@platforms//os:windows",
458500
"@platforms//cpu:x86_64",
459501
],
460-
env = {"platform_version": "0"},
502+
want_abis = [],
503+
platform_tags = ["win_amd64"],
504+
env = {
505+
"platform_version": "0",
506+
},
461507
)
462508
return struct(**defaults)
463509

@@ -527,14 +573,15 @@ You cannot use both the additive_build_content and additive_build_content_file a
527573
env = tag.env,
528574
os_name = tag.os_name,
529575
platform = tag.platform,
576+
platform_tags = tag.platform_tags,
577+
want_abis = tag.want_abis,
530578
override = mod.is_root,
531579
# TODO @aignas 2025-05-19: add more attr groups:
532580
# * for AUTH - the default `netrc` usage could be configured through a common
533581
# attribute.
534582
# * for index/downloader config. This includes all of those attributes for
535583
# overrides, etc. Index overrides per platform could be also used here.
536-
# * for whl selection - selecting preferences of which `platform_tag`s we should use
537-
# for what. We could also model the `cp313t` freethreaded as separate platforms.
584+
# * for whl selection - We could also model the `cp313t` freethreaded as separate platforms.
538585
)
539586

540587
config = _create_config(defaults)
@@ -666,7 +713,14 @@ You cannot use both the additive_build_content and additive_build_content_file a
666713
for whl_name, aliases in out.extra_aliases.items():
667714
extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
668715
exposed_packages.setdefault(hub_name, {}).update(out.exposed_packages)
669-
whl_libraries.update(out.whl_libraries)
716+
for whl_name, lib in out.whl_libraries.items():
717+
if enable_pipstar:
718+
whl_libraries.setdefault(whl_name, lib)
719+
elif whl_name in lib:
720+
fail("'{}' already in created".format(whl_name))
721+
else:
722+
# replicate whl_libraries.update(out.whl_libraries)
723+
whl_libraries[whl_name] = lib
670724

671725
# TODO @aignas 2024-04-05: how do we support different requirement
672726
# cycles for different abis/oses? For now we will need the users to
@@ -829,25 +883,6 @@ The list of labels to `config_setting` targets that need to be matched for the p
829883
selected.
830884
""",
831885
),
832-
"os_name": attr.string(
833-
doc = """\
834-
The OS name to be used.
835-
836-
:::{note}
837-
Either this or the appropriate `env` keys should be specified.
838-
:::
839-
""",
840-
),
841-
"platform": attr.string(
842-
doc = """\
843-
A platform identifier which will be used as the unique identifier within the extension evaluation.
844-
If you are defining custom platforms in your project and don't want things to clash, use extension
845-
[isolation] feature.
846-
847-
[isolation]: https://bazel.build/rules/lib/globals/module#use_extension.isolate
848-
""",
849-
),
850-
} | {
851886
"env": attr.string_dict(
852887
doc = """\
853888
The values to use for environment markers when evaluating an expression.
@@ -873,6 +908,40 @@ This is only used if the {envvar}`RULES_PYTHON_ENABLE_PIPSTAR` is enabled.
873908
""",
874909
),
875910
# The values for PEP508 env marker evaluation during the lock file parsing
911+
"os_name": attr.string(
912+
doc = """\
913+
The OS name to be used.
914+
915+
:::{note}
916+
Either this or the appropriate `env` keys should be specified.
917+
:::
918+
""",
919+
),
920+
"platform": attr.string(
921+
doc = """\
922+
A platform identifier which will be used as the unique identifier within the extension evaluation.
923+
If you are defining custom platforms in your project and don't want things to clash, use extension
924+
[isolation] feature.
925+
926+
[isolation]: https://bazel.build/rules/lib/globals/module#use_extension.isolate
927+
""",
928+
),
929+
"platform_tags": attr.string_list(
930+
doc = """\
931+
A list of `platform_tag` matchers so that we can select the best wheel based on the user
932+
preference. Per platform we will select a single wheel and the last match from this list will
933+
take precedence.
934+
935+
The items in this list can contain a single `*` character that is equivalent to `.*` regex match.
936+
""",
937+
),
938+
"want_abis": attr.string_list(
939+
doc = """\
940+
A list of ABIs to select wheels for. The values can be either strings or include template
941+
parameters like `{0}` which will be replaced with python version parts. e.g. `cp{0}{1}` will
942+
result in `cp313` given the full python version is `3.13.5`.
943+
""",
944+
),
876945
}
877946

878947
_SUPPORTED_PEP508_KEYS = [

0 commit comments

Comments
 (0)