refactor(toolchains): share Python version config settings

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,

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,
     )
 

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,29 +215,55 @@ def python_toolchain_build_file_content(
             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(