Skip to content

Commit 454b5a6

Browse files
rd4398claude
andcommitted
(refactor): Move caching from bootstrapper to resolver
This commit improves cohesion by moving the session-level resolution cache (_resolved_requirements) from Bootstrapper into RequirementResolver. The cache now lives alongside the resolution logic it supports. Changes: - Added _resolved_requirements dict to RequirementResolver.__init__ - Updated resolve_source() and resolve_prebuilt() to check cache first and update cache before returning - Added cache_resolution() public method for externally-resolved git URLs - Removed _resolved_requirements from Bootstrapper - Updated Bootstrapper.resolve_version() to use resolver.cache_resolution() for git URL results This acts as next step towards implementing multiple version bootstrap Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
1 parent e655192 commit 454b5a6

2 files changed

Lines changed: 87 additions & 37 deletions

File tree

src/fromager/bootstrapper.py

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,6 @@ def __init__(
122122
# package.
123123
self._seen_requirements: set[SeenKey] = set()
124124

125-
# Track requirements we have already resolved so we don't resolve them again.
126-
self._resolved_requirements: dict[str, tuple[str, Version]] = {}
127-
128125
self._build_order_filename = self.ctx.work_dir / "build-order.json"
129126

130127
# Track failed packages in test mode (list of typed dicts for JSON export)
@@ -186,41 +183,42 @@ def resolve_version(
186183
build orchestration (BuildEnvironment, build dependencies).
187184
Delegates PyPI/graph resolution to RequirementResolver.
188185
"""
189-
req_str = str(req)
190-
if req_str in self._resolved_requirements:
191-
logger.debug(f"resolved {req_str} from cache")
192-
return self._resolved_requirements[req_str]
193-
194186
if req.url:
187+
# Git URLs must be resolved in Bootstrapper (orchestration concern)
195188
if req_type != RequirementType.TOP_LEVEL:
196189
raise ValueError(
197190
f"{req} includes a URL, but is not a top-level dependency"
198191
)
192+
193+
# Check cache first to avoid re-resolving
194+
cached_result = self._resolver.get_cached_resolution(req)
195+
if cached_result is not None:
196+
logger.debug(f"resolved {req} from cache")
197+
return cached_result
198+
199199
logger.info("resolving source via URL, ignoring any plugins")
200200
source_url, resolved_version = self._resolve_version_from_git_url(req=req)
201-
self._resolved_requirements[req_str] = (source_url, resolved_version)
201+
# Cache the git URL resolution
202+
self._resolver.cache_resolution(req, (source_url, resolved_version))
202203
return source_url, resolved_version
203204

204-
# Delegate to RequirementResolver
205+
# Delegate to RequirementResolver (handles caching internally)
205206
parent_req = self.why[-1][1] if self.why else None
206207
pbi = self.ctx.package_build_info(req)
207208

208209
if pbi.pre_built:
209-
source_url, resolved_version = self._resolver.resolve_prebuilt(
210+
return self._resolver.resolve_prebuilt(
210211
req=req,
211212
req_type=req_type,
212213
parent_req=parent_req,
213214
)
214215
else:
215-
source_url, resolved_version = self._resolver.resolve_source(
216+
return self._resolver.resolve_source(
216217
req=req,
217218
req_type=req_type,
218219
parent_req=parent_req,
219220
)
220221

221-
self._resolved_requirements[req_str] = (source_url, resolved_version)
222-
return source_url, resolved_version
223-
224222
def _processing_build_requirement(self, current_req_type: RequirementType) -> bool:
225223
"""Are we currently processing a build requirement?
226224

src/fromager/requirement_resolver.py

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ def __init__(
4949
"""
5050
self.ctx = ctx
5151
self.prev_graph = prev_graph
52+
# Session-level resolution cache to avoid re-resolving same requirements
53+
self._resolved_requirements: dict[str, tuple[str, Version]] = {}
5254

5355
def resolve_source(
5456
self,
@@ -59,8 +61,9 @@ def resolve_source(
5961
"""Resolve source package (sdist).
6062
6163
Tries resolution strategies in order:
62-
1. Previous dependency graph
63-
2. PyPI source resolution
64+
1. Session cache (if previously resolved)
65+
2. Previous dependency graph
66+
3. PyPI source resolution
6467
6568
Args:
6669
req: Package requirement (must NOT have URL)
@@ -78,7 +81,13 @@ def resolve_source(
7881
f"Git URL requirements must be handled by Bootstrapper: {req}"
7982
)
8083

81-
# Try graph first
84+
# Check session cache first
85+
cached_result = self.get_cached_resolution(req)
86+
if cached_result is not None:
87+
logger.debug(f"resolved {req} from cache")
88+
return cached_result
89+
90+
# Try graph
8291
cached_resolution = self._resolve_from_graph(
8392
req=req,
8493
req_type=req_type,
@@ -88,15 +97,18 @@ def resolve_source(
8897
if cached_resolution:
8998
source_url, resolved_version = cached_resolution
9099
logger.debug(f"resolved from previous bootstrap to {resolved_version}")
91-
return source_url, 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+
)
92108

93-
# Fallback to PyPI
94-
source_url, resolved_version = sources.resolve_source(
95-
ctx=self.ctx,
96-
req=req,
97-
sdist_server_url=resolver.PYPI_SERVER_URL,
98-
req_type=req_type,
99-
)
109+
# Cache the result
110+
result = (source_url, resolved_version)
111+
self.cache_resolution(req, result)
100112
return source_url, resolved_version
101113

102114
def resolve_prebuilt(
@@ -108,8 +120,9 @@ def resolve_prebuilt(
108120
"""Resolve pre-built package (wheels only).
109121
110122
Tries resolution strategies in order:
111-
1. Previous dependency graph
112-
2. PyPI wheel resolution
123+
1. Session cache (if previously resolved)
124+
2. Previous dependency graph
125+
3. PyPI wheel resolution
113126
114127
Args:
115128
req: Package requirement
@@ -122,7 +135,13 @@ def resolve_prebuilt(
122135
Raises:
123136
ValueError: If unable to resolve
124137
"""
125-
# Try graph first
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
143+
144+
# Try graph
126145
cached_resolution = self._resolve_from_graph(
127146
req=req,
128147
req_type=req_type,
@@ -133,17 +152,50 @@ def resolve_prebuilt(
133152
if cached_resolution and not req.url:
134153
wheel_url, resolved_version = cached_resolution
135154
logger.debug(f"resolved from previous bootstrap to {resolved_version}")
136-
return wheel_url, resolved_version
155+
else:
156+
# Fallback to PyPI prebuilt resolution
157+
servers = wheels.get_wheel_server_urls(
158+
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
159+
)
160+
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
161+
ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
162+
)
137163

138-
# Fallback to PyPI prebuilt resolution
139-
servers = wheels.get_wheel_server_urls(
140-
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
141-
)
142-
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
143-
ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
144-
)
164+
# Cache the result
165+
result = (wheel_url, resolved_version)
166+
self.cache_resolution(req, result)
145167
return wheel_url, resolved_version
146168

169+
def get_cached_resolution(
170+
self,
171+
req: Requirement,
172+
) -> tuple[str, Version] | None:
173+
"""Get a cached resolution result if it exists.
174+
175+
Args:
176+
req: Package requirement to look up in cache
177+
178+
Returns:
179+
Tuple of (source_url, resolved_version) if cached, None otherwise
180+
"""
181+
return self._resolved_requirements.get(str(req))
182+
183+
def cache_resolution(
184+
self,
185+
req: Requirement,
186+
result: tuple[str, Version],
187+
) -> None:
188+
"""Cache a resolution result.
189+
190+
Used by Bootstrapper to cache git URL resolutions that are
191+
handled externally (outside this resolver).
192+
193+
Args:
194+
req: Package requirement to cache
195+
result: Tuple of (source_url, resolved_version)
196+
"""
197+
self._resolved_requirements[str(req)] = result
198+
147199
def _resolve_from_graph(
148200
self,
149201
req: Requirement,

0 commit comments

Comments
 (0)