fix(pypi): propagate fails if overrides are passed only one index is used

CHANGELOG.md

@@ -67,6 +67,10 @@ END_UNRELEASED_TEMPLATE
 Other changes:
 * (pypi) Update dependencies used for `compile_pip_requirements`, building
   sdists in the `whl_library` rule and fetching wheels using `pip`.
+* (pypi) We will set `allow_fail` to `False` if the {attr}`experimental_index_url_overrides` is set
+  to a non-empty value. This means that failures will be no-longer cached in this particular case.
+  ([#3260](https://github.com/bazel-contrib/rules_python/issues/3260) and 
+  [#2632](https://github.com/bazel-contrib/rules_python/issues/2632))
 
 {#v0-0-0-fixed}
 ### Fixed

python/private/pypi/simpleapi_download.bzl

@@ -71,16 +71,21 @@ def simpleapi_download(
         for p, i in (attr.index_url_overrides or {}).items()
     }
 
-    download_kwargs = {}
-    if bazel_features.external_deps.download_has_block_param:
-        download_kwargs["block"] = not parallel_download
-
     # NOTE @aignas 2024-03-31: we are not merging results from multiple indexes
     # to replicate how `pip` would handle this case.
     contents = {}
     index_urls = [attr.index_url] + attr.extra_index_urls
     read_simpleapi = read_simpleapi or _read_simpleapi
 
+    download_kwargs = {}
+    if bazel_features.external_deps.download_has_block_param:
+        download_kwargs["block"] = not parallel_download
+
+    if len(index_urls) == 1 or index_url_overrides:
+        download_kwargs["allow_fail"] = False
+    else:
+        download_kwargs["allow_fail"] = True
+
     input_sources = attr.sources
 
     found_on_index = {}
@@ -225,7 +230,6 @@ def _read_simpleapi(ctx, url, attr, cache, versions, get_auth = None, **download
         url = [real_url],
         output = output,
         auth = get_auth(ctx, [real_url], ctx_attr = attr),
-        allow_fail = True,
         **download_kwargs
     )
 

tests/pypi/simpleapi_download/simpleapi_download_tests.bzl

@@ -23,13 +23,10 @@ _tests = []
 def _test_simple(env):
     calls = []
 
-    def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block):
-        _ = ctx  # buildifier: disable=unused-variable
-        _ = attr
-        _ = cache
-        _ = get_auth
-        _ = versions
+    def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block, allow_fail):
+        _ = ctx, attr, cache, get_auth, versions  # buildifier: disable=unused-variable
         env.expect.that_bool(block).equals(False)
+        env.expect.that_bool(allow_fail).equals(True)
         calls.append(url)
         if "foo" in url and "main" in url:
             return struct(
@@ -96,13 +93,10 @@ def _test_fail(env):
     calls = []
     fails = []
 
-    def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block):
-        _ = ctx  # buildifier: disable=unused-variable
-        _ = attr
-        _ = cache
-        _ = get_auth
-        _ = versions
+    def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block, allow_fail):
+        _ = ctx, attr, cache, get_auth, versions  # buildifier: disable=unused-variable
         env.expect.that_bool(block).equals(False)
+        env.expect.that_bool(allow_fail).equals(True)
         calls.append(url)
         if "foo" in url:
             return struct(
@@ -130,9 +124,7 @@ def _test_fail(env):
             report_progress = lambda _: None,
         ),
         attr = struct(
-            index_url_overrides = {
-                "foo": "invalid",
-            },
+            index_url_overrides = {},
             index_url = "main",
             extra_index_urls = ["extra"],
             sources = {"bar": None, "baz": None, "foo": None},
@@ -149,7 +141,7 @@ def _test_fail(env):
 Failed to download metadata of the following packages from urls:
 {
     "bar": ["main", "extra"],
-    "foo": "invalid",
+    "foo": ["main", "extra"],
 }
 
 If you would like to skip downloading metadata for these packages please add 'simpleapi_skip=[
@@ -159,15 +151,82 @@ If you would like to skip downloading metadata for these packages please add 'si
 """,
     ])
     env.expect.that_collection(calls).contains_exactly([
-        "invalid/foo/",
+        "main/foo/",
         "main/bar/",
         "main/baz/",
-        "invalid/foo/",
+        "extra/foo/",
         "extra/bar/",
     ])
 
 _tests.append(_test_fail)
 
+def _test_allow_fail_single_index(env):
+    calls = []
+    fails = []
+
+    def read_simpleapi(ctx, *, url, versions, attr, cache, get_auth, block, allow_fail):
+        _ = ctx, attr, cache, get_auth, versions  # buildifier: disable=unused-variable
+        env.expect.that_bool(block).equals(False)
+        env.expect.that_bool(allow_fail).equals(False)
+        calls.append(url)
+        return struct(
+            output = struct(
+                sdists = {"deadbeef": url.strip("/").split("/")[-1]},
+                whls = {"deadb33f": url.strip("/").split("/")[-1]},
+                sha256s_by_version = {"fizz": url.strip("/").split("/")[-1]},
+            ),
+            success = True,
+        )
+
+    contents = simpleapi_download(
+        ctx = struct(
+            getenv = {}.get,
+            report_progress = lambda _: None,
+        ),
+        attr = struct(
+            index_url_overrides = {
+                "foo": "extra",
+            },
+            index_url = "main",
+            extra_index_urls = [],
+            sources = {"bar": None, "baz": None, "foo": None},
+            envsubst = [],
+        ),
+        cache = pypi_cache(),
+        parallel_download = True,
+        read_simpleapi = read_simpleapi,
+        _fail = fails.append,
+    )
+
+    env.expect.that_collection(fails).contains_exactly([])
+    env.expect.that_collection(calls).contains_exactly([
+        "main/bar/",
+        "main/baz/",
+        "extra/foo/",
+    ])
+    env.expect.that_dict(contents).contains_exactly({
+        "bar": struct(
+            index_url = "main/bar/",
+            sdists = {"deadbeef": "bar"},
+            sha256s_by_version = {"fizz": "bar"},
+            whls = {"deadb33f": "bar"},
+        ),
+        "baz": struct(
+            index_url = "main/baz/",
+            sdists = {"deadbeef": "baz"},
+            sha256s_by_version = {"fizz": "baz"},
+            whls = {"deadb33f": "baz"},
+        ),
+        "foo": struct(
+            index_url = "extra/foo/",
+            sdists = {"deadbeef": "foo"},
+            sha256s_by_version = {"fizz": "foo"},
+            whls = {"deadb33f": "foo"},
+        ),
+    })
+
+_tests.append(_test_allow_fail_single_index)
+
 def _test_download_url(env):
     downloads = {}