From 301183ab83574a3b645e39ef2a465da2420dd877 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Thu, 10 Sep 2026 10:19:18 -0400 Subject: [PATCH 1/2] refactor(toolchains): share Python version config settings Generate each distinct Python version and flag predicate once per toolchain repository and reuse it across py_toolchain_suite calls. In our repo, this removes 79MB of retained heap. --- python/private/py_toolchain_suite.bzl | 61 +++++------------------ python/private/pythons_hub.bzl | 6 +-- python/private/toolchains_repo.bzl | 70 +++++++++++++++++---------- 3 files changed, 59 insertions(+), 78 deletions(-) diff --git a/python/private/py_toolchain_suite.bzl b/python/private/py_toolchain_suite.bzl index fa73d5daa3..8958614295 100644 --- a/python/private/py_toolchain_suite.bzl +++ b/python/private/py_toolchain_suite.bzl @@ -32,8 +32,7 @@ def py_toolchain_suite( prefix, user_repository_name, python_version, - set_python_version_constraint, - flag_values, + version_settings, target_settings = [], target_compatible_with = []): """For internal use only. @@ -44,57 +43,21 @@ def py_toolchain_suite( implementation (it's assumed to have particular target names within it). Does not include the leading "@". python_version: The full (X.Y.Z) version of the interpreter. - set_python_version_constraint: True or False as a string. - flag_values: Extra flag values to match for this toolchain. These - are prepended to target_settings. + version_settings: Labels of the shared Python version predicates. target_settings: Extra target_settings to match for this toolchain. target_compatible_with: list constraints the toolchains are compatible with. """ - # We have to use a String value here because bzlmod is passing in a - # string as we cannot have list of bools in build rule attributes. - # This if statement does not appear to work unless it is in the - # toolchain file. - if set_python_version_constraint in ["True", "False"]: - major_minor, _, _ = python_version.rpartition(".") - python_versions = [major_minor, python_version] - if set_python_version_constraint == "False": - python_versions.append("") - - match_any = [] - for i, v in enumerate(python_versions): - name = "{prefix}_{python_version}_{i}".format( - prefix = prefix, - python_version = python_version, - i = i, - ) - match_any.append(name) - native.config_setting( - name = name, - flag_values = flag_values | { - Label("@rules_python//python/config_settings:python_version"): v, - }, - visibility = ["//visibility:private"], - ) - - name = "{prefix}_version_setting_{python_version}".format( - prefix = prefix, - python_version = python_version, - visibility = ["//visibility:private"], - ) - selects.config_setting_group( - name = name, - match_any = match_any, - visibility = ["//visibility:private"], - ) - target_settings = [name] + target_settings - else: - fail(("Invalid set_python_version_constraint value: got {} {}, wanted " + - "either the string 'True' or the string 'False'; " + - "(did you convert bool to string?)").format( - type(set_python_version_constraint), - repr(set_python_version_constraint), - )) + name = "{prefix}_version_setting_{python_version}".format( + prefix = prefix, + python_version = python_version, + ) + selects.config_setting_group( + name = name, + match_any = version_settings, + visibility = ["//visibility:private"], + ) + target_settings = [name] + target_settings _internal_toolchain_suite( prefix = prefix, diff --git a/python/private/pythons_hub.bzl b/python/private/pythons_hub.bzl index c64abf9887..cd699e7851 100644 --- a/python/private/pythons_hub.bzl +++ b/python/private/pythons_hub.bzl @@ -17,7 +17,7 @@ load("//python:versions.bzl", "PLATFORMS") load(":pbs_manifest.bzl", "parse_runtime_manifest") load(":text_util.bzl", "render") -load(":toolchains_repo.bzl", "toolchain_suite_content") +load(":toolchains_repo.bzl", "toolchain_suites_content") def _have_same_length(*lists): if not lists: @@ -69,7 +69,7 @@ def _hub_build_file_content(rctx): else: flag_values = {} - toolchains.append(toolchain_suite_content( + toolchains.append(struct( prefix = "_{}_{}".format(render.left_pad_zero(i, pad_length), base_name), user_repository_name = rctx.attr.toolchain_repo_names[key], target_compatible_with = rctx.attr.toolchain_target_compatible_with_map[key], @@ -80,7 +80,7 @@ def _hub_build_file_content(rctx): )) return _HUB_BUILD_FILE_TEMPLATE.format( - toolchains = "\n".join(toolchains), + toolchains = toolchain_suites_content(toolchains), rules_python = rctx.attr._rules_python_workspace.repo_name, ) diff --git a/python/private/toolchains_repo.bzl b/python/private/toolchains_repo.bzl index 1338e9c569..b9b9f9b51f 100644 --- a/python/private/toolchains_repo.bzl +++ b/python/private/toolchains_repo.bzl @@ -35,11 +35,10 @@ load(":text_util.bzl", "render") _SUITE_TEMPLATE = """ py_toolchain_suite( - flag_values = {flag_values}, + version_settings = {version_settings}, target_settings = {target_settings}, prefix = {prefix}, python_version = {python_version}, - set_python_version_constraint = {set_python_version_constraint}, target_compatible_with = {target_compatible_with}, user_repository_name = {user_repository_name}, ) @@ -205,9 +204,9 @@ def python_toolchain_build_file_content( build_content: Text containing toolchain definitions """ - entries = [] + suites = [] for platform, meta in loaded_platforms.items(): - entries.append(toolchain_suite_content( + suites.append(struct( target_compatible_with = meta.compatible_with, flag_values = meta.flag_values, prefix = "{}{}".format(prefix, platform), @@ -216,30 +215,49 @@ def python_toolchain_build_file_content( set_python_version_constraint = set_python_version_constraint, target_settings = meta.target_settings, )) + return toolchain_suites_content(suites) + +def toolchain_suites_content(suites): + """Render shared version predicates followed by their toolchain suites.""" + suite_settings = [] + for suite in suites: + if suite.set_python_version_constraint not in ["True", "False"]: + fail("set_python_version_constraint must be the string 'True' or 'False'") + major_minor, _, _ = suite.python_version.rpartition(".") + versions = [major_minor, suite.python_version] + if suite.set_python_version_constraint == "False": + versions.append("") + values_list = [] + for version in versions: + values = suite.flag_values | { + Label("//python/config_settings:python_version"): version, + } + values_list.append(tuple(sorted([(str(label), value) for label, value in values.items()]))) + suite_settings.append(values_list) + + # Assign names before rendering either the predicates or their consumers. + settings = {values: None for values_list in suite_settings for values in values_list} + settings = {values: "_python_version_{}".format(i) for i, values in enumerate(settings)} + entries = [ + render.call( + "config_setting", + name = render.str(name), + flag_values = render.dict(dict(values)), + visibility = render.list(["//visibility:private"]), + ) + for values, name in settings.items() + ] + for suite, values in zip(suites, suite_settings): + entries.append(_SUITE_TEMPLATE.format( + prefix = render.str(suite.prefix), + user_repository_name = render.str(suite.user_repository_name), + target_compatible_with = render.indent(render.list(suite.target_compatible_with)).lstrip(), + version_settings = render.list([settings[value] for value in values], hanging_indent = " "), + target_settings = render.list(suite.target_settings, hanging_indent = " "), + python_version = render.str(suite.python_version), + )) return "\n\n".join(entries) -def toolchain_suite_content( - *, - flag_values, - prefix, - python_version, - set_python_version_constraint, - target_compatible_with, - target_settings, - user_repository_name): - return _SUITE_TEMPLATE.format( - prefix = render.str(prefix), - user_repository_name = render.str(user_repository_name), - target_compatible_with = render.indent(render.list(target_compatible_with)).lstrip(), - flag_values = render.indent(render.dict( - flag_values, - key_repr = lambda x: repr(str(x)), # this is to correctly display labels - )).lstrip(), - target_settings = render.list(target_settings, hanging_indent = " "), - set_python_version_constraint = render.str(set_python_version_constraint), - python_version = render.str(python_version), - ) - def _toolchains_repo_impl(rctx): build_content = _WORKSPACE_TOOLCHAINS_BUILD_TEMPLATE.format( rules_python = rctx.attr._rules_python_workspace.repo_name, From 7e46001c18b54a4f347c5b6b6e4a2f904810e949 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Thu, 10 Sep 2026 10:28:34 -0400 Subject: [PATCH 2/2] docs(toolchains): document shared version setting renderer --- python/private/toolchains_repo.bzl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/python/private/toolchains_repo.bzl b/python/private/toolchains_repo.bzl index b9b9f9b51f..359fc51feb 100644 --- a/python/private/toolchains_repo.bzl +++ b/python/private/toolchains_repo.bzl @@ -218,7 +218,14 @@ def python_toolchain_build_file_content( return toolchain_suites_content(suites) def toolchain_suites_content(suites): - """Render shared version predicates followed by their toolchain suites.""" + """Render shared version predicates followed by their toolchain suites. + + Args: + suites: Toolchain suite data with version and platform settings. + + Returns: + BUILD file content with shared config_setting predicates and suites. + """ suite_settings = [] for suite in suites: if suite.set_python_version_constraint not in ["True", "False"]: