Skip to content

Commit 3eed051

Browse files
committed
(refactor): RequirementsResolver should have only one method resolve()
This commits attempts to refactor the RequirementsResolver to have only one method resolve() instead of separate methods It is consistent with existing code in resolver.py where there is only single method for resolution Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
1 parent c509b13 commit 3eed051

3 files changed

Lines changed: 53 additions & 67 deletions

File tree

src/fromager/bootstrapper.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ def resolve_version(
183183
Delegates PyPI/graph resolution to RequirementResolver.
184184
"""
185185
if req.url:
186-
# Git URLs must be resolved in Bootstrapper (orchestration concern)
187186
if req_type != RequirementType.TOP_LEVEL:
188187
raise ValueError(
189188
f"{req} includes a URL, but is not a top-level dependency"
@@ -201,22 +200,14 @@ def resolve_version(
201200
self._resolver.cache_resolution(req, (source_url, resolved_version))
202201
return source_url, resolved_version
203202

204-
# Delegate to RequirementResolver (handles caching internally)
203+
# Delegate to RequirementResolver
205204
parent_req = self.why[-1][1] if self.why else None
206-
pbi = self.ctx.package_build_info(req)
207205

208-
if pbi.pre_built:
209-
return self._resolver.resolve_prebuilt(
210-
req=req,
211-
req_type=req_type,
212-
parent_req=parent_req,
213-
)
214-
else:
215-
return self._resolver.resolve_source(
216-
req=req,
217-
req_type=req_type,
218-
parent_req=parent_req,
219-
)
206+
return self._resolver.resolve(
207+
req=req,
208+
req_type=req_type,
209+
parent_req=parent_req,
210+
)
220211

221212
def _processing_build_requirement(self, current_req_type: RequirementType) -> bool:
222213
"""Are we currently processing a build requirement?
@@ -927,10 +918,11 @@ def _handle_test_mode_failure(
927918

928919
try:
929920
parent_req = self.why[-1][1] if self.why else None
930-
wheel_url, fallback_version = self._resolver.resolve_prebuilt(
921+
wheel_url, fallback_version = self._resolver.resolve(
931922
req=req,
932923
req_type=req_type,
933924
parent_req=parent_req,
925+
pre_built=True, # Force prebuilt for test mode fallback
934926
)
935927

936928
if fallback_version != resolved_version:

src/fromager/requirement_resolver.py

Lines changed: 41 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,29 @@ def __init__(
5252
# Session-level resolution cache to avoid re-resolving same requirements
5353
self._resolved_requirements: dict[str, tuple[str, Version]] = {}
5454

55-
def resolve_source(
55+
def resolve(
5656
self,
5757
req: Requirement,
5858
req_type: RequirementType,
5959
parent_req: Requirement | None = None,
60+
pre_built: bool | None = None,
6061
) -> tuple[str, Version]:
61-
"""Resolve source package (sdist).
62+
"""Resolve package requirement.
6263
6364
Tries resolution strategies in order:
6465
1. Session cache (if previously resolved)
6566
2. Previous dependency graph
66-
3. PyPI source resolution
67+
3. PyPI resolution (source or prebuilt based on package build info)
6768
6869
Args:
6970
req: Package requirement (must NOT have URL)
7071
req_type: Type of requirement
7172
parent_req: Parent requirement from dependency chain
73+
pre_built: Optional override to force prebuilt (True) or source (False).
74+
If None (default), uses package build info to determine.
7275
7376
Returns:
74-
Tuple of (source_url, resolved_version)
77+
Tuple of (url, resolved_version)
7578
7679
Raises:
7780
ValueError: If req contains a URL (must use Bootstrapper for git URLs)
@@ -87,84 +90,74 @@ def resolve_source(
8790
logger.debug(f"resolved {req} from cache")
8891
return cached_result
8992

90-
# Try graph
91-
cached_resolution = self._resolve_from_graph(
92-
req=req,
93-
req_type=req_type,
94-
pre_built=False,
95-
parent_req=parent_req,
96-
)
97-
if cached_resolution:
98-
source_url, resolved_version = cached_resolution
99-
logger.debug(f"resolved from previous bootstrap to {resolved_version}")
100-
else:
101-
# Fallback to PyPI
102-
source_url, resolved_version = sources.resolve_source(
103-
ctx=self.ctx,
104-
req=req,
105-
sdist_server_url=resolver.PYPI_SERVER_URL,
106-
req_type=req_type,
107-
)
93+
# Resolve using strategies
94+
url, resolved_version = self._resolve(req, req_type, parent_req, pre_built)
10895

10996
# Cache the result
110-
result = (source_url, resolved_version)
97+
result = (url, resolved_version)
11198
self.cache_resolution(req, result)
112-
return source_url, resolved_version
99+
return url, resolved_version
113100

114-
def resolve_prebuilt(
101+
def _resolve(
115102
self,
116103
req: Requirement,
117104
req_type: RequirementType,
118-
parent_req: Requirement | None = None,
105+
parent_req: Requirement | None,
106+
pre_built: bool | None,
119107
) -> tuple[str, Version]:
120-
"""Resolve pre-built package (wheels only).
108+
"""Internal resolution logic without caching.
121109
122110
Tries resolution strategies in order:
123-
1. Session cache (if previously resolved)
124-
2. Previous dependency graph
125-
3. PyPI wheel resolution
111+
1. Previous dependency graph
112+
2. PyPI resolution (source or prebuilt)
126113
127114
Args:
128115
req: Package requirement
129116
req_type: Type of requirement
130117
parent_req: Parent requirement from dependency chain
118+
pre_built: Optional override to force prebuilt (True) or source (False).
119+
If None, uses package build info to determine.
131120
132121
Returns:
133-
Tuple of (source_url, resolved_version)
134-
135-
Raises:
136-
ValueError: If unable to resolve
122+
Tuple of (url, resolved_version)
137123
"""
138-
# Check session cache first
139-
cached_result = self.get_cached_resolution(req)
140-
if cached_result is not None:
141-
logger.debug(f"resolved {req} from cache")
142-
return cached_result
124+
# Determine whether to resolve prebuilt or source
125+
if pre_built is None:
126+
pbi = self.ctx.package_build_info(req)
127+
pre_built = pbi.pre_built
143128

144129
# Try graph
145130
cached_resolution = self._resolve_from_graph(
146131
req=req,
147132
req_type=req_type,
148-
pre_built=True,
133+
pre_built=pre_built,
149134
parent_req=parent_req,
150135
)
151136

152137
if cached_resolution and not req.url:
153-
wheel_url, resolved_version = cached_resolution
138+
url, resolved_version = cached_resolution
154139
logger.debug(f"resolved from previous bootstrap to {resolved_version}")
155-
else:
156-
# Fallback to PyPI prebuilt resolution
140+
return url, resolved_version
141+
142+
# Fallback to PyPI
143+
if pre_built:
144+
# Resolve prebuilt wheel
157145
servers = wheels.get_wheel_server_urls(
158146
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
159147
)
160-
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
148+
url, resolved_version = wheels.resolve_prebuilt_wheel(
161149
ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
162150
)
151+
else:
152+
# Resolve source (sdist)
153+
url, resolved_version = sources.resolve_source(
154+
ctx=self.ctx,
155+
req=req,
156+
sdist_server_url=resolver.PYPI_SERVER_URL,
157+
req_type=req_type,
158+
)
163159

164-
# Cache the result
165-
result = (wheel_url, resolved_version)
166-
self.cache_resolution(req, result)
167-
return wheel_url, resolved_version
160+
return url, resolved_version
168161

169162
def get_cached_resolution(
170163
self,

tests/test_requirement_resolver.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,16 @@ def test_resolve_from_graph_no_previous_graph(tmp_context: WorkContext) -> None:
248248
)
249249

250250

251-
def test_resolve_source_rejects_git_urls(tmp_context: WorkContext) -> None:
252-
"""RequirementResolver.resolve_source() rejects git URLs."""
251+
def test_resolve_rejects_git_urls(tmp_context: WorkContext) -> None:
252+
"""RequirementResolver.resolve() rejects git URLs."""
253253
resolver = RequirementResolver(tmp_context)
254254

255255
with pytest.raises(
256256
ValueError, match="Git URL requirements must be handled by Bootstrapper"
257257
):
258-
resolver.resolve_source(
258+
resolver.resolve(
259259
req=Requirement("package @ git+https://github.com/example/repo.git"),
260260
req_type=RequirementType.TOP_LEVEL,
261+
pre_built=False,
261262
parent_req=None,
262263
)

0 commit comments

Comments
 (0)