Skip to content
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

feat(toolchain): support freethreaded toolchains #2372

Merged
merged 27 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c53ada9
chore: add freethreaded interpreters to 3.13 def
aignas Oct 14, 2024
b182b52
fixup! chore: add freethreaded interpreters to 3.13 def
aignas Nov 4, 2024
02e84f7
update the hermetic_runtime_repo_setup to support free-threaded inter…
aignas Nov 5, 2024
0b57e4d
finish adding the freethreaded toolchains
aignas Nov 5, 2024
330b9f2
fixup
aignas Nov 5, 2024
503acab
revert the inclusion of 3.13 toolchain
aignas Nov 5, 2024
fe35320
Pass around the list of loaded platforms for each toolchain
aignas Nov 5, 2024
0cee209
Merge branch 'main' into exp/freethreading
aignas Nov 5, 2024
aef8f47
finish bumping the toolchains
aignas Nov 5, 2024
e58413c
buildifier
aignas Nov 5, 2024
0d6366a
fixup changelog
aignas Nov 5, 2024
1c54116
add a merge-conflict pre-commit-hook
aignas Nov 5, 2024
472cc40
comment: use enum
aignas Nov 6, 2024
c319b2b
comment: use a constant
aignas Nov 6, 2024
5fbbf9e
comment: remove sorting
aignas Nov 6, 2024
2d6e11a
fix: use the coverage enabled config setting
aignas Nov 6, 2024
465111e
comment: loaded_platform strings
aignas Nov 7, 2024
0f9954e
comment: pass loaded_platform structs
aignas Nov 7, 2024
daaa4de
use a function to define platforms
aignas Nov 7, 2024
66a102a
move the URL generation to a function instead of list comprehension
aignas Nov 7, 2024
b53073a
simplify
aignas Nov 7, 2024
ffe70cb
comment: use a constant
aignas Nov 7, 2024
b39ff67
Merge branch 'main' into exp/freethreading
aignas Nov 7, 2024
a90dd54
add docs
aignas Nov 7, 2024
c2663ba
chore: add coverage deps
aignas Nov 7, 2024
8585991
Merge branch 'main' into exp/freethreading
aignas Nov 11, 2024
f9609e9
comment: improve docs and add suggestions from rickeylev
aignas Nov 11, 2024
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
4 changes: 4 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0 # Use the ref you want to point at
hooks:
- id: check-merge-conflict
- repo: https://github.com/keith/pre-commit-buildifier
rev: 6.1.0
hooks:
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ A brief description of the categories of changes:
[`pip.parse#extra_pip_args`](https://rules-python.readthedocs.io/en/latest/api/rules_python/python/extensions/pip.html#pip.parse.extra_pip_args)
* (pip.parse) {attr}`pip.parse.whl_modifications` now normalizes the given whl names
and now `pyyaml` and `PyYAML` will both work.
* (toolchains) Use the latest indygreg toolchain release [20241016] for Python versions:
* 3.9.20
* 3.10.15
* 3.11.10
* 3.12.7
* 3.13.0

[20241016]: https://github.com/indygreg/python-build-standalone/releases/tag/20241016

{#v0-0-0-fixed}
### Fixed
Expand All @@ -63,6 +71,7 @@ A brief description of the categories of changes:
* (pip.parse) {attr}`pip.parse.extra_hub_aliases` can now be used to expose extra
targets created by annotations in whl repositories.
Fixes [#2187](https://github.com/bazelbuild/rules_python/issues/2187).
* (toolchain) The support for freethreaded Python toolchains is now available.

{#v0-0-0-removed}
### Removed
Expand Down
18 changes: 18 additions & 0 deletions python/config_settings/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,24 @@ string_flag(
visibility = ["//visibility:public"],
)

string_flag(
name = "py_freethreaded",
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
build_setting_default = "no",
values = [
"yes",
"no",
],
aignas marked this conversation as resolved.
Show resolved Hide resolved
visibility = ["//visibility:public"],
)

config_setting(
name = "is_py_freethreaded",
flag_values = {
":py_freethreaded": "yes",
},
visibility = ["//visibility:public"],
)

# pip.parse related flags

string_flag(
Expand Down
83 changes: 72 additions & 11 deletions python/private/hermetic_runtime_repo_setup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ def define_hermetic_runtime_toolchain_impl(
python_version: {type}`str` The Python version, in `major.minor.micro`
format.
python_bin: {type}`str` The path to the Python binary within the
repositoroy.
repository.
coverage_tool: {type}`str` optional target to the coverage tool to
use.
"""
_ = name # @unused
is_freethreaded = Label("//python/config_settings:is_py_freethreaded")
aignas marked this conversation as resolved.
Show resolved Hide resolved
version_info = semver(python_version)
version_dict = version_info.to_dict()
native.filegroup(
Expand All @@ -65,21 +66,27 @@ def define_hermetic_runtime_toolchain_impl(
# Platform-agnostic filegroup can't match on all patterns.
allow_empty = True,
exclude = [
# Unused shared libraries. `python` executable and the `:libpython` target
aignas marked this conversation as resolved.
Show resolved Hide resolved
# depend on `libpython{python_version}.so.1.0`.
"lib/libpython{major}.{minor}.so".format(**version_dict),
# static libraries
"lib/**/*.a",
# During pyc creation, temp files named *.pyc.NNN are created
"**/__pycache__/*.pyc.*",
# Do exclusions for all known ABIs
# Unused shared libraries. `python` executable and the `:libpython` target
# depend on `libpython{python_version}.so.1.0`.
"lib/libpython{major}.{minor}*.so".format(**version_dict),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking/curious: I'm guessing the libX.Y* pattern works because the extracted standalone interpreters only contain appropriately relevant .so files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is for exclusion, that is why libX.Y*.so is just as good as libX.Y.so.

"lib/libpython{major}.{minor}*.so".format(**version_dict),
# tests for the standard libraries.
"lib/python{major}.{minor}/**/test/**".format(**version_dict),
"lib/python{major}.{minor}/**/tests/**".format(**version_dict),
"**/__pycache__/*.pyc.*", # During pyc creation, temp files named *.pyc.NNN are created
"lib/python{major}.{minor}*/**/test/**".format(**version_dict),
"lib/python{major}.{minor}*/**/tests/**".format(**version_dict),
] + glob_excludes.version_dependent_exclusions() + extra_files_glob_exclude,
),
)
cc_import(
name = "interface",
interface_library = "libs/python{major}{minor}.lib".format(**version_dict),
interface_library = select({
is_freethreaded: "libs/python{major}{minor}t.lib".format(**version_dict),
"//conditions:default": "libs/python{major}{minor}.lib".format(**version_dict),
}),
system_provided = True,
)

Expand All @@ -96,14 +103,65 @@ def define_hermetic_runtime_toolchain_impl(
hdrs = [":includes"],
includes = [
"include",
"include/python{major}.{minor}".format(**version_dict),
"include/python{major}.{minor}m".format(**version_dict),
] + select({
is_freethreaded: [
"include/python{major}.{minor}t".format(**version_dict),
# FIXME @aignas 2024-11-05: the following looks fishy - should
# we have a config setting for `m` (i.e. DEBUG builds)?
"include/python{major}.{minor}tm".format(**version_dict),
aignas marked this conversation as resolved.
Show resolved Hide resolved
],
"//conditions:default": [
"include/python{major}.{minor}".format(**version_dict),
"include/python{major}.{minor}m".format(**version_dict),
],
}),
)
native.config_setting(
name = "is_freethreaded_linux",
flag_values = {
Label("//python/config_settings:py_freethreaded"): "yes",
},
constraint_values = [
"@platforms//os:linux",
],
visibility = ["//visibility:private"],
)
native.config_setting(
name = "is_freethreaded_osx",
flag_values = {
Label("//python/config_settings:py_freethreaded"): "yes",
},
constraint_values = [
"@platforms//os:osx",
],
visibility = ["//visibility:private"],
)
native.config_setting(
name = "is_freethreaded_windows",
flag_values = {
Label("//python/config_settings:py_freethreaded"): "yes",
},
constraint_values = [
"@platforms//os:windows",
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
],
visibility = ["//visibility:private"],
)

cc_library(
name = "libpython",
hdrs = [":includes"],
srcs = select({
":is_freethreaded_linux": [
"lib/libpython{major}.{minor}t.so".format(**version_dict),
"lib/libpython{major}.{minor}t.so.1.0".format(**version_dict),
],
":is_freethreaded_osx": [
"lib/libpython{major}.{minor}t.dylib".format(**version_dict),
],
":is_freethreaded_windows": [
"python3.dll",
"libs/python{major}{minor}t.lib".format(**version_dict),
],
"@platforms//os:linux": [
"lib/libpython{major}.{minor}.so".format(**version_dict),
"lib/libpython{major}.{minor}.so.1.0".format(**version_dict),
Expand Down Expand Up @@ -137,7 +195,10 @@ def define_hermetic_runtime_toolchain_impl(
python_version = "PY3",
implementation_name = "cpython",
# See https://peps.python.org/pep-3147/ for pyc tag infix format
pyc_tag = "cpython-{major}{minor}".format(**version_dict),
pyc_tag = select({
is_freethreaded: "cpython-{major}{minor}t".format(**version_dict),
"//conditions:default": "cpython-{major}{minor}".format(**version_dict),
}),
)

py_runtime_pair(
Expand Down
6 changes: 4 additions & 2 deletions python/private/python.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ def parse_modules(*, module_ctx, _fail = fail):
def _python_impl(module_ctx):
py = parse_modules(module_ctx = module_ctx)

loaded_platforms = {}
for toolchain_info in py.toolchains:
# Ensure that we pass the full version here.
full_python_version = full_version(
Expand All @@ -228,7 +229,7 @@ def _python_impl(module_ctx):
kwargs.update(py.config.kwargs.get(toolchain_info.python_version, {}))
kwargs.update(py.config.kwargs.get(full_python_version, {}))
kwargs.update(py.config.default)
python_register_toolchains(
loaded_platforms[full_python_version] = python_register_toolchains(
name = toolchain_info.name,
_internal_bzlmod_toolchain_call = True,
**kwargs
Expand Down Expand Up @@ -257,6 +258,7 @@ def _python_impl(module_ctx):
for i in range(len(py.toolchains))
],
toolchain_user_repository_names = [t.name for t in py.toolchains],
loaded_platforms = loaded_platforms,
)

# This is require in order to support multiple version py_test
Expand Down Expand Up @@ -464,7 +466,7 @@ def _get_toolchain_config(*, modules, _fail = fail):
"url": {
platform: [item["url"]]
for platform in item["sha256"]
},
} if type(item["url"]) == type("") else item["url"],
}
for version, item in TOOL_VERSIONS.items()
}
Expand Down
7 changes: 6 additions & 1 deletion python/private/python_register_toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ def python_register_toolchains(
minor_mapping: {type}`dict[str, str]` contains a mapping from `X.Y` to `X.Y.Z`
version.
**kwargs: passed to each {obj}`python_repository` call.

Returns:
on bzlmod this returns the loaded platform labels.
aignas marked this conversation as resolved.
Show resolved Hide resolved
"""
bzlmod_toolchain_call = kwargs.pop("_internal_bzlmod_toolchain_call", False)
if bzlmod_toolchain_call:
Expand Down Expand Up @@ -168,11 +171,13 @@ def python_register_toolchains(

# in bzlmod we write out our own toolchain repos
if bzlmod_toolchain_call:
return
return loaded_platforms

toolchains_repo(
name = toolchain_repo_name,
python_version = python_version,
set_python_version_constraint = set_python_version_constraint,
user_repository_name = name,
platforms = loaded_platforms,
)
return None
6 changes: 5 additions & 1 deletion python/private/python_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@ def _python_repository_impl(rctx):
platform = rctx.attr.platform
python_version = rctx.attr.python_version
python_version_info = python_version.split(".")
python_short_version = "{0}.{1}".format(*python_version_info)
release_filename = rctx.attr.release_filename
version_suffix = "t" if "freethreaded" in release_filename else ""
python_short_version = "{0}.{1}{suffix}".format(
suffix = version_suffix,
*python_version_info
)
urls = rctx.attr.urls or [rctx.attr.url]
auth = get_auth(rctx, urls)

Expand Down
8 changes: 7 additions & 1 deletion python/private/pythons_hub.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ def _hub_build_file_content(
python_versions,
set_python_version_constraints,
user_repository_names,
workspace_location):
workspace_location,
loaded_platforms):
"""This macro iterates over each of the lists and returns the toolchain content.

python_toolchain_build_file_content is called to generate each of the toolchain
Expand All @@ -65,6 +66,7 @@ def _hub_build_file_content(
python_version = python_versions[i],
set_python_version_constraint = set_python_version_constraints[i],
user_repository_name = user_repository_names[i],
loaded_platforms = loaded_platforms[python_versions[i]],
)
for i in range(len(python_versions))
],
Expand Down Expand Up @@ -103,6 +105,7 @@ def _hub_repo_impl(rctx):
rctx.attr.toolchain_set_python_version_constraints,
rctx.attr.toolchain_user_repository_names,
rctx.attr._rules_python_workspace,
rctx.attr.loaded_platforms,
),
executable = False,
)
Expand Down Expand Up @@ -149,6 +152,9 @@ This rule also writes out the various toolchains for the different Python versio
doc = "Default Python version for the build in `X.Y` or `X.Y.Z` format.",
mandatory = True,
),
"loaded_platforms": attr.string_list_dict(
doc = "The mapping from platform to version",
aignas marked this conversation as resolved.
Show resolved Hide resolved
),
"minor_mapping": attr.string_dict(
doc = "The minor mapping of the `X.Y` to `X.Y.Z` format that is used in config settings.",
mandatory = True,
Expand Down
11 changes: 10 additions & 1 deletion python/private/toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ def python_toolchain_build_file_content(
prefix,
python_version,
set_python_version_constraint,
user_repository_name):
user_repository_name,
loaded_platforms):
"""Creates the content for toolchain definitions for a build file.

Args:
Expand All @@ -51,6 +52,8 @@ def python_toolchain_build_file_content(
have the Python version constraint added as a requirement for
matching the toolchain, "False" if not.
user_repository_name: names for the user repos
loaded_platforms: the list of platform identifiers for which to generate
the toolchain targets.

Returns:
build_content: Text containing toolchain definitions
Expand Down Expand Up @@ -78,6 +81,7 @@ py_toolchain_suite(
python_version = python_version,
)
for platform, meta in PLATFORMS.items()
if platform in loaded_platforms
aignas marked this conversation as resolved.
Show resolved Hide resolved
])

def _toolchains_repo_impl(rctx):
Expand All @@ -100,6 +104,7 @@ load("@{rules_python}//python/private:py_toolchain_suite.bzl", "py_toolchain_sui
python_version = rctx.attr.python_version,
set_python_version_constraint = str(rctx.attr.set_python_version_constraint),
user_repository_name = rctx.attr.user_repository_name,
loaded_platforms = rctx.attr.platforms,
)

rctx.file("BUILD.bazel", build_content + toolchains)
Expand All @@ -109,6 +114,7 @@ toolchains_repo = repository_rule(
doc = "Creates a repository with toolchain definitions for all known platforms " +
"which can be registered or selected.",
attrs = {
"platforms": attr.string_list(doc = "List of platforms for which the toolchain definitions shall be created"),
"python_version": attr.string(doc = "The Python version."),
"set_python_version_constraint": attr.bool(doc = "if target_compatible_with for the toolchain should set the version constraint"),
"user_repository_name": attr.string(doc = "what the user chose for the base name"),
Expand Down Expand Up @@ -390,6 +396,9 @@ def _get_host_platform(os_name, arch):
"""
host_platform = None
for platform, meta in PLATFORMS.items():
if "freethreaded" in platform:
aignas marked this conversation as resolved.
Show resolved Hide resolved
continue

if meta.os_name == os_name and meta.arch == arch:
host_platform = platform
if not host_platform:
Expand Down
Loading