feat(pypi): actually start using env_marker_setting

Summary:
- `pep508_deps` is now much simpler, because the hard work is done in
  analysis phase
- `whl_library` BUILD.bazel tests now also have a test for the legacy
  flow.

One thing that I noticed is that now we have an implicit dependency on
the python toolchain when getting all of the `whl` target tree. This is
a filegroup target that includes dependent wheels. However, we fallback
to the flag values if we don't have the toolchain, so we should be good
in general.

Overall I like how this is turning out because we don't need to pipe the
`target_platforms` anymore when we enable `PIPSTAR` feature. This means
that we can start creating fewer whl_library instances - e.g. a
`py3-none-any` wheel can be fetched once instead of once per python
interpreter version. I'll leave this optimization for a later time.

Work towards bazel-contrib#260

Author: aignas

CHANGELOG.md

@@ -97,6 +97,7 @@ END_UNRELEASED_TEMPLATE
 * (pypi) Starlark-based evaluation of environment markers (requirements.txt conditionals)
   available (not enabled by default) for improved multi-platform build support.
   Set the `RULES_PYTHON_ENABLE_PIPSTAR=1` environment variable to enable it.
+  Work towards [#260](https://github.com/bazel-contrib/rules_python/issues/260).
 
 {#v0-0-0-removed}
 ### Removed

python/private/pypi/BUILD.bazel

@@ -236,13 +236,9 @@ bzl_library(
     name = "pep508_deps_bzl",
     srcs = ["pep508_deps.bzl"],
     deps = [
-        ":pep508_env_bzl",
         ":pep508_evaluate_bzl",
-        ":pep508_platform_bzl",
         ":pep508_requirement_bzl",
-        "//python/private:full_version_bzl",
         "//python/private:normalize_name_bzl",
-        "@pythons_hub//:versions_bzl",
     ],
 )
 

python/private/pypi/extension.bzl

@@ -17,6 +17,7 @@
 load("@bazel_features//:features.bzl", "bazel_features")
 load("@pythons_hub//:interpreters.bzl", "INTERPRETER_LABELS")
 load("@pythons_hub//:versions.bzl", "MINOR_MAPPING")
+load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config")
 load("//python/private:auth.bzl", "AUTH_ATTRS")
 load("//python/private:full_version.bzl", "full_version")
 load("//python/private:normalize_name.bzl", "normalize_name")
@@ -216,7 +217,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,
@@ -227,6 +227,9 @@ def _create_whl_repos(
                 for p, args in whl_overrides.get(whl_name, {}).items()
             },
         )
+        if not rp_config.enable_pipstar:
+            maybe_args["experimental_target_platforms"] = pip_attr.experimental_target_platforms
+
         whl_library_args.update({k: v for k, v in maybe_args.items() if v})
         maybe_args_with_default = dict(
             # The following values have defaults next to them
@@ -305,13 +308,14 @@ 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"] = [
-            # Get rid of the version fot the target platforms because we are
-            # passing the interpreter any way. Ideally we should search of ways
-            # how to pass the target platforms through the hub repo.
-            p.partition("_")[2]
-            for p in requirement.target_platforms
-        ]
+        if not rp_config.enable_pipstar:
+            args["experimental_target_platforms"] = [
+                # Get rid of the version fot the target platforms because we are
+                # passing the interpreter any way. Ideally we should search of ways
+                # how to pass the target platforms through the hub repo.
+                p.partition("_")[2]
+                for p in requirement.target_platforms
+            ]
 
         # Pure python wheels or sdists may need to have a platform here
         target_platforms = None

python/private/pypi/generate_whl_library_build_bazel.bzl

@@ -26,10 +26,11 @@ _RENDER = {
     "entry_points": render.dict,
     "extras": render.list,
     "group_deps": render.list,
+    "include": str,
     "requires_dist": render.list,
     "srcs_exclude": render.list,
     "tags": render.list,
-    "target_platforms": lambda x: render.list(x) if x else "target_platforms",
+    "target_platforms": render.list,
 }
 
 # NOTE @aignas 2024-10-25: We have to keep this so that files in
@@ -62,28 +63,44 @@ def generate_whl_library_build_bazel(
         A complete BUILD file as a string
     """
 
-    fn = "whl_library_targets"
+    loads = []
     if kwargs.get("tags"):
+        fn = "whl_library_targets"
+
         # legacy path
         unsupported_args = [
             "requires",
             "metadata_name",
             "metadata_version",
+            "include",
         ]
     else:
-        fn = "{}_from_requires".format(fn)
+        fn = "whl_library_targets_from_requires"
         unsupported_args = [
             "dependencies",
             "dependencies_by_platform",
+            "target_platforms",
+            "default_python_version",
         ]
+        dep_template = kwargs.get("dep_template")
+        loads.append(
+            """load("{}", "{}")""".format(
+                dep_template.format(
+                    name = "",
+                    target = "requirements.bzl",
+                ),
+                "all_whl_requirements_by_package",
+            ),
+        )
+        kwargs["include"] = "sorted(all_whl_requirements_by_package)"
 
     for arg in unsupported_args:
         if kwargs.get(arg):
             fail("BUG, unsupported arg: '{}'".format(arg))
 
-    loads = [
+    loads.extend([
         """load("@rules_python//python/private/pypi:whl_library_targets.bzl", "{}")""".format(fn),
-    ]
+    ])
 
     additional_content = []
     if annotation:

python/private/pypi/pep508_deps.bzl

@@ -15,36 +15,27 @@
 """This module is for implementing PEP508 compliant METADATA deps parsing.
 """
 
-load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION", "MINOR_MAPPING")
-load("//python/private:full_version.bzl", "full_version")
 load("//python/private:normalize_name.bzl", "normalize_name")
-load(":pep508_env.bzl", "env")
 load(":pep508_evaluate.bzl", "evaluate")
-load(":pep508_platform.bzl", "platform", "platform_from_str")
 load(":pep508_requirement.bzl", "requirement")
 
 def deps(
         name,
         *,
         requires_dist,
-        platforms = [],
         extras = [],
         excludes = [],
-        default_python_version = None,
-        minor_mapping = MINOR_MAPPING):
+        include = []):
     """Parse the RequiresDist from wheel METADATA
 
     Args:
         name: {type}`str` the name of the wheel.
         requires_dist: {type}`list[str]` the list of RequiresDist lines from the
             METADATA file.
         excludes: {type}`list[str]` what packages should we exclude.
+        include: {type}`list[str]` what packages should we exclude. If it is not
+            specified, then we will include all deps from `requires_dist`.
         extras: {type}`list[str]` the requested extras to generate targets for.
-        platforms: {type}`list[str]` the list of target platform strings.
-        default_python_version: {type}`str` the host python version.
-        minor_mapping: {type}`type[str, str]` the minor mapping to use when
-            resolving to the full python version as DEFAULT_PYTHON_VERSION can by
-            of format `3.x`.
 
     Returns:
         A struct with attributes:
@@ -60,39 +51,20 @@ def deps(
     deps_select = {}
     name = normalize_name(name)
     want_extras = _resolve_extras(name, reqs, extras)
+    include = [normalize_name(n) for n in include]
 
     # drop self edges
     excludes = [name] + [normalize_name(x) for x in excludes]
 
-    default_python_version = default_python_version or DEFAULT_PYTHON_VERSION
-    if default_python_version:
-        # if it is not bzlmod, then DEFAULT_PYTHON_VERSION may be unset
-        default_python_version = full_version(
-            version = default_python_version,
-            minor_mapping = minor_mapping,
-        )
-    platforms = [
-        platform_from_str(p, python_version = default_python_version)
-        for p in platforms
-    ]
-
-    abis = sorted({p.abi: True for p in platforms if p.abi})
-    if default_python_version and len(abis) > 1:
-        _, _, tail = default_python_version.partition(".")
-        default_abi = "cp3" + tail
-    elif len(abis) > 1:
-        fail(
-            "all python versions need to be specified explicitly, got: {}".format(platforms),
-        )
-    else:
-        default_abi = None
-
     reqs_by_name = {}
 
     for req in reqs:
         if req.name_ in excludes:
             continue
 
+        if include and req.name_ not in include:
+            continue
+
         reqs_by_name.setdefault(req.name, []).append(req)
 
     for name, reqs in reqs_by_name.items():
@@ -102,55 +74,25 @@ def deps(
             normalize_name(name),
             reqs,
             extras = want_extras,
-            platforms = platforms,
-            default_abi = default_abi,
         )
 
     return struct(
         deps = sorted(deps),
         deps_select = {
-            _platform_str(p): sorted(deps)
-            for p, deps in deps_select.items()
+            d: markers
+            for d, markers in sorted(deps_select.items())
         },
     )
 
-def _platform_str(self):
-    if self.abi == None:
-        return "{}_{}".format(self.os, self.arch)
-
-    return "{}_{}_{}".format(
-        self.abi,
-        self.os or "anyos",
-        self.arch or "anyarch",
-    )
-
-def _add(deps, deps_select, dep, platform):
+def _add(deps, deps_select, dep, markers = None):
     dep = normalize_name(dep)
 
-    if platform == None:
+    if not markers:
         deps[dep] = True
-
-        # If the dep is in the platform-specific list, remove it from the select.
-        pop_keys = []
-        for p, _deps in deps_select.items():
-            if dep not in _deps:
-                continue
-
-            _deps.pop(dep)
-            if not _deps:
-                pop_keys.append(p)
-
-        for p in pop_keys:
-            deps_select.pop(p)
-        return
-
-    if dep in deps:
-        # If the dep is already in the main dependency list, no need to add it in the
-        # platform-specific dependency list.
-        return
-
-    # Add the platform-specific branch
-    deps_select.setdefault(platform, {})[dep] = True
+    elif len(markers) == 1:
+        deps_select[dep] = markers[0]
+    else:
+        deps_select[dep] = "({})".format(") or (".join(sorted(markers)))
 
 def _resolve_extras(self_name, reqs, extras):
     """Resolve extras which are due to depending on self[some_other_extra].
@@ -207,37 +149,24 @@ def _resolve_extras(self_name, reqs, extras):
     # Poor mans set
     return sorted({x: None for x in extras})
 
-def _add_reqs(deps, deps_select, dep, reqs, *, extras, platforms, default_abi = None):
+def _add_reqs(deps, deps_select, dep, reqs, *, extras):
     for req in reqs:
         if not req.marker:
-            _add(deps, deps_select, dep, None)
+            _add(deps, deps_select, dep)
             return
 
-    platforms_to_add = {}
-    for plat in platforms:
-        if plat in platforms_to_add:
-            # marker evaluation is more expensive than this check
-            continue
-
-        added = False
-        for extra in extras:
-            if added:
+    markers = {}
+    for req in reqs:
+        for x in extras:
+            m = evaluate(req.marker, env = {"extra": x}, strict = False)
+            if m == False:
+                continue
+            elif m == True:
+                _add(deps, deps_select, dep)
                 break
+            else:
+                markers[m] = None
+                continue
 
-            for req in reqs:
-                if evaluate(req.marker, env = env(target_platform = plat, extra = extra)):
-                    platforms_to_add[plat] = True
-                    added = True
-                    break
-
-    if len(platforms_to_add) == len(platforms):
-        # the dep is in all target platforms, let's just add it to the regular
-        # list
-        _add(deps, deps_select, dep, None)
-        return
-
-    for plat in platforms_to_add:
-        if default_abi:
-            _add(deps, deps_select, dep, plat)
-        if plat.abi == default_abi or not default_abi:
-            _add(deps, deps_select, dep, platform(os = plat.os, arch = plat.arch))
+    if markers:
+        _add(deps, deps_select, dep, sorted(markers))

python/private/pypi/whl_installer/wheel_installer.py

@@ -126,7 +126,6 @@ def _extract_wheel(
         _setup_namespace_pkg_compatibility(installation_dir)
 
     metadata = {
-        "python_version": f"{sys.version_info[0]}.{sys.version_info[1]}.{sys.version_info[2]}",
         "entry_points": [
             {
                 "name": name,

python/private/pypi/whl_library.bzl

@@ -22,7 +22,6 @@ load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils")
 load(":attrs.bzl", "ATTRS", "use_isolated")
 load(":deps.bzl", "all_repo_names", "record_files")
 load(":generate_whl_library_build_bazel.bzl", "generate_whl_library_build_bazel")
-load(":parse_requirements.bzl", "host_platform")
 load(":parse_whl_name.bzl", "parse_whl_name")
 load(":patch_whl.bzl", "patch_whl")
 load(":pypi_repo_utils.bzl", "pypi_repo_utils")
@@ -362,7 +361,6 @@ def _whl_library_impl(rctx):
 
         metadata = json.decode(rctx.read("metadata.json"))
         rctx.delete("metadata.json")
-        python_version = metadata["python_version"]
 
         # NOTE @aignas 2024-06-22: this has to live on until we stop supporting
         # passing `twine` as a `:pkg` library via the `WORKSPACE` builds.
@@ -400,9 +398,7 @@ def _whl_library_impl(rctx):
             entry_points = entry_points,
             metadata_name = metadata.name,
             metadata_version = metadata.version,
-            default_python_version = python_version,
             requires_dist = metadata.requires_dist,
-            target_platforms = rctx.attr.experimental_target_platforms or [host_platform(rctx)],
             # TODO @aignas 2025-04-14: load through the hub:
             annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))),
             data_exclude = rctx.attr.pip_data_exclude,