From 20999a054d9fa28de7301fe0de43439ab3a216ec Mon Sep 17 00:00:00 2001 From: Shanmukh Pawan Date: Tue, 12 May 2026 18:13:48 -0400 Subject: [PATCH] feat(wheels): separate cache wheel resolution from pre-built path The local wheel server and --cache-wheel-server-url only hold wheels Fromager already built. Cooldown was applied when the sdist was resolved. The cache upload-time is when we uploaded the wheel, not when the package was published, so --min-release-age rejects a wheel we should reuse. A package wheel_server_url is an upstream pre-built index, so that lookup stays on the pre-built path and cooldown still applies. Follows up on #1143 and closes #1140. Co-Authored-By: Claude Co-Authored-By: Cursor Grok 4.7 Signed-off-by: Shanmukh Pawan --- src/fromager/commands/build.py | 69 +++- src/fromager/commands/download_sequence.py | 29 +- src/fromager/wheels.py | 32 +- tests/test_cooldown.py | 383 +++++++++++++++++++++ 4 files changed, 478 insertions(+), 35 deletions(-) diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index 2e33f8f0e..67f96439a 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -346,17 +346,13 @@ def _build( pbi = wkctx.package_build_info(req) prebuilt = pbi.pre_built - wheel_server_urls = wheels.get_wheel_server_urls( - wkctx, req, cache_wheel_server_url=cache_wheel_server_url - ) - # See if we can reuse an existing wheel. if not force: wheel_filename = _is_wheel_built( wkctx, req.name, resolved_version, - wheel_server_urls, + cache_wheel_server_url=cache_wheel_server_url, ) if wheel_filename: logger.info("using existing wheel from %s", wheel_filename) @@ -469,23 +465,62 @@ def _is_wheel_built( wkctx: context.WorkContext, dist_name: str, resolved_version: Version, - wheel_server_urls: list[str], + *, + cache_wheel_server_url: str | None = None, ) -> pathlib.Path | None: req = Requirement(f"{dist_name}=={resolved_version}") + pbi = wkctx.package_build_info(req) + # No package index and no local or job cache means there is nothing to reuse. + if ( + not pbi.wheel_server_url + and not wkctx.wheel_server_url + and not cache_wheel_server_url + ): + logger.debug( + "no wheel server configured for %s, skipping existing-wheel check", + req, + ) + return None try: + # Package wheel_server_url is exclusive: it replaces local and cache. + servers = wheels.get_wheel_server_urls( + wkctx, + req, + cache_wheel_server_url=cache_wheel_server_url, + ) logger.info( "checking if a suitable wheel for %s was already built on %s", req, - wheel_server_urls, - ) - url, _ = wheels.resolve_prebuilt_wheel( - ctx=wkctx, - req=req, - wheel_server_urls=wheel_server_urls, + servers, ) + + url: str | None = None + for server_url in servers: + try: + if pbi.wheel_server_url: + # Upstream pre-built index. Release-age cooldown stays on. + url, _ = wheels.resolve_prebuilt_wheel( + ctx=wkctx, + req=req, + wheel_server_urls=[server_url], + ) + else: + # Local server and --cache-wheel-server-url are trusted caches. + url, _ = wheels.resolve_cached_wheel( + ctx=wkctx, + req=req, + cache_server_url=server_url, + ) + break + except Exception: + logger.debug("wheel not found on %s", server_url, exc_info=True) + + if url is None: + logger.info("could not locate existing wheel") + return None + logger.info("found candidate wheel %s", url) - pbi = wkctx.package_build_info(req) build_tag_from_settings = pbi.build_tag(resolved_version) build_tag = build_tag_from_settings if build_tag_from_settings else (0, "") wheel_basename = downloads.extract_filename_from_url(url) @@ -505,7 +540,7 @@ def _is_wheel_built( return None wheel_filename: pathlib.Path | None = None - if url.startswith(wkctx.wheel_server_url): + if wkctx.wheel_server_url and url.startswith(wkctx.wheel_server_url): logging.debug("found wheel on local server") wheel_filename = wkctx.wheels_downloads / wheel_basename if not wheel_filename.exists(): @@ -513,20 +548,18 @@ def _is_wheel_built( wheel_filename = None if not wheel_filename: - # if the found wheel was on an external server, then download it logger.info("downloading wheel from %s", url) wheel_filename = wheels.download_wheel(req, url, wkctx.wheels_downloads) return wheel_filename except Exception: logger.debug( - "could not locate prebuilt wheel %s-%s on %s", + "could not locate existing wheel %s-%s", dist_name, resolved_version, - wheel_server_urls, exc_info=True, ) - logger.info("could not locate prebuilt wheel") + logger.info("could not locate existing wheel") return None diff --git a/src/fromager/commands/download_sequence.py b/src/fromager/commands/download_sequence.py index 9409c4b35..177440fd5 100644 --- a/src/fromager/commands/download_sequence.py +++ b/src/fromager/commands/download_sequence.py @@ -51,10 +51,13 @@ def download_sequence( the build order file that have source_url_type as sdist. """ - if wkctx.wheel_server_url: - wheel_servers = [wkctx.wheel_server_url] - else: - wheel_servers = [sdist_server_url] + # The local wheel server is a trusted cache (wheels built in this or + # prior runs). An external sdist_server_url used as a wheel fallback + # is an upstream index that needs cooldown and override hooks. + use_cache_resolution = bool(wkctx.wheel_server_url) + wheel_server_url = ( + wkctx.wheel_server_url if use_cache_resolution else sdist_server_url + ) logger.info("reading build order from %s", build_order_file) with read.open_file_or_url(build_order_file) as f: @@ -82,7 +85,6 @@ def download_one(entry: dict[str, typing.Any]) -> None: except Exception as err: logger.error(f"failed to download sdist for {req}: {err}") if not ignore_missing_sdists: - # Re-raise with package context since context var is lost across threads raise RuntimeError(f"Failed to download sdist for {req}") from err else: logger.info( @@ -91,12 +93,21 @@ def download_one(entry: dict[str, typing.Any]) -> None: if include_wheels: try: - wheel_url, _ = wheels.resolve_prebuilt_wheel( - ctx=wkctx, req=req, wheel_server_urls=wheel_servers - ) + if use_cache_resolution: + resolved_url, _ = wheels.resolve_cached_wheel( + ctx=wkctx, + req=req, + cache_server_url=wheel_server_url, + ) + else: + resolved_url, _ = wheels.resolve_prebuilt_wheel( + ctx=wkctx, + req=req, + wheel_server_urls=[wheel_server_url], + ) wheels.download_wheel( req=req, - wheel_url=wheel_url, + wheel_url=resolved_url, output_directory=wkctx.wheels_downloads, ) except Exception as err: diff --git a/src/fromager/wheels.py b/src/fromager/wheels.py index dc9bd5241..6c3535e56 100644 --- a/src/fromager/wheels.py +++ b/src/fromager/wheels.py @@ -24,6 +24,7 @@ dependencies, downloads, external_commands, + finders, metrics, overrides, packagesettings, @@ -508,6 +509,26 @@ def get_prebuilt_wheel_provider( ) +@metrics.timeit(description="resolve wheel") +def resolve_cached_wheel( + *, + ctx: context.WorkContext, + req: Requirement, + cache_server_url: str, +) -> tuple[str, Version]: + """Resolve a wheel from a trusted cache server (local or remote). + + Uses ``PyPICacheProvider`` -- no cooldown, no hooks, no upload-time checks. + """ + provider = finders.PyPICacheProvider( + cache_server_url=cache_server_url, + constraints=ctx.constraints, + ) + results = resolver.find_all_matching_from_provider(provider, req) + wheel_url, version = results[0] + return str(wheel_url), version + + def resolve_all_prebuilt_wheels( *, ctx: context.WorkContext, @@ -532,11 +553,6 @@ def resolve_all_prebuilt_wheels( provider.cooldown = resolver.resolve_package_cooldown( ctx, req, req_type=req_type ) - # The local fromager wheel server is PEP 503-only and serves - # packages that were already resolved and vetted earlier in the - # same run. Don't fail-closed on missing upload_time there. - if ctx.wheel_server_url and url == ctx.wheel_server_url: - provider.supports_upload_time = False # Get all matching candidates from provider results = resolver.find_all_matching_from_provider(provider, req) @@ -559,10 +575,10 @@ def resolve_prebuilt_wheel( wheel_server_urls: list[str], req_type: requirements_file.RequirementType | None = None, ) -> tuple[str, Version]: - """Return (URL, version) for the best matching wheel version. + """Return (URL, version) for the best matching pre-built wheel. - Tries wheel servers in order and returns result from the first that succeeds. - Returns the highest matching version. + Tries wheel servers in order and returns the highest matching version + from the first server that succeeds. """ results = resolve_all_prebuilt_wheels( ctx=ctx, req=req, wheel_server_urls=wheel_server_urls, req_type=req_type diff --git a/tests/test_cooldown.py b/tests/test_cooldown.py index 5ced99785..dfa1305d7 100644 --- a/tests/test_cooldown.py +++ b/tests/test_cooldown.py @@ -27,6 +27,7 @@ sources, wheels, ) +from fromager.commands import build as build_command from fromager.requirements_file import RequirementType _BOOTSTRAP_TIME = datetime.datetime(2026, 3, 26, 0, 0, 0, tzinfo=datetime.UTC) @@ -699,6 +700,388 @@ def test_local_wheel_server_allows_without_upload_time( assert "cooldown cannot be enforced" not in caplog.text +def test_local_wheel_server_skips_cooldown_entirely( + tmp_path: pathlib.Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """resolve_cached_wheel() disables cooldown for the local wheel server. + + The local fromager wheel server serves packages that were already resolved + and built earlier in the same run. They are trusted, so resolve_cached_wheel + uses PyPICacheProvider (cooldown=None) and no cooldown-related warnings are + emitted. + """ + local_server_url = "http://127.0.0.1:9999/simple/" + ctx = context.WorkContext( + active_settings=None, + patches_dir=tmp_path / "patches", + sdists_repo=tmp_path / "sdists-repo", + wheels_repo=tmp_path / "wheels-repo", + work_dir=tmp_path / "work-dir", + cooldown=_COOLDOWN, + ) + + no_timestamp_response = { + "meta": {"api-version": "1.1"}, + "name": "test-pkg", + "files": [ + { + "filename": "test_pkg-1.3.2-py3-none-any.whl", + "url": f"{local_server_url}test-pkg/test_pkg-1.3.2-py3-none-any.whl", + "hashes": {"sha256": "bbb"}, + }, + ], + } + with caplog.at_level(logging.WARNING, logger="fromager.resolver"): + with requests_mock.Mocker() as r: + r.get( + f"{local_server_url}test-pkg/", + json=no_timestamp_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + _url, version = wheels.resolve_cached_wheel( + ctx=ctx, + req=Requirement("test-pkg"), + cache_server_url=local_server_url, + ) + + assert str(version) == "1.3.2" + assert "cooldown" not in caplog.text.lower() + + +def test_cache_wheel_server_skips_cooldown_entirely( + tmp_path: pathlib.Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """resolve_cached_wheel() disables cooldown for the cache wheel server. + + The cache wheel server contains wheels from previous fromager runs that + were already vetted during original sdist resolution. Cooldown is not + applicable. + """ + cache_server_url = "https://registry.test/packages/pypi/simple/" + ctx = context.WorkContext( + active_settings=None, + patches_dir=tmp_path / "patches", + sdists_repo=tmp_path / "sdists-repo", + wheels_repo=tmp_path / "wheels-repo", + work_dir=tmp_path / "work-dir", + cooldown=_COOLDOWN, + ) + + no_timestamp_response = { + "meta": {"api-version": "1.1"}, + "name": "test-pkg", + "files": [ + { + "filename": "test_pkg-1.3.2-py3-none-any.whl", + "url": f"{cache_server_url}test-pkg/test_pkg-1.3.2-py3-none-any.whl", + "hashes": {"sha256": "bbb"}, + }, + ], + } + with caplog.at_level(logging.WARNING, logger="fromager.resolver"): + with requests_mock.Mocker() as r: + r.get( + f"{cache_server_url}test-pkg/", + json=no_timestamp_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + _url, version = wheels.resolve_cached_wheel( + ctx=ctx, + req=Requirement("test-pkg"), + cache_server_url=cache_server_url, + ) + + assert str(version) == "1.3.2" + assert "cooldown" not in caplog.text.lower() + + +def test_non_cache_server_retains_cooldown( + tmp_path: pathlib.Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """resolve_all_prebuilt_wheels() keeps cooldown active for non-cache servers. + + When a server URL is an external/upstream index (not local or cache), + cooldown remains active. Candidates without upload timestamps trigger a + warning because the server does not support upload_time. + """ + external_server_url = "https://external.test/simple/" + ctx = context.WorkContext( + active_settings=None, + patches_dir=tmp_path / "patches", + sdists_repo=tmp_path / "sdists-repo", + wheels_repo=tmp_path / "wheels-repo", + work_dir=tmp_path / "work-dir", + cooldown=_COOLDOWN, + ) + + no_timestamp_response = { + "meta": {"api-version": "1.1"}, + "name": "test-pkg", + "files": [ + { + "filename": "test_pkg-1.3.2-py3-none-any.whl", + "url": f"{external_server_url}test-pkg/test_pkg-1.3.2-py3-none-any.whl", + "hashes": {"sha256": "bbb"}, + }, + ], + } + with caplog.at_level(logging.WARNING, logger="fromager.resolver"): + with requests_mock.Mocker() as r: + r.get( + f"{external_server_url}test-pkg/", + json=no_timestamp_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + results = wheels.resolve_all_prebuilt_wheels( + ctx=ctx, + req=Requirement("test-pkg"), + wheel_server_urls=[external_server_url], + ) + + assert len(results) == 1 + _, version = results[0] + assert str(version) == "1.3.2" + assert "cooldown cannot be enforced" in caplog.text + + +_LOCAL_WHEEL_SERVER = "http://127.0.0.1:8080/simple/" +_CACHE_WHEEL_SERVER = "https://cache.test/simple/" +_PACKAGE_WHEEL_SERVER = "https://custom.test/simple/" +_RECENT_UPLOAD_TIME = "2026-03-25T00:00:00+00:00" +_OLD_UPLOAD_TIME = "2026-03-15T00:00:00+00:00" + + +def _simple_wheel_index( + server_url: str, + *, + upload_time: str | None = None, + version: str = "1.3.2", +) -> dict[str, typing.Any]: + filename = f"test_pkg-{version}-py3-none-any.whl" + file_entry: dict[str, typing.Any] = { + "filename": filename, + "url": f"{server_url}test-pkg/{filename}", + "hashes": {"sha256": "bbb"}, + } + if upload_time is not None: + file_entry["upload-time"] = upload_time + return { + "meta": {"api-version": "1.1"}, + "name": "test-pkg", + "files": [file_entry], + } + + +def _empty_index() -> dict[str, typing.Any]: + return {"meta": {"api-version": "1.1"}, "name": "test-pkg", "files": []} + + +def _context_for_existing_wheel( + tmp_path: pathlib.Path, + *, + package_wheel_server_url: str | None = None, +) -> context.WorkContext: + settings_dir = tmp_path / "settings" + settings_dir.mkdir() + if package_wheel_server_url is not None: + (settings_dir / "test-pkg.yaml").write_text( + f"variants:\n cpu:\n wheel_server_url: {package_wheel_server_url}\n", + encoding="utf-8", + ) + settings = packagesettings.Settings.from_files( + settings_file=tmp_path / "settings.yaml", + settings_dir=settings_dir, + variant="cpu", + patches_dir=tmp_path / "patches", + max_jobs=None, + ) + return context.WorkContext( + active_settings=settings, + patches_dir=tmp_path / "patches", + sdists_repo=tmp_path / "sdists-repo", + wheels_repo=tmp_path / "wheels-repo", + work_dir=tmp_path / "work-dir", + wheel_server_url=_LOCAL_WHEEL_SERVER, + cooldown=_COOLDOWN, + ) + + +def _record_download( + seen: list[str], +) -> typing.Callable[[Requirement, str, pathlib.Path], pathlib.Path]: + def _download( + req: Requirement, + wheel_url: str, + output_directory: pathlib.Path, + ) -> pathlib.Path: + del req + seen.append(wheel_url) + output_directory.mkdir(parents=True, exist_ok=True) + path = output_directory / wheel_url.rsplit("/", 1)[-1] + path.write_bytes(b"") + return path + + return _download + + +def test_is_wheel_built_uses_package_wheel_server_url( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A package wheel_server_url is the only index consulted. + + The job cache and local server are not fallbacks. A missing upload + timestamp warns and still allows the wheel, because this path uses the + pre-built resolver rather than the trusted cache. + """ + ctx = _context_for_existing_wheel( + tmp_path, + package_wheel_server_url=_PACKAGE_WHEEL_SERVER, + ) + seen: list[str] = [] + monkeypatch.setattr(build_command.wheels, "download_wheel", _record_download(seen)) + + with caplog.at_level(logging.WARNING, logger="fromager.resolver"): + with requests_mock.Mocker() as r: + r.get( + f"{_PACKAGE_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index(_PACKAGE_WHEEL_SERVER), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + r.get( + f"{_CACHE_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index( + _CACHE_WHEEL_SERVER, + upload_time=_OLD_UPLOAD_TIME, + ), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + r.get( + f"{_LOCAL_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index(_LOCAL_WHEEL_SERVER), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + found = build_command._is_wheel_built( + ctx, + "test-pkg", + Version("1.3.2"), + cache_wheel_server_url=_CACHE_WHEEL_SERVER, + ) + + assert found is not None + assert found.name == "test_pkg-1.3.2-py3-none-any.whl" + assert seen == [f"{_PACKAGE_WHEEL_SERVER}test-pkg/test_pkg-1.3.2-py3-none-any.whl"] + requested = [request.url for request in r.request_history] + assert any(url.startswith(_PACKAGE_WHEEL_SERVER) for url in requested) + assert all(not url.startswith(_CACHE_WHEEL_SERVER) for url in requested) + assert all(not url.startswith(_LOCAL_WHEEL_SERVER) for url in requested) + assert "cooldown cannot be enforced" in caplog.text + + +def test_is_wheel_built_package_wheel_server_keeps_cooldown( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A too-new wheel on a package index is rejected and the cache is not used. + + The cache holds an older matching wheel. Falling through to it would hide + the cooldown that must stay on for an upstream pre-built index. + """ + ctx = _context_for_existing_wheel( + tmp_path, + package_wheel_server_url=_PACKAGE_WHEEL_SERVER, + ) + seen: list[str] = [] + monkeypatch.setattr(build_command.wheels, "download_wheel", _record_download(seen)) + + with caplog.at_level(logging.WARNING, logger="fromager.resolver"): + with requests_mock.Mocker() as r: + r.get( + f"{_PACKAGE_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index( + _PACKAGE_WHEEL_SERVER, + upload_time=_RECENT_UPLOAD_TIME, + ), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + r.get( + f"{_CACHE_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index( + _CACHE_WHEEL_SERVER, + upload_time=_OLD_UPLOAD_TIME, + ), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + r.get( + f"{_LOCAL_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index( + _LOCAL_WHEEL_SERVER, + upload_time=_OLD_UPLOAD_TIME, + ), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + found = build_command._is_wheel_built( + ctx, + "test-pkg", + Version("1.3.2"), + cache_wheel_server_url=_CACHE_WHEEL_SERVER, + ) + + assert found is None + assert seen == [] + requested = [request.url for request in r.request_history] + assert any(url.startswith(_PACKAGE_WHEEL_SERVER) for url in requested) + assert all(not url.startswith(_CACHE_WHEEL_SERVER) for url in requested) + assert all(not url.startswith(_LOCAL_WHEEL_SERVER) for url in requested) + assert "cooldown blocked" in caplog.text + + +def test_is_wheel_built_cache_ignores_recent_upload_time( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """The job cache is trusted even when its upload-time is inside the cooldown. + + Pulp reports the time the artifact was added to the index, not the PyPI + publish date. That timestamp must not cause a cached wheel to be rebuilt. + """ + ctx = _context_for_existing_wheel(tmp_path) + seen: list[str] = [] + monkeypatch.setattr(build_command.wheels, "download_wheel", _record_download(seen)) + + with caplog.at_level(logging.WARNING, logger="fromager.resolver"): + with requests_mock.Mocker() as r: + r.get( + f"{_LOCAL_WHEEL_SERVER}test-pkg/", + json=_empty_index(), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + r.get( + f"{_CACHE_WHEEL_SERVER}test-pkg/", + json=_simple_wheel_index( + _CACHE_WHEEL_SERVER, + upload_time=_RECENT_UPLOAD_TIME, + ), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + found = build_command._is_wheel_built( + ctx, + "test-pkg", + Version("1.3.2"), + cache_wheel_server_url=_CACHE_WHEEL_SERVER, + ) + + assert found is not None + assert seen == [f"{_CACHE_WHEEL_SERVER}test-pkg/test_pkg-1.3.2-py3-none-any.whl"] + assert "cooldown" not in caplog.text.lower() + + # --------------------------------------------------------------------------- # max-release-age tests # ---------------------------------------------------------------------------