From 19ecb6f2a0d20a57afb7df676369c4648ff73f82 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 15 Jun 2025 23:25:02 +0900 Subject: [PATCH] feat: support arbitrary config_setting labels in our platforms 3/n With this PR we can support arbitrary target settings instead of just plain `constraint_values`. We still have custom logic to ensure that all of the tests pass. However, the plan is to remove those tests once we have simplified the wheel selection mechanisms and the `pkg_aliases` macro. I.e. if we have at most 1 wheel per platform that the `pypi` bzlmod extension passes to the `pkg_aliases` macro, then we can just have a simple `selects.with_or` where we list out all of the target platform values. This PR may result in us creating more targets but that is the price that we have to pay if we want to do this incrementally. The last remaining thing for #2548 is to make sure that we are not parsing the user-friendly strings to get the `os` and the `cpu`. Work towards #2747 Work towards #2548 Work towards #260 --- CHANGELOG.md | 2 +- python/private/pypi/BUILD.bazel | 1 + python/private/pypi/config_settings.bzl | 41 ++++++++++++++++--- python/private/pypi/extension.bzl | 26 +++++++----- python/private/pypi/hub_repository.bzl | 4 +- python/private/pypi/render_pkg_aliases.bzl | 14 +++---- .../config_settings/config_settings_tests.bzl | 2 +- tests/pypi/extension/extension_tests.bzl | 8 ++-- tests/pypi/pkg_aliases/pkg_aliases_test.bzl | 23 +++++++---- .../render_pkg_aliases_test.bzl | 4 +- 10 files changed, 83 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da3dcc8efc..f2fa98f73f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,7 @@ END_UNRELEASED_TEMPLATE ### Added * (pypi) To configure the environment for `requirements.txt` evaluation, use the newly added developer preview of the `pip.default` tag class. Only `rules_python` and root modules can use - this feature. You can also configure `constraint_values` using `pip.default`. + this feature. You can also configure custom `config_settings` using `pip.default`. {#v0-0-0-removed} ### Removed diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index b569b2217c..2666197786 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -64,6 +64,7 @@ bzl_library( deps = [ ":flags_bzl", "//python/private:flags_bzl", + "@bazel_skylib//lib:selects", ], ) diff --git a/python/private/pypi/config_settings.bzl b/python/private/pypi/config_settings.bzl index 7edc578d7a..f4826007f8 100644 --- a/python/private/pypi/config_settings.bzl +++ b/python/private/pypi/config_settings.bzl @@ -70,6 +70,7 @@ suffix. ::: """ +load("@bazel_skylib//lib:selects.bzl", "selects") load("//python/private:flags.bzl", "LibcFlag") load(":flags.bzl", "INTERNAL_FLAGS", "UniversalWhlFlag") @@ -112,7 +113,7 @@ def config_settings( muslc_versions = [], osx_versions = [], name = None, - platform_constraint_values = {}, + platform_config_settings = {}, **kwargs): """Generate all of the pip config settings. @@ -126,7 +127,7 @@ def config_settings( configure config settings for. osx_versions (list[str]): The list of OSX OS versions to configure config settings for. - platform_constraint_values: {type}`dict[str, list[str]]` the constraint + platform_config_settings: {type}`dict[str, list[str]]` the constraint values to use instead of the default ones. Key are platform names (a human-friendly platform string). Values are lists of `constraint_value` label strings. @@ -142,13 +143,24 @@ def config_settings( # TODO @aignas 2025-06-15: allowing universal2 and platform specific wheels in one # closure is making things maybe a little bit too complicated. "osx_universal2": ["@platforms//os:osx"], - } | platform_constraint_values + } | platform_config_settings for python_version in python_versions: - for platform_name, constraint_values in target_platforms.items(): + for platform_name, config_settings in target_platforms.items(): suffix = "_{}".format(platform_name) if platform_name else "" os, _, cpu = platform_name.partition("_") + # We parse the target settings and if there is a "platforms//os" or + # "platforms//cpu" value in here, we also add it into the constraint_values + # + # this is to ensure that we can still pass all of the unit tests for config + # setting specialization. + constraint_values = [] + for setting in config_settings: + setting_label = Label(setting) + if setting_label.repo_name == "platforms" and setting_label.package in ["os", "cpu"]: + constraint_values.append(setting) + _dist_config_settings( suffix = suffix, plat_flag_values = _plat_flag_values( @@ -158,6 +170,7 @@ def config_settings( glibc_versions = glibc_versions, muslc_versions = muslc_versions, ), + config_settings = config_settings, constraint_values = constraint_values, python_version = python_version, **kwargs @@ -318,7 +331,7 @@ def _plat_flag_values(os, cpu, osx_versions, glibc_versions, muslc_versions): return ret -def _dist_config_setting(*, name, compatible_with = None, native = native, **kwargs): +def _dist_config_setting(*, name, compatible_with = None, selects = selects, native = native, config_settings = None, **kwargs): """A macro to create a target for matching Python binary and source distributions. Args: @@ -327,6 +340,12 @@ def _dist_config_setting(*, name, compatible_with = None, native = native, **kwa compatible with the given dist config setting. For example, if only non-freethreaded python builds are allowed, add FLAGS._is_py_freethreaded_no here. + config_settings: {type}`list[str | Label]` the list of target settings that must + be matched before we try to evaluate the config_setting that we may create in + this function. + selects (struct): The struct containing config_setting_group function + to use for creating config setting groups. Can be overridden for unit tests + reasons. native (struct): The struct containing alias and config_setting rules to use for creating the objects. Can be overridden for unit tests reasons. @@ -346,4 +365,14 @@ def _dist_config_setting(*, name, compatible_with = None, native = native, **kwa ) name = dist_config_setting_name - native.config_setting(name = name, **kwargs) + # first define the config setting that has all of the constraint values + _name = "_" + name + native.config_setting( + name = _name, + **kwargs + ) + selects.config_setting_group( + name = name, + match_all = config_settings + [_name], + visibility = kwargs.get("visibility"), + ) diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 78511b4c27..a0095f8f15 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -372,7 +372,7 @@ def _whl_repo(*, src, whl_library_args, is_multiple_versions, download_only, net ), ) -def _configure(config, *, platform, os_name, arch_name, constraint_values, env = {}, override = False): +def _configure(config, *, platform, os_name, arch_name, config_settings, env = {}, override = False): """Set the value in the config if the value is provided""" config.setdefault("platforms", {}) if platform: @@ -387,7 +387,7 @@ def _configure(config, *, platform, os_name, arch_name, constraint_values, env = name = platform.replace("-", "_").lower(), os_name = os_name, arch_name = arch_name, - constraint_values = constraint_values, + config_settings = config_settings, env = env, ) else: @@ -414,7 +414,7 @@ def _create_config(defaults): arch_name = cpu, os_name = "linux", platform = "linux_{}".format(cpu), - constraint_values = [ + config_settings = [ "@platforms//os:linux", "@platforms//cpu:{}".format(cpu), ], @@ -431,7 +431,7 @@ def _create_config(defaults): # See https://endoflife.date/macos os_name = "osx", platform = "osx_{}".format(cpu), - constraint_values = [ + config_settings = [ "@platforms//os:osx", "@platforms//cpu:{}".format(cpu), ], @@ -443,7 +443,7 @@ def _create_config(defaults): arch_name = "x86_64", os_name = "windows", platform = "windows_x86_64", - constraint_values = [ + config_settings = [ "@platforms//os:windows", "@platforms//cpu:x86_64", ], @@ -513,7 +513,7 @@ You cannot use both the additive_build_content and additive_build_content_file a _configure( defaults, arch_name = tag.arch_name, - constraint_values = tag.constraint_values, + config_settings = tag.config_settings, env = tag.env, os_name = tag.os_name, platform = tag.platform, @@ -693,9 +693,9 @@ You cannot use both the additive_build_content and additive_build_content_file a } for hub_name, extra_whl_aliases in extra_aliases.items() }, - platform_constraint_values = { + platform_config_settings = { hub_name: { - platform_name: sorted([str(Label(cv)) for cv in p.constraint_values]) + platform_name: sorted([str(Label(cv)) for cv in p.config_settings]) for platform_name, p in config.platforms.items() } for hub_name in hub_whl_map @@ -790,7 +790,7 @@ def _pip_impl(module_ctx): for key, values in whl_map.items() }, packages = mods.exposed_packages.get(hub_name, []), - platform_constraint_values = mods.platform_constraint_values.get(hub_name, {}), + platform_config_settings = mods.platform_config_settings.get(hub_name, {}), groups = mods.hub_group_map.get(hub_name), ) @@ -812,10 +812,11 @@ Either this or {attr}`env` `platform_machine` key should be specified. ::: """, ), - "constraint_values": attr.label_list( + "config_settings": attr.label_list( mandatory = True, doc = """\ -The constraint_values to use in select statements. +The list of labels to `config_setting` targets that need to be matched for the platform to be +selected. """, ), "os_name": attr.string( @@ -1145,6 +1146,9 @@ terms used in this extension. [environment_markers]: https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers ::: + +:::{versionadded} VERSION_NEXT_FEATURE +::: """, ), "override": _override_tag, diff --git a/python/private/pypi/hub_repository.bzl b/python/private/pypi/hub_repository.bzl index 4398d7b597..75f3ec98d7 100644 --- a/python/private/pypi/hub_repository.bzl +++ b/python/private/pypi/hub_repository.bzl @@ -34,7 +34,7 @@ def _impl(rctx): }, extra_hub_aliases = rctx.attr.extra_hub_aliases, requirement_cycles = rctx.attr.groups, - platform_constraint_values = rctx.attr.platform_constraint_values, + platform_config_settings = rctx.attr.platform_config_settings, ) for path, contents in aliases.items(): rctx.file(path, contents) @@ -84,7 +84,7 @@ hub_repository = repository_rule( The list of packages that will be exposed via all_*requirements macros. Defaults to whl_map keys. """, ), - "platform_constraint_values": attr.string_list_dict( + "platform_config_settings": attr.string_list_dict( doc = "The constraint values for each platform name. The values are string canonical string Label representations", mandatory = False, ), diff --git a/python/private/pypi/render_pkg_aliases.bzl b/python/private/pypi/render_pkg_aliases.bzl index 267d7ce85d..e743fc20f7 100644 --- a/python/private/pypi/render_pkg_aliases.bzl +++ b/python/private/pypi/render_pkg_aliases.bzl @@ -155,14 +155,14 @@ def _major_minor_versions(python_versions): # Use a dict as a simple set return sorted({_major_minor(v): None for v in python_versions}) -def render_multiplatform_pkg_aliases(*, aliases, platform_constraint_values = {}, **kwargs): +def render_multiplatform_pkg_aliases(*, aliases, platform_config_settings = {}, **kwargs): """Render the multi-platform pkg aliases. Args: aliases: dict[str, list(whl_config_setting)] A list of aliases that will be transformed from ones having `filename` to ones having `config_setting`. - platform_constraint_values: {type}`dict[str, list[str]]` contains all of the - target platforms and their appropriate `constraint_values`. + platform_config_settings: {type}`dict[str, list[str]]` contains all of the + target platforms and their appropriate `target_settings`. **kwargs: extra arguments passed to render_pkg_aliases. Returns: @@ -189,20 +189,20 @@ def render_multiplatform_pkg_aliases(*, aliases, platform_constraint_values = {} muslc_versions = flag_versions.get("muslc_versions", []), osx_versions = flag_versions.get("osx_versions", []), python_versions = _major_minor_versions(flag_versions.get("python_versions", [])), - platform_constraint_values = platform_constraint_values, + platform_config_settings = platform_config_settings, visibility = ["//:__subpackages__"], ) return contents -def _render_config_settings(platform_constraint_values, **kwargs): +def _render_config_settings(platform_config_settings, **kwargs): return """\ load("@rules_python//python/private/pypi:config_settings.bzl", "config_settings") {}""".format(render.call( "config_settings", name = repr("config_settings"), - platform_constraint_values = render.dict( - platform_constraint_values, + platform_config_settings = render.dict( + platform_config_settings, value_repr = render.list, ), **_repr_dict(value_repr = render.list, **kwargs) diff --git a/tests/pypi/config_settings/config_settings_tests.bzl b/tests/pypi/config_settings/config_settings_tests.bzl index 9551d42d10..a15f6b4d32 100644 --- a/tests/pypi/config_settings/config_settings_tests.bzl +++ b/tests/pypi/config_settings/config_settings_tests.bzl @@ -657,7 +657,7 @@ def config_settings_test_suite(name): # buildifier: disable=function-docstring glibc_versions = [(2, 14), (2, 17)], muslc_versions = [(1, 1)], osx_versions = [(10, 9), (11, 0)], - platform_constraint_values = { + platform_config_settings = { "linux_aarch64": [ "@platforms//cpu:aarch64", "@platforms//os:linux", diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl index 231e8cab41..146293ee8d 100644 --- a/tests/pypi/extension/extension_tests.bzl +++ b/tests/pypi/extension/extension_tests.bzl @@ -78,19 +78,17 @@ def _parse_modules(env, enable_pipstar = 0, **kwargs): def _default( arch_name = None, - constraint_values = None, + config_settings = None, os_name = None, platform = None, - target_settings = None, env = None, whl_limit = None, whl_platforms = None): return struct( arch_name = arch_name, - constraint_values = constraint_values, os_name = os_name, platform = platform, - target_settings = target_settings, + config_settings = config_settings, env = env or {}, whl_platforms = whl_platforms, whl_limit = whl_limit, @@ -1051,7 +1049,7 @@ def _test_pipstar_platforms(env): default = [ _default( platform = "{}_{}".format(os, cpu), - constraint_values = [ + config_settings = [ "@platforms//os:{}".format(os), "@platforms//cpu:{}".format(cpu), ], diff --git a/tests/pypi/pkg_aliases/pkg_aliases_test.bzl b/tests/pypi/pkg_aliases/pkg_aliases_test.bzl index 0fbcd4e7a6..123ee725f8 100644 --- a/tests/pypi/pkg_aliases/pkg_aliases_test.bzl +++ b/tests/pypi/pkg_aliases/pkg_aliases_test.bzl @@ -392,6 +392,9 @@ _tests.append(_test_multiplatform_whl_aliases_filename_versioned) def _mock_alias(container): return lambda name, **kwargs: container.append(name) +def _mock_config_setting_group(container): + return lambda name, **kwargs: container.append(name) + def _mock_config_setting(container): def _inner(name, flag_values = None, constraint_values = None, **_): if flag_values or constraint_values: @@ -417,9 +420,12 @@ def _test_config_settings_exist_legacy(env): python_versions = ["3.11"], native = struct( alias = _mock_alias(available_config_settings), - config_setting = _mock_config_setting(available_config_settings), + config_setting = _mock_config_setting([]), ), - platform_constraint_values = { + selects = struct( + config_setting_group = _mock_config_setting_group(available_config_settings), + ), + platform_config_settings = { "linux_aarch64": [ "@platforms//cpu:aarch64", "@platforms//os:linux", @@ -454,7 +460,7 @@ def _test_config_settings_exist(env): "any": {}, "macosx_11_0_arm64": { "osx_versions": [(11, 0)], - "platform_constraint_values": { + "platform_config_settings": { "osx_aarch64": [ "@platforms//cpu:aarch64", "@platforms//os:osx", @@ -463,7 +469,7 @@ def _test_config_settings_exist(env): }, "manylinux_2_17_x86_64": { "glibc_versions": [(2, 17), (2, 18)], - "platform_constraint_values": { + "platform_config_settings": { "linux_x86_64": [ "@platforms//cpu:x86_64", "@platforms//os:linux", @@ -472,7 +478,7 @@ def _test_config_settings_exist(env): }, "manylinux_2_18_x86_64": { "glibc_versions": [(2, 17), (2, 18)], - "platform_constraint_values": { + "platform_config_settings": { "linux_x86_64": [ "@platforms//cpu:x86_64", "@platforms//os:linux", @@ -481,7 +487,7 @@ def _test_config_settings_exist(env): }, "musllinux_1_1_aarch64": { "muslc_versions": [(1, 2), (1, 1), (1, 0)], - "platform_constraint_values": { + "platform_config_settings": { "linux_aarch64": [ "@platforms//cpu:aarch64", "@platforms//os:linux", @@ -500,7 +506,10 @@ def _test_config_settings_exist(env): python_versions = ["3.11"], native = struct( alias = _mock_alias(available_config_settings), - config_setting = _mock_config_setting(available_config_settings), + config_setting = _mock_config_setting([]), + ), + selects = struct( + config_setting_group = _mock_config_setting_group(available_config_settings), ), **kwargs ) diff --git a/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl b/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl index c262ed6823..ad7f36aed6 100644 --- a/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl +++ b/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl @@ -93,7 +93,7 @@ def _test_bzlmod_aliases(env): }, }, extra_hub_aliases = {"bar_baz": ["foo"]}, - platform_constraint_values = { + platform_config_settings = { "linux_x86_64": [ "@platforms//os:linux", "@platforms//cpu:x86_64", @@ -136,7 +136,7 @@ load("@rules_python//python/private/pypi:config_settings.bzl", "config_settings" config_settings( name = "config_settings", - platform_constraint_values = { + platform_config_settings = { "linux_x86_64": [ "@platforms//os:linux", "@platforms//cpu:x86_64",