From abcfd231acfa79f5110914be795d159fae6da440 Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Thu, 14 May 2026 14:52:23 -0400 Subject: [PATCH 1/2] fix(resolver): exempt top-level pinned versions from transitive cooldown Add `exempt_versions` field to `Cooldown` so that only the specific version(s) approved via a top-level `==` pin bypass the cooldown age check when resolved as transitive dependencies. Other versions remain subject to cooldown filtering. Fixes: #1153 #1155 Co-Authored-By: Claude Signed-off-by: Lalatendu Mohanty --- src/fromager/candidate.py | 9 +- src/fromager/resolver.py | 90 +++++++-- tests/test_cooldown.py | 396 +++++++++++++++++++++++++++++++++++++- 3 files changed, 466 insertions(+), 29 deletions(-) diff --git a/src/fromager/candidate.py b/src/fromager/candidate.py index 36d5955a..140fa597 100644 --- a/src/fromager/candidate.py +++ b/src/fromager/candidate.py @@ -15,18 +15,25 @@ logger = logging.getLogger(__name__) -@dataclasses.dataclass +@dataclasses.dataclass(frozen=True) class Cooldown: """Policy for rejecting recently-published package versions. + Frozen so that cooldown policy cannot be accidentally weakened after + construction — all parameters are set once and shared read-only. + bootstrap_time is fixed at construction so all resolutions in a single run share the same cutoff. + + exempt_versions bypasses the age check for specific versions that were + already approved via a top-level exact pin. """ min_age: datetime.timedelta bootstrap_time: datetime.datetime = dataclasses.field( default_factory=lambda: datetime.datetime.now(datetime.UTC) ) + exempt_versions: frozenset[Version] = dataclasses.field(default_factory=frozenset) @dataclasses.dataclass(frozen=True, order=True, slots=True, repr=False, kw_only=True) diff --git a/src/fromager/resolver.py b/src/fromager/resolver.py index 0ac959f8..f5d6ea34 100644 --- a/src/fromager/resolver.py +++ b/src/fromager/resolver.py @@ -147,6 +147,52 @@ def _has_equality_pin(req: Requirement) -> bool: return len(specs) == 1 and specs[0].operator == "==" and "*" not in specs[0].version +def _get_toplevel_pinned_versions( + ctx: context.WorkContext, req: Requirement +) -> frozenset[Version]: + """Return versions of *req* that have a top-level exact ``==`` pin in the graph.""" + top_level_edges = ctx.dependency_graph.get_root_node().get_outgoing_edges( + req.name, RequirementType.TOP_LEVEL + ) + return frozenset( + edge.destination_node.version + for edge in top_level_edges + if _has_equality_pin(edge.req) + ) + + +def _resolve_cooldown_params( + ctx: context.WorkContext, + req: Requirement, +) -> tuple[datetime.timedelta, datetime.datetime] | None: + """Resolve min_age and bootstrap_time for a package's cooldown. + + Returns ``None`` when cooldown is disabled (no global/per-package + configuration, or a per-package override of 0). Otherwise returns + the effective ``(min_age, bootstrap_time)`` pair. + """ + min_age_override = ctx.package_build_info(req).resolver_min_release_age + + if ctx.cooldown is None and min_age_override is None: + return None + if min_age_override == 0: + return None + + if min_age_override is not None: + min_age = datetime.timedelta(days=min_age_override) + elif ctx.cooldown is not None: + min_age = ctx.cooldown.min_age + else: + return None + + bootstrap_time = ( + ctx.cooldown.bootstrap_time + if ctx.cooldown is not None + else datetime.datetime.now(datetime.UTC) + ) + return min_age, bootstrap_time + + def resolve_package_cooldown( ctx: context.WorkContext, req: Requirement, @@ -154,35 +200,36 @@ def resolve_package_cooldown( ) -> Cooldown | None: """Compute the effective cooldown for a single package. - Args: - ctx: The current work context (provides the global cooldown). - req: The package requirement being resolved. - req_type: The requirement type (top-level, install, etc.). + Returns ``None`` (cooldown disabled) when: + + * The requirement is a top-level exact ``==`` pin — the user explicitly + approved that version. + * A per-package ``min_release_age=0`` override disables cooldown. + * No global cooldown is configured and no per-package override enables one. - Returns: - The cooldown to pass to the provider, or ``None`` if disabled. + Otherwise a ``Cooldown`` is returned with: + + * *min_age* from the per-package override (if set) or the global cooldown. + * *bootstrap_time* inherited from the global cooldown (for a consistent + cutoff across the entire run) or the current time. + * *exempt_versions* populated from top-level exact-pinned entries in the + dependency graph, so transitive resolutions of the same package honour + the user's explicit pin. """ if req_type == RequirementType.TOP_LEVEL and _has_equality_pin(req): if ctx.cooldown is not None: logger.info("cooldown bypassed as the top-level requirement uses == pin") return None - per_package_days = ctx.package_build_info(req).resolver_min_release_age - global_cooldown = ctx.cooldown - if per_package_days is None: - return global_cooldown - if per_package_days == 0: + params = _resolve_cooldown_params(ctx, req) + if params is None: return None - # Per-package positive override: inherit bootstrap_time from global so all - # resolutions in a single run share the same fixed cutoff point. - bootstrap_time = ( - global_cooldown.bootstrap_time - if global_cooldown is not None - else datetime.datetime.now(datetime.UTC) - ) + + min_age, bootstrap_time = params return Cooldown( - min_age=datetime.timedelta(days=per_package_days), + min_age=min_age, bootstrap_time=bootstrap_time, + exempt_versions=_get_toplevel_pinned_versions(ctx, req), ) @@ -685,11 +732,12 @@ def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> boo def is_blocked_by_cooldown(self, candidate: Candidate) -> bool: """Return True if the candidate is rejected by the release-age cooldown.""" - # a cooldown is not specified... if self.cooldown is None: return False - # the target candidate doesn't provide a valid upload timestamp + if candidate.version in self.cooldown.exempt_versions: + return False + if candidate.upload_time is None: if not self.supports_upload_time: # this provider does not yet support timestamp retrieval (e.g. GitHub). diff --git a/tests/test_cooldown.py b/tests/test_cooldown.py index 098081a3..e2a9f173 100644 --- a/tests/test_cooldown.py +++ b/tests/test_cooldown.py @@ -378,10 +378,13 @@ def _make_ctx( def test_resolve_package_cooldown_inherits_global(tmp_path: pathlib.Path) -> None: - """No per-package override returns the global cooldown unchanged.""" + """No per-package override returns a cooldown equal to the global one.""" ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) result = resolver.resolve_package_cooldown(ctx, Requirement("test-pkg")) - assert result is _COOLDOWN + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset() def test_resolve_package_cooldown_disabled_per_package(tmp_path: pathlib.Path) -> None: @@ -896,7 +899,10 @@ def test_resolve_package_cooldown_enforced_transitive_equality_pin( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.3.2"), req_type=RequirementType.INSTALL ) - assert result is _COOLDOWN + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset() def test_resolve_package_cooldown_enforced_toplevel_no_pin( @@ -907,7 +913,10 @@ def test_resolve_package_cooldown_enforced_toplevel_no_pin( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.TOP_LEVEL ) - assert result is _COOLDOWN + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset() def test_resolve_package_cooldown_none_req_type_not_exempt( @@ -918,7 +927,10 @@ def test_resolve_package_cooldown_none_req_type_not_exempt( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.3.2"), req_type=None ) - assert result is _COOLDOWN + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset() def test_resolve_package_cooldown_toplevel_wildcard_equality_not_exempt( @@ -929,7 +941,10 @@ def test_resolve_package_cooldown_toplevel_wildcard_equality_not_exempt( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.*"), req_type=RequirementType.TOP_LEVEL ) - assert result is _COOLDOWN + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset() def test_resolve_package_cooldown_toplevel_compound_specifier_not_exempt( @@ -940,4 +955,371 @@ def test_resolve_package_cooldown_toplevel_compound_specifier_not_exempt( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.0,>0.9"), req_type=RequirementType.TOP_LEVEL ) - assert result is _COOLDOWN + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset() + + +def test_get_toplevel_pinned_versions_empty(tmp_path: pathlib.Path) -> None: + """No top-level pin in the graph returns an empty frozenset.""" + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + result = resolver._get_toplevel_pinned_versions(ctx, Requirement("test-pkg")) + assert result == frozenset() + + +def test_get_toplevel_pinned_versions_ignores_wildcard_pin( + tmp_path: pathlib.Path, +) -> None: + """A top-level wildcard pin (==1.*) is not an exact pin and must be excluded.""" + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.*"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + result = resolver._get_toplevel_pinned_versions(ctx, Requirement("test-pkg")) + assert result == frozenset() + + +def test_non_exact_toplevel_entry_does_not_exempt(tmp_path: pathlib.Path) -> None: + """A top-level >= entry is not an exact pin — no version is exempted.""" + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg>=1.0"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + result = resolver.resolve_package_cooldown( + ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL + ) + assert result is not None + assert result.exempt_versions == frozenset() + + +def test_wildcard_toplevel_pin_does_not_exempt(tmp_path: pathlib.Path) -> None: + """A top-level ==1.* entry is not a true exact pin — no version is exempted.""" + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.*"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + result = resolver.resolve_package_cooldown( + ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL + ) + assert result is not None + assert result.exempt_versions == frozenset() + + +def test_name_normalization_across_requirement_and_graph( + tmp_path: pathlib.Path, +) -> None: + """Exemption works even when requirement and graph use different name forms. + + The transitive requirement uses ``Test_Pkg`` while the graph entry uses + the canonical ``test-pkg``. Name normalization in ``get_outgoing_edges`` + must handle this transparently. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.3.2"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + result = resolver.resolve_package_cooldown( + ctx, Requirement("Test_Pkg>=1.0"), req_type=RequirementType.INSTALL + ) + assert result is not None + assert result.exempt_versions == frozenset({Version("1.3.2")}) + + +def test_toplevel_pin_takes_precedence_over_per_package_override( + tmp_path: pathlib.Path, +) -> None: + """Top-level == pin bypasses cooldown even with a per-package min_release_age. + + The per-package setting (30 days) is a weaker signal than an explicit + top-level pin. The pin should win and disable cooldown entirely. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN, min_release_age=30) + result = resolver.resolve_package_cooldown( + ctx, Requirement("test-pkg==1.3.2"), req_type=RequirementType.TOP_LEVEL + ) + assert result is None + + +def test_transitive_dep_cooldown_blocks_non_pinned_version( + tmp_path: pathlib.Path, +) -> None: + """Cooldown must block non-pinned versions even when a top-level pin exists. + + Pin test-pkg==1.3.2 top-level. A transitive dep asks for test-pkg>=1.0. + Version 2.0.0 (2 days old) is within the 7-day cooldown window and is NOT + the pinned version, so cooldown must block it. The resolver should select + 1.3.2 (the pinned version, 11 days old, outside cooldown). + + This is the scenario where PR #1154's blanket bypass was too broad — it + disabled cooldown for all versions of the package instead of only the + pinned version. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.3.2"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + + with requests_mock.Mocker() as r: + r.get( + "https://pypi.org/simple/test-pkg/", + json=_cooldown_json_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + + _, version = resolver.resolve( + ctx=ctx, + req=Requirement("test-pkg>=1.0"), + sdist_server_url="https://pypi.org/simple/", + include_sdists=True, + include_wheels=True, + req_type=RequirementType.INSTALL, + ) + assert str(version) == "1.3.2" + + +def test_transitive_dep_cooldown_not_bypassed_for_all_versions( + tmp_path: pathlib.Path, +) -> None: + """Cooldown for transitive deps must not be fully bypassed by a top-level pin. + + When a top-level pin exists, `resolve_package_cooldown` should still return + a cooldown (not None) for transitive deps so that non-pinned versions + remain subject to cooldown filtering. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==2.0.0"), + req_version=Version("2.0.0"), + download_url="https://files.pythonhosted.org/packages/test_pkg-2.0.0-py3-none-any.whl", + pre_built=False, + ) + + result = resolver.resolve_package_cooldown( + ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL + ) + assert result is not None + assert result.min_age == _COOLDOWN.min_age + assert result.bootstrap_time == _COOLDOWN.bootstrap_time + assert result.exempt_versions == frozenset({Version("2.0.0")}) + + +def test_transitive_dep_cooldown_unpinned_transitive_spec( + tmp_path: pathlib.Path, +) -> None: + """Transitive dep with no version spec still respects cooldown. + + Top-level pins test-pkg==1.3.2. A transitive dep asks for bare + ``test-pkg`` (no specifier). Version 2.0.0 (within cooldown) must be + blocked; 1.3.2 (outside cooldown) should be selected. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.3.2"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + + with requests_mock.Mocker() as r: + r.get( + "https://pypi.org/simple/test-pkg/", + json=_cooldown_json_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + + _, version = resolver.resolve( + ctx=ctx, + req=Requirement("test-pkg"), + sdist_server_url="https://pypi.org/simple/", + include_sdists=True, + include_wheels=True, + req_type=RequirementType.INSTALL, + ) + assert str(version) == "1.3.2" + + +def test_transitive_dep_cooldown_lower_bound_matches_pin( + tmp_path: pathlib.Path, +) -> None: + """Transitive dep whose lower bound matches the pin still respects cooldown. + + Top-level pins test-pkg==1.3.2. A transitive dep asks for + test-pkg>=1.3.2. Version 2.0.0 (within cooldown) must be blocked; + 1.3.2 (outside cooldown, matches the pin) should be selected. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.3.2"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + + with requests_mock.Mocker() as r: + r.get( + "https://pypi.org/simple/test-pkg/", + json=_cooldown_json_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + + _, version = resolver.resolve( + ctx=ctx, + req=Requirement("test-pkg>=1.3.2"), + sdist_server_url="https://pypi.org/simple/", + include_sdists=True, + include_wheels=True, + req_type=RequirementType.INSTALL, + ) + assert str(version) == "1.3.2" + + +def test_transitive_dep_cooldown_conflict_with_pin( + tmp_path: pathlib.Path, +) -> None: + """Transitive dep that conflicts with the pin fails regardless of cooldown. + + Top-level pins test-pkg==1.3.2. A transitive dep asks for + test-pkg>=2.0. No version satisfies both — 2.0.0 is blocked by cooldown + and nothing else matches >=2.0. Resolution should fail. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==1.3.2"), + req_version=Version("1.3.2"), + download_url="https://files.pythonhosted.org/packages/test_pkg-1.3.2-py3-none-any.whl", + pre_built=False, + ) + + with requests_mock.Mocker() as r: + r.get( + "https://pypi.org/simple/test-pkg/", + json=_cooldown_json_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + + with pytest.raises(resolvelib.resolvers.ResolverException): + resolver.resolve( + ctx=ctx, + req=Requirement("test-pkg>=2.0"), + sdist_server_url="https://pypi.org/simple/", + include_sdists=True, + include_wheels=True, + req_type=RequirementType.INSTALL, + ) + + +def test_transitive_dep_exempts_pinned_version_from_cooldown( + tmp_path: pathlib.Path, +) -> None: + """Transitive dep should exempt only the pinned version from cooldown. + + If a requirements file pins test-pkg==2.0.0 (top-level) and another + top-level package depends on test-pkg>=1.0 (transitive), cooldown should + remain active but exempt version 2.0.0 — the user already explicitly + approved that version via the pin. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==2.0.0"), + req_version=Version("2.0.0"), + download_url="https://files.pythonhosted.org/packages/test_pkg-2.0.0-py3-none-any.whl", + pre_built=False, + ) + + result = resolver.resolve_package_cooldown( + ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL + ) + assert result is not None + assert result.exempt_versions == frozenset({Version("2.0.0")}) + + +def test_transitive_dep_resolves_to_toplevel_pinned_version( + tmp_path: pathlib.Path, +) -> None: + """End-to-end: transitive dep selects the top-level pinned version, not an older one. + + With cooldown active, test-pkg 2.0.0 (2 days old) is within the cooldown + window. A top-level pin test-pkg==2.0.0 exempts 2.0.0 from cooldown. + When the same package appears as a transitive dependency (test-pkg>=1.0), + it should resolve to 2.0.0 — not fall back to 1.3.2. + """ + ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) + + ctx.dependency_graph.add_dependency( + parent_name=None, + parent_version=None, + req_type=RequirementType.TOP_LEVEL, + req=Requirement("test-pkg==2.0.0"), + req_version=Version("2.0.0"), + download_url="https://files.pythonhosted.org/packages/test_pkg-2.0.0-py3-none-any.whl", + pre_built=False, + ) + + with requests_mock.Mocker() as r: + r.get( + "https://pypi.org/simple/test-pkg/", + json=_cooldown_json_response, + headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, + ) + + _, version = resolver.resolve( + ctx=ctx, + req=Requirement("test-pkg>=1.0"), + sdist_server_url="https://pypi.org/simple/", + include_sdists=True, + include_wheels=True, + req_type=RequirementType.INSTALL, + ) + assert str(version) == "2.0.0" From 12b8635cdb6a254d0d5e702f64f5cadd09820d14 Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Tue, 19 May 2026 18:45:13 -0400 Subject: [PATCH 2/2] refactor(resolver): make Cooldown always present, never None Replace the Optional[Cooldown] pattern with a Null Object: a Cooldown with min_age=0 is effectively disabled since every package age exceeds zero. Callers no longer need None checks, and each WorkContext/provider gets a fresh instance so bootstrap_time is never stale. Co-Authored-By: Claude Signed-off-by: Lalatendu Mohanty --- src/fromager/__main__.py | 6 +--- src/fromager/candidate.py | 9 ++++++ src/fromager/context.py | 9 ++++-- src/fromager/finders.py | 1 - src/fromager/resolver.py | 63 +++++++++++++++------------------------ tests/test_cooldown.py | 52 ++++++++++++++++---------------- tests/test_finders.py | 2 +- 7 files changed, 67 insertions(+), 75 deletions(-) diff --git a/src/fromager/__main__.py b/src/fromager/__main__.py index 2ac6f4b5..153687a1 100644 --- a/src/fromager/__main__.py +++ b/src/fromager/__main__.py @@ -283,11 +283,7 @@ def main( network_isolation=network_isolation, max_jobs=jobs, settings_dir=settings_dir, - cooldown=( - candidate.Cooldown(min_age=datetime.timedelta(days=min_release_age)) - if min_release_age > 0 - else None - ), + cooldown=candidate.Cooldown(min_age=datetime.timedelta(days=min_release_age)), ) wkctx.setup() ctx.obj = wkctx diff --git a/src/fromager/candidate.py b/src/fromager/candidate.py index 140fa597..5a67d62a 100644 --- a/src/fromager/candidate.py +++ b/src/fromager/candidate.py @@ -22,6 +22,10 @@ class Cooldown: Frozen so that cooldown policy cannot be accidentally weakened after construction — all parameters are set once and shared read-only. + A cooldown with ``min_age`` of zero (or negative) is effectively disabled — + every package age exceeds the threshold. Use :meth:`disabled` as a + convenient factory instead of ``None``. + bootstrap_time is fixed at construction so all resolutions in a single run share the same cutoff. @@ -35,6 +39,11 @@ class Cooldown: ) exempt_versions: frozenset[Version] = dataclasses.field(default_factory=frozenset) + @classmethod + def disabled(cls) -> "Cooldown": + """Return a cooldown that never filters any candidate.""" + return cls(min_age=datetime.timedelta(0)) + @dataclasses.dataclass(frozen=True, order=True, slots=True, repr=False, kw_only=True) class Candidate: diff --git a/src/fromager/context.py b/src/fromager/context.py index 58866aed..c2c6b1b3 100644 --- a/src/fromager/context.py +++ b/src/fromager/context.py @@ -20,9 +20,10 @@ external_commands, packagesettings, ) +from .candidate import Cooldown if typing.TYPE_CHECKING: - from . import build_environment, candidate + from . import build_environment logger = logging.getLogger(__name__) @@ -47,7 +48,7 @@ def __init__( max_jobs: int | None = None, settings_dir: pathlib.Path | None = None, wheel_server_url: str = "", - cooldown: candidate.Cooldown | None = None, + cooldown: Cooldown | None = None, max_release_age: datetime.timedelta | None = None, ): if active_settings is None: @@ -95,7 +96,9 @@ def __init__( self._parallel_builds = False - self.cooldown: candidate.Cooldown | None = cooldown + self.cooldown: Cooldown = ( + cooldown if cooldown is not None else Cooldown.disabled() + ) self._max_release_age: datetime.timedelta | None = max_release_age @property diff --git a/src/fromager/finders.py b/src/fromager/finders.py index 69c31e33..6729bc85 100644 --- a/src/fromager/finders.py +++ b/src/fromager/finders.py @@ -61,7 +61,6 @@ def __init__( ignore_platform=False, use_resolver_cache=use_resolver_cache, override_download_url=None, - cooldown=None, supports_upload_time=False, ) diff --git a/src/fromager/resolver.py b/src/fromager/resolver.py index f5d6ea34..f17aca81 100644 --- a/src/fromager/resolver.py +++ b/src/fromager/resolver.py @@ -164,68 +164,57 @@ def _get_toplevel_pinned_versions( def _resolve_cooldown_params( ctx: context.WorkContext, req: Requirement, -) -> tuple[datetime.timedelta, datetime.datetime] | None: +) -> tuple[datetime.timedelta, datetime.datetime]: """Resolve min_age and bootstrap_time for a package's cooldown. - Returns ``None`` when cooldown is disabled (no global/per-package - configuration, or a per-package override of 0). Otherwise returns - the effective ``(min_age, bootstrap_time)`` pair. + Always returns a ``(min_age, bootstrap_time)`` pair. When cooldown is + disabled (per-package override of 0, or no global/per-package config), + ``min_age`` will be zero — every package age exceeds that threshold. """ min_age_override = ctx.package_build_info(req).resolver_min_release_age - if ctx.cooldown is None and min_age_override is None: - return None if min_age_override == 0: - return None - - if min_age_override is not None: + min_age = datetime.timedelta(0) + elif min_age_override is not None: min_age = datetime.timedelta(days=min_age_override) - elif ctx.cooldown is not None: - min_age = ctx.cooldown.min_age else: - return None + min_age = ctx.cooldown.min_age - bootstrap_time = ( - ctx.cooldown.bootstrap_time - if ctx.cooldown is not None - else datetime.datetime.now(datetime.UTC) - ) - return min_age, bootstrap_time + return min_age, ctx.cooldown.bootstrap_time def resolve_package_cooldown( ctx: context.WorkContext, req: Requirement, req_type: RequirementType | None = None, -) -> Cooldown | None: +) -> Cooldown: """Compute the effective cooldown for a single package. - Returns ``None`` (cooldown disabled) when: + Always returns a ``Cooldown`` instance. A ``min_age`` of zero means + cooldown is effectively disabled — every package age exceeds zero. + + Returns a *disabled* cooldown (min_age=0) when: * The requirement is a top-level exact ``==`` pin — the user explicitly approved that version. * A per-package ``min_release_age=0`` override disables cooldown. * No global cooldown is configured and no per-package override enables one. - Otherwise a ``Cooldown`` is returned with: + Otherwise returns an *active* cooldown with: * *min_age* from the per-package override (if set) or the global cooldown. * *bootstrap_time* inherited from the global cooldown (for a consistent - cutoff across the entire run) or the current time. + cutoff across the entire run). * *exempt_versions* populated from top-level exact-pinned entries in the dependency graph, so transitive resolutions of the same package honour the user's explicit pin. """ if req_type == RequirementType.TOP_LEVEL and _has_equality_pin(req): - if ctx.cooldown is not None: + if ctx.cooldown.min_age > datetime.timedelta(0): logger.info("cooldown bypassed as the top-level requirement uses == pin") - return None - - params = _resolve_cooldown_params(ctx, req) - if params is None: - return None + return Cooldown.disabled() - min_age, bootstrap_time = params + min_age, bootstrap_time = _resolve_cooldown_params(ctx, req) return Cooldown( min_age=min_age, bootstrap_time=bootstrap_time, @@ -243,12 +232,7 @@ def _compute_max_age_cutoff( """ if ctx.max_release_age is None: return None - bootstrap_time = ( - ctx.cooldown.bootstrap_time - if ctx.cooldown is not None - else datetime.datetime.now(datetime.UTC) - ) - return bootstrap_time - ctx.max_release_age + return ctx.cooldown.bootstrap_time - ctx.max_release_age def extract_filename_from_url(url: str) -> str: @@ -609,8 +593,9 @@ def __init__( self.req_type = req_type self.use_cache_candidates = use_resolver_cache - # cooldown specific settings - self.cooldown = cooldown + self.cooldown: Cooldown = ( + cooldown if cooldown is not None else Cooldown.disabled() + ) # Does this provider supply upload timestamps for candidates? # Defaults to False (safe/unknown). Subclasses that reliably populate # upload_time on every candidate should set this to True in their __init__. @@ -732,7 +717,7 @@ def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> boo def is_blocked_by_cooldown(self, candidate: Candidate) -> bool: """Return True if the candidate is rejected by the release-age cooldown.""" - if self.cooldown is None: + if self.cooldown.min_age <= datetime.timedelta(0): return False if candidate.version in self.cooldown.exempt_versions: @@ -981,7 +966,7 @@ def _get_no_match_error_message( # If a cooldown is active, check whether it's responsible for the # failure so we can give a more actionable error message. - if self.cooldown is not None: + if self.cooldown.min_age > datetime.timedelta(0): cutoff = self.cooldown.bootstrap_time - self.cooldown.min_age all_candidates = list(self._find_cached_candidates(identifier)) missing_time = [c for c in all_candidates if c.upload_time is None] diff --git a/tests/test_cooldown.py b/tests/test_cooldown.py index e2a9f173..85b1b286 100644 --- a/tests/test_cooldown.py +++ b/tests/test_cooldown.py @@ -128,7 +128,7 @@ def test_cooldown_disabled_selects_latest() -> None: json=_cooldown_json_response, headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE}, ) - provider = resolver.PyPIProvider(include_sdists=True, cooldown=None) + provider = resolver.PyPIProvider(include_sdists=True) rslvr = resolvelib.Resolver(provider, resolvelib.BaseReporter()) result = rslvr.resolve([Requirement("test-pkg")]) @@ -351,7 +351,7 @@ def test_non_pypi_index_allows_without_upload_time( def _make_ctx( tmp_path: pathlib.Path, *, - cooldown: candidate.Cooldown | None, + cooldown: candidate.Cooldown | None = None, min_release_age: int | None = None, ) -> context.WorkContext: """Build a WorkContext with an optional per-package min_release_age setting.""" @@ -381,7 +381,7 @@ def test_resolve_package_cooldown_inherits_global(tmp_path: pathlib.Path) -> Non """No per-package override returns a cooldown equal to the global one.""" ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN) result = resolver.resolve_package_cooldown(ctx, Requirement("test-pkg")) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset() @@ -391,21 +391,21 @@ def test_resolve_package_cooldown_disabled_per_package(tmp_path: pathlib.Path) - """min_release_age=0 disables the cooldown for the package even when global is set.""" ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN, min_release_age=0) result = resolver.resolve_package_cooldown(ctx, Requirement("test-pkg")) - assert result is None + assert not result.min_age def test_resolve_package_cooldown_disabled_no_global(tmp_path: pathlib.Path) -> None: - """min_release_age=0 with no global cooldown still returns None.""" - ctx = _make_ctx(tmp_path, cooldown=None, min_release_age=0) + """min_release_age=0 with no global cooldown returns a disabled cooldown.""" + ctx = _make_ctx(tmp_path, min_release_age=0) result = resolver.resolve_package_cooldown(ctx, Requirement("test-pkg")) - assert result is None + assert not result.min_age def test_resolve_package_cooldown_override_days(tmp_path: pathlib.Path) -> None: """Positive per-package override creates a new Cooldown with the given days.""" ctx = _make_ctx(tmp_path, cooldown=_COOLDOWN, min_release_age=30) result = resolver.resolve_package_cooldown(ctx, Requirement("test-pkg")) - assert result is not None + assert result.min_age assert result.min_age.days == 30 # bootstrap_time is inherited from the global cooldown for a consistent cutoff. assert result.bootstrap_time == _COOLDOWN.bootstrap_time @@ -413,9 +413,9 @@ def test_resolve_package_cooldown_override_days(tmp_path: pathlib.Path) -> None: def test_resolve_package_cooldown_override_no_global(tmp_path: pathlib.Path) -> None: """Positive per-package override works even without a global cooldown.""" - ctx = _make_ctx(tmp_path, cooldown=None, min_release_age=14) + ctx = _make_ctx(tmp_path, min_release_age=14) result = resolver.resolve_package_cooldown(ctx, Requirement("test-pkg")) - assert result is not None + assert result.min_age assert result.min_age.days == 14 @@ -533,7 +533,7 @@ def test_per_package_cooldown_disable_via_ctx(tmp_path: pathlib.Path) -> None: def _make_gitlab_provider( - cooldown: candidate.Cooldown | None, + cooldown: candidate.Cooldown | None = None, ) -> resolver.GitLabTagProvider: return resolver.GitLabTagProvider( project_path="test/pkg", @@ -568,7 +568,7 @@ def test_gitlab_cooldown_disabled_selects_latest() -> None: """Without a cooldown, GitLabTagProvider selects the latest tag.""" with requests_mock.Mocker() as r: r.get(_GITLAB_API_URL, text=_gitlab_tags_response) - provider = _make_gitlab_provider(cooldown=None) + provider = _make_gitlab_provider() rslvr = resolvelib.Resolver(provider, resolvelib.BaseReporter()) result = rslvr.resolve([Requirement("test-pkg")]) assert str(result.mapping["test-pkg"].version) == "0.0.3" @@ -863,8 +863,8 @@ def test_compute_max_age_cutoff_with_cooldown( def test_compute_max_age_cutoff_without_cooldown( tmp_context: context.WorkContext, ) -> None: - """_compute_max_age_cutoff uses current time when no cooldown is set.""" - tmp_context.cooldown = None + """_compute_max_age_cutoff uses disabled cooldown's bootstrap_time.""" + tmp_context.cooldown = candidate.Cooldown.disabled() tmp_context.set_max_release_age(30) cutoff = resolver._compute_max_age_cutoff(tmp_context) assert cutoff is not None @@ -888,7 +888,7 @@ def test_resolve_package_cooldown_exempt_toplevel_equality_pin( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.3.2"), req_type=RequirementType.TOP_LEVEL ) - assert result is None + assert not result.min_age def test_resolve_package_cooldown_enforced_transitive_equality_pin( @@ -899,7 +899,7 @@ def test_resolve_package_cooldown_enforced_transitive_equality_pin( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.3.2"), req_type=RequirementType.INSTALL ) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset() @@ -913,7 +913,7 @@ def test_resolve_package_cooldown_enforced_toplevel_no_pin( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.TOP_LEVEL ) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset() @@ -927,7 +927,7 @@ def test_resolve_package_cooldown_none_req_type_not_exempt( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.3.2"), req_type=None ) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset() @@ -941,7 +941,7 @@ def test_resolve_package_cooldown_toplevel_wildcard_equality_not_exempt( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.*"), req_type=RequirementType.TOP_LEVEL ) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset() @@ -955,7 +955,7 @@ def test_resolve_package_cooldown_toplevel_compound_specifier_not_exempt( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.0,>0.9"), req_type=RequirementType.TOP_LEVEL ) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset() @@ -1001,7 +1001,7 @@ def test_non_exact_toplevel_entry_does_not_exempt(tmp_path: pathlib.Path) -> Non result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL ) - assert result is not None + assert result.min_age assert result.exempt_versions == frozenset() @@ -1020,7 +1020,7 @@ def test_wildcard_toplevel_pin_does_not_exempt(tmp_path: pathlib.Path) -> None: result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL ) - assert result is not None + assert result.min_age assert result.exempt_versions == frozenset() @@ -1046,7 +1046,7 @@ def test_name_normalization_across_requirement_and_graph( result = resolver.resolve_package_cooldown( ctx, Requirement("Test_Pkg>=1.0"), req_type=RequirementType.INSTALL ) - assert result is not None + assert result.min_age assert result.exempt_versions == frozenset({Version("1.3.2")}) @@ -1062,7 +1062,7 @@ def test_toplevel_pin_takes_precedence_over_per_package_override( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg==1.3.2"), req_type=RequirementType.TOP_LEVEL ) - assert result is None + assert not result.min_age def test_transitive_dep_cooldown_blocks_non_pinned_version( @@ -1133,7 +1133,7 @@ def test_transitive_dep_cooldown_not_bypassed_for_all_versions( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL ) - assert result is not None + assert result.min_age assert result.min_age == _COOLDOWN.min_age assert result.bootstrap_time == _COOLDOWN.bootstrap_time assert result.exempt_versions == frozenset({Version("2.0.0")}) @@ -1281,7 +1281,7 @@ def test_transitive_dep_exempts_pinned_version_from_cooldown( result = resolver.resolve_package_cooldown( ctx, Requirement("test-pkg>=1.0"), req_type=RequirementType.INSTALL ) - assert result is not None + assert result.min_age assert result.exempt_versions == frozenset({Version("2.0.0")}) diff --git a/tests/test_finders.py b/tests/test_finders.py index 110ccfa1..4879070e 100644 --- a/tests/test_finders.py +++ b/tests/test_finders.py @@ -123,7 +123,7 @@ def test_pypi_cache_provider() -> None: assert provider.include_wheels is True assert provider.ignore_platform is False assert provider.override_download_url is None - assert provider.cooldown is None + assert not provider.cooldown.min_age assert provider.supports_upload_time is False # sdists only with req_type