fix(pip): do not add a pip-fallback when there is no sdist (#3432)

Before this PR the user would have to specify `download = True` in order
to not
fallback to pip, which is admittedly an odd interface design. With this
PR we
correctly do not add a `pip` fallback if there is no `sdist` to be used.
With
this in place we are better placed to enable the
`experimental_index_url` by
default. What is more we can more reliably detect when we should use a
special
repository rule to prepare for building from sdist.

Summary:
- Add a test that shows the problem and then adjust the torch
experimental URL
  test.
- Cleanup unused marker stubs in the tests.
- Fix the code to better handle the case when there is no sdist.

Work towards #260
Work towards #2410

Author: aignas

python/private/pypi/parse_requirements.bzl

@@ -188,19 +188,35 @@ def parse_requirements(
             for p in r.target_platforms:
                 requirement_target_platforms[p] = None
 
+        package_srcs = _package_srcs(
+            name = name,
+            reqs = reqs,
+            index_urls = index_urls,
+            platforms = platforms,
+            extract_url_srcs = extract_url_srcs,
+            logger = logger,
+        )
+
+        # FIXME @aignas 2025-11-24: we can get the list of target platforms here
+        #
+        # However it is likely that we may stop exposing packages like torch in here
+        # which do not have wheels for all osx platforms.
+        #
+        # If users specify the target platforms accurately, then it is a different
+        # (better) story, but we may not be able to guarantee this
+        #
+        # target_platforms = [
+        #     p
+        #     for dist in package_srcs
+        #     for p in dist.target_platforms
+        # ]
+
         item = struct(
             # Return normalized names
             name = normalize_name(name),
             is_exposed = len(requirement_target_platforms) == len(requirements),
             is_multiple_versions = len(reqs.values()) > 1,
-            srcs = _package_srcs(
-                name = name,
-                reqs = reqs,
-                index_urls = index_urls,
-                platforms = platforms,
-                extract_url_srcs = extract_url_srcs,
-                logger = logger,
-            ),
+            srcs = package_srcs,
         )
         ret.append(item)
         if not item.is_exposed and logger:
@@ -234,7 +250,7 @@ def _package_srcs(
                     platforms.keys(),
                 ))
 
-            dist = _add_dists(
+            dist, can_fallback = _add_dists(
                 requirement = r,
                 target_platform = platforms.get(target_platform),
                 index_urls = index_urls.get(name),
@@ -244,14 +260,16 @@ def _package_srcs(
 
             if extract_url_srcs and dist:
                 req_line = r.srcs.requirement
-            else:
+            elif can_fallback:
                 dist = struct(
                     url = "",
                     filename = "",
                     sha256 = "",
                     yanked = False,
                 )
                 req_line = r.srcs.requirement_line
+            else:
+                continue
 
             key = (
                 dist.filename,
@@ -337,14 +355,22 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
         index_urls: The result of simpleapi_download.
         target_platform: The target_platform information.
         logger: A logger for printing diagnostic info.
+
+    Returns:
+        (dist, can_fallback_to_pip): a struct with distribution details and how to fetch
+        it and a boolean flag to tell the other layers if we should add an entry to
+        fallback for pip if there are no supported whls found - if there is an sdist, we
+        can attempt the fallback, otherwise better to not, because the pip command will
+        fail and the error message will be confusing. What is more that would lead to
+        breakage of the bazel query.
     """
 
     if requirement.srcs.url:
         if not requirement.srcs.filename:
             logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format(
                 requirement.srcs.url,
             ))
-            return None
+            return None, True
 
         # Handle direct URLs in requirements
         dist = struct(
@@ -354,13 +380,10 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
             yanked = False,
         )
 
-        if dist.filename.endswith(".whl"):
-            return dist
-        else:
-            return dist
+        return dist, False
 
     if not index_urls:
-        return None
+        return None, True
 
     whls = []
     sdist = None
@@ -403,7 +426,14 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
 
     if not target_platform:
         # The pipstar platforms are undefined here, so we cannot do any matching
-        return sdist
+        return sdist, True
+
+    if not whls and not sdist:
+        # If there are no suitable wheels to handle for now allow fallback to pip, it
+        # may be a little bit more helpful when debugging? Most likely something is
+        # going a bit wrong here, should we raise an error because the sha256 have most
+        # likely mismatched? We are already printing a warning above.
+        return None, True
 
     # Select a single wheel that can work on the target_platform
     return select_whl(
@@ -413,4 +443,4 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
         whl_abi_tags = target_platform.whl_abi_tags,
         whl_platform_tags = target_platform.whl_platform_tags,
         logger = logger,
-    ) or sdist
+    ) or sdist, sdist != None

tests/pypi/hub_builder/hub_builder_tests.bzl

@@ -566,6 +566,9 @@ def _test_torch_experimental_index_url(env):
                 for (os, cpu), whl_platform_tags in {
                     ("linux", "x86_64"): ["linux_x86_64", "manylinux_*_x86_64"],
                     ("linux", "aarch64"): ["linux_aarch64", "manylinux_*_aarch64"],
+                    # this should be ignored as well because there is no sdist and no whls
+                    # for intel Macs
+                    ("osx", "x86_64"): ["macosx_*_x86_64"],
                     ("osx", "aarch64"): ["macosx_*_arm64"],
                     ("windows", "x86_64"): ["win_amd64"],
                     ("windows", "aarch64"): ["win_arm64"],  # this should be ignored
@@ -576,15 +579,6 @@ def _test_torch_experimental_index_url(env):
             "python_3_12_host": "unit_test_interpreter_target",
         },
         minor_mapping = {"3.12": "3.12.19"},
-        evaluate_markers_fn = lambda _, requirements, **__: {
-            # todo once 2692 is merged, this is going to be easier to test.
-            key: [
-                platform
-                for platform in platforms
-                if ("x86_64" in platform and "platform_machine ==" in key) or ("x86_64" not in platform and "platform_machine !=" in key)
-            ]
-            for key, platforms in requirements.items()
-        },
         simpleapi_download_fn = mocksimpleapi_download,
     )
     builder.pip_parse(
@@ -621,7 +615,6 @@ torch==2.4.1+cpu ; platform_machine == 'x86_64' \
         _parse(
             hub_name = "pypi",
             python_version = "3.12",
-            download_only = True,
             experimental_index_url = "https://torch.index",
             requirements_lock = "universal.txt",
         ),
@@ -800,6 +793,20 @@ def _test_simple_get_index(env):
         got_simpleapi_download_args.extend(args)
         got_simpleapi_download_kwargs.update(kwargs)
         return {
+            "plat_pkg": struct(
+                whls = {
+                    "deadb44f": struct(
+                        yanked = False,
+                        filename = "plat-pkg-0.0.4-py3-none-linux_x86_64.whl",
+                        sha256 = "deadb44f",
+                        url = "example2.org/index/plat_pkg/",
+                    ),
+                },
+                sdists = {},
+                sha256s_by_version = {
+                    "0.0.4": ["deadb44f"],
+                },
+            ),
             "simple": struct(
                 whls = {
                     "deadb00f": struct(
@@ -850,6 +857,7 @@ some_pkg==0.0.1 @ example-direct.org/some_pkg-0.0.1-py3-none-any.whl \
     --hash=sha256:deadbaaf
 direct_without_sha==0.0.1 @ example-direct.org/direct_without_sha-0.0.1-py3-none-any.whl
 some_other_pkg==0.0.1
+plat_pkg==0.0.4
 pip_fallback==0.0.1
 direct_sdist_without_sha @ some-archive/any-name.tar.gz
 git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef
@@ -873,6 +881,7 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef
         "direct_without_sha",
         "git_dep",
         "pip_fallback",
+        "plat_pkg",
         "simple",
         "some_other_pkg",
         "some_pkg",
@@ -921,6 +930,17 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef
                 ),
             ],
         },
+        "plat_pkg": {
+            "pypi_315_plat_py3_none_linux_x86_64_deadb44f": [
+                whl_config_setting(
+                    target_platforms = [
+                        "cp315_linux_x86_64",
+                        "cp315_linux_x86_64_freethreaded",
+                    ],
+                    version = "3.15",
+                ),
+            ],
+        },
         "simple": {
             "pypi_315_simple_py3_none_any_deadb00f": [
                 whl_config_setting(
@@ -998,6 +1018,15 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef
             "python_interpreter_target": "unit_test_interpreter_target",
             "requirement": "pip_fallback==0.0.1",
         },
+        "pypi_315_plat_py3_none_linux_x86_64_deadb44f": {
+            "config_load": "@pypi//:config.bzl",
+            "dep_template": "@pypi//{name}:{target}",
+            "filename": "plat-pkg-0.0.4-py3-none-linux_x86_64.whl",
+            "python_interpreter_target": "unit_test_interpreter_target",
+            "requirement": "plat_pkg==0.0.4",
+            "sha256": "deadb44f",
+            "urls": ["example2.org/index/plat_pkg/"],
+        },
         "pypi_315_simple_py3_none_any_deadb00f": {
             "config_load": "@pypi//:config.bzl",
             "dep_template": "@pypi//{name}:{target}",
@@ -1036,7 +1065,7 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef
                 index_url = "pypi.org",
                 index_url_overrides = {},
                 netrc = None,
-                sources = ["simple", "pip_fallback", "some_other_pkg"],
+                sources = ["simple", "plat_pkg", "pip_fallback", "some_other_pkg"],
             ),
             "cache": {},
             "parallel_download": False,
@@ -1048,14 +1077,6 @@ _tests.append(_test_simple_get_index)
 def _test_optimum_sys_platform_extra(env):
     builder = hub_builder(
         env,
-        evaluate_markers_fn = lambda _, requirements, **__: {
-            key: [
-                platform
-                for platform in platforms
-                if ("darwin" in key and "osx" in platform) or ("linux" in key and "linux" in platform)
-            ]
-            for key, platforms in requirements.items()
-        },
     )
     builder.pip_parse(
         _mock_mctx(