Repository: python-wheel-build/fromager

Issue:
add caching in resolver
This will allow us to reuse already seen candidates if they match the current conditions of the package we are trying to resolve. 

Advantages:
- clean data: reduce the number of different package versions we see in graph.json and build-order.json. This will help in #340 
- micro-optimization: we might not need to make a network connection for package resolution

Discussion:

Patch:
diff --git a/docs/customization.md b/docs/customization.md
index 8050701..11cd0db 100644
--- a/docs/customization.md
+++ b/docs/customization.md
@@ -137,24 +137,16 @@ Settings common to all variants of a given package can be placed in the the
 top-level `env` mapping. Variant env vars override global env vars.
 
 Environment files support simple parameter expansions `$NAME` and
-`${NAME}` as well as default values `${NAME:-}` (empty string) and
-`${NAME:-somedefault}`. Values are taken from previous lines, then global env
-map, and finally process environment. Sub shell expression `$(cmd)` and
-extended parameter expansions like `${NAME:+alternative}` are not
-implemented. A literal `$` must be quoted as `$$`.
-
-```{versionchanged} 0.32.0
-
-Added support for default value syntax `${NAME:-}`.
-```
+`${NAME}`. Values are taken from previous lines, then global env map, and
+finally process environment. Sub shell expression `$(cmd)` and extended
+parameter expansions like `${NAME:-default}` are not implemented. A literal
+`$` must be quoted as `$$`.
 
 ```yaml
 # example
 env:
     # pre-pend '/global/bin' to PATH
     PATH: "/global/bin:$PATH"
-    # default CFLAGS to empty string and append " -g"
-    CFLAGS: "${CFLAGS:-} -g"
 variants:
     cpu:
         env:
diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py
index 1863b45..b18ae8d 100644
--- a/src/fromager/bootstrapper.py
+++ b/src/fromager/bootstrapper.py
@@ -333,7 +333,10 @@ class Bootstrapper:
             )
         else:
             source_url, resolved_version = sources.resolve_source(
-                ctx=self.ctx, req=req, sdist_server_url=resolver.PYPI_SERVER_URL
+                ctx=self.ctx,
+                req=req,
+                sdist_server_url=resolver.PYPI_SERVER_URL,
+                req_type=req_type,
             )
         return (source_url, resolved_version)
 
@@ -356,7 +359,7 @@ class Bootstrapper:
         else:
             servers = wheels.get_wheel_server_urls(self.ctx, req)
             wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
-                ctx=self.ctx, req=req, wheel_server_urls=servers
+                ctx=self.ctx, req=req, wheel_server_urls=servers, req_type=req_type
             )
         return (wheel_url, resolved_version)
 
@@ -445,6 +448,7 @@ class Bootstrapper:
         if not version_source:
             return None
         try:
+            # no need to pass req type to enable caching since we are already using the graph as our cache
             provider = resolver.GenericProvider(
                 version_source=lambda x, y, z: version_source,
                 constraints=self.ctx.constraints,
diff --git a/src/fromager/build_environment.py b/src/fromager/build_environment.py
index c8b890b..de512c3 100644
--- a/src/fromager/build_environment.py
+++ b/src/fromager/build_environment.py
@@ -53,7 +53,10 @@ class MissingDependency(Exception):  # noqa: N818
         for r in all_reqs:
             try:
                 _, version = resolver.resolve(
-                    ctx=ctx, req=r, sdist_server_url=resolver.PYPI_SERVER_URL
+                    ctx=ctx,
+                    req=r,
+                    sdist_server_url=resolver.PYPI_SERVER_URL,
+                    req_type=req_type,
                 )
             except Exception as err:
                 resolutions.append(f"{r} -> {err}")
diff --git a/src/fromager/packagesettings.py b/src/fromager/packagesettings.py
index 97ad560..6660027 100644
--- a/src/fromager/packagesettings.py
+++ b/src/fromager/packagesettings.py
@@ -1,7 +1,6 @@
 import logging
 import os
 import pathlib
-import re
 import string
 import types
 import typing
@@ -421,28 +420,6 @@ def _resolve_template(
         raise
 
 
-_DEFAULT_PATTERN_RE = re.compile(
-    r"(?<!\$)"  # not preceeded by a second '$'
-    r"\$\{(?P<name>[a-z0-9_]+)"  # '${name'
-    r"(:-(?P<default>[^\}:]*))?"  # optional ':-default', capture value
-    r"\}",  # closing '}'
-    flags=re.ASCII | re.IGNORECASE,
-)
-
-
-def substitute_template(value: str, template_env: dict[str, str]) -> str:
-    """Substitute ${var} and ${var:-default} in value string"""
-    localdefault = template_env.copy()
-    for mo in _DEFAULT_PATTERN_RE.finditer(value):
-        modict = mo.groupdict()
-        name = modict["name"]
-        # add to local default, keep existing default
-        localdefault.setdefault(name, modict["default"])
-        # remove ":-default"
-        value = value.replace(mo.group(0), f"${{{name}}}")
-    return string.Template(value).substitute(localdefault)
-
-
 def get_cpu_count() -> int:
     """CPU count from scheduler affinity"""
     if hasattr(os, "sched_getaffinity"):
@@ -650,7 +627,7 @@ class PackageBuildInfo:
             entries.extend(vi.env.items())
 
         for key, value in entries:
-            value = substitute_template(value, template_env)
+            value = string.Template(value).substitute(template_env)
             extra_environ[key] = value
             # subsequent key-value pairs can depend on previously vars.
             template_env[key] = value
diff --git a/src/fromager/requirements_file.py b/src/fromager/requirements_file.py
index 52c2553..46241c6 100644
--- a/src/fromager/requirements_file.py
+++ b/src/fromager/requirements_file.py
@@ -29,6 +29,9 @@ class RequirementType(StrEnum):
                 return self.value in allowed_values and other.value in allowed_values
         return super.__eq__(self, other)
 
+    def __ne__(self, value: object) -> bool:
+        return not self == value
+
 
 class SourceType(StrEnum):
     PREBUILT = "prebuilt"
diff --git a/src/fromager/resolver.py b/src/fromager/resolver.py
index ac7272c..4f957d1 100644
--- a/src/fromager/resolver.py
+++ b/src/fromager/resolver.py
@@ -8,6 +8,7 @@ from __future__ import annotations
 import logging
 import os
 import typing
+from collections import defaultdict
 from operator import attrgetter
 from platform import python_version
 from urllib.parse import urljoin, urlparse
@@ -31,6 +32,7 @@ from .candidate import Candidate
 from .constraints import Constraints
 from .extras_provider import ExtrasProvider
 from .request_session import session
+from .requirements_file import RequirementType
 
 if typing.TYPE_CHECKING:
     from . import context
@@ -51,6 +53,7 @@ def resolve(
     sdist_server_url: str,
     include_sdists: bool = True,
     include_wheels: bool = True,
+    req_type: RequirementType | None = None,
 ) -> tuple[str, Version]:
     # Create the (reusable) resolver.
     provider = overrides.find_and_invoke(
@@ -62,6 +65,7 @@ def resolve(
         include_sdists=include_sdists,
         include_wheels=include_wheels,
         sdist_server_url=sdist_server_url,
+        req_type=req_type,
     )
     return resolve_from_provider(provider, req)
 
@@ -72,6 +76,7 @@ def default_resolver_provider(
     sdist_server_url: str,
     include_sdists: bool,
     include_wheels: bool,
+    req_type: RequirementType | None = None,
 ) -> PyPIProvider | GenericProvider | GitHubTagProvider:
     """Lookup resolver provider to resolve package versions"""
     return PyPIProvider(
@@ -79,6 +84,7 @@ def default_resolver_provider(
         include_wheels=include_wheels,
         sdist_server_url=sdist_server_url,
         constraints=ctx.constraints,
+        req_type=req_type,
     )
 
 
@@ -200,12 +206,14 @@ class BaseProvider(ExtrasProvider):
         include_wheels: bool = True,
         sdist_server_url: str = "https://pypi.org/simple/",
         constraints: Constraints | None = None,
+        req_type: RequirementType | None = None,
     ):
         super().__init__()
         self.include_sdists = include_sdists
         self.include_wheels = include_wheels
         self.sdist_server_url = sdist_server_url
         self.constraints = constraints or Constraints({})
+        self.req_type = req_type
 
     def identify(self, requirement_or_candidate: Requirement | Candidate) -> str:
         return canonicalize_name(requirement_or_candidate.name)
@@ -222,6 +230,72 @@ class BaseProvider(ExtrasProvider):
     def get_base_requirement(self, candidate: Candidate) -> Requirement:
         return Requirement(f"{candidate.name}=={candidate.version}")
 
+    def validate_candidate(
+        self,
+        identifier: str,
+        requirements: RequirementsMap,
+        incompatibilities: CandidatesMap,
+        candidate: Candidate,
+    ) -> bool:
+        identifier_reqs = list(requirements[identifier])
+        bad_versions = {c.version for c in incompatibilities[identifier]}
+        allow_prerelease = self.constraints.allow_prerelease(identifier)
+
+        # Skip versions that are known bad
+        if candidate.version in bad_versions:
+            if DEBUG_RESOLVER:
+                logger.debug(
+                    f"{identifier}: skipping bad version {candidate.version} from {bad_versions}"
+                )
+            return False
+        # Skip versions that do not match the requirement. Allow prereleases only if constraints allow prereleases
+        if not all(
+            r.specifier.contains(
+                candidate.version,
+                prereleases=(allow_prerelease or bool(r.specifier.prereleases)),
+            )
+            for r in identifier_reqs
+        ):
+            if DEBUG_RESOLVER:
+                logger.debug(
+                    f"{identifier}: skipping {candidate.version} because it does not match {identifier_reqs}"
+                )
+            return False
+        # Skip versions that do not match the constraint
+        if not self.constraints.is_satisfied_by(identifier, candidate.version):
+            if DEBUG_RESOLVER:
+                c = self.constraints.get_constraint(identifier)
+                logger.debug(
+                    f"{identifier}: skipping {candidate.version} due to constraint {c}"
+                )
+            return False
+        return True
+
+    def get_cache(self) -> dict[str, list[Candidate]]:
+        raise NotImplementedError()
+
+    def get_from_cache(
+        self,
+        identifier: str,
+        requirements: RequirementsMap,
+        incompatibilities: CandidatesMap,
+    ) -> list[Candidate]:
+        cache = self.get_cache()
+        # we only want caching for build reqs because for install time reqs we always want to get the latest version
+        # we can't guarantee that the latest version is available in the cache so install time reqs cannot use the cache
+        if self.req_type != RequirementType.BUILD:
+            return []
+        return [
+            c
+            for c in cache[identifier]
+            if self.validate_candidate(identifier, requirements, incompatibilities, c)
+        ]
+
+    def add_to_cache(self, identifier: str, candidates: list[Candidate]) -> None:
+        # we can add candidates to cache even for install type reqs because build time reqs are
+        # allowed to use candidates seen when we were resolving the same req as an install type
+        self.get_cache()[identifier].extend(candidates)
+
     def get_preference(
         self,
         identifier: str,
@@ -262,93 +336,94 @@ class BaseProvider(ExtrasProvider):
 class PyPIProvider(BaseProvider):
     """Lookup package and versions from a simple Python index (PyPI)"""
 
+    pypi_resolver_cache: typing.ClassVar[dict[str, list[Candidate]]] = defaultdict(list)
+
     def __init__(
         self,
         include_sdists: bool = True,
         include_wheels: bool = True,
         sdist_server_url: str = "https://pypi.org/simple/",
         constraints: Constraints | None = None,
+        req_type: RequirementType | None = None,
     ):
-        super().__init__()
-        self.include_sdists = include_sdists
-        self.include_wheels = include_wheels
-        self.sdist_server_url = sdist_server_url
-        self.constraints = constraints or Constraints({})
+        super().__init__(
+            include_sdists=include_sdists,
+            include_wheels=include_wheels,
+            sdist_server_url=sdist_server_url,
+            constraints=constraints,
+            req_type=req_type,
+        )
 
-    def find_matches(
+    def get_cache(self) -> dict[str, list[Candidate]]:
+        return PyPIProvider.pypi_resolver_cache
+
+    def validate_candidate(
         self,
         identifier: str,
         requirements: RequirementsMap,
         incompatibilities: CandidatesMap,
-    ) -> typing.Iterable[Candidate]:
-        identifier_reqs = list(requirements[identifier])
-        bad_versions = {c.version for c in incompatibilities[identifier]}
-
-        # Need to pass the extras to the search, so they
-        # are added to the candidate at creation - we
-        # treat candidates as immutable once created.
-        candidates = []
-        for candidate in get_project_from_pypi(
-            identifier, set(), self.sdist_server_url
+        candidate: Candidate,
+    ) -> bool:
+        if not super().validate_candidate(
+            identifier, requirements, incompatibilities, candidate
         ):
-            allow_prerelease = self.constraints.allow_prerelease(identifier)
-            # Skip versions that are known bad
-            if candidate.version in bad_versions:
-                if DEBUG_RESOLVER:
-                    logger.debug(
-                        f"{identifier}: skipping bad version {candidate.version} from {bad_versions}"
-                    )
-                continue
-            # Skip versions that do not match the requirement. Allow prereleases only if constraints allow prereleases
-            if not all(
-                r.specifier.contains(
-                    candidate.version,
-                    prereleases=(allow_prerelease or bool(r.specifier.prereleases)),
+            return False
+        # Only include sdists if we're asked to
+        if candidate.is_sdist and not self.include_sdists:
+            if DEBUG_RESOLVER:
+                logger.debug(
+                    f"{identifier}: skipping {candidate} because it is an sdist"
                 )
-                for r in identifier_reqs
+            return False
+        # Only include wheels if we're asked to
+        if not candidate.is_sdist and not self.include_wheels:
+            if DEBUG_RESOLVER:
+                logger.debug(
+                    f"{identifier}: skipping {candidate} because it is a wheel"
+                )
+            return False
+        return True
+
+    def find_matches(
+        self,
+        identifier: str,
+        requirements: RequirementsMap,
+        incompatibilities: CandidatesMap,
+    ) -> typing.Iterable[Candidate]:
+        candidates = self.get_from_cache(identifier, requirements, incompatibilities)
+        if not candidates:
+            # Need to pass the extras to the search, so they
+            # are added to the candidate at creation - we
+            # treat candidates as immutable once created.
+            for candidate in get_project_from_pypi(
+                identifier, set(), self.sdist_server_url
             ):
-                if DEBUG_RESOLVER:
-                    logger.debug(
-                        f"{identifier}: skipping {candidate.version} because it does not match {identifier_reqs}"
-                    )
-                continue
-            # Skip versions that do not match the constraint
-            if not self.constraints.is_satisfied_by(identifier, candidate.version):
-                if DEBUG_RESOLVER:
-                    c = self.constraints.get_constraint(identifier)
-                    logger.debug(
-                        f"{identifier}: skipping {candidate.version} due to constraint {c}"
-                    )
-                continue
-            # Only include sdists if we're asked to
-            if candidate.is_sdist and not self.include_sdists:
-                if DEBUG_RESOLVER:
-                    logger.debug(
-                        f"{identifier}: skipping {candidate} because it is an sdist"
-                    )
-                continue
-            # Only include wheels if we're asked to
-            if not candidate.is_sdist and not self.include_wheels:
-                if DEBUG_RESOLVER:
-                    logger.debug(
-                        f"{identifier}: skipping {candidate} because it is a wheel"
-                    )
-                continue
-            candidates.append(candidate)
+                if self.validate_candidate(
+                    identifier, requirements, incompatibilities, candidate
+                ):
+                    candidates.append(candidate)
+            self.add_to_cache(identifier, candidates)
         return sorted(candidates, key=attrgetter("version", "build_tag"), reverse=True)
 
 
 class GenericProvider(BaseProvider):
     """Lookup package and version by using a callback"""
 
+    generic_resolver_cache: typing.ClassVar[dict[str, list[Candidate]]] = defaultdict(
+        list
+    )
+
     def __init__(
         self,
         version_source: VersionSource,
         constraints: Constraints | None = None,
+        req_type: RequirementType | None = None,
     ):
-        super().__init__()
+        super().__init__(constraints=constraints, req_type=req_type)
         self._version_source = version_source
-        self.constraints = constraints or Constraints({})
+
+    def get_cache(self) -> dict[str, list[Candidate]]:
+        return GenericProvider.generic_resolver_cache
 
     def find_matches(
         self,
@@ -356,57 +431,32 @@ class GenericProvider(BaseProvider):
         requirements: RequirementsMap,
         incompatibilities: CandidatesMap,
     ) -> typing.Iterable[Candidate]:
-        identifier_reqs = list(requirements[identifier])
-        bad_versions = {c.version for c in incompatibilities[identifier]}
-
-        # Need to pass the extras to the search, so they
-        # are added to the candidate at creation - we
-        # treat candidates as immutable once created.
-        candidates = []
-        for url, item in self._version_source(
-            identifier, requirements, incompatibilities
-        ):
-            if isinstance(item, Version):
-                version = item
-            else:
-                try:
-                    version = Version(item)
-                except Exception as err:
-                    logger.debug(
-                        f"{identifier}: could not parse version from {item}: {err}"
-                    )
-                    continue
-            allow_prerelease = self.constraints.allow_prerelease(identifier)
-
-            # Skip versions that are known bad
-            if version in bad_versions:
-                if DEBUG_RESOLVER:
-                    logger.debug(
-                        f"{identifier}: skipping bad version {version} from {bad_versions}"
-                    )
-                continue
-            # Skip versions that do not match the requirement
-            if not all(
-                r.specifier.contains(
-                    version,
-                    prereleases=(allow_prerelease or bool(r.specifier.prereleases)),
-                )
-                for r in identifier_reqs
+        candidates = self.get_from_cache(identifier, requirements, incompatibilities)
+
+        if not candidates:
+            # Need to pass the extras to the search, so they
+            # are added to the candidate at creation - we
+            # treat candidates as immutable once created.
+            for url, item in self._version_source(
+                identifier, requirements, incompatibilities
             ):
-                if DEBUG_RESOLVER:
-                    logger.debug(
-                        f"{identifier}: skipping {version} because it does not match {identifier_reqs}"
-                    )
-                continue
-            # Skip versions that do not match the constraint
-            if not self.constraints.is_satisfied_by(identifier, version):
-                if DEBUG_RESOLVER:
-                    c = self.constraints.get_constraint(identifier)
-                    logger.debug(
-                        f"{identifier}: skipping {version} due to constraint {c}"
-                    )
-                continue
-            candidates.append(Candidate(identifier, version, url=url))
+                if isinstance(item, Version):
+                    version = item
+                else:
+                    try:
+                        version = Version(item)
+                    except Exception as err:
+                        logger.debug(
+                            f"{identifier}: could not parse version from {item}: {err}"
+                        )
+                        continue
+                candidate = Candidate(identifier, version, url=url)
+                if self.validate_candidate(
+                    identifier, requirements, incompatibilities, candidate
+                ):
+                    candidates.append(candidate)
+                self.add_to_cache(identifier, candidates)
+
         return sorted(candidates, key=attrgetter("version"), reverse=True)
 
 
@@ -418,6 +468,9 @@ class GitHubTagProvider(GenericProvider):
 
     host = "github.com:443"
     api_url = "https://api.{self.host}/repos/{self.organization}/{self.repo}/tags"
+    github_resolver_cache: typing.ClassVar[dict[str, list[Candidate]]] = defaultdict(
+        list
+    )
 
     def __init__(
         self, organization: str, repo: str, constraints: Constraints | None = None
@@ -426,6 +479,9 @@ class GitHubTagProvider(GenericProvider):
         self.organization = organization
         self.repo = repo
 
+    def get_cache(self) -> dict[str, list[Candidate]]:
+        return GitHubTagProvider.github_resolver_cache
+
     def _find_tags(
         self,
         identifier: str,
diff --git a/src/fromager/sources.py b/src/fromager/sources.py
index 447868f..ca62b98 100644
--- a/src/fromager/sources.py
+++ b/src/fromager/sources.py
@@ -26,6 +26,7 @@ from . import (
     vendor_rust,
 )
 from .request_session import session
+from .requirements_file import RequirementType
 
 if typing.TYPE_CHECKING:
     from . import build_environment, context
@@ -76,6 +77,7 @@ def resolve_source(
     ctx: context.WorkContext,
     req: Requirement,
     sdist_server_url: str,
+    req_type: RequirementType | None = None,
 ) -> tuple[str, Version]:
     "Return URL to source and its version."
     constraint = ctx.constraints.get_constraint(req.name)
@@ -91,6 +93,7 @@ def resolve_source(
             ctx=ctx,
             req=req,
             sdist_server_url=sdist_server_url,
+            req_type=req_type,
         )
     except (
         resolvelib.InconsistentCandidate,
@@ -114,7 +117,10 @@ def resolve_source(
 
 
 def default_resolve_source(
-    ctx: context.WorkContext, req: Requirement, sdist_server_url: str
+    ctx: context.WorkContext,
+    req: Requirement,
+    sdist_server_url: str,
+    req_type: RequirementType | None = None,
 ) -> tuple[str, Version]:
     "Return URL to source and its version."
 
@@ -127,6 +133,7 @@ def default_resolve_source(
         sdist_server_url=override_sdist_server_url,
         include_sdists=pbi.resolver_include_sdists,
         include_wheels=pbi.resolver_include_wheels,
+        req_type=req_type,
     )
     return url, version
 
diff --git a/src/fromager/wheels.py b/src/fromager/wheels.py
index a4e12d0..1577957 100644
--- a/src/fromager/wheels.py
+++ b/src/fromager/wheels.py
@@ -19,12 +19,7 @@ from packaging.requirements import Requirement
 from packaging.utils import canonicalize_name, parse_wheel_filename
 from packaging.version import Version
 
-from . import (
-    external_commands,
-    overrides,
-    resolver,
-    sources,
-)
+from . import external_commands, overrides, requirements_file, resolver, sources
 
 if typing.TYPE_CHECKING:
     from . import build_environment, context
@@ -385,6 +380,7 @@ def resolve_prebuilt_wheel(
     ctx: context.WorkContext,
     req: Requirement,
     wheel_server_urls: list[str],
+    req_type: requirements_file.RequirementType | None = None,
 ) -> tuple[str, Version]:
     "Return URL to wheel and its version."
     for url in wheel_server_urls:
@@ -395,6 +391,7 @@ def resolve_prebuilt_wheel(
                 sdist_server_url=url,
                 include_sdists=False,
                 include_wheels=True,
+                req_type=req_type,
             )
         except Exception:
             continue


Test patch:
diff --git a/tests/test_build_environment.py b/tests/test_build_environment.py
index ec721ff..9b17675 100644
--- a/tests/test_build_environment.py
+++ b/tests/test_build_environment.py
@@ -18,7 +18,7 @@ def test_missing_dependency_format(
         "flit_core": "3.9.0",
         "setuptools": "69.5.1",
     }
-    resolve_dist.side_effect = lambda ctx, req, sdist_server_url: (
+    resolve_dist.side_effect = lambda ctx, req, sdist_server_url, req_type: (
         "",
         Version(resolutions[req.name]),
     )
diff --git a/tests/test_packagesettings.py b/tests/test_packagesettings.py
index ed4cfe0..bb31591 100644
--- a/tests/test_packagesettings.py
+++ b/tests/test_packagesettings.py
@@ -15,7 +15,6 @@ from fromager.packagesettings import (
     PackageSettings,
     SettingsFile,
     Variant,
-    substitute_template,
 )
 
 TEST_PKG = "test-pkg"
@@ -42,7 +41,6 @@ FULL_EXPECTED: dict[str, typing.Any] = {
         "EGG_AGAIN": "$EGG",
         "SPAM": "alot $EXTRA",
         "QUOTES": "A\"BC'$$EGG",
-        "DEF": "${DEF:-default}",
     },
     "name": "test-pkg",
     "has_config": True,
@@ -151,18 +149,6 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam spam",
             "QUOTES": "A\"BC'$EGG",  # $$EGG is transformed into $EGG
             "SPAM": "alot extra",
-            "DEF": "default",
-        }
-        | parallel
-    )
-    assert (
-        pbi.get_extra_environ(template_env={"EXTRA": "extra", "DEF": "nondefault"})
-        == {
-            "EGG": "spam spam",
-            "EGG_AGAIN": "spam spam",
-            "QUOTES": "A\"BC'$EGG",  # $$EGG is transformed into $EGG
-            "SPAM": "alot extra",
-            "DEF": "nondefault",
         }
         | parallel
     )
@@ -176,7 +162,6 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam",
             "QUOTES": "A\"BC'$EGG",
             "SPAM": "",
-            "DEF": "default",
         }
         | parallel
     )
@@ -190,7 +175,6 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam",
             "QUOTES": "A\"BC'$EGG",
             "SPAM": "alot spam",
-            "DEF": "default",
         }
         | parallel
     )
@@ -208,7 +192,6 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam",
             "QUOTES": "A\"BC'$EGG",
             "SPAM": "alot spam",
-            "DEF": "default",
             "PATH": f"{build_env.path / 'bin'}:/sbin:/bin",
             "VIRTUAL_ENV": str(build_env.path),
         }
@@ -421,19 +404,3 @@ def test_parallel_jobs(
     testdata_context.settings.max_jobs = 4
     pbi = testdata_context.settings.package_build_info(TEST_PKG)
     assert pbi.parallel_jobs() == 4
-
-
-@pytest.mark.parametrize(
-    "value,template_env,expected",
-    [
-        ("", {}, ""),
-        ("${var}", {"var": "value"}, "value"),
-        ("$${var}", {"var": "value"}, "${var}"),
-        ("${var:-}", {}, ""),
-        ("${var:-default}", {}, "default"),
-        ("${var:-default}", {"var": "value"}, "value"),
-        ("$${var:-default}", {}, "${var:-default}"),
-    ],
-)
-def test_substitute_template(value: str, template_env: dict[str, str], expected: str):
-    assert substitute_template(value, template_env) == expected
diff --git a/tests/test_resolver.py b/tests/test_resolver.py
index ef61b9f..5ae87db 100644
--- a/tests/test_resolver.py
+++ b/tests/test_resolver.py
@@ -1,9 +1,12 @@
+import collections
+
 import pytest
 import requests_mock
 import resolvelib
 from packaging.requirements import Requirement
 
 from fromager import constraints, resolver
+from fromager.requirements_file import RequirementType
 
 _hydra_core_simple_response = """
 <!DOCTYPE html>
@@ -31,6 +34,13 @@ _hydra_core_simple_response = """
 """
 
 
+@pytest.fixture(autouse=True)
+def reset_cache():
+    resolver.PyPIProvider.pypi_resolver_cache = collections.defaultdict(list)
+    resolver.GenericProvider.generic_resolver_cache = collections.defaultdict(list)
+    resolver.GitHubTagProvider.github_resolver_cache = collections.defaultdict(list)
+
+
 def test_provider_choose_wheel():
     with requests_mock.Mocker() as r:
         r.get(
@@ -53,6 +63,74 @@ def test_provider_choose_wheel():
         assert str(candidate.version) == "1.3.2"
 
 
+def test_provider_cache():
+    with requests_mock.Mocker() as r:
+        r.get(
+            "https://pypi.org/simple/hydra-core/",
+            text=_hydra_core_simple_response,
+        )
+
+        # fill the cache
+        provider = resolver.PyPIProvider(include_sdists=False)
+        reporter = resolvelib.BaseReporter()
+        rslvr = resolvelib.Resolver(provider, reporter)
+        result = rslvr.resolve([Requirement("hydra-core<1.3")])
+        candidate = result.mapping["hydra-core"]
+        assert str(candidate.version) == "1.2.2"
+        assert "hydra-core" in resolver.PyPIProvider.pypi_resolver_cache
+        assert len(resolver.PyPIProvider.pypi_resolver_cache["hydra-core"]) == 1
+
+        # store a copy of the cache
+        cache_copy = {
+            "hydra-core": resolver.PyPIProvider.pypi_resolver_cache["hydra-core"][:]
+        }
+
+        # resolve for build requirement should end up with the already seen older version
+        provider = resolver.PyPIProvider(
+            include_sdists=False, req_type=RequirementType.BUILD_SDIST
+        )
+        reporter = resolvelib.BaseReporter()
+        rslvr = resolvelib.Resolver(provider, reporter)
+        result = rslvr.resolve([Requirement("hydra-core>=1.2")])
+        candidate = result.mapping["hydra-core"]
+        assert str(candidate.version) == "1.2.2"
+        assert "hydra-core" in resolver.PyPIProvider.pypi_resolver_cache
+        assert len(resolver.PyPIProvider.pypi_resolver_cache["hydra-core"]) == 1
+
+        # resolve for install requirement should ignore the already seen older version
+        provider = resolver.PyPIProvider(
+            include_sdists=False, req_type=RequirementType.INSTALL
+        )
+        reporter = resolvelib.BaseReporter()
+        rslvr = resolvelib.Resolver(provider, reporter)
+        result = rslvr.resolve([Requirement("hydra-core>=1.2")])
+        candidate = result.mapping["hydra-core"]
+        assert str(candidate.version) == "1.3.2"
+
+        # have to restore the cache so that 1.3.2 doesn't get picked up from there
+        resolver.PyPIProvider.pypi_resolver_cache = cache_copy
+
+        # double check that the restoration worked
+        provider = resolver.PyPIProvider(
+            include_sdists=False, req_type=RequirementType.BUILD_SDIST
+        )
+        reporter = resolvelib.BaseReporter()
+        rslvr = resolvelib.Resolver(provider, reporter)
+        result = rslvr.resolve([Requirement("hydra-core>=1.2")])
+        candidate = result.mapping["hydra-core"]
+        assert str(candidate.version) == "1.2.2"
+
+        # if resolving for build but with different conditions, don't use cache
+        provider = resolver.PyPIProvider(
+            include_wheels=False, req_type=RequirementType.BUILD_SDIST
+        )
+        reporter = resolvelib.BaseReporter()
+        rslvr = resolvelib.Resolver(provider, reporter)
+        result = rslvr.resolve([Requirement("hydra-core>=1.2")])
+        candidate = result.mapping["hydra-core"]
+        assert str(candidate.version) == "1.3.2"
+
+
 def test_provider_choose_wheel_prereleases():
     with requests_mock.Mocker() as r:
         r.get(
diff --git a/tests/test_sources.py b/tests/test_sources.py
index 68805c5..9d0aa9b 100644
--- a/tests/test_sources.py
+++ b/tests/test_sources.py
@@ -40,6 +40,7 @@ def test_default_download_source_from_settings(
         sdist_server_url=sdist_server_url,
         include_sdists=True,
         include_wheels=False,
+        req_type=None,
     )
 
     sources.default_download_source(
@@ -82,6 +83,7 @@ def test_default_download_source_with_predefined_resolve_dist(
         sdist_server_url="url",
         include_sdists=False,
         include_wheels=True,
+        req_type=None,
     )
 
 
diff --git a/tests/testdata/context/overrides/settings/test_pkg.yaml b/tests/testdata/context/overrides/settings/test_pkg.yaml
index 542e873..2f1a77c 100644
--- a/tests/testdata/context/overrides/settings/test_pkg.yaml
+++ b/tests/testdata/context/overrides/settings/test_pkg.yaml
@@ -14,7 +14,6 @@ env:
     EGG_AGAIN: "$EGG"
     SPAM: "alot $EXTRA"
     QUOTES: "A\"BC'$$EGG"
-    DEF: "${DEF:-default}"
 download_source:
     url: https://egg.test/${canonicalized_name}/v${version}.tar.gz
     destination_filename: ${canonicalized_name}-${version}.tar.gz