fix(pypi): correctly handle different package versions

After merging bazel-contrib#3058, I wanted to try creating a `universal` requirements
file for our `sphinx` setup and have an integration test in that way. It
seems that `alabaster` has different versions for different python
versions and we were not handling this correctly. In bazel-contrib#3058 I have
attempted adding a unit test for this but it escaped me that the only
time where this bug manifests itself is when the `parse_requirements` is
used multiple times, once per each `python_version`.

With this I think it is safe to say that the bazel-contrib#2797 is fixed, because we
found a bug, where we were not skipping requirements.

As an added counter measure, I have added an extra check elsewhere to
catch a regression.

Fixes bazel-contrib#2797

Author: aignas

CHANGELOG.md

@@ -120,6 +120,9 @@ END_UNRELEASED_TEMPLATE
   installations (Mac frameworks, missing dynamic libraries, and other
   esoteric cases, see
   [#3148](https://github.com/bazel-contrib/rules_python/pull/3148) for details).
+* (pypi) Support `requirements.txt` files that use different versions of the same
+  package targeting different target platforms.
+  ([#2797](https://github.com/bazel-contrib/rules_python/issues/2797)).
 
 {#v0-0-0-added}
 ### Added

MODULE.bazel

@@ -241,21 +241,25 @@ dev_pip = use_extension(
     "pip",
     dev_dependency = True,
 )
-dev_pip.parse(
-    download_only = True,
-    experimental_index_url = "https://pypi.org/simple",
-    hub_name = "dev_pip",
-    parallel_download = False,
-    python_version = "3.11",
-    requirements_lock = "//docs:requirements.txt",
-)
-dev_pip.parse(
-    download_only = True,
-    experimental_index_url = "https://pypi.org/simple",
-    hub_name = "dev_pip",
-    python_version = "3.13",
-    requirements_lock = "//docs:requirements.txt",
-)
+
+[
+    dev_pip.parse(
+        download_only = True,
+        experimental_index_url = "https://pypi.org/simple",
+        hub_name = "dev_pip",
+        parallel_download = False,
+        python_version = python_version,
+        requirements_lock = "//docs:requirements.txt",
+    )
+    for python_version in [
+        "3.9",
+        "3.10",
+        "3.11",
+        "3.12",
+        "3.13",
+    ]
+]
+
 dev_pip.parse(
     download_only = True,
     experimental_index_url = "https://pypi.org/simple",

docs/BUILD.bazel

@@ -186,5 +186,6 @@ lock(
         "--universal",
         "--upgrade",
     ],
+    python_version = "3.9",  # select the lowest version
     visibility = ["//:__subpackages__"],
 )

docs/requirements.txt

@@ -346,7 +346,17 @@ def _create_whl_repos(
                 ))
 
             whl_libraries[repo_name] = repo.args
-            whl_map.setdefault(whl.name, {})[repo.config_setting] = repo_name
+            mapping = whl_map.setdefault(whl.name, {})
+            if repo.config_setting in mapping and mapping[repo.config_setting] != repo_name:
+                fail(
+                    "attempting to override an existing repo '{}' for config setting '{}' with a new repo '{}'".format(
+                        mapping[repo.config_setting],
+                        repo.config_setting,
+                        repo_name,
+                    ),
+                )
+            else:
+                mapping[repo.config_setting] = repo_name
 
     return struct(
         whl_map = whl_map,

python/private/pypi/extension.bzl

@@ -89,8 +89,7 @@ def parse_requirements(
     options = {}
     requirements = {}
     for file, plats in requirements_by_platform.items():
-        if logger:
-            logger.trace(lambda: "Using {} for {}".format(file, plats))
+        logger.trace(lambda: "Using {} for {}".format(file, plats))
         contents = ctx.read(file)
 
         # Parse the requirements file directly in starlark to get the information
@@ -162,11 +161,10 @@ def parse_requirements(
     # URL of the files to download things from. This should be important for
     # VCS package references.
     env_marker_target_platforms = evaluate_markers(ctx, reqs_with_env_markers)
-    if logger:
-        logger.trace(lambda: "Evaluated env markers from:\n{}\n\nTo:\n{}".format(
-            reqs_with_env_markers,
-            env_marker_target_platforms,
-        ))
+    logger.trace(lambda: "Evaluated env markers from:\n{}\n\nTo:\n{}".format(
+        reqs_with_env_markers,
+        env_marker_target_platforms,
+    ))
 
     index_urls = {}
     if get_index_urls:
@@ -212,8 +210,7 @@ def parse_requirements(
                 sorted(requirements),
             ))
 
-    if logger:
-        logger.debug(lambda: "Will configure whl repos: {}".format([w.name for w in ret]))
+    logger.debug(lambda: "Will configure whl repos: {}".format([w.name for w in ret]))
 
     return ret
 
@@ -229,7 +226,10 @@ def _package_srcs(
     """A function to return sources for a particular package."""
     srcs = {}
     for r in sorted(reqs.values(), key = lambda r: r.requirement_line):
-        target_platforms = env_marker_target_platforms.get(r.requirement_line, r.target_platforms)
+        if ";" in r.requirement_line:
+            target_platforms = env_marker_target_platforms.get(r.requirement_line, [])
+        else:
+            target_platforms = r.target_platforms
         extra_pip_args = tuple(r.extra_pip_args)
 
         for target_platform in target_platforms:
@@ -245,8 +245,7 @@ def _package_srcs(
                 index_urls = index_urls.get(name),
                 logger = logger,
             )
-            if logger:
-                logger.debug(lambda: "The whl dist is: {}".format(dist.filename if dist else dist))
+            logger.debug(lambda: "The whl dist is: {}".format(dist.filename if dist else dist))
 
             if extract_url_srcs and dist:
                 req_line = r.srcs.requirement
@@ -347,10 +346,9 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
 
     if requirement.srcs.url:
         if not requirement.srcs.filename:
-            if logger:
-                logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format(
-                    requirement.srcs.url,
-                ))
+            logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format(
+                requirement.srcs.url,
+            ))
             return None
 
         # Handle direct URLs in requirements
@@ -377,8 +375,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
     if not shas_to_use:
         version = requirement.srcs.version
         shas_to_use = index_urls.sha256s_by_version.get(version, [])
-        if logger:
-            logger.warn(lambda: "requirement file has been generated without hashes, will use all hashes for the given version {} that could find on the index:\n    {}".format(version, shas_to_use))
+        logger.warn(lambda: "requirement file has been generated without hashes, will use all hashes for the given version {} that could find on the index:\n    {}".format(version, shas_to_use))
 
     for sha256 in shas_to_use:
         # For now if the artifact is marked as yanked we just ignore it.
@@ -395,8 +392,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
             sdist = maybe_sdist
             continue
 
-        if logger:
-            logger.warn(lambda: "Could not find a whl or an sdist with sha256={}".format(sha256))
+        logger.warn(lambda: "Could not find a whl or an sdist with sha256={}".format(sha256))
 
     yanked = {}
     for dist in whls + [sdist]:

python/private/pypi/parse_requirements.bzl

@@ -639,7 +639,7 @@ def _test_get_index_urls_different_versions(env):
         platforms = {
             "cp310_linux_x86_64": struct(
                 env = pep508_env(
-                    python_version = "3.9.0",
+                    python_version = "3.10.0",
                     os = "linux",
                     arch = "x86_64",
                 ),
@@ -686,6 +686,7 @@ def _test_get_index_urls_different_versions(env):
                 ),
             },
         ),
+        debug = True,
     )
 
     env.expect.that_collection(got).contains_exactly([
@@ -720,6 +721,70 @@ def _test_get_index_urls_different_versions(env):
 
 _tests.append(_test_get_index_urls_different_versions)
 
+def _test_get_index_urls_single_py_version(env):
+    got = parse_requirements(
+        requirements_by_platform = {
+            "requirements_multi_version": [
+                "cp310_linux_x86_64",
+            ],
+        },
+        platforms = {
+            "cp310_linux_x86_64": struct(
+                env = pep508_env(
+                    python_version = "3.10.0",
+                    os = "linux",
+                    arch = "x86_64",
+                ),
+                whl_abi_tags = ["none"],
+                whl_platform_tags = ["any"],
+            ),
+        },
+        get_index_urls = lambda _, __: {
+            "foo": struct(
+                sdists = {},
+                whls = {
+                    "deadb11f": struct(
+                        url = "super2",
+                        sha256 = "deadb11f",
+                        filename = "foo-0.0.2-py3-none-any.whl",
+                        yanked = False,
+                    ),
+                },
+            ),
+        },
+        evaluate_markers = lambda _, requirements: evaluate_markers(
+            requirements = requirements,
+            platforms = {
+                "cp310_linux_x86_64": struct(
+                    env = {"python_full_version": "3.10.0"},
+                ),
+            },
+        ),
+        debug = True,
+    )
+
+    env.expect.that_collection(got).contains_exactly([
+        struct(
+            is_exposed = True,
+            is_multiple_versions = True,
+            name = "foo",
+            srcs = [
+                struct(
+                    distribution = "foo",
+                    extra_pip_args = [],
+                    filename = "foo-0.0.2-py3-none-any.whl",
+                    requirement_line = "foo==0.0.2",
+                    sha256 = "deadb11f",
+                    target_platforms = ["cp310_linux_x86_64"],
+                    url = "super2",
+                    yanked = False,
+                ),
+            ],
+        ),
+    ])
+
+_tests.append(_test_get_index_urls_single_py_version)
+
 def parse_requirements_test_suite(name):
     """Create the test suite.