From 2eb4005a306eb5291936c3b35b0a9263a645decf Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 20 Apr 2026 17:59:27 +0900 Subject: [PATCH 1/3] fix(pypi): correctly write the used facts back Summary: 1. Added 3 new tests to pypi_cache_tests.bzl 2. Fixed `_cache` helper to handle both struct and dict return values 3. Fixed `_filter_packages`: Returns None for empty dict results instead of empty dict 4. Fixed `_get_from_facts`: Now stores facts when reading index_urls from the dict case Fixes #3711 Work towards #3707 --- python/private/pypi/pypi_cache.bzl | 8 +- tests/pypi/pypi_cache/pypi_cache_tests.bzl | 98 ++++++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/python/private/pypi/pypi_cache.bzl b/python/private/pypi/pypi_cache.bzl index 7b24102263..972bbe04ae 100644 --- a/python/private/pypi/pypi_cache.bzl +++ b/python/private/pypi/pypi_cache.bzl @@ -128,11 +128,12 @@ def _filter_packages(dists, requested_versions): return dists if type(dists) == "dict": - return { + result = { pkg: url for pkg, url in dists.items() if pkg in requested_versions } + return result if result else None sha256s_by_version = {} whls = {} @@ -206,10 +207,13 @@ def _get_from_facts(facts, known_facts, index_url, requested_versions, facts_ver return None if type(requested_versions) == "dict": - return _filter_packages( + result = _filter_packages( dists = known_facts.get("index_urls", {}).get(index_url, {}), requested_versions = requested_versions, ) + if result: + _store_facts(facts, facts_version, index_url, result) + return result known_sources = {} diff --git a/tests/pypi/pypi_cache/pypi_cache_tests.bzl b/tests/pypi/pypi_cache/pypi_cache_tests.bzl index 59ef661ab7..89ed5693e2 100644 --- a/tests/pypi/pypi_cache/pypi_cache_tests.bzl +++ b/tests/pypi/pypi_cache/pypi_cache_tests.bzl @@ -20,6 +20,9 @@ def _cache(env, **kwargs): if not value: return env.expect.that_str(value) + if type(value) == "dict": + return env.expect.that_dict(value) + return env.expect.that_struct( value, attrs = attrs, @@ -266,6 +269,101 @@ def _test_pypi_cache_reads_from_facts(env): _tests.append(_test_pypi_cache_reads_from_facts) +def _test_memory_cache_index_urls(env): + """Verifies that the cache returns stored values for index_urls.""" + store = {} + cache = _cache(env, mctx = None, store = store) + + fake_result = { + "pkg-a": "https://pypi.org/simple/pkg-a/", + "pkg_b": "https://pypi.org/simple/pkg-b/", + } + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None, "pkg_b": None}) + + cache.setdefault(key, fake_result) + + got = cache.get(key) + got.contains_exactly(fake_result) + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None}) + got = cache.get(key) + got.contains_exactly({"pkg-a": "https://pypi.org/simple/pkg-a/"}) + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-c": None}) + cache.get(key).equals(None) + +_tests.append(_test_memory_cache_index_urls) + +def _test_pypi_cache_writes_index_urls_to_facts(env): + """Verifies that setting index_urls in the cache also populates the facts store.""" + mock_ctx = mocks.mctx(facts = {}) + cache = _cache(env, mctx = mock_ctx) + + fake_result = { + "pkg-a": "https://pypi.org/simple/pkg-a/", + "pkg_b": "https://pypi.org/simple/pkg-b/", + } + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None}) + + cache.setdefault(key, fake_result) + + cache.get_facts().contains_exactly({ + "fact_version": "v1", + "index_urls": { + "https://pypi.org/simple/": { + "pkg-a": "https://pypi.org/simple/pkg-a/", + }, + }, + }) + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg_b": None}) + cache.setdefault(key, fake_result) + + cache.get_facts().contains_exactly({ + "fact_version": "v1", + "index_urls": { + "https://pypi.org/simple/": { + "pkg-a": "https://pypi.org/simple/pkg-a/", + "pkg_b": "https://pypi.org/simple/pkg-b/", + }, + }, + }) + +_tests.append(_test_pypi_cache_writes_index_urls_to_facts) + +def _test_pypi_cache_reads_index_urls_from_facts(env): + """Verifies that reading index_urls from facts works correctly.""" + mock_ctx = mocks.mctx(facts = { + "fact_version": "v1", + "index_urls": { + "https://pypi.org/simple/": { + "pkg-a": "https://pypi.org/simple/pkg-a/", + "pkg-b": "https://pypi.org/simple/pkg-b/", + }, + }, + }) + cache = _cache(env, mctx = mock_ctx) + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None}) + got = cache.get(key) + got.contains_exactly({"pkg-a": "https://pypi.org/simple/pkg-a/"}) + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None, "pkg-b": None}) + got = cache.get(key) + got.contains_exactly({ + "pkg-a": "https://pypi.org/simple/pkg-a/", + "pkg-b": "https://pypi.org/simple/pkg-b/", + }) + + key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-c": None}) + cache.get(key).equals(None) + + cache.get_facts().contains_exactly(mock_ctx.facts) + +_tests.append(_test_pypi_cache_reads_index_urls_from_facts) + def pypi_cache_test_suite(name): test_suite( name = name, From f14f63080db978be8eb9166bd711347785f7a70a Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 20 Apr 2026 18:26:08 +0900 Subject: [PATCH 2/3] Update python/private/pypi/pypi_cache.bzl Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- python/private/pypi/pypi_cache.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/private/pypi/pypi_cache.bzl b/python/private/pypi/pypi_cache.bzl index 972bbe04ae..a6e5b7f9e5 100644 --- a/python/private/pypi/pypi_cache.bzl +++ b/python/private/pypi/pypi_cache.bzl @@ -129,9 +129,9 @@ def _filter_packages(dists, requested_versions): if type(dists) == "dict": result = { - pkg: url - for pkg, url in dists.items() - if pkg in requested_versions + pkg: dists[pkg] + for pkg in requested_versions + if pkg in dists } return result if result else None From 0482daf823297f027dcb6a08688f67838493fcc7 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 20 Apr 2026 18:26:50 +0900 Subject: [PATCH 3/3] Revert "Update python/private/pypi/pypi_cache.bzl" This reverts commit f14f63080db978be8eb9166bd711347785f7a70a. Can't do that because the requested packages may come from anywhere, so the dict access may fail. --- python/private/pypi/pypi_cache.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/private/pypi/pypi_cache.bzl b/python/private/pypi/pypi_cache.bzl index a6e5b7f9e5..972bbe04ae 100644 --- a/python/private/pypi/pypi_cache.bzl +++ b/python/private/pypi/pypi_cache.bzl @@ -129,9 +129,9 @@ def _filter_packages(dists, requested_versions): if type(dists) == "dict": result = { - pkg: dists[pkg] - for pkg in requested_versions - if pkg in dists + pkg: url + for pkg, url in dists.items() + if pkg in requested_versions } return result if result else None