refactor(pypi): move config setting processing to the macro (#2424)

Before hand we would pass around `whl_alias` struct instances and it
would go through rounds of starlark functions which would end up
rendered as BUILD.bazel files in the hub repository. This means that
every time the output of the said functions would change, the
`MODULE.bazel.lock` would also possibly change.

Since we now have much better unit testing framework, we start relying
on these functions being evaluated at loading phase instead of repo rule
phase, which means that we can make the `BUILD.bazel` files not change a
lot when the underlying code changes, this should make the code more
maintainable in the long run and unblocks the code to accept extra
things, like env marker values or similar.

Summary:
- Remove the `repo` field from `whl_alias`.
- Rename `whl_alias` to `whl_config_setting`
- Move the tests around
- Simplify the `pkg_aliases` tests to use `contains` instead of exact
  equality to make things less brittle.
- Simplify the `pkg_aliases` tests to use different mocks to make
  expectations easier to understand.
- Make `whl_config_setting` hashable by using tuples instead of lists.
- Adjust how we store the `whl_config_setting` in the PyPI extension
  and optimize the passing to the `hub_repository`.

This is needed for #2319 and #2423.

To be in future PRs:
* Remove the need to pass `osx_versions`, etc to the `pkg_aliases`
macro.

Author: aignas

python/private/pypi/BUILD.bazel

@@ -64,6 +64,7 @@ bzl_library(
         ":parse_whl_name_bzl",
         ":pip_repository_attrs_bzl",
         ":simpleapi_download_bzl",
+        ":whl_config_setting_bzl",
         ":whl_library_bzl",
         ":whl_repo_name_bzl",
         "//python/private:full_version_bzl",
@@ -156,7 +157,7 @@ bzl_library(
     name = "multi_pip_parse_bzl",
     srcs = ["multi_pip_parse.bzl"],
     deps = [
-        "pip_repository_bzl",
+        ":pip_repository_bzl",
         "//python/private:text_util_bzl",
     ],
 )
@@ -230,6 +231,7 @@ bzl_library(
         ":parse_requirements_bzl",
         ":pip_repository_attrs_bzl",
         ":render_pkg_aliases_bzl",
+        ":whl_config_setting_bzl",
         "//python/private:normalize_name_bzl",
         "//python/private:repo_utils_bzl",
         "//python/private:text_util_bzl",
@@ -257,6 +259,7 @@ bzl_library(
     deps = [
         ":generate_group_library_build_bazel_bzl",
         ":parse_whl_name_bzl",
+        ":whl_config_setting_bzl",
         ":whl_target_platforms_bzl",
         "//python/private:normalize_name_bzl",
         "//python/private:text_util_bzl",
@@ -283,6 +286,11 @@ bzl_library(
     ],
 )
 
+bzl_library(
+    name = "whl_config_setting_bzl",
+    srcs = ["whl_config_setting.bzl"],
+)
+
 bzl_library(
     name = "whl_library_alias_bzl",
     srcs = ["whl_library_alias.bzl"],

python/private/pypi/extension.bzl

@@ -23,13 +23,13 @@ load("//python/private:semver.bzl", "semver")
 load("//python/private:version_label.bzl", "version_label")
 load(":attrs.bzl", "use_isolated")
 load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
-load(":hub_repository.bzl", "hub_repository")
+load(":hub_repository.bzl", "hub_repository", "whl_config_settings_to_json")
 load(":parse_requirements.bzl", "parse_requirements")
 load(":parse_whl_name.bzl", "parse_whl_name")
 load(":pip_repository_attrs.bzl", "ATTRS")
-load(":render_pkg_aliases.bzl", "whl_alias")
 load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
 load(":simpleapi_download.bzl", "simpleapi_download")
+load(":whl_config_setting.bzl", "whl_config_setting")
 load(":whl_library.bzl", "whl_library")
 load(":whl_repo_name.bzl", "pypi_repo_name", "whl_repo_name")
 
@@ -87,7 +87,7 @@ def _create_whl_repos(
     Returns a {type}`struct` with the following attributes:
         whl_map: {type}`dict[str, list[struct]]` the output is keyed by the
             normalized package name and the values are the instances of the
-            {bzl:obj}`whl_alias` return values.
+            {bzl:obj}`whl_config_setting` return values.
         exposed_packages: {type}`dict[str, Any]` this is just a way to
             represent a set of string values.
         whl_libraries: {type}`dict[str, dict[str, Any]]` the keys are the
@@ -304,14 +304,11 @@ def _create_whl_repos(
 
                     whl_libraries[repo_name] = args
 
-                    whl_map.setdefault(whl_name, []).append(
-                        whl_alias(
-                            repo = repo_name,
-                            version = major_minor,
-                            filename = distribution.filename,
-                            target_platforms = target_platforms,
-                        ),
-                    )
+                    whl_map.setdefault(whl_name, {})[whl_config_setting(
+                        version = major_minor,
+                        filename = distribution.filename,
+                        target_platforms = target_platforms,
+                    )] = repo_name
 
             if found_something:
                 if is_exposed:
@@ -339,13 +336,10 @@ def _create_whl_repos(
                 *target_platforms
             )
             whl_libraries[repo_name] = args
-            whl_map.setdefault(whl_name, []).append(
-                whl_alias(
-                    repo = repo_name,
-                    version = major_minor,
-                    target_platforms = target_platforms or None,
-                ),
-            )
+            whl_map.setdefault(whl_name, {})[whl_config_setting(
+                version = major_minor,
+                target_platforms = target_platforms or None,
+            )] = repo_name
 
         if is_exposed:
             exposed_packages[whl_name] = None
@@ -488,7 +482,8 @@ You cannot use both the additive_build_content and additive_build_content_file a
             )
             hub_whl_map.setdefault(hub_name, {})
             for key, settings in out.whl_map.items():
-                hub_whl_map[hub_name].setdefault(key, []).extend(settings)
+                for setting, repo in settings.items():
+                    hub_whl_map[hub_name].setdefault(key, {}).setdefault(repo, []).append(setting)
             extra_aliases.setdefault(hub_name, {})
             for whl_name, aliases in out.extra_aliases.items():
                 extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
@@ -508,7 +503,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
         whl_mods = dict(sorted(whl_mods.items())),
         hub_whl_map = {
             hub_name: {
-                whl_name: sorted(settings, key = lambda x: (x.version, x.filename))
+                whl_name: dict(settings)
                 for whl_name, settings in sorted(whl_map.items())
             }
             for hub_name, whl_map in sorted(hub_whl_map.items())
@@ -538,20 +533,6 @@ You cannot use both the additive_build_content and additive_build_content_file a
         is_reproducible = is_reproducible,
     )
 
-def _alias_dict(a):
-    ret = {
-        "repo": a.repo,
-    }
-    if a.config_setting:
-        ret["config_setting"] = a.config_setting
-    if a.filename:
-        ret["filename"] = a.filename
-    if a.target_platforms:
-        ret["target_platforms"] = a.target_platforms
-    if a.version:
-        ret["version"] = a.version
-    return ret
-
 def _pip_impl(module_ctx):
     """Implementation of a class tag that creates the pip hub and corresponding pip spoke whl repositories.
 
@@ -632,8 +613,8 @@ def _pip_impl(module_ctx):
             repo_name = hub_name,
             extra_hub_aliases = mods.extra_aliases.get(hub_name, {}),
             whl_map = {
-                key: json.encode([_alias_dict(a) for a in aliases])
-                for key, aliases in whl_map.items()
+                key: whl_config_settings_to_json(values)
+                for key, values in whl_map.items()
             },
             packages = mods.exposed_packages.get(hub_name, []),
             groups = mods.hub_group_map.get(hub_name),

python/private/pypi/hub_repository.bzl

@@ -15,11 +15,8 @@
 ""
 
 load("//python/private:text_util.bzl", "render")
-load(
-    ":render_pkg_aliases.bzl",
-    "render_multiplatform_pkg_aliases",
-    "whl_alias",
-)
+load(":render_pkg_aliases.bzl", "render_multiplatform_pkg_aliases")
+load(":whl_config_setting.bzl", "whl_config_setting")
 
 _BUILD_FILE_CONTENTS = """\
 package(default_visibility = ["//visibility:public"])
@@ -32,7 +29,7 @@ def _impl(rctx):
     bzl_packages = rctx.attr.packages or rctx.attr.whl_map.keys()
     aliases = render_multiplatform_pkg_aliases(
         aliases = {
-            key: [whl_alias(**v) for v in json.decode(values)]
+            key: _whl_config_settings_from_json(values)
             for key, values in rctx.attr.whl_map.items()
         },
         extra_hub_aliases = rctx.attr.extra_hub_aliases,
@@ -97,3 +94,45 @@ in the pip.parse tag class.
     doc = """A rule for bzlmod mulitple pip repository creation. PRIVATE USE ONLY.""",
     implementation = _impl,
 )
+
+def _whl_config_settings_from_json(repo_mapping_json):
+    """Deserialize the serialized values with whl_config_settings_to_json.
+
+    Args:
+        repo_mapping_json: {type}`str`
+
+    Returns:
+        What `whl_config_settings_to_json` accepts.
+    """
+    return {
+        whl_config_setting(**v): repo
+        for repo, values in json.decode(repo_mapping_json).items()
+        for v in values
+    }
+
+def whl_config_settings_to_json(repo_mapping):
+    """A function to serialize the aliases so that `hub_repository` can accept them.
+
+    Args:
+        repo_mapping: {type}`dict[str, list[struct]]` repo to
+            {obj}`whl_config_setting` mapping.
+
+    Returns:
+        A deserializable JSON string
+    """
+    return json.encode({
+        repo: [_whl_config_setting_dict(s) for s in settings]
+        for repo, settings in repo_mapping.items()
+    })
+
+def _whl_config_setting_dict(a):
+    ret = {}
+    if a.config_setting:
+        ret["config_setting"] = a.config_setting
+    if a.filename:
+        ret["filename"] = a.filename
+    if a.target_platforms:
+        ret["target_platforms"] = a.target_platforms
+    if a.version:
+        ret["version"] = a.version
+    return ret

python/private/pypi/pip_repository.bzl

@@ -21,7 +21,7 @@ load("//python/private:text_util.bzl", "render")
 load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
 load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
 load(":pip_repository_attrs.bzl", "ATTRS")
-load(":render_pkg_aliases.bzl", "render_pkg_aliases", "whl_alias")
+load(":render_pkg_aliases.bzl", "render_pkg_aliases")
 load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
 
 def _get_python_interpreter_attr(rctx):
@@ -174,7 +174,7 @@ def _pip_repository_impl(rctx):
 
     aliases = render_pkg_aliases(
         aliases = {
-            pkg: [whl_alias(repo = rctx.attr.name + "_" + pkg)]
+            pkg: rctx.attr.name + "_" + pkg
             for pkg in bzl_packages or []
         },
         extra_hub_aliases = rctx.attr.extra_hub_aliases,