diff --git a/CHANGELOG.md b/CHANGELOG.md index e5748704b6..ac5c8146df 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/python/private/pypi/simpleapi_download.bzl b/python/private/pypi/simpleapi_download.bzl index ff18887ec1..20d79ba9b4 100644 --- a/python/private/pypi/simpleapi_download.bzl +++ b/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 ) diff --git a/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl b/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl index 953df5c107..e43041e9b1 100644 --- a/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl +++ b/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl @@ -23,13 +23,14 @@ _tests = [] def _test_simple(env): calls = [] - def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block): + def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block, allow_fail): _ = ctx # buildifier: disable=unused-variable _ = attr _ = cache _ = get_auth _ = versions 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 +97,14 @@ def _test_fail(env): calls = [] fails = [] - def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block): + def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block, allow_fail): _ = ctx # buildifier: disable=unused-variable _ = attr _ = cache _ = get_auth _ = versions 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 +132,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 +149,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 +159,86 @@ 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 # buildifier: disable=unused-variable + _ = attr + _ = cache + _ = get_auth + _ = versions + 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 = {}