Skip to content

refactor: explicitly define host platform ordering #2890

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
acf4136
refactor: make bzlmod directly aware of created toolchain repo names
rickeylev May 16, 2025
e9d5d86
refactor: make bzlmod create host repos
rickeylev May 16, 2025
47b115e
create host repo in bzlmod
rickeylev May 16, 2025
e36105c
refactor: make bzlmod pass platform mapping to host repo creation
rickeylev May 16, 2025
ce3027c
format toolchains_repo.bzl
rickeylev May 16, 2025
7f42fe7
refactor: explicitly define host platform ordering selection
rickeylev May 16, 2025
f88fc59
verions.bzl: sort dict keys
rickeylev May 16, 2025
d26897c
refactor: make bzlmod create host repos
rickeylev May 16, 2025
56b09f5
create host repo in bzlmod
rickeylev May 16, 2025
d7d434f
Merge branch 'refactor.bzlmod.creates.host.repos' into refactor.bzlmo…
rickeylev May 19, 2025
a715d46
rename to arch_names
rickeylev May 19, 2025
a4fc891
Merge branch 'main' of https://github.com/bazel-contrib/rules_python …
rickeylev May 19, 2025
103bebe
rename archs to arch_names
rickeylev May 19, 2025
2d8a38e
Merge branch 'main' of https://github.com/bazel-contrib/rules_python …
rickeylev May 19, 2025
9ac39b6
Merge branch 'refactor.bzlmod.pass.platforms.to.host.repo' into refac…
rickeylev May 19, 2025
1b659bc
add missing symbol; use updated attr name
rickeylev May 19, 2025
d23a77c
Merge branch 'main' of https://github.com/bazel-contrib/rules_python …
rickeylev May 19, 2025
72161e6
Revert "verions.bzl: sort dict keys"
rickeylev May 20, 2025
adc300e
consts with hyphen, sort that supports ft+musl
rickeylev May 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions python/private/python.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ load(":full_version.bzl", "full_version")
load(":python_register_toolchains.bzl", "python_register_toolchains")
load(":pythons_hub.bzl", "hub_repo")
load(":repo_utils.bzl", "repo_utils")
load(":toolchains_repo.bzl", "host_toolchain", "multi_toolchain_aliases")
load(":toolchains_repo.bzl", "host_toolchain", "multi_toolchain_aliases", "sorted_host_platforms")
load(":util.bzl", "IS_BAZEL_6_4_OR_HIGHER")
load(":version.bzl", "version")

Expand Down Expand Up @@ -298,9 +298,8 @@ def _python_impl(module_ctx):
_internal_bzlmod_toolchain_call = True,
**kwargs
)
host_platforms = []
host_os_names = {}
host_archs = {}

host_platforms = {}
for repo_name, (platform_name, platform_info) in register_result.impl_repos.items():
toolchain_impls.append(struct(
# str: The base name to use for the toolchain() target
Expand All @@ -319,17 +318,21 @@ def _python_impl(module_ctx):
set_python_version_constraint = is_last,
))
if _is_compatible_with_host(module_ctx, platform_info):
host_key = str(len(host_platforms))
host_platforms.append(platform_name)
host_os_names[host_key] = platform_info.os_name
host_archs[host_key] = platform_info.arch
host_platforms[platform_name] = platform_info

host_platforms = sorted_host_platforms(host_platforms)
host_toolchain(
name = toolchain_info.name + "_host",
# NOTE: Order matters. The first found to be compatible is (usually) used.
platforms = host_platforms,
os_names = host_os_names,
arch_names = host_archs,
platforms = host_platforms.keys(),
os_names = {
str(i): platform_info.os_name
for i, platform_info in enumerate(host_platforms.values())
},
arch_names = {
str(i): platform_info.arch
for i, platform_info in enumerate(host_platforms.values())
},
python_version = full_python_version,
)

Expand Down
42 changes: 41 additions & 1 deletion python/private/toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ platform-specific repositories.

load(
"//python:versions.bzl",
"FREETHREADED",
"MUSL",
"PLATFORMS",
"WINDOWS_NAME",
)
Expand Down Expand Up @@ -433,6 +435,44 @@ multi_toolchain_aliases = repository_rule(
},
)

def sorted_host_platforms(platform_map):
"""Sort the keys in the platform map to give correct precedence.

The order of keys in the platform mapping matters for the host toolchain
selection. When multiple runtimes are compatible with the host, we take the
first that is compatible (usually; there's also the
`RULES_PYTHON_REPO_TOOLCHAIN_*` environment variables). The historical
behavior carefully constructed the ordering of platform keys such that
the ordering was:
* Regular platforms
* The "-freethreaded" suffix
* The "-musl" suffix

Here, we formalize that so it isn't subtly encoded in the ordering of keys
in a dict that autoformatters like to clobber and whose only documentation
is an innocous looking formatter disable directive.

Args:
platform_map: a mapping of platforms and their metadata.

Returns:
dict; the same values, but with the keys inserted in the desired
order so that iteration happens in the desired order.
"""

def platform_keyer(name):
# Ascending sort: lower is higher precedence
return (
1 if MUSL in name else 0,
1 if FREETHREADED in name else 0,
)

sorted_platform_keys = sorted(platform_map.keys(), key = platform_keyer)
return {
key: platform_map[key]
for key in sorted_platform_keys
}

def _get_host_platform(*, rctx, logger, python_version, os_name, cpu_name, platforms):
"""Gets the host platform.

Expand All @@ -455,7 +495,7 @@ def _get_host_platform(*, rctx, logger, python_version, os_name, cpu_name, platf
arch = rctx.attr.arch_names[key],
)
else:
platform_map = PLATFORMS
platform_map = sorted_host_platforms(PLATFORMS)

candidates = []
for platform in platforms:
Expand Down
12 changes: 7 additions & 5 deletions python/versions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
MACOS_NAME = "osx"
LINUX_NAME = "linux"
WINDOWS_NAME = "windows"
FREETHREADED = "freethreaded"

FREETHREADED = "-freethreaded"
MUSL = "-musl"
INSTALL_ONLY = "install_only"

DEFAULT_RELEASE_BASE_URL = "https://github.com/astral-sh/python-build-standalone/releases/download"
Expand Down Expand Up @@ -845,7 +847,7 @@ def _generate_platforms():
for p, v in platforms.items()
for suffix, freethreadedness in {
"": is_freethreaded_no,
"-" + FREETHREADED: is_freethreaded_yes,
FREETHREADED: is_freethreaded_yes,
}.items()
}

Expand Down Expand Up @@ -879,11 +881,11 @@ def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_U
release_filename = None
rendered_urls = []
for u in url:
p, _, _ = platform.partition("-" + FREETHREADED)
p, _, _ = platform.partition(FREETHREADED)

if FREETHREADED in platform:
if FREETHREADED.lstrip("-") in platform:
build = "{}+{}-full".format(
FREETHREADED,
FREETHREADED.lstrip("-"),
{
"aarch64-apple-darwin": "pgo+lto",
"aarch64-unknown-linux-gnu": "lto",
Expand Down