From 7db7b5ab9a701998d5b63d763839ad3cee9a85e0 Mon Sep 17 00:00:00 2001 From: Shanmukh Pawan Date: Mon, 5 Oct 2026 09:00:57 -0400 Subject: [PATCH] fix(build): reuse pre-built wheels that carry a build number Pre-built packages have an empty changelog build tag, which the reuse check treated as 0. A publisher build number higher than 0 then failed the job instead of downloading the wheel. Skip that comparison for pre-built packages and keep it for wheels Fromager builds. Co-Authored-By: Grok Signed-off-by: Shanmukh Pawan --- src/fromager/commands/build.py | 49 ++++++++++-------- tests/test_cooldown.py | 93 +++++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 24 deletions(-) diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index 05284bc3..a5ef79f0 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -535,30 +535,35 @@ def _is_wheel_built( logger.debug("could not parse candidate wheel %s", url, exc_info=True) continue - # Keep hook validation errors visible instead of treating them as - # a cache miss. - expected_tag = wheels.get_build_tag( - ctx=wkctx, - req=req, - version=candidate_version, - wheel_tags=wheel_tags, - ) - build_tag = expected_tag if expected_tag else (0, "") - existing_build_tag = build_tag_from_name if build_tag_from_name else (0, "") - if ( - existing_build_tag[0] > build_tag[0] - and existing_build_tag[1] == build_tag[1] - ): - raise ValueError( - f"{dist_name}: changelog for version {candidate_version} is inconsistent. Found build tag {existing_build_tag} but expected {build_tag}" + # The publisher owns the build number of a pre-built wheel. + # Changelog tags apply only to wheels Fromager builds. + if not pbi.pre_built: + # Keep hook validation errors visible instead of treating them as + # a cache miss. + expected_tag = wheels.get_build_tag( + ctx=wkctx, + req=req, + version=candidate_version, + wheel_tags=wheel_tags, ) - if existing_build_tag != build_tag: - logger.info( - "candidate wheel build tag %s does not match expected build tag %s", - existing_build_tag, - build_tag, + build_tag = expected_tag if expected_tag else (0, "") + existing_build_tag = ( + build_tag_from_name if build_tag_from_name else (0, "") ) - continue + if ( + existing_build_tag[0] > build_tag[0] + and existing_build_tag[1] == build_tag[1] + ): + raise ValueError( + f"{dist_name}: changelog for version {candidate_version} is inconsistent. Found build tag {existing_build_tag} but expected {build_tag}" + ) + if existing_build_tag != build_tag: + logger.info( + "candidate wheel build tag %s does not match expected build tag %s", + existing_build_tag, + build_tag, + ) + continue wheel_filename: pathlib.Path | None = None if wkctx.wheel_server_url and url.startswith(wkctx.wheel_server_url): diff --git a/tests/test_cooldown.py b/tests/test_cooldown.py index 4eee08b9..7a384b4d 100644 --- a/tests/test_cooldown.py +++ b/tests/test_cooldown.py @@ -901,12 +901,18 @@ def _context_for_existing_wheel( tmp_path: pathlib.Path, *, package_wheel_server_url: str | None = None, + pre_built: bool = False, ) -> context.WorkContext: settings_dir = tmp_path / "settings" settings_dir.mkdir() - if package_wheel_server_url is not None: + if package_wheel_server_url is not None or pre_built: + variant_lines = ["variants:", " cpu:"] + if pre_built: + variant_lines.append(" pre_built: true") + if package_wheel_server_url is not None: + variant_lines.append(f" wheel_server_url: {package_wheel_server_url}") (settings_dir / "test-pkg.yaml").write_text( - f"variants:\n cpu:\n wheel_server_url: {package_wheel_server_url}\n", + "\n".join(variant_lines) + "\n", encoding="utf-8", ) settings = packagesettings.Settings.from_files( @@ -1080,6 +1086,89 @@ def test_is_wheel_built_propagates_inconsistent_build_tag( build_command._is_wheel_built(ctx, "test-pkg", version) +def test_is_wheel_built_prebuilt_keeps_publisher_build_tag( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A pre-built wheel with a publisher build number is reused. + + The changelog tag is empty for pre-built packages. That must not reject + a higher build number, and the highest published build number is used. + """ + ctx = _context_for_existing_wheel( + tmp_path, + package_wheel_server_url=_PACKAGE_WHEEL_SERVER, + pre_built=True, + ) + version = Version("1.3.2") + older_name = "test_pkg-1.3.2-1-py3-none-any.whl" + newer_name = "test_pkg-1.3.2-2-py3-none-any.whl" + newer_url = f"{_PACKAGE_WHEEL_SERVER}test-pkg/{newer_name}" + downloaded: list[str] = [] + monkeypatch.setattr( + build_command.wheels, "download_wheel", _record_download(downloaded) + ) + + with requests_mock.Mocker() as r: + r.get( + f"{_PACKAGE_WHEEL_SERVER}test-pkg/", + json=_wheel_index_for_filenames( + _PACKAGE_WHEEL_SERVER, [older_name, newer_name] + ), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + found = build_command._is_wheel_built(ctx, "test-pkg", version) + + assert found is not None + assert found.name == newer_name + assert downloaded == [newer_url] + + +def test_is_wheel_built_prebuilt_reuses_cache_build_tag( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A pre-built package reuses a build-tagged wheel from the job cache. + + No package wheel_server_url is set, so the lookup uses the local server + and --cache-wheel-server-url. The publisher build number must not raise. + """ + ctx = _context_for_existing_wheel(tmp_path, pre_built=True) + version = Version("1.3.2") + wheel_name = "test_pkg-1.3.2-2-py3-none-any.whl" + wheel_url = f"{_CACHE_WHEEL_SERVER}test-pkg/{wheel_name}" + downloaded: list[str] = [] + monkeypatch.setattr( + build_command.wheels, "download_wheel", _record_download(downloaded) + ) + + 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=_wheel_index_for_filenames(_CACHE_WHEEL_SERVER, [wheel_name]), + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + found = build_command._is_wheel_built( + ctx, + "test-pkg", + version, + cache_wheel_server_url=_CACHE_WHEEL_SERVER, + ) + + assert found is not None + assert found.name == wheel_name + assert downloaded == [wheel_url] + assert [request.url for request in r.request_history] == [ + f"{_LOCAL_WHEEL_SERVER}test-pkg/", + f"{_CACHE_WHEEL_SERVER}test-pkg/", + ] + + def test_is_wheel_built_uses_package_wheel_server_url( tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch,