Merge pull request #941 from rd4398/refactor-move-cache-to-resolver

(refactor): Move caching from bootstrapper to resolver

Author: rd4398

src/fromager/bootstrapper.py

@@ -121,9 +121,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)
@@ -185,41 +182,42 @@ def resolve_version(
         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]
-
         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"
                 )
+
+            # Check cache first to avoid re-resolving
+            cached_result = self._resolver.get_cached_resolution(req)
+            if cached_result is not None:
+                logger.debug(f"resolved {req} from cache")
+                return cached_result
+
             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
+            self._resolver.cache_resolution(req, (source_url, resolved_version))
             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?
 

src/fromager/requirement_resolver.py

@@ -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,
@@ -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)
@@ -78,7 +81,13 @@ def resolve_source(
                 f"Git URL requirements must be handled by Bootstrapper: {req}"
             )
 
-        # Try graph first
+        # Check session cache first
+        cached_result = self.get_cached_resolution(req)
+        if cached_result is not None:
+            logger.debug(f"resolved {req} from cache")
+            return cached_result
+
+        # Try graph
         cached_resolution = self._resolve_from_graph(
             req=req,
             req_type=req_type,
@@ -88,15 +97,18 @@ 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
+        result = (source_url, resolved_version)
+        self.cache_resolution(req, result)
         return source_url, resolved_version
 
     def resolve_prebuilt(
@@ -108,8 +120,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
@@ -122,7 +135,13 @@ def resolve_prebuilt(
         Raises:
             ValueError: If unable to resolve
         """
-        # Try graph first
+        # Check session cache first
+        cached_result = self.get_cached_resolution(req)
+        if cached_result is not None:
+            logger.debug(f"resolved {req} from cache")
+            return cached_result
+
+        # Try graph
         cached_resolution = self._resolve_from_graph(
             req=req,
             req_type=req_type,
@@ -133,17 +152,50 @@ 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
+        result = (wheel_url, resolved_version)
+        self.cache_resolution(req, result)
         return wheel_url, resolved_version
 
+    def get_cached_resolution(
+        self,
+        req: Requirement,
+    ) -> tuple[str, Version] | None:
+        """Get a cached resolution result if it exists.
+
+        Args:
+            req: Package requirement to look up in cache
+
+        Returns:
+            Tuple of (source_url, resolved_version) if cached, None otherwise
+        """
+        return self._resolved_requirements.get(str(req))
+
+    def cache_resolution(
+        self,
+        req: Requirement,
+        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: Package requirement to cache
+            result: Tuple of (source_url, resolved_version)
+        """
+        self._resolved_requirements[str(req)] = result
+
     def _resolve_from_graph(
         self,
         req: Requirement,