update CHANGELOG and the implementation

aignas · aignas · commit 9a6079cd474e · 2025-03-29T22:48:38.000+09:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,7 +61,11 @@ Unreleased changes template.
 
 {#v0-0-0-added}
 ### Added
-* (pypi) Added version-based fetching in `_add_dists` when SHA256 hashes are unavailable, with unit tests in `//tests/pypi/parse_requirements`[#2023](https://github.com/bazel-contrib/rules_python/issues/2023).
+* (pypi) From now on `sha256` values in the `requirements.txt` is no longer
+  mandatory when enabling {attr}`pip.parse.experimental_index_url` feature.
+  This means that `rules_python` will attempt to fetch metadata for all
+  packages through SimpleAPI unless they are pulled through direct URL
+  references. Fixes [#2023](https://github.com/bazel-contrib/rules_python/issues/2023).
 
 {#v0-0-0-removed}
 ### Removed
diff --git a/docs/pypi-dependencies.md b/docs/pypi-dependencies.md
@@ -386,11 +386,13 @@ This does not mean that `rules_python` is fetching the wheels eagerly, but it
 rather means that it is calling the PyPI server to get the Simple API response
 to get the list of all available source and wheel distributions. Once it has
 got all of the available distributions, it will select the right ones depending
-on the `sha256` values in your `requirements_lock.txt` file. The compatible
-distribution URLs will be then written to the `MODULE.bazel.lock` file. Currently
-users wishing to use the lock file with `rules_python` with this feature have
-to set an environment variable `RULES_PYTHON_OS_ARCH_LOCK_FILE=0` which will
-become default in the next release.
+on the `sha256` values in your `requirements_lock.txt` file. If `sha256` hashes
+are not present in the requirements file, we will fallback to matching by version
+specified in the lock file. The compatible distribution URLs will be then
+written to the `MODULE.bazel.lock` file. Currently users wishing to use the
+lock file with `rules_python` with this feature have to set an environment
+variable `RULES_PYTHON_OS_ARCH_LOCK_FILE=0` which will become default in the
+next release.
 
 Fetching the distribution information from the PyPI allows `rules_python` to
 know which `whl` should be used on which target platform and it will determine
diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl
@@ -184,7 +184,7 @@ def parse_requirements(
                 req.distribution: None
                 for reqs in requirements_by_platform.values()
                 for req in reqs.values()
-                if req.srcs.shas
+                if not req.srcs.url
             }),
         )
 
@@ -316,45 +316,27 @@ def _add_dists(*, requirement, index_urls, logger = None):
     sdist = None
 
     # First try to find distributions by SHA256 if provided
-    if requirement.srcs.shas:
-        for sha256 in requirement.srcs.shas:
-            # For now if the artifact is marked as yanked we just ignore it.
-            #
-            # See https://packaging.python.org/en/latest/specifications/simple-repository-api/#adding-yank-support-to-the-simple-api
-
-            maybe_whl = index_urls.whls.get(sha256)
-            if maybe_whl and not maybe_whl.yanked:
-                whls.append(maybe_whl)
-                continue
-
-            maybe_sdist = index_urls.sdists.get(sha256)
-            if maybe_sdist and not maybe_sdist.yanked:
-                sdist = maybe_sdist
-                continue
-
-            if logger:
-                logger.warn(lambda: "Could not find a whl or an sdist with sha256={}".format(sha256))
-    else:
-        # If no SHA256s provided, try to find distributions by version
-        version = requirement.srcs.version
-        if version:
-            # Look for wheels matching the version
-            for whl in index_urls.whls.values():
-                # Extract package name from wheel filename (format: package_name-version-python_tag-abi_tag-platform_tag.whl)
-                whl_name = whl.filename.split("-")[0]
-                if whl_name == requirement.distribution and whl.version == version and not whl.yanked:
-                    whls.append(whl)
-
-            # Look for source distributions matching the version
-            for sdist_dist in index_urls.sdists.values():
-                # Extract package name from sdist filename (format: package_name-version.tar.gz or package_name-version.zip)
-                sdist_name = sdist_dist.filename.split("-")[0]
-                if sdist_name == requirement.distribution and sdist_dist.version == version and not sdist_dist.yanked:
-                    sdist = sdist_dist
-                    break
-
-            if not whls and not sdist and logger:
-                logger.warn(lambda: "Could not find any distributions for version={}".format(version))
+    shas_to_use = requirement.srcs.shas
+    if not shas_to_use:
+        shas_to_use = index_urls.sha256s_by_version.get(requirement.srcs.version, [])
+
+    for sha256 in shas_to_use:
+        # For now if the artifact is marked as yanked we just ignore it.
+        #
+        # See https://packaging.python.org/en/latest/specifications/simple-repository-api/#adding-yank-support-to-the-simple-api
+
+        maybe_whl = index_urls.whls.get(sha256)
+        if maybe_whl and not maybe_whl.yanked:
+            whls.append(maybe_whl)
+            continue
+
+        maybe_sdist = index_urls.sdists.get(sha256)
+        if maybe_sdist and not maybe_sdist.yanked:
+            sdist = maybe_sdist
+            continue
+
+        if logger:
+            logger.warn(lambda: "Could not find a whl or an sdist with sha256={}".format(sha256))
 
     yanked = {}
     for dist in whls + [sdist]:
diff --git a/python/private/pypi/parse_simpleapi_html.bzl b/python/private/pypi/parse_simpleapi_html.bzl
@@ -26,6 +26,7 @@ def parse_simpleapi_html(*, url, content):
     Returns:
         A list of structs with:
         * filename: The filename of the artifact.
+        * version: The version of the artifact.
         * url: The URL to download the artifact.
         * sha256: The sha256 of the artifact.
         * metadata_sha256: The whl METADATA sha256 if we can download it. If this is
@@ -51,15 +52,20 @@ def parse_simpleapi_html(*, url, content):
 
     # Each line follows the following pattern
     # <a href="https://...#sha256=..." attribute1="foo" ... attributeN="bar">filename</a><br />
+    sha256_by_version = {}
     for line in lines[1:]:
         dist_url, _, tail = line.partition("#sha256=")
+        dist_url = _absolute_url(url, dist_url)
+
         sha256, _, tail = tail.partition("\"")
 
         # See https://packaging.python.org/en/latest/specifications/simple-repository-api/#adding-yank-support-to-the-simple-api
         yanked = "data-yanked" in line
 
         head, _, _ = tail.rpartition("</a>")
         maybe_metadata, _, filename = head.rpartition(">")
+        version = _version(filename)
+        sha256_by_version.setdefault(version, []).append(sha256)
 
         metadata_sha256 = ""
         metadata_url = ""
@@ -75,7 +81,8 @@ def parse_simpleapi_html(*, url, content):
         if filename.endswith(".whl"):
             whls[sha256] = struct(
                 filename = filename,
-                url = _absolute_url(url, dist_url),
+                version = version,
+                url = dist_url,
                 sha256 = sha256,
                 metadata_sha256 = metadata_sha256,
                 metadata_url = _absolute_url(url, metadata_url) if metadata_url else "",
@@ -84,7 +91,8 @@ def parse_simpleapi_html(*, url, content):
         else:
             sdists[sha256] = struct(
                 filename = filename,
-                url = _absolute_url(url, dist_url),
+                version = version,
+                url = dist_url,
                 sha256 = sha256,
                 metadata_sha256 = "",
                 metadata_url = "",
@@ -94,8 +102,31 @@ def parse_simpleapi_html(*, url, content):
     return struct(
         sdists = sdists,
         whls = whls,
+        sha256_by_version = sha256_by_version,
     )
 
+_SDIST_EXTS = [
+    ".tar",  # handles any compression
+    ".zip",
+]
+
+def _version(filename):
+    # See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#binary-distribution-format
+
+    _, _, tail = filename.partition("-")
+    version, _, _ = tail.partition("-")
+    if version != tail:
+        # The format is {name}-{version}-{whl_specifiers}.whl
+        return version
+
+    # NOTE @aignas 2025-03-29: most of the files are wheels, so this is not the common path
+
+    # {name}-{version}.{ext}
+    for ext in _SDIST_EXTS:
+        version, _, _ = version.partition(ext)  # build or name
+
+    return version
+
 def _get_root_directory(url):
     scheme_end = url.find("://")
     if scheme_end == -1:
diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl
@@ -461,6 +461,21 @@ def _test_simple_get_index(env):
                     ),
                 },
             ),
+            "some_other_pkg": struct(
+                whls = {
+                    "deadb33f": struct(
+                        yanked = False,
+                        filename = "some-other-pkg-0.0.1-py3-none-any.whl",
+                        sha256 = "deadb33f",
+                        url = "example2.org/index/some_other_pkg/",
+                    ),
+                },
+                sdists = {},
+                sha256s_by_version = {
+                    "0.0.1": ["deadb33f"],
+                    "0.0.3": ["deadbeef"],
+                },
+            ),
         }
 
     pypi = _parse_modules(
@@ -485,7 +500,10 @@ def _test_simple_get_index(env):
 simple==0.0.1 \
     --hash=sha256:deadbeef \
     --hash=sha256:deadb00f
-some_pkg==0.0.1
+some_pkg==0.0.1 @ example-direct.org/some_pkg-0.0.1-py3-none-any.whl \
+    --hash=sha256:deadbaaf
+some_other_pkg==0.0.1
+pip_fallback==0.0.1
 """,
             }[x],
         ),
@@ -496,42 +514,71 @@ some_pkg==0.0.1
     )
 
     pypi.is_reproducible().equals(False)
-    pypi.exposed_packages().contains_exactly({"pypi": ["simple", "some_pkg"]})
+    pypi.exposed_packages().contains_exactly({"pypi": ["pip_fallback", "simple", "some_other_pkg", "some_pkg"]})
     pypi.hub_group_map().contains_exactly({"pypi": {}})
     pypi.hub_whl_map().contains_exactly({
         "pypi": {
+            "pip_fallback": {
+                "pypi_315_pip_fallback": [
+                    struct(
+                        config_setting = None,
+                        filename = None,
+                        target_platforms = None,
+                        version = "3.15",
+                    ),
+                ],
+            },
             "simple": {
                 "pypi_315_simple_py3_none_any_deadb00f": [
-                    whl_config_setting(
+                    struct(
+                        config_setting = None,
                         filename = "simple-0.0.1-py3-none-any.whl",
+                        target_platforms = None,
                         version = "3.15",
                     ),
                 ],
                 "pypi_315_simple_sdist_deadbeef": [
-                    whl_config_setting(
+                    struct(
+                        config_setting = None,
                         filename = "simple-0.0.1.tar.gz",
+                        target_platforms = None,
+                        version = "3.15",
+                    ),
+                ],
+            },
+            "some_other_pkg": {
+                "pypi_315_some_py3_none_any_deadb33f": [
+                    struct(
+                        config_setting = None,
+                        filename = "some-other-pkg-0.0.1-py3-none-any.whl",
+                        target_platforms = None,
                         version = "3.15",
                     ),
                 ],
             },
             "some_pkg": {
-                "pypi_315_some_pkg": [whl_config_setting(version = "3.15")],
+                "pypi_315_some_pkg_py3_none_any_deadbaaf": [
+                    struct(
+                        config_setting = None,
+                        filename = "some_pkg-0.0.1-py3-none-any.whl",
+                        target_platforms = None,
+                        version = "3.15",
+                    ),
+                ],
             },
         },
     })
     pypi.whl_libraries().contains_exactly({
+        "pypi_315_pip_fallback": {
+            "dep_template": "@pypi//{name}:{target}",
+            "extra_pip_args": ["--extra-args-for-sdist-building"],
+            "python_interpreter_target": "unit_test_interpreter_target",
+            "repo": "pypi_315",
+            "requirement": "pip_fallback==0.0.1",
+        },
         "pypi_315_simple_py3_none_any_deadb00f": {
             "dep_template": "@pypi//{name}:{target}",
-            "experimental_target_platforms": [
-                "cp315_linux_aarch64",
-                "cp315_linux_arm",
-                "cp315_linux_ppc",
-                "cp315_linux_s390x",
-                "cp315_linux_x86_64",
-                "cp315_osx_aarch64",
-                "cp315_osx_x86_64",
-                "cp315_windows_x86_64",
-            ],
+            "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"],
             "filename": "simple-0.0.1-py3-none-any.whl",
             "python_interpreter_target": "unit_test_interpreter_target",
             "repo": "pypi_315",
@@ -541,16 +588,7 @@ some_pkg==0.0.1
         },
         "pypi_315_simple_sdist_deadbeef": {
             "dep_template": "@pypi//{name}:{target}",
-            "experimental_target_platforms": [
-                "cp315_linux_aarch64",
-                "cp315_linux_arm",
-                "cp315_linux_ppc",
-                "cp315_linux_s390x",
-                "cp315_linux_x86_64",
-                "cp315_osx_aarch64",
-                "cp315_osx_x86_64",
-                "cp315_windows_x86_64",
-            ],
+            "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"],
             "extra_pip_args": ["--extra-args-for-sdist-building"],
             "filename": "simple-0.0.1.tar.gz",
             "python_interpreter_target": "unit_test_interpreter_target",
@@ -559,29 +597,43 @@ some_pkg==0.0.1
             "sha256": "deadbeef",
             "urls": ["example.org"],
         },
-        # We are falling back to regular `pip`
-        "pypi_315_some_pkg": {
+        "pypi_315_some_pkg_py3_none_any_deadbaaf": {
             "dep_template": "@pypi//{name}:{target}",
-            "extra_pip_args": ["--extra-args-for-sdist-building"],
+            "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"],
+            "filename": "some_pkg-0.0.1-py3-none-any.whl",
             "python_interpreter_target": "unit_test_interpreter_target",
             "repo": "pypi_315",
-            "requirement": "some_pkg==0.0.1",
+            "requirement": "some_pkg==0.0.1 @ example-direct.org/some_pkg-0.0.1-py3-none-any.whl --hash=sha256:deadbaaf",
+            "sha256": "deadbaaf",
+            "urls": ["example-direct.org/some_pkg-0.0.1-py3-none-any.whl"],
+        },
+        "pypi_315_some_py3_none_any_deadb33f": {
+            "dep_template": "@pypi//{name}:{target}",
+            "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"],
+            "filename": "some-other-pkg-0.0.1-py3-none-any.whl",
+            "python_interpreter_target": "unit_test_interpreter_target",
+            "repo": "pypi_315",
+            "requirement": "some_other_pkg==0.0.1",
+            "sha256": "deadb33f",
+            "urls": ["example2.org/index/some_other_pkg/"],
         },
     })
     pypi.whl_mods().contains_exactly({})
-    env.expect.that_dict(got_simpleapi_download_kwargs).contains_exactly({
-        "attr": struct(
-            auth_patterns = {},
-            envsubst = {},
-            extra_index_urls = [],
-            index_url = "pypi.org",
-            index_url_overrides = {},
-            netrc = None,
-            sources = ["simple"],
-        ),
-        "cache": {},
-        "parallel_download": False,
-    })
+    env.expect.that_dict(got_simpleapi_download_kwargs).contains_exactly(
+        {
+            "attr": struct(
+                auth_patterns = {},
+                envsubst = {},
+                extra_index_urls = [],
+                index_url = "pypi.org",
+                index_url_overrides = {},
+                netrc = None,
+                sources = ["simple", "pip_fallback", "some_other_pkg"],
+            ),
+            "cache": {},
+            "parallel_download": False,
+        },
+    )
 
 _tests.append(_test_simple_get_index)
 
diff --git a/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl b/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl
