fix: load target_platforms through the hub (#2781)

This PR moves the parsing of `Requires-Dist` to the loading phase
within the `whl_library_targets_from_requires` macro. The original
`whl_library_targets` macro has been left unchanged so that I don't have
to reinvent the unit tests - it is well covered under tests.

Before this PR we had to wire the `target_platforms` via the
`experimental_target_platforms` attr in the `whl_library`, which means
that whenever this would change (e.g. the minor Python version changes),
the wheel would be re-extracted even though the final result may be the
same.

This refactor uncovered that the dependency graph creation was incorrect
if we had multiple target Python versions due to various heuristics that
this had. In hindsight I had them to make the generated `BUILD.bazel`
files more readable when the unit test coverage was not great. Now this
is unnecessary and since everything is happening in Starlark I thought
that having a simpler algorithm that does the right thing always is the
best way.

This also cleans up the code by removing left over TODO notes or code
that no longer make sense.

Work towards #260, #2319

Author: aignas

CHANGELOG.md

@@ -105,6 +105,13 @@ END_UNRELEASED_TEMPLATE
   [PR #2746](https://github.com/bazel-contrib/rules_python/pull/2746).
 * (rules) {attr}`py_binary.srcs` and {attr}`py_test.srcs` is no longer mandatory when
   `main_module` is specified (for `--bootstrap_impl=script`)
+* (pypi) From now on the `Requires-Dist` from the wheel metadata is analysed in
+  the loading phase instead of repository rule phase giving better caching
+  performance when the target platforms are changed (e.g. target python
+  versions). This is preparatory work for stabilizing the cross-platform wheel
+  support. From now on the usage of `experimental_target_platforms` should be
+  avoided and the `requirements_by_platform` values should be instead used to
+  specify the target platforms for the given dependencies.
 
 [20250317]: https://github.com/astral-sh/python-build-standalone/releases/tag/20250317
 

config.bzl.tmpl.bzlmod

@@ -212,15 +212,6 @@ bzl_library(
     ],
 )
 
-bzl_library(
-    name = "pep508_bzl",
-    srcs = ["pep508.bzl"],
-    deps = [
-        ":pep508_env_bzl",
-        ":pep508_evaluate_bzl",
-    ],
-)
-
 bzl_library(
     name = "pep508_deps_bzl",
     srcs = ["pep508_deps.bzl"],
@@ -378,13 +369,12 @@ bzl_library(
         ":attrs_bzl",
         ":deps_bzl",
         ":generate_whl_library_build_bazel_bzl",
-        ":parse_whl_name_bzl",
         ":patch_whl_bzl",
-        ":pep508_deps_bzl",
+        ":pep508_requirement_bzl",
         ":pypi_repo_utils_bzl",
         ":whl_metadata_bzl",
-        ":whl_target_platforms_bzl",
         "//python/private:auth_bzl",
+        "//python/private:bzlmod_enabled_bzl",
         "//python/private:envsubst_bzl",
         "//python/private:is_standalone_interpreter_bzl",
         "//python/private:repo_utils_bzl",

python/private/pypi/BUILD.bazel

@@ -123,6 +123,9 @@ Warning:
     "experimental_target_platforms": attr.string_list(
         default = [],
         doc = """\
+*NOTE*: This will be removed in the next major version, so please consider migrating
+to `bzlmod` and rely on {attr}`pip.parse.requirements_by_platform` for this feature.
+
 A list of platforms that we will generate the conditional dependency graph for
 cross platform wheels by parsing the wheel metadata. This will generate the
 correct dependencies for packages like `sphinx` or `pylint`, which include

python/private/pypi/attrs.bzl

@@ -0,0 +1,9 @@
+"""Extra configuration values that are exposed from the hub repository for spoke repositories to access.
+
+NOTE: This is internal `rules_python` API and if you would like to depend on it, please raise an issue
+with your usecase. This may change in between rules_python versions without any notice.
+
+@generated by rules_python pip.parse bzlmod extension.
+"""
+
+target_platforms = %%TARGET_PLATFORMS%%

python/private/pypi/config.bzl.tmpl.bzlmod

@@ -32,7 +32,6 @@ 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")
-load(":whl_target_platforms.bzl", "whl_target_platforms")
 
 def _major_minor_version(version):
     version = semver(version)
@@ -68,7 +67,6 @@ def _create_whl_repos(
         *,
         pip_attr,
         whl_overrides,
-        evaluate_markers = evaluate_markers,
         available_interpreters = INTERPRETER_LABELS,
         get_index_urls = None):
     """create all of the whl repositories
@@ -77,7 +75,6 @@ def _create_whl_repos(
         module_ctx: {type}`module_ctx`.
         pip_attr: {type}`struct` - the struct that comes from the tag class iteration.
         whl_overrides: {type}`dict[str, struct]` - per-wheel overrides.
-        evaluate_markers: the function to use to evaluate markers.
         get_index_urls: A function used to get the index URLs
         available_interpreters: {type}`dict[str, Label]` The dictionary of available
             interpreters that have been registered using the `python` bzlmod extension.
@@ -162,14 +159,12 @@ def _create_whl_repos(
             requirements_osx = pip_attr.requirements_darwin,
             requirements_windows = pip_attr.requirements_windows,
             extra_pip_args = pip_attr.extra_pip_args,
+            # TODO @aignas 2025-04-15: pass the full version into here
             python_version = major_minor,
             logger = logger,
         ),
         extra_pip_args = pip_attr.extra_pip_args,
         get_index_urls = get_index_urls,
-        # NOTE @aignas 2025-02-24: we will use the "cp3xx_os_arch" platform labels
-        # for converting to the PEP508 environment and will evaluate them in starlark
-        # without involving the interpreter at all.
         evaluate_markers = evaluate_markers,
         logger = logger,
     )
@@ -191,7 +186,6 @@ def _create_whl_repos(
             enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs,
             environment = pip_attr.environment,
             envsubst = pip_attr.envsubst,
-            experimental_target_platforms = pip_attr.experimental_target_platforms,
             group_deps = group_deps,
             group_name = group_name,
             pip_data_exclude = pip_attr.pip_data_exclude,
@@ -244,6 +238,12 @@ def _create_whl_repos(
         },
         extra_aliases = extra_aliases,
         whl_libraries = whl_libraries,
+        target_platforms = {
+            plat: None
+            for reqs in requirements_by_platform.values()
+            for req in reqs
+            for plat in req.target_platforms
+        },
     )
 
 def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patterns, multiple_requirements_for_whl = False, python_version):
@@ -274,20 +274,11 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt
         args["urls"] = [distribution.url]
         args["sha256"] = distribution.sha256
         args["filename"] = distribution.filename
-        args["experimental_target_platforms"] = requirement.target_platforms
 
         # Pure python wheels or sdists may need to have a platform here
         target_platforms = None
         if distribution.filename.endswith(".whl") and not distribution.filename.endswith("-any.whl"):
-            parsed_whl = parse_whl_name(distribution.filename)
-            whl_platforms = whl_target_platforms(
-                platform_tag = parsed_whl.platform_tag,
-            )
-            args["experimental_target_platforms"] = [
-                p
-                for p in requirement.target_platforms
-                if [None for wp in whl_platforms if p.endswith(wp.target_platform)]
-            ]
+            pass
         elif multiple_requirements_for_whl:
             target_platforms = requirement.target_platforms
 
@@ -416,6 +407,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
     hub_group_map = {}
     exposed_packages = {}
     extra_aliases = {}
+    target_platforms = {}
     whl_libraries = {}
 
     for mod in module_ctx.modules:
@@ -498,6 +490,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
             for whl_name, aliases in out.extra_aliases.items():
                 extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
             exposed_packages.setdefault(hub_name, {}).update(out.exposed_packages)
+            target_platforms.setdefault(hub_name, {}).update(out.target_platforms)
             whl_libraries.update(out.whl_libraries)
 
             # TODO @aignas 2024-04-05: how do we support different requirement
@@ -535,6 +528,10 @@ You cannot use both the additive_build_content and additive_build_content_file a
             }
             for hub_name, extra_whl_aliases in extra_aliases.items()
         },
+        target_platforms = {
+            hub_name: sorted(p)
+            for hub_name, p in target_platforms.items()
+        },
         whl_libraries = {
             k: dict(sorted(args.items()))
             for k, args in sorted(whl_libraries.items())
@@ -626,15 +623,13 @@ def _pip_impl(module_ctx):
             },
             packages = mods.exposed_packages.get(hub_name, []),
             groups = mods.hub_group_map.get(hub_name),
+            target_platforms = mods.target_platforms.get(hub_name, []),
         )
 
     if bazel_features.external_deps.extension_metadata_has_reproducible:
-        # If we are not using the `experimental_index_url feature, the extension is fully
-        # deterministic and we don't need to create a lock entry for it.
-        #
-        # In order to be able to dogfood the `experimental_index_url` feature before it gets
-        # stabilized, we have created the `_pip_non_reproducible` function, that will result
-        # in extra entries in the lock file.
+        # NOTE @aignas 2025-04-15: this is set to be reproducible, because the
+        # results after calling the PyPI index should be reproducible on each
+        # machine.
         return module_ctx.extension_metadata(reproducible = True)
     else:
         return None

python/private/pypi/extension.bzl

@@ -21,42 +21,56 @@ _RENDER = {
     "copy_files": render.dict,
     "data": render.list,
     "data_exclude": render.list,
-    "dependencies": render.list,
-    "dependencies_by_platform": lambda x: render.dict(x, value_repr = render.list),
     "entry_points": render.dict,
+    "extras": render.list,
     "group_deps": render.list,
+    "requires_dist": render.list,
     "srcs_exclude": render.list,
-    "tags": render.list,
+    "target_platforms": lambda x: render.list(x) if x else "target_platforms",
 }
 
 # NOTE @aignas 2024-10-25: We have to keep this so that files in
 # this repository can be publicly visible without the need for
 # export_files
 _TEMPLATE = """\
-load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets")
+{loads}
 
 package(default_visibility = ["//visibility:public"])
 
-whl_library_targets(
+whl_library_targets_from_requires(
 {kwargs}
 )
 """
 
 def generate_whl_library_build_bazel(
         *,
         annotation = None,
+        default_python_version = None,
         **kwargs):
     """Generate a BUILD file for an unzipped Wheel
 
     Args:
         annotation: The annotation for the build file.
+        default_python_version: The python version to use to parse the METADATA.
         **kwargs: Extra args serialized to be passed to the
             {obj}`whl_library_targets`.
 
     Returns:
         A complete BUILD file as a string
     """
 
+    loads = [
+        """load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires")""",
+    ]
+    if not kwargs.setdefault("target_platforms", None):
+        dep_template = kwargs["dep_template"]
+        loads.append(
+            "load(\"{}\", \"{}\")".format(
+                dep_template.format(name = "", target = "config.bzl"),
+                "target_platforms",
+            ),
+        )
+
     additional_content = []
     if annotation:
         kwargs["data"] = annotation.data
@@ -66,10 +80,13 @@ def generate_whl_library_build_bazel(
         kwargs["srcs_exclude"] = annotation.srcs_exclude_glob
         if annotation.additive_build_content:
             additional_content.append(annotation.additive_build_content)
+    if default_python_version:
+        kwargs["default_python_version"] = default_python_version
 
     contents = "\n".join(
         [
             _TEMPLATE.format(
+                loads = "\n".join(loads),
                 kwargs = render.indent("\n".join([
                     "{} = {},".format(k, _RENDER.get(k, repr)(v))
                     for k, v in sorted(kwargs.items())

python/private/pypi/generate_whl_library_build_bazel.bzl

@@ -45,7 +45,14 @@ def _impl(rctx):
     macro_tmpl = "@@{name}//{{}}:{{}}".format(name = rctx.attr.name)
 
     rctx.file("BUILD.bazel", _BUILD_FILE_CONTENTS)
-    rctx.template("requirements.bzl", rctx.attr._template, substitutions = {
+    rctx.template(
+        "config.bzl",
+        rctx.attr._config_template,
+        substitutions = {
+            "%%TARGET_PLATFORMS%%": render.list(rctx.attr.target_platforms),
+        },
+    )
+    rctx.template("requirements.bzl", rctx.attr._requirements_bzl_template, substitutions = {
         "%%ALL_DATA_REQUIREMENTS%%": render.list([
             macro_tmpl.format(p, "data")
             for p in bzl_packages
@@ -80,14 +87,21 @@ The list of packages that will be exposed via all_*requirements macros. Defaults
             mandatory = True,
             doc = "The apparent name of the repo. This is needed because in bzlmod, the name attribute becomes the canonical name.",
         ),
+        "target_platforms": attr.string_list(
+            mandatory = True,
+            doc = "All of the target platforms for the hub repo",
+        ),
         "whl_map": attr.string_dict(
             mandatory = True,
             doc = """\
 The wheel map where values are json.encoded strings of the whl_map constructed
 in the pip.parse tag class.
 """,
         ),
-        "_template": attr.label(
+        "_config_template": attr.label(
+            default = ":config.bzl.tmpl.bzlmod",
+        ),
+        "_requirements_bzl_template": attr.label(
             default = ":requirements.bzl.tmpl.bzlmod",
         ),
     },