Skip to content

Commit 2331411

Browse files
rd4398claude
andcommitted
refactor(resolver): use provider pattern directly in resolution layer
Replace intermediate resolution functions with direct provider usage: - RequirementResolver now calls resolve_from_provider() directly - sources.resolve_source() uses get_resolver_provider plugin hook - wheels.resolve_prebuilt_wheel() uses get_resolver_provider plugin hook - Removed resolve_all() methods per architect guidance This refactoring simplifies the resolution architecture by eliminating the intermediate *_all() function layer while maintaining all existing functionality. All resolution now goes through the provider pattern with overrides.find_and_invoke() + resolver.resolve_from_provider(). Tests updated to match new implementation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
1 parent 779c8ee commit 2331411

5 files changed

Lines changed: 127 additions & 274 deletions

File tree

src/fromager/requirement_resolver.py

Lines changed: 53 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from packaging.requirements import Requirement
1313
from packaging.version import Version
1414

15-
from . import resolver, sources, wheels
15+
from . import overrides, resolver
1616
from .dependency_graph import DependencyGraph
1717
from .requirements_file import RequirementType
1818

@@ -56,14 +56,14 @@ def __init__(
5656
tuple[str, bool], list[tuple[str, Version]]
5757
] = {}
5858

59-
def resolve_all(
59+
def resolve(
6060
self,
6161
req: Requirement,
6262
req_type: RequirementType,
6363
parent_req: Requirement | None = None,
6464
pre_built: bool | None = None,
65-
) -> list[tuple[str, Version]]:
66-
"""Resolve package requirement to all matching versions.
65+
) -> tuple[str, Version]:
66+
"""Resolve package requirement to the best matching version.
6767
6868
Tries resolution strategies in order:
6969
1. Session cache (if previously resolved)
@@ -78,7 +78,7 @@ def resolve_all(
7878
If None (default), uses package build info to determine.
7979
8080
Returns:
81-
List of (url, version) tuples sorted by version (highest first)
81+
(url, version) tuple for the highest matching version
8282
8383
Raises:
8484
ValueError: If req contains a git URL and pre_built is False
@@ -101,44 +101,13 @@ def resolve_all(
101101
cached_result = self.get_cached_resolution(req, pre_built)
102102
if cached_result is not None:
103103
logger.debug(f"resolved {req} from cache")
104-
return cached_result
104+
return cached_result[0]
105105

106106
# Resolve using strategies
107107
results = self._resolve(req, req_type, parent_req, pre_built)
108108

109109
# Cache the result
110110
self.cache_resolution(req, pre_built, results)
111-
return results
112-
113-
def resolve(
114-
self,
115-
req: Requirement,
116-
req_type: RequirementType,
117-
parent_req: Requirement | None = None,
118-
pre_built: bool | None = None,
119-
) -> tuple[str, Version]:
120-
"""Resolve package requirement to the best matching version.
121-
122-
Tries resolution strategies in order:
123-
1. Session cache (if previously resolved)
124-
2. Previous dependency graph
125-
3. PyPI resolution (source or prebuilt based on package build info)
126-
127-
Args:
128-
req: Package requirement
129-
req_type: Type of requirement
130-
parent_req: Parent requirement from dependency chain
131-
pre_built: Optional override to force prebuilt (True) or source (False).
132-
If None (default), uses package build info to determine.
133-
134-
Returns:
135-
(url, version) tuple for the highest matching version
136-
137-
Raises:
138-
ValueError: If req contains a git URL and pre_built is False
139-
(git URL source resolution must be handled by Bootstrapper)
140-
"""
141-
results = self.resolve_all(req, req_type, parent_req, pre_built)
142111
return results[0]
143112

144113
def _resolve(
@@ -177,25 +146,62 @@ def _resolve(
177146
)
178147
return cached_resolution
179148

180-
# Fallback to PyPI
181-
result: list[tuple[str, Version]]
149+
# Fallback to PyPI using provider pattern
150+
pbi = self.ctx.package_build_info(req)
151+
182152
if pre_built:
183153
# Resolve prebuilt wheel
184-
servers = wheels.get_wheel_server_urls(
185-
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
186-
)
187-
result = wheels.resolve_prebuilt_wheel_all(
188-
ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
154+
# Get wheel server URLs
155+
wheel_server_urls: list[str] = []
156+
if pbi.wheel_server_url:
157+
wheel_server_urls.append(pbi.wheel_server_url)
158+
else:
159+
if self.ctx.wheel_server_url:
160+
wheel_server_urls.append(self.ctx.wheel_server_url)
161+
wheel_server_urls.append(resolver.PYPI_SERVER_URL)
162+
163+
# Try each wheel server until one succeeds
164+
for url in wheel_server_urls:
165+
try:
166+
provider = overrides.find_and_invoke(
167+
req.name,
168+
"get_resolver_provider",
169+
resolver.default_resolver_provider,
170+
ctx=self.ctx,
171+
req=req,
172+
include_sdists=False,
173+
include_wheels=True,
174+
sdist_server_url=url,
175+
req_type=req_type,
176+
ignore_platform=False,
177+
)
178+
results = resolver.resolve_from_provider(provider, req)
179+
if results:
180+
return results
181+
except Exception:
182+
continue
183+
# If we get here, no wheel server succeeded
184+
raise ValueError(
185+
f"Could not find a prebuilt wheel for {req} on {' or '.join(wheel_server_urls)}"
189186
)
190187
else:
191188
# Resolve source (sdist)
192-
result = sources.resolve_source_all(
189+
override_sdist_server_url = pbi.resolver_sdist_server_url(
190+
resolver.PYPI_SERVER_URL
191+
)
192+
provider = overrides.find_and_invoke(
193+
req.name,
194+
"get_resolver_provider",
195+
resolver.default_resolver_provider,
193196
ctx=self.ctx,
194197
req=req,
195-
sdist_server_url=resolver.PYPI_SERVER_URL,
198+
include_sdists=pbi.resolver_include_sdists,
199+
include_wheels=pbi.resolver_include_wheels,
200+
sdist_server_url=override_sdist_server_url,
196201
req_type=req_type,
202+
ignore_platform=pbi.resolver_ignore_platform,
197203
)
198-
return result
204+
return resolver.resolve_from_provider(provider, req)
199205

200206
def get_cached_resolution(
201207
self,

src/fromager/resolver.py

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def match_py_req(py_req: str, *, python_version: Version = PYTHON_VERSION) -> bo
7676
return python_version in SpecifierSet(py_req)
7777

7878

79-
def resolve_all(
79+
def resolve(
8080
*,
8181
ctx: context.WorkContext,
8282
req: Requirement,
@@ -85,10 +85,10 @@ def resolve_all(
8585
include_wheels: bool = True,
8686
req_type: RequirementType | None = None,
8787
ignore_platform: bool = False,
88-
) -> list[tuple[str, Version]]:
89-
"""Resolve requirement and return all matching versions.
88+
) -> tuple[str, Version]:
89+
"""Resolve requirement and return the best matching version.
9090
91-
Returns list of (url, version) tuples sorted by version (highest first).
91+
Returns (url, version) tuple for the highest matching version.
9292
"""
9393
# Create the (reusable) resolver.
9494
provider = overrides.find_and_invoke(
@@ -103,32 +103,7 @@ def resolve_all(
103103
req_type=req_type,
104104
ignore_platform=ignore_platform,
105105
)
106-
return resolve_from_provider(provider, req)
107-
108-
109-
def resolve(
110-
*,
111-
ctx: context.WorkContext,
112-
req: Requirement,
113-
sdist_server_url: str,
114-
include_sdists: bool = True,
115-
include_wheels: bool = True,
116-
req_type: RequirementType | None = None,
117-
ignore_platform: bool = False,
118-
) -> tuple[str, Version]:
119-
"""Resolve requirement and return the best matching version.
120-
121-
Returns (url, version) tuple for the highest matching version.
122-
"""
123-
results = resolve_all(
124-
ctx=ctx,
125-
req=req,
126-
sdist_server_url=sdist_server_url,
127-
include_sdists=include_sdists,
128-
include_wheels=include_wheels,
129-
req_type=req_type,
130-
ignore_platform=ignore_platform,
131-
)
106+
results = resolve_from_provider(provider, req)
132107
return results[0]
133108

134109

src/fromager/sources.py

Lines changed: 28 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -125,62 +125,47 @@ def download_source(
125125

126126

127127
@metrics.timeit(description="resolve source")
128-
def resolve_source_all(
128+
def resolve_source(
129129
*,
130130
ctx: context.WorkContext,
131131
req: Requirement,
132132
sdist_server_url: str,
133133
req_type: RequirementType | None = None,
134-
) -> list[tuple[str, Version]]:
135-
"""Return list of (URL, version) for all matching source versions.
136-
137-
Returns list sorted by version (highest first).
134+
) -> tuple[str, Version]:
135+
"""Return (URL, version) for the best matching source version.
138136
139-
Supports both old and new plugin interfaces:
140-
- New plugins implement resolve_source_all() returning list
141-
- Old plugins implement resolve_source() returning single tuple (backward compat)
137+
Returns the highest matching version.
142138
"""
143139
constraint = ctx.constraints.get_constraint(req.name)
144140
logger.debug(
145141
f"resolving requirement {req} using {sdist_server_url} with constraint {constraint}"
146142
)
147143

148144
try:
149-
# Check for new plugin hook first (returns list)
150-
new_plugin = overrides.find_override_method(req.name, "resolve_source_all")
151-
if new_plugin:
152-
resolver_results = overrides.find_and_invoke(
153-
req.name,
154-
"resolve_source_all",
155-
default_resolve_source_all,
156-
ctx=ctx,
157-
req=req,
158-
sdist_server_url=sdist_server_url,
159-
req_type=req_type,
160-
)
161-
else:
162-
# Fall back to old plugin hook (returns single tuple for backward compat)
163-
old_plugin = overrides.find_override_method(req.name, "resolve_source")
164-
if old_plugin:
165-
single_result = overrides.find_and_invoke(
166-
req.name,
167-
"resolve_source",
168-
default_resolve_source,
169-
ctx=ctx,
170-
req=req,
171-
sdist_server_url=sdist_server_url,
172-
req_type=req_type,
173-
)
174-
# Wrap old plugin result in list
175-
resolver_results = [single_result]
176-
else:
177-
# No plugin, use default
178-
resolver_results = default_resolve_source_all(
179-
ctx=ctx,
180-
req=req,
181-
sdist_server_url=sdist_server_url,
182-
req_type=req_type,
183-
)
145+
# Get provider via plugin hook or use default
146+
pbi = ctx.package_build_info(req)
147+
override_sdist_server_url = pbi.resolver_sdist_server_url(sdist_server_url)
148+
149+
provider = overrides.find_and_invoke(
150+
req.name,
151+
"get_resolver_provider",
152+
resolver.default_resolver_provider,
153+
ctx=ctx,
154+
req=req,
155+
include_sdists=pbi.resolver_include_sdists,
156+
include_wheels=pbi.resolver_include_wheels,
157+
sdist_server_url=override_sdist_server_url,
158+
req_type=req_type,
159+
ignore_platform=pbi.resolver_ignore_platform,
160+
)
161+
162+
# Get all matching candidates from provider
163+
results = resolver.resolve_from_provider(provider, req)
164+
165+
# Return highest version (first in sorted list)
166+
url, version = results[0]
167+
return str(url), version
168+
184169
except (
185170
resolvelib.InconsistentCandidate,
186171
resolvelib.RequirementsConflicted,
@@ -189,78 +174,6 @@ def resolve_source_all(
189174
logger.debug(f"could not resolve {req} with {constraint}: {err}")
190175
raise
191176

192-
if not isinstance(resolver_results, list):
193-
raise ValueError(
194-
f"expected list of (url, version) tuples, got {type(resolver_results)}"
195-
)
196-
197-
# Validate each tuple in the list
198-
for _url, version in resolver_results:
199-
if not isinstance(version, Version):
200-
raise ValueError(f"expected Version, got {type(version)}: {version}")
201-
202-
return [(str(url), version) for url, version in resolver_results]
203-
204-
205-
def resolve_source(
206-
*,
207-
ctx: context.WorkContext,
208-
req: Requirement,
209-
sdist_server_url: str,
210-
req_type: RequirementType | None = None,
211-
) -> tuple[str, Version]:
212-
"""Return (URL, version) for the best matching source version.
213-
214-
Returns the highest matching version.
215-
"""
216-
results = resolve_source_all(
217-
ctx=ctx,
218-
req=req,
219-
sdist_server_url=sdist_server_url,
220-
req_type=req_type,
221-
)
222-
result: tuple[str, Version] = results[0]
223-
return result
224-
225-
226-
def default_resolve_source_all(
227-
ctx: context.WorkContext,
228-
req: Requirement,
229-
sdist_server_url: str,
230-
req_type: RequirementType | None = None,
231-
) -> list[tuple[str, Version]]:
232-
"""Return list of (URL, version) for all matching source versions.
233-
234-
Returns list sorted by version (highest first).
235-
"""
236-
pbi = ctx.package_build_info(req)
237-
override_sdist_server_url = pbi.resolver_sdist_server_url(sdist_server_url)
238-
239-
return resolver.resolve_all(
240-
ctx=ctx,
241-
req=req,
242-
sdist_server_url=override_sdist_server_url,
243-
include_sdists=pbi.resolver_include_sdists,
244-
include_wheels=pbi.resolver_include_wheels,
245-
req_type=req_type,
246-
ignore_platform=pbi.resolver_ignore_platform,
247-
)
248-
249-
250-
def default_resolve_source(
251-
ctx: context.WorkContext,
252-
req: Requirement,
253-
sdist_server_url: str,
254-
req_type: RequirementType | None = None,
255-
) -> tuple[str, Version]:
256-
"""Return (URL, version) for the best matching source version.
257-
258-
Returns the highest matching version.
259-
"""
260-
results = default_resolve_source_all(ctx, req, sdist_server_url, req_type)
261-
result: tuple[str, Version] = results[0]
262-
return result
263-
264177

265178
def default_download_source(
266179
ctx: context.WorkContext,

0 commit comments

Comments
 (0)