diff --git a/docs/concepts/resolver-architecture.rst b/docs/concepts/resolver-architecture.rst index 67fefa0e..1865151a 100644 --- a/docs/concepts/resolver-architecture.rst +++ b/docs/concepts/resolver-architecture.rst @@ -90,10 +90,16 @@ CLI commands interact with providers through a common Used when versions are already known, e.g. from a prior resolution or a settings-provided URL template. +.. versionchanged:: 0.95.0 + Packages with ``source:`` configuration now select their provider through + the configured source resolver. + Per-package settings in YAML can select which provider to use and -configure its parameters (index URL, tag pattern, etc.). Override -plugins can replace the provider entirely for a specific package via -the ``get_resolver_provider`` hook. +configure its parameters (index URL, tag pattern, etc.). When a package +has a ``source:`` resolver configured, that resolver creates the provider. +Otherwise, override plugins can replace the provider for a specific package +via the ``get_resolver_provider`` hook, after which the legacy resolver +settings are used. Version Filtering Window ------------------------- diff --git a/src/fromager/packagesettings/_resolver.py b/src/fromager/packagesettings/_resolver.py index 213cf073..0059b167 100644 --- a/src/fromager/packagesettings/_resolver.py +++ b/src/fromager/packagesettings/_resolver.py @@ -67,7 +67,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.BaseProvider: """Return a resolver provider for the given requirement.""" raise NotImplementedError @@ -198,7 +198,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.PyPIProvider: return resolver.PyPIProvider( include_sdists=True, @@ -249,7 +249,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.PyPIProvider: return resolver.PyPIProvider( include_sdists=False, @@ -322,7 +322,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.PyPIProvider: return resolver.PyPIProvider( include_sdists=True, @@ -404,7 +404,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.PyPIProvider: download_url = f"git+{self.clone_url}@refs/tags/{self.tag}" return resolver.PyPIProvider( @@ -510,7 +510,7 @@ def _github_provider( self, *, ctx: context.WorkContext, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, override_download_url: str | None = None, ) -> resolver.GitHubTagProvider: if self.project_url.host != "github.com": @@ -534,7 +534,7 @@ def _gitlab_provider( self, *, ctx: context.WorkContext, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, override_download_url: str | None = None, ) -> resolver.GitLabTagProvider: if not self.project_url.path: @@ -571,7 +571,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.GitHubTagProvider: return self._github_provider( ctx=ctx, @@ -612,7 +612,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.GitHubTagProvider: return self._github_provider( ctx=ctx, @@ -651,7 +651,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.GitLabTagProvider: return self._gitlab_provider( ctx=ctx, @@ -692,7 +692,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.GitLabTagProvider: return self._gitlab_provider( ctx=ctx, @@ -719,7 +719,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.BaseProvider: raise ValueError(f"package {req.name} is not available") @@ -743,7 +743,7 @@ def resolver_provider( self, ctx: context.WorkContext, req: Requirement, - req_type: requirements_file.RequirementType, + req_type: requirements_file.RequirementType | None, ) -> resolver.BaseProvider: # TODO raise NotImplementedError("Hook resolver needs a hook") diff --git a/src/fromager/resolver.py b/src/fromager/resolver.py index 9f2cfa1b..d9a9a2c6 100644 --- a/src/fromager/resolver.py +++ b/src/fromager/resolver.py @@ -157,6 +157,36 @@ def _has_equality_pin(req: Requirement) -> bool: return len(specs) == 1 and specs[0].operator == "==" and "*" not in specs[0].version +def _effective_cooldown( + global_cooldown: Cooldown | None, + per_package_days: int | None, +) -> Cooldown | None: + """Merge a per-package cooldown override with the global cooldown. + + Args: + global_cooldown: The global cooldown from ``ctx.cooldown``. + per_package_days: Per-package override in days — ``None`` inherits + the global value, ``0`` disables cooldown, a positive integer + overrides it. + + Returns: + The effective cooldown, or ``None`` if disabled. + """ + if per_package_days is None: + return global_cooldown + if per_package_days == 0: + return None + bootstrap_time = ( + global_cooldown.bootstrap_time + if global_cooldown is not None + else datetime.datetime.now(datetime.UTC) + ) + return Cooldown( + min_age=datetime.timedelta(days=per_package_days), + bootstrap_time=bootstrap_time, + ) + + def resolve_package_cooldown( ctx: context.WorkContext, req: Requirement, @@ -178,22 +208,7 @@ def resolve_package_cooldown( 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: - 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) - ) - return Cooldown( - min_age=datetime.timedelta(days=per_package_days), - bootstrap_time=bootstrap_time, - ) + return _effective_cooldown(ctx.cooldown, per_package_days) def _compute_max_age_cutoff( diff --git a/src/fromager/sources.py b/src/fromager/sources.py index 534573d4..6cd8633a 100644 --- a/src/fromager/sources.py +++ b/src/fromager/sources.py @@ -116,8 +116,25 @@ def get_source_provider( Returns a provider configured according to the package's resolver settings (sdist/wheel inclusion, platform matching, server URL override). + + Packages with a ``source:`` resolver use that resolver's provider + configuration. Packages without one retain the legacy override-hook and + ``resolver_dist`` behavior. """ pbi = ctx.package_build_info(req) + + source_resolver = pbi.source_resolver + if source_resolver is not None: + provider = source_resolver.resolver_provider(ctx, req, req_type) + if req_type == RequirementType.TOP_LEVEL and resolver._has_equality_pin(req): + provider.cooldown = None + else: + per_package_days = getattr(source_resolver, "min_release_age", None) + provider.cooldown = resolver._effective_cooldown( + ctx.cooldown, per_package_days + ) + return provider + override_sdist_server_url = pbi.resolver_sdist_server_url(sdist_server_url) provider = typing.cast( diff --git a/tests/test_bootstrap_requirement_resolver.py b/tests/test_bootstrap_requirement_resolver.py index 7222a6fb..22374e1e 100644 --- a/tests/test_bootstrap_requirement_resolver.py +++ b/tests/test_bootstrap_requirement_resolver.py @@ -494,7 +494,7 @@ def test_resolve_auto_routes_to_source( mock_pbi.resolver_min_release_age = None with patch.object(tmp_context, "package_build_info", return_value=mock_pbi): - resolver = BootstrapRequirementResolver(tmp_context) + brr = BootstrapRequirementResolver(tmp_context) # Mock source resolution to return expected result (as list) mock_resolve.return_value = [ @@ -502,15 +502,24 @@ def test_resolve_auto_routes_to_source( ] # Call resolve with pre_built=None (should auto-detect) - results = resolver.resolve( - req=req, - req_type=RequirementType.INSTALL, - parent_req=None, - pre_built=None, - ) + with patch( + "fromager.bootstrap_requirement_resolver.sources.get_source_provider" + ) as get_source_provider: + results = brr.resolve( + req=req, + req_type=RequirementType.INSTALL, + parent_req=None, + pre_built=None, + ) # Verify resolution was called mock_resolve.assert_called_once() + get_source_provider.assert_called_once_with( + ctx=tmp_context, + req=req, + sdist_server_url="https://pypi.org/simple", + req_type=RequirementType.INSTALL, + ) assert len(results) == 1 url, version = results[0] assert url == "https://files.pythonhosted.org/mypackage-2.0.tar.gz" diff --git a/tests/test_sources.py b/tests/test_sources.py index 1ed0904b..74107abc 100644 --- a/tests/test_sources.py +++ b/tests/test_sources.py @@ -1,14 +1,193 @@ +import datetime import pathlib import sys import tarfile import zipfile -from unittest.mock import Mock, patch +from unittest.mock import Mock, PropertyMock, patch +import pydantic import pytest from packaging.requirements import Requirement from packaging.version import Version from fromager import context, packagesettings, resolver, sources +from fromager.candidate import Cooldown +from fromager.requirements_file import RequirementType + + +def test_get_source_provider_uses_configured_source_resolver( + tmp_context: context.WorkContext, +) -> None: + """Use the configured source resolver instead of legacy hooks.""" + req = Requirement("test-pkg") + provider = Mock(spec=resolver.BaseProvider) + source_resolver = Mock() + source_resolver.resolver_provider.return_value = provider + source_resolver.min_release_age = None + + with ( + patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ), + patch("fromager.sources.overrides.find_and_invoke") as find_and_invoke, + ): + result = sources.get_source_provider( + ctx=tmp_context, + req=req, + sdist_server_url=resolver.PYPI_SERVER_URL, + ) + + assert result is provider + source_resolver.resolver_provider.assert_called_once_with(tmp_context, req, None) + find_and_invoke.assert_not_called() + + +def test_get_source_provider_uses_pypi_sdist_source_resolver( + tmp_context: context.WorkContext, +) -> None: + """Configure a PyPI sdist provider from the source resolver settings.""" + req = Requirement("test-pkg") + source_resolver = packagesettings.PyPISDistResolver( + provider="pypi-sdist", + index_url=pydantic.HttpUrl("https://pypi.test/simple"), + ) + + with patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ): + provider = sources.get_source_provider( + ctx=tmp_context, + req=req, + sdist_server_url=resolver.PYPI_SERVER_URL, + ) + + assert isinstance(provider, resolver.PyPIProvider) + assert provider.include_sdists is True + assert provider.include_wheels is False + assert provider.sdist_server_url == "https://pypi.test/simple" + + +def test_get_source_provider_forwards_req_type_to_source_resolver( + tmp_context: context.WorkContext, +) -> None: + """Forward req_type to the source resolver provider.""" + req = Requirement("test-pkg") + source_resolver = packagesettings.PyPISDistResolver( + provider="pypi-sdist", + index_url=pydantic.HttpUrl("https://pypi.test/simple"), + ) + + with patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ): + provider = sources.get_source_provider( + ctx=tmp_context, + req=req, + sdist_server_url=resolver.PYPI_SERVER_URL, + req_type=RequirementType.INSTALL, + ) + + assert isinstance(provider, resolver.PyPIProvider) + assert provider.req_type is RequirementType.INSTALL + + +def test_get_source_provider_source_resolver_inherits_global_cooldown( + tmp_context: context.WorkContext, +) -> None: + """Source resolver with no per-package cooldown inherits the global one.""" + req = Requirement("test-pkg") + global_cooldown = Cooldown(min_age=datetime.timedelta(days=7)) + tmp_context.cooldown = global_cooldown + + source_resolver = packagesettings.PyPISDistResolver( + provider="pypi-sdist", + index_url=pydantic.HttpUrl("https://pypi.test/simple"), + ) + + with patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ): + provider = sources.get_source_provider( + ctx=tmp_context, + req=req, + sdist_server_url=resolver.PYPI_SERVER_URL, + ) + + assert provider.cooldown is global_cooldown + + +def test_get_source_provider_source_resolver_toplevel_pin_bypasses_cooldown( + tmp_context: context.WorkContext, +) -> None: + """Top-level == pin disables cooldown even with a source resolver.""" + req = Requirement("test-pkg==1.0") + tmp_context.cooldown = Cooldown(min_age=datetime.timedelta(days=7)) + + source_resolver = packagesettings.PyPISDistResolver( + provider="pypi-sdist", + index_url=pydantic.HttpUrl("https://pypi.test/simple"), + min_release_age=14, + ) + + with patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ): + provider = sources.get_source_provider( + ctx=tmp_context, + req=req, + sdist_server_url=resolver.PYPI_SERVER_URL, + req_type=RequirementType.TOP_LEVEL, + ) + + assert provider.cooldown is None + + +@patch("fromager.resolver.find_all_matching_from_provider") +def test_resolve_source_uses_configured_source_resolver( + find_all_matching_from_provider: Mock, + tmp_context: context.WorkContext, +) -> None: + """Resolve versions through a configured source resolver provider.""" + req = Requirement("test-pkg>=1.0") + provider = Mock(spec=resolver.BaseProvider) + source_resolver = Mock() + source_resolver.resolver_provider.return_value = provider + source_resolver.min_release_age = None + find_all_matching_from_provider.return_value = [("url", Version("1.0"))] + + with patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ): + url, version = sources.resolve_source( + ctx=tmp_context, + req=req, + sdist_server_url=resolver.PYPI_SERVER_URL, + ) + + assert (url, version) == ("url", Version("1.0")) + find_all_matching_from_provider.assert_called_once_with( + provider, + req, + max_age_cutoff=None, + ) @patch("fromager.resolver.find_all_matching_from_provider")