Skip to content

(refactor): Move caching from bootstrapper to resolver #941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rd4398 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

(refactor): Move caching from bootstrapper to resolver #941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 9 additions & 15 deletions src/fromager/bootstrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,6 @@ def __init__(
# package.
self._seen_requirements: set[SeenKey] = set()

# Track requirements we have already resolved so we don't resolve them again.
self._resolved_requirements: dict[str, tuple[str, Version]] = {}

self._build_order_filename = self.ctx.work_dir / "build-order.json"

# Track failed packages in test mode (list of typed dicts for JSON export)
Expand Down Expand Up @@ -185,42 +182,39 @@ def resolve_version(
Git URL resolution stays in Bootstrapper because it requires
build orchestration (BuildEnvironment, build dependencies).
Delegates PyPI/graph resolution to RequirementResolver.
"""
req_str = str(req)
if req_str in self._resolved_requirements:
logger.debug(f"resolved {req_str} from cache")
return self._resolved_requirements[req_str]

Resolution caching is handled by RequirementResolver for all
Copy link
Member

@dhellmann dhellmann Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The agent is leaving comments about the refactoring again, but they're not really relevant to someone looking at the code after that refactoring. See if you can get it to stop doing that generally and update the changes here to remove them.

resolution types (PyPI, graph, and git URLs).
"""
if req.url:
# Git URLs must be resolved in Bootstrapper (orchestration concern)
if req_type != RequirementType.TOP_LEVEL:
raise ValueError(
f"{req} includes a URL, but is not a top-level dependency"
)
logger.info("resolving source via URL, ignoring any plugins")
source_url, resolved_version = self._resolve_version_from_git_url(req=req)
self._resolved_requirements[req_str] = (source_url, resolved_version)
# Cache the git URL resolution in the resolver
self._resolver.cache_resolution(str(req), (source_url, resolved_version))
Copy link
Member

@dhellmann dhellmann Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will save the cached results, but the cache isn't checked as part of this code path so we may end up resolving the version repeatedly.

return source_url, resolved_version

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

if pbi.pre_built:
source_url, resolved_version = self._resolver.resolve_prebuilt(
return self._resolver.resolve_prebuilt(
req=req,
req_type=req_type,
parent_req=parent_req,
)
else:
source_url, resolved_version = self._resolver.resolve_source(
return self._resolver.resolve_source(
req=req,
req_type=req_type,
parent_req=parent_req,
)

self._resolved_requirements[req_str] = (source_url, resolved_version)
return source_url, resolved_version

def _processing_build_requirement(self, current_req_type: RequirementType) -> bool:
"""Are we currently processing a build requirement?

Expand Down
80 changes: 58 additions & 22 deletions src/fromager/requirement_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ def __init__(
"""
self.ctx = ctx
self.prev_graph = prev_graph
# Session-level resolution cache to avoid re-resolving same requirements
self._resolved_requirements: dict[str, tuple[str, Version]] = {}

def resolve_source(
self,
Expand All @@ -59,8 +61,9 @@ def resolve_source(
"""Resolve source package (sdist).

Tries resolution strategies in order:
1. Previous dependency graph
2. PyPI source resolution
1. Session cache (if previously resolved)
2. Previous dependency graph
3. PyPI source resolution

Args:
req: Package requirement (must NOT have URL)
Expand All @@ -78,7 +81,13 @@ def resolve_source(
f"Git URL requirements must be handled by Bootstrapper: {req}"
)

# Try graph first
# Check session cache first
req_str = str(req)
if req_str in self._resolved_requirements:
logger.debug(f"resolved {req_str} from cache")
return self._resolved_requirements[req_str]

# Try graph
cached_resolution = self._resolve_from_graph(
req=req,
req_type=req_type,
Expand All @@ -88,15 +97,17 @@ def resolve_source(
if cached_resolution:
source_url, resolved_version = cached_resolution
logger.debug(f"resolved from previous bootstrap to {resolved_version}")
return source_url, resolved_version
else:
# Fallback to PyPI
source_url, resolved_version = sources.resolve_source(
ctx=self.ctx,
req=req,
sdist_server_url=resolver.PYPI_SERVER_URL,
req_type=req_type,
)

# Fallback to PyPI
source_url, resolved_version = sources.resolve_source(
ctx=self.ctx,
req=req,
sdist_server_url=resolver.PYPI_SERVER_URL,
req_type=req_type,
)
# Cache the result
self._resolved_requirements[req_str] = (source_url, resolved_version)
return source_url, resolved_version

def resolve_prebuilt(
Expand All @@ -108,8 +119,9 @@ def resolve_prebuilt(
"""Resolve pre-built package (wheels only).

Tries resolution strategies in order:
1. Previous dependency graph
2. PyPI wheel resolution
1. Session cache (if previously resolved)
2. Previous dependency graph
3. PyPI wheel resolution

Args:
req: Package requirement
Expand All @@ -122,7 +134,13 @@ def resolve_prebuilt(
Raises:
ValueError: If unable to resolve
"""
# Try graph first
# Check session cache first
req_str = str(req)
if req_str in self._resolved_requirements:
logger.debug(f"resolved {req_str} from cache")
return self._resolved_requirements[req_str]

# Try graph
cached_resolution = self._resolve_from_graph(
req=req,
req_type=req_type,
Expand All @@ -133,17 +151,35 @@ def resolve_prebuilt(
if cached_resolution and not req.url:
wheel_url, resolved_version = cached_resolution
logger.debug(f"resolved from previous bootstrap to {resolved_version}")
return wheel_url, resolved_version
else:
# Fallback to PyPI prebuilt resolution
servers = wheels.get_wheel_server_urls(
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
)
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
)

# Fallback to PyPI prebuilt resolution
servers = wheels.get_wheel_server_urls(
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
)
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
)
# Cache the result
self._resolved_requirements[req_str] = (wheel_url, resolved_version)
return wheel_url, resolved_version

def cache_resolution(
self,
req_str: str,
Copy link
Member

@dhellmann dhellmann Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The read and write API for the cache should take a Requirement and then convert it to a string. That way we're only doing the conversion inside this class, and we can change how we index the cache if we want to.

result: tuple[str, Version],
) -> None:
"""Cache a resolution result.

Used by Bootstrapper to cache git URL resolutions that are
handled externally (outside this resolver).

Args:
req_str: String representation of the requirement
result: Tuple of (source_url, resolved_version)
"""
self._resolved_requirements[req_str] = result

def _resolve_from_graph(
self,
req: Requirement,
Expand Down
Loading