refactor(toolchains): share Python version config settings (#4159) Generated toolchain BUILD files repeat Python version `config_setting` predicates for each suite. `toolchain_suites_content` now emits each distinct predicate once and passes its label to `py_toolchain_suite`, preserving version matching while reducing retained configuration data. In our repo, this removes 79MB of retained heap. Validation: `buildifier -mode=check` on the three changed files and `git diff --check` passed. `bazel test --config=fast-tests --jobs=4 //tests/toolchains/... //tests/config_settings/...` passed (36 passed, 1 skipped).
diff --git a/python/private/py_toolchain_suite.bzl b/python/private/py_toolchain_suite.bzl index fa73d5d..8958614 100644 --- a/python/private/py_toolchain_suite.bzl +++ b/python/private/py_toolchain_suite.bzl
@@ -32,8 +32,7 @@ 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 @@ 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 c64abf9..cd699e7 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 @@ 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 @@ )) 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 1338e9c..359fc51 100644 --- a/python/private/toolchains_repo.bzl +++ b/python/private/toolchains_repo.bzl
@@ -35,11 +35,10 @@ _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 @@ 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,29 +215,55 @@ set_python_version_constraint = set_python_version_constraint, target_settings = meta.target_settings, )) - return "\n\n".join(entries) + return toolchain_suites_content(suites) -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 toolchain_suites_content(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"]: + 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 _toolchains_repo_impl(rctx): build_content = _WORKSPACE_TOOLCHAINS_BUILD_TEMPLATE.format(