refactor: load target_platforms through the hub

This PR moves the parsing of `Requires-Dist` to the analysis 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 also cleans up the code by removing left over TODO notes or code
that no longer make sense.

Work towards bazel-contrib#260, bazel-contrib#2319

Author: aignas

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

@@ -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,23 +21,23 @@ _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}
 )
 """
@@ -57,6 +57,18 @@ def generate_whl_library_build_bazel(
         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
@@ -70,6 +82,7 @@ def generate_whl_library_build_bazel(
     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",
         ),
     },

python/private/pypi/hub_repository.bzl

@@ -15,6 +15,7 @@
 """This module is for implementing PEP508 compliant METADATA deps parsing.
 """
 
+load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION")
 load("//python/private:normalize_name.bzl", "normalize_name")
 load(":pep508_env.bzl", "env")
 load(":pep508_evaluate.bzl", "evaluate")
@@ -35,7 +36,7 @@ _ALL_ARCH_VALUES = [
     "x86_64",
 ]
 
-def deps(name, *, requires_dist, platforms = [], extras = [], host_python_version = None):
+def deps(name, *, requires_dist, platforms = [], extras = [], default_python_version = None):
     """Parse the RequiresDist from wheel METADATA
 
     Args:
@@ -44,46 +45,59 @@ def deps(name, *, requires_dist, platforms = [], extras = [], host_python_versio
             METADATA file.
         extras: {type}`list[str]` the requested extras to generate targets for.
         platforms: {type}`list[str]` the list of target platform strings.
-        host_python_version: {type}`str` the host python version.
+        default_python_version: {type}`str` the host python version.
 
     Returns:
         A struct with attributes:
         * deps: {type}`list[str]` dependencies to include unconditionally.
         * deps_select: {type}`dict[str, list[str]]` dependencies to include on particular
               subset of target platforms.
     """
-    reqs = sorted(
-        [requirement(r) for r in requires_dist],
-        key = lambda x: "{}:{}:".format(x.name, sorted(x.extras), x.marker),
-    )
-    deps = {}
-    deps_select = {}
-    name = normalize_name(name)
-    want_extras = _resolve_extras(name, reqs, extras)
-
-    # drop self edges
-    reqs = [r for r in reqs if r.name != name]
-
+    if not platforms:
+        fail("'platforms' arg is mandatory")
+
+    # TODO @aignas 2025-04-16: this `default_abi` variable is only
+    # here so that we can have selects without "is_python_version" conditions.
+    # This would happen in the `multi_pip_parse` logic in WORKSPACE.
+    #
+    # In followup PRs I would like to remove this logic and mainly rely on the
+    # fact that there will be only a single ABI when internal rules pass the
+    # values in the WORKSPACE case and in that case we can get the default
+    # version from the target platform strings. What is more when we drop
+    # WORKSPACE, we can simplify the logic in `_add_req` substantially and get
+    # rid of `default_abi` all together.
+    default_python_version = default_python_version or DEFAULT_PYTHON_VERSION
     platforms = [
-        platform_from_str(p, python_version = host_python_version)
+        platform_from_str(p, python_version = default_python_version)
         for p in platforms
-    ] or [
-        platform_from_str("", python_version = host_python_version),
     ]
 
     abis = sorted({p.abi: True for p in platforms if p.abi})
-    if host_python_version and len(abis) > 1:
-        _, _, minor_version = host_python_version.partition(".")
+    if default_python_version and len(abis) > 1:
+        _, _, minor_version = default_python_version.partition(".")
         minor_version, _, _ = minor_version.partition(".")
         default_abi = "cp3" + minor_version
     elif len(abis) > 1:
         fail(
-            "all python versions need to be specified explicitly, got: {}".format(platforms),
+            "all python versions need to be specified explicitly with the default_python_version, got: {}, {}".format(platforms, default_python_version),
         )
     else:
         default_abi = None
 
+    reqs = sorted(
+        [requirement(r) for r in requires_dist],
+        key = lambda x: "{}:{}:".format(x.name, sorted(x.extras), x.marker),
+    )
+    deps = {}
+    deps_select = {}
+    name = normalize_name(name)
+    want_extras = _resolve_extras(name, reqs, extras)
+
     for req in reqs:
+        if req.name == name:
+            # drop self edges
+            continue
+
         _add_req(
             deps,
             deps_select,
@@ -280,11 +294,6 @@ def _add_req(deps, deps_select, req, *, extras, platforms, default_abi = None):
         _add(deps, deps_select, req.name, None)
         return
 
-    # NOTE @aignas 2023-12-08: in order to have reasonable select statements
-    # we do have to have some parsing of the markers, so it begs the question
-    # if packaging should be reimplemented in Starlark to have the best solution
-    # for now we will implement it in Python and see what the best parsing result
-    # can be before making this decision.
     match_os = len([
         tag
         for tag in [