refactor(bzlmod): generate fewer targets for pip_config_settings (#1974)

Remove the `python_version` from `flag_values` and reduces the number of
targets we create by a lot because we no longer need to create all of
the micro
version combinations to also match when we want to match e.g. 3.10 on a
particular platform by using a trick I've learned in #1968:
```starlark
select({
    "is_python_3.x": "my_config_setting"
    "//conditions:default": "is_python_3.x"
})
```

Then further optimization was done on how we create the aliases and
config
settings and optimizing the usage of `pip_whl` flag finally reduced the
internal number targets. The number config settings targets in
`//tests/private/pip_config_settings/...` where we have multiple python
versions has changed:

* `:is_*` - from **995** to **996** (the extra addition being
is_python_default).
* `:_.*is_*` - from **28272** to **2480** (just python version) and then
to **496** (final).

Work towards #260

Author: aignas

CHANGELOG.md

@@ -26,6 +26,8 @@ A brief description of the categories of changes:
 
 ### Changed
 * `protobuf`/`com_google_protobuf` dependency bumped to `v24.4`
+* (bzlmod): optimize the creation of config settings used in pip to
+  reduce the total number of targets in the hub repo.
 
 ### Fixed
 * (bzlmod): Targets in `all_requirements` now use the same form as targets returned by the `requirement` macro.

python/config_settings/BUILD.bazel

@@ -88,6 +88,33 @@ string_flag(
     visibility = ["//visibility:public"],
 )
 
+config_setting(
+    name = "is_pip_whl_auto",
+    flag_values = {
+        ":pip_whl": UseWhlFlag.AUTO,
+    },
+    # NOTE: Only public because it is used in pip hub repos.
+    visibility = ["//visibility:public"],
+)
+
+config_setting(
+    name = "is_pip_whl_no",
+    flag_values = {
+        ":pip_whl": UseWhlFlag.NO,
+    },
+    # NOTE: Only public because it is used in pip hub repos.
+    visibility = ["//visibility:public"],
+)
+
+config_setting(
+    name = "is_pip_whl_only",
+    flag_values = {
+        ":pip_whl": UseWhlFlag.ONLY,
+    },
+    # NOTE: Only public because it is used in pip hub repos.
+    visibility = ["//visibility:public"],
+)
+
 string_flag(
     name = "pip_whl_osx_arch",
     build_setting_default = UniversalWhlFlag.ARCH,

python/private/config_settings.bzl

@@ -181,6 +181,14 @@ def construct_config_settings(name = None):  # buildifier: disable=function-docs
         visibility = ["//visibility:public"],
     )
 
+    native.config_setting(
+        name = "is_python_version_unset",
+        flag_values = {
+            Label("//python/config_settings:python_version"): "",
+        },
+        visibility = ["//visibility:public"],
+    )
+
     for version, matching_versions in VERSION_FLAG_VALUES.items():
         is_python_config_setting(
             name = "is_python_{}".format(version),

python/private/pip_config_settings.bzl

@@ -39,12 +39,10 @@ Note, that here the specialization of musl vs manylinux wheels is the same in
 order to ensure that the matching fails if the user requests for `musl` and we don't have it or vice versa.
 """
 
-load(":config_settings.bzl", "is_python_config_setting")
 load(
     ":pip_flags.bzl",
     "INTERNAL_FLAGS",
     "UniversalWhlFlag",
-    "UseWhlFlag",
     "WhlLibcFlag",
 )
 
@@ -53,12 +51,14 @@ FLAGS = struct(
         f: str(Label("//python/config_settings:" + f))
         for f in [
             "python_version",
-            "pip_whl",
             "pip_whl_glibc_version",
             "pip_whl_muslc_version",
             "pip_whl_osx_arch",
             "pip_whl_osx_version",
             "py_linux_libc",
+            "is_pip_whl_no",
+            "is_pip_whl_only",
+            "is_pip_whl_auto",
         ]
     }
 )
@@ -82,8 +82,7 @@ def pip_config_settings(
         target_platforms = [],
         name = None,
         visibility = None,
-        alias_rule = None,
-        config_setting_rule = None):
+        native = native):
     """Generate all of the pip config settings.
 
     Args:
@@ -100,10 +99,9 @@ def pip_config_settings(
             constraint values for each condition.
         visibility (list[str], optional): The visibility to be passed to the
             exposed labels. All other labels will be private.
-        alias_rule (rule): The alias rule to use for creating the
-            objects. Can be overridden for unit tests reasons.
-        config_setting_rule (rule): The config setting rule to use for creating the
-            objects. 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.
     """
 
     glibc_versions = [""] + glibc_versions
@@ -114,43 +112,25 @@ def pip_config_settings(
         for t in target_platforms
     ]
 
-    alias_rule = alias_rule or native.alias
-
-    for version in python_versions:
-        is_python = "is_python_{}".format(version)
-        alias_rule(
+    for python_version in [""] + python_versions:
+        is_python = "is_python_{}".format(python_version or "version_unset")
+        native.alias(
             name = is_python,
             actual = Label("//python/config_settings:" + is_python),
             visibility = visibility,
         )
 
-    for os, cpu in target_platforms:
-        constraint_values = []
-        suffix = ""
-        if os:
-            constraint_values.append("@platforms//os:" + os)
-            suffix += "_" + os
-        if cpu:
-            constraint_values.append("@platforms//cpu:" + cpu)
-            suffix += "_" + cpu
-
-        _sdist_config_setting(
-            name = "sdist" + suffix,
-            constraint_values = constraint_values,
-            visibility = visibility,
-            config_setting_rule = config_setting_rule,
-        )
-        for python_version in python_versions:
-            _sdist_config_setting(
-                name = "cp{}_sdist{}".format(python_version, suffix),
-                python_version = python_version,
-                constraint_values = constraint_values,
-                visibility = visibility,
-                config_setting_rule = config_setting_rule,
-            )
-
-        for python_version in [""] + python_versions:
-            _whl_config_settings(
+        for os, cpu in target_platforms:
+            constraint_values = []
+            suffix = ""
+            if os:
+                constraint_values.append("@platforms//os:" + os)
+                suffix += "_" + os
+            if cpu:
+                constraint_values.append("@platforms//cpu:" + cpu)
+                suffix += "_" + cpu
+
+            _dist_config_settings(
                 suffix = suffix,
                 plat_flag_values = _plat_flag_values(
                     os = os,
@@ -161,34 +141,45 @@ def pip_config_settings(
                 ),
                 constraint_values = constraint_values,
                 python_version = python_version,
+                is_python = is_python,
                 visibility = visibility,
-                config_setting_rule = config_setting_rule,
+                native = native,
             )
 
-def _whl_config_settings(*, suffix, plat_flag_values, **kwargs):
-    # With the following three we cover different per-version wheels
-    python_version = kwargs.get("python_version")
-    py = "cp{}_py".format(python_version) if python_version else "py"
-    pycp = "cp{}_cp3x".format(python_version) if python_version else "cp3x"
-
-    flag_values = {}
-
-    for n, f in {
-        "{}_none_any{}".format(py, suffix): None,
-        "{}3_none_any{}".format(py, suffix): _flags.whl_py3,
-        "{}3_abi3_any{}".format(py, suffix): _flags.whl_py3_abi3,
-        "{}_none_any{}".format(pycp, suffix): _flags.whl_pycp3x,
-        "{}_abi3_any{}".format(pycp, suffix): _flags.whl_pycp3x_abi3,
-        "{}_cp_any{}".format(pycp, suffix): _flags.whl_pycp3x_abicp,
-    }.items():
-        if f and f in flag_values:
-            fail("BUG")
-        elif f:
+def _dist_config_settings(*, suffix, plat_flag_values, **kwargs):
+    flag_values = {_flags.dist: ""}
+
+    # First create an sdist, we will be building upon the flag values, which
+    # will ensure that each sdist config setting is the least specialized of
+    # all. However, we need at least one flag value to cover the case where we
+    # have `sdist` for any platform, hence we have a non-empty `flag_values`
+    # here.
+    _dist_config_setting(
+        name = "sdist{}".format(suffix),
+        flag_values = flag_values,
+        is_pip_whl = FLAGS.is_pip_whl_no,
+        **kwargs
+    )
+
+    for name, f in [
+        ("py_none", _flags.whl_py2_py3),
+        ("py3_none", _flags.whl_py3),
+        ("py3_abi3", _flags.whl_py3_abi3),
+        ("cp3x_none", _flags.whl_pycp3x),
+        ("cp3x_abi3", _flags.whl_pycp3x_abi3),
+        ("cp3x_cp", _flags.whl_pycp3x_abicp),
+    ]:
+        if f in flag_values:
+            # This should never happen as all of the different whls should have
+            # unique flag values.
+            fail("BUG: the flag {} is attempted to be added twice to the list".format(f))
+        else:
             flag_values[f] = ""
 
-        _whl_config_setting(
-            name = n,
+        _dist_config_setting(
+            name = "{}_any{}".format(name, suffix),
             flag_values = flag_values,
+            is_pip_whl = FLAGS.is_pip_whl_only,
             **kwargs
         )
 
@@ -197,22 +188,25 @@ def _whl_config_settings(*, suffix, plat_flag_values, **kwargs):
     for (suffix, flag_values) in plat_flag_values:
         flag_values = flag_values | generic_flag_values
 
-        for n, f in {
-            "{}_none_{}".format(py, suffix): _flags.whl_plat,
-            "{}3_none_{}".format(py, suffix): _flags.whl_plat_py3,
-            "{}3_abi3_{}".format(py, suffix): _flags.whl_plat_py3_abi3,
-            "{}_none_{}".format(pycp, suffix): _flags.whl_plat_pycp3x,
-            "{}_abi3_{}".format(pycp, suffix): _flags.whl_plat_pycp3x_abi3,
-            "{}_cp_{}".format(pycp, suffix): _flags.whl_plat_pycp3x_abicp,
-        }.items():
-            if f and f in flag_values:
-                fail("BUG")
-            elif f:
+        for name, f in [
+            ("py_none", _flags.whl_plat),
+            ("py3_none", _flags.whl_plat_py3),
+            ("py3_abi3", _flags.whl_plat_py3_abi3),
+            ("cp3x_none", _flags.whl_plat_pycp3x),
+            ("cp3x_abi3", _flags.whl_plat_pycp3x_abi3),
+            ("cp3x_cp", _flags.whl_plat_pycp3x_abicp),
+        ]:
+            if f in flag_values:
+                # This should never happen as all of the different whls should have
+                # unique flag values.
+                fail("BUG: the flag {} is attempted to be added twice to the list".format(f))
+            else:
                 flag_values[f] = ""
 
-            _whl_config_setting(
-                name = n,
+            _dist_config_setting(
+                name = "{}_{}".format(name, suffix),
                 flag_values = flag_values,
+                is_pip_whl = FLAGS.is_pip_whl_only,
                 **kwargs
             )
 
@@ -282,85 +276,50 @@ def _plat_flag_values(os, cpu, osx_versions, glibc_versions, muslc_versions):
 
     return ret
 
-def _whl_config_setting(*, name, flag_values, visibility, config_setting_rule = None, **kwargs):
-    config_setting_rule = config_setting_rule or _config_setting_or
-    config_setting_rule(
-        name = "is_" + name,
-        flag_values = flag_values | {
-            FLAGS.pip_whl: UseWhlFlag.ONLY,
-        },
-        default = flag_values | {
-            _flags.whl_py2_py3: "",
-            FLAGS.pip_whl: UseWhlFlag.AUTO,
-        },
-        visibility = visibility,
-        **kwargs
-    )
-
-def _sdist_config_setting(*, name, visibility, config_setting_rule = None, **kwargs):
-    config_setting_rule = config_setting_rule or _config_setting_or
-    config_setting_rule(
-        name = "is_" + name,
-        flag_values = {FLAGS.pip_whl: UseWhlFlag.NO},
-        default = {FLAGS.pip_whl: UseWhlFlag.AUTO},
-        visibility = visibility,
-        **kwargs
-    )
+def _dist_config_setting(*, name, is_pip_whl, is_python, python_version, native = native, **kwargs):
+    """A macro to create a target that matches is_pip_whl_auto and one more value.
 
-def _config_setting_or(*, name, flag_values, default, visibility, **kwargs):
-    match_name = "_{}".format(name)
-    default_name = "_{}_default".format(name)
+    Args:
+        name: The name of the public target.
+        is_pip_whl: The config setting to match in addition to
+            `is_pip_whl_auto` when evaluating the config setting.
+        is_python: The python version config_setting to match.
+        python_version: The python version name.
+        native (struct): The struct containing alias and config_setting rules
+            to use for creating the objects. Can be overridden for unit tests
+            reasons.
+        **kwargs: The kwargs passed to the config_setting rule. Visibility of
+            the main alias target is also taken from the kwargs.
+    """
+    _name = "_is_" + name
 
+    visibility = kwargs.get("visibility")
     native.alias(
-        name = name,
+        name = "is_cp{}_{}".format(python_version, name) if python_version else "is_{}".format(name),
         actual = select({
-            "//conditions:default": default_name,
-            match_name: match_name,
+            # First match by the python version
+            is_python: _name,
+            "//conditions:default": is_python,
         }),
         visibility = visibility,
     )
 
-    _config_setting(
-        name = match_name,
-        flag_values = flag_values,
-        visibility = visibility,
-        **kwargs
-    )
-    _config_setting(
-        name = default_name,
-        flag_values = default,
+    if python_version:
+        # Reuse the config_setting targets that we use with the default
+        # `python_version` setting.
+        return
+
+    config_setting_name = _name + "_setting"
+    native.config_setting(name = config_setting_name, **kwargs)
+
+    # Next match by the `pip_whl` flag value and then match by the flags that
+    # are intrinsic to the distribution.
+    native.alias(
+        name = _name,
+        actual = select({
+            "//conditions:default": FLAGS.is_pip_whl_auto,
+            FLAGS.is_pip_whl_auto: config_setting_name,
+            is_pip_whl: config_setting_name,
+        }),
         visibility = visibility,
-        **kwargs
     )
-
-def _config_setting(python_version = "", **kwargs):
-    if python_version:
-        # NOTE @aignas 2024-05-26: with this we are getting about 24k internal
-        # config_setting targets in our unit tests. Whilst the number of the
-        # external dependencies does not dictate this number, it does mean that
-        # bazel will take longer to parse stuff. This would be especially
-        # noticeable in repos, which use multiple hub repos within a single
-        # workspace.
-        #
-        # A way to reduce the number of targets would be:
-        # * put them to a central location and teach this code to just alias them,
-        #   maybe we should create a pip_config_settings repo within the pip
-        #   extension, which would collect config settings for all hub_repos.
-        # * put them in rules_python - this has the drawback of exposing things like
-        #   is_cp3.10_linux and users may start depending upon the naming
-        #   convention and this API is very unstable.
-        is_python_config_setting(
-            python_version = python_version,
-            **kwargs
-        )
-    else:
-        # We need this to ensure that there are no ambiguous matches when python_version
-        # is unset, which usually happens when we are not using the python version aware
-        # rules.
-        flag_values = kwargs.pop("flag_values", {}) | {
-            FLAGS.python_version: "",
-        }
-        native.config_setting(
-            flag_values = flag_values,
-            **kwargs
-        )