Skip to content

Commit 5aa8c93

Browse files
andre-mottaclaude
andcommitted
fix(resolver): prefer sdists in cache server fallback
`_resolve_from_cache_server` queried for both wheels and sdists, picking whichever had the higher version. When only a wheel existed, the wheel URL was returned as `source_url` but written to build-order.json with `source_url_type: "sdist"`, causing downstream consumers to fail on the `.whl` basename. Now only sdists are queried. If the cache server has only wheels for the package, an empty list is returned — the package is already built and does not need to appear in the build order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Andre Lustosa <alustosa@redhat.com>
1 parent ed6da67 commit 5aa8c93

2 files changed

Lines changed: 56 additions & 30 deletions

File tree

src/fromager/bootstrap_requirement_resolver.py

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -189,40 +189,45 @@ def resolve(
189189
return [results[0]]
190190

191191
def _resolve_from_cache_server(self, req: Requirement) -> list[tuple[str, Version]]:
192-
"""Fall back to the remote wheel cache server for a cached version.
192+
"""Fall back to the remote wheel cache server for a cached sdist.
193193
194194
When age filtering removes all candidates in multi-version mode,
195-
queries the remote cache server for the newest available wheel.
195+
queries the remote cache server for the newest available sdist.
196196
Returns at most one version so that transitive dependencies are
197197
re-processed without rebuilding every old version.
198+
199+
Only sdists are returned. If the cache server has only wheels for
200+
the package, an empty list is returned because the package is
201+
already built and does not need to appear in the build order.
198202
"""
199203
logger.info(
200204
"checking cache server %s for existing build",
201205
self.cache_wheel_server_url,
202206
)
203-
best: tuple[str, Version] | None = None
204-
for include_sdists, include_wheels in [(False, True), (True, False)]:
205-
try:
206-
provider = finders.PyPICacheProvider(
207-
cache_server_url=self.cache_wheel_server_url,
208-
constraints=self.ctx.constraints,
209-
include_sdists=include_sdists,
210-
include_wheels=include_wheels,
211-
)
212-
results = resolver.find_all_matching_from_provider(provider, req)
213-
if results:
214-
url, version = results[0]
215-
if best is None or version > best[1]:
216-
best = (url, version)
217-
except Exception as err:
218-
logger.warning(
219-
"error checking cache server %s: %s",
220-
self.cache_wheel_server_url,
221-
err,
222-
)
223-
if best is not None:
224-
logger.info("found version %s on cache server", best[1])
225-
return [best]
207+
try:
208+
provider = finders.PyPICacheProvider(
209+
cache_server_url=self.cache_wheel_server_url,
210+
constraints=self.ctx.constraints,
211+
include_sdists=True,
212+
include_wheels=False,
213+
)
214+
results = resolver.find_all_matching_from_provider(provider, req)
215+
if results:
216+
url, version = results[0]
217+
logger.info("found sdist version %s on cache server", version)
218+
return [(url, version)]
219+
except Exception as err:
220+
logger.warning(
221+
"error checking cache server %s: %s",
222+
self.cache_wheel_server_url,
223+
err,
224+
)
225+
226+
logger.debug(
227+
"no sdist found on cache server %s for %s",
228+
self.cache_wheel_server_url,
229+
req.name,
230+
)
226231
return []
227232

228233
def get_cached_resolution(

tests/test_bootstrap_requirement_resolver.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -699,8 +699,8 @@ def test_resolve_prebuilt_after_source_uses_separate_cache(
699699
class TestResolveFromCacheServer:
700700
"""Tests for the cache server fallback in _resolve_from_cache_server."""
701701

702-
def test_returns_newest_version_from_cache(self, tmp_context: WorkContext) -> None:
703-
"""Falls back to cache server and returns only the newest version."""
702+
def test_returns_newest_sdist_from_cache(self, tmp_context: WorkContext) -> None:
703+
"""Falls back to cache server and returns only the newest sdist."""
704704
brr = BootstrapRequirementResolver(
705705
tmp_context,
706706
multiple_versions=True,
@@ -712,14 +712,35 @@ def test_returns_newest_version_from_cache(self, tmp_context: WorkContext) -> No
712712
"fromager.bootstrap_requirement_resolver.resolver"
713713
".find_all_matching_from_provider",
714714
return_value=[
715-
("http://cache.test/testpkg-3.0.whl", Version("3.0")),
716-
("http://cache.test/testpkg-2.0.whl", Version("2.0")),
715+
("http://cache.test/testpkg-3.0.tar.gz", Version("3.0")),
716+
("http://cache.test/testpkg-2.0.tar.gz", Version("2.0")),
717717
],
718718
):
719719
result = brr._resolve_from_cache_server(req)
720720

721721
assert len(result) == 1
722722
assert result[0][1] == Version("3.0")
723+
assert result[0][0].endswith(".tar.gz")
724+
725+
def test_returns_empty_when_only_wheels_on_cache(
726+
self, tmp_context: WorkContext
727+
) -> None:
728+
"""Returns empty list when cache server has only wheels (already built)."""
729+
brr = BootstrapRequirementResolver(
730+
tmp_context,
731+
multiple_versions=True,
732+
cache_wheel_server_url="http://cache.test/simple",
733+
)
734+
req = Requirement("testpkg")
735+
736+
with patch(
737+
"fromager.bootstrap_requirement_resolver.resolver"
738+
".find_all_matching_from_provider",
739+
return_value=[],
740+
):
741+
result = brr._resolve_from_cache_server(req)
742+
743+
assert result == []
723744

724745
def test_returns_empty_when_cache_has_no_match(
725746
self, tmp_context: WorkContext
@@ -783,7 +804,7 @@ def test_resolve_uses_cache_fallback_when_age_filter_empties(
783804
patch.object(
784805
brr,
785806
"_resolve_from_cache_server",
786-
return_value=[("http://cache.test/testpkg-1.0.whl", Version("1.0"))],
807+
return_value=[("http://cache.test/testpkg-1.0.tar.gz", Version("1.0"))],
787808
) as mock_cache,
788809
):
789810
result = brr.resolve(

0 commit comments

Comments
 (0)