From 370c3c86476576b6ab71dc91e057c99c093b6c91 Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Wed, 19 Nov 2025 13:24:04 -0500 Subject: [PATCH] fix(resolver): indicate resolver type in error messages Add provider description to error messages when resolution fails. Previously, errors from custom resolvers (GitHub, GitLab, etc.) were indistinguishable from PyPI resolver errors, making debugging difficult. Closes: #858 Signed-off-by: Lalatendu Mohanty --- src/fromager/resolver.py | 103 ++++++++++++++++++++++++++++----------- tests/test_resolver.py | 69 +++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 30 deletions(-) diff --git a/src/fromager/resolver.py b/src/fromager/resolver.py index f909e5fd..ca75b47f 100644 --- a/src/fromager/resolver.py +++ b/src/fromager/resolver.py @@ -163,8 +163,12 @@ def resolve_from_provider( result = rslvr.resolve([req]) except resolvelib.resolvers.ResolverException as err: constraint = provider.constraints.get_constraint(req.name) + provider_desc = provider.get_provider_description() + # Include the original error message to preserve detailed information + # (e.g., file types, pre-release info from PyPIProvider) + original_msg = str(err) raise resolvelib.resolvers.ResolverException( - f"Unable to resolve requirement specifier {req} with constraint {constraint}" + f"Unable to resolve requirement specifier {req} with constraint {constraint} using {provider_desc}: {original_msg}" ) from err # resolvelib actually just returns one candidate per requirement. # result.mapping is map from an identifier to its resolved candidate @@ -373,6 +377,7 @@ def get_project_from_pypi( class BaseProvider(ExtrasProvider): resolver_cache: typing.ClassVar[ResolverCache] = {} + provider_description: typing.ClassVar[str] def __init__( self, @@ -395,6 +400,20 @@ def cache_key(self) -> str: """ raise NotImplementedError() + def get_provider_description(self) -> str: + """Return a human-readable description of the provider type + + This is used in error messages to indicate what resolver was being used. + The ClassVar `provider_description` must be set by each subclass. + If it contains format placeholders like {self.attr}, it will be formatted + with the instance. + """ + try: + return self.provider_description.format(self=self) + except (KeyError, AttributeError): + # No format placeholders or invalid format, return as-is + return self.provider_description + def find_candidates(self, identifier: str) -> Candidates: """Find unfiltered candidates""" raise NotImplementedError() @@ -505,6 +524,7 @@ def _get_cached_candidates(self, identifier: str) -> list[Candidate]: def _find_cached_candidates(self, identifier: str) -> Candidates: """Find candidates with caching""" + cached_candidates: list[Candidate] = [] if self.use_cache_candidates: cached_candidates = self._get_cached_candidates(identifier) if cached_candidates: @@ -531,6 +551,16 @@ def _find_cached_candidates(self, identifier: str) -> Candidates: ) return candidates + def _get_no_match_error_message( + self, identifier: str, requirements: RequirementsMap + ) -> str: + """Generate an error message when no candidates are found. + + Subclasses should override this to provide provider-specific error details. + """ + r = next(iter(requirements[identifier])) + return f"found no match for {r} using {self.get_provider_description()}" + def find_matches( self, identifier: str, @@ -546,12 +576,20 @@ def find_matches( identifier, requirements, incompatibilities, candidate ) ] + if not candidates: + raise resolvelib.resolvers.ResolverException( + self._get_no_match_error_message(identifier, requirements) + ) return sorted(candidates, key=attrgetter("version", "build_tag"), reverse=True) class PyPIProvider(BaseProvider): """Lookup package and versions from a simple Python index (PyPI)""" + provider_description: typing.ClassVar[str] = ( + "PyPI resolver (searching at {self.sdist_server_url})" + ) + def __init__( self, include_sdists: bool = True, @@ -616,39 +654,39 @@ def validate_candidate( return False return True + def _get_no_match_error_message( + self, identifier: str, requirements: RequirementsMap + ) -> str: + """Generate a PyPI-specific error message with file type and pre-release details.""" + r = next(iter(requirements[identifier])) + + # Determine if pre-releases are allowed + req_allows_prerelease = bool(r.specifier) and bool(r.specifier.prereleases) + allow_prerelease = ( + self.constraints.allow_prerelease(r.name) or req_allows_prerelease + ) + prerelease_info = "including" if allow_prerelease else "ignoring" + + # Determine the file type that was allowed + if self.include_sdists and self.include_wheels: + file_type_info = "any file type" + elif self.include_sdists: + file_type_info = "sdists" + else: + file_type_info = "wheels" + + return ( + f"found no match for {r} using {self.get_provider_description()}, " + f"searching for {file_type_info}, {prerelease_info} pre-release versions" + ) + def find_matches( self, identifier: str, requirements: RequirementsMap, incompatibilities: CandidatesMap, ) -> Candidates: - candidates = super().find_matches(identifier, requirements, incompatibilities) - if not candidates: - # Try to construct a meaningful error message that points out the - # type(s) of files the resolver has been told it can choose as a - # hint in case that should be adjusted for the package that does not - # resolve. - r = next(iter(requirements[identifier])) - - # Determine if pre-releases are allowed - req_allows_prerelease = bool(r.specifier) and bool(r.specifier.prereleases) - allow_prerelease = ( - self.constraints.allow_prerelease(r.name) or req_allows_prerelease - ) - prerelease_info = "including" if allow_prerelease else "ignoring" - - # Determine the file type that was allowed - if self.include_sdists and self.include_wheels: - file_type_info = "any file type" - elif self.include_sdists: - file_type_info = "sdists" - else: - file_type_info = "wheels" - - raise resolvelib.resolvers.ResolverException( - f"found no match for {r}, searching for {file_type_info}, {prerelease_info} pre-release versions, in cache or at {self.sdist_server_url}" - ) - return sorted(candidates, key=attrgetter("version", "build_tag"), reverse=True) + return super().find_matches(identifier, requirements, incompatibilities) class MatchFunction(typing.Protocol): @@ -707,6 +745,8 @@ def _re_match_function( logger.debug(f"{identifier}: could not parse version from {value}: {err}") return None + provider_description: typing.ClassVar[str] = "custom resolver (GenericProvider)" + @property def cache_key(self) -> str: raise NotImplementedError("GenericProvider does not implement caching") @@ -734,6 +774,9 @@ class GitHubTagProvider(GenericProvider): Assumes that upstream uses version tags `1.2.3` or `v1.2.3`. """ + provider_description: typing.ClassVar[str] = ( + "GitHub tag resolver (repository: {self.organization}/{self.repo})" + ) host = "github.com:443" api_url = "https://api.{self.host}/repos/{self.organization}/{self.repo}/tags" @@ -799,6 +842,10 @@ def _find_tags( class GitLabTagProvider(GenericProvider): """Lookup tarball and version from GitLab git tags""" + provider_description: typing.ClassVar[str] = ( + "GitLab tag resolver (project: {self.server_url}/{self.project_path})" + ) + def __init__( self, project_path: str, diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 4d89b864..89b203b7 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -730,7 +730,7 @@ def test_github_constraint_mismatch() -> None: reporter: resolvelib.BaseReporter = resolvelib.BaseReporter() rslvr = resolvelib.Resolver(provider, reporter) - with pytest.raises(resolvelib.resolvers.ResolutionImpossible): + with pytest.raises(resolvelib.resolvers.ResolverException): rslvr.resolve([Requirement("fromager")]) @@ -931,7 +931,7 @@ def test_gitlab_constraint_mismatch() -> None: reporter: resolvelib.BaseReporter = resolvelib.BaseReporter() rslvr = resolvelib.Resolver(provider, reporter) - with pytest.raises(resolvelib.resolvers.ResolutionImpossible): + with pytest.raises(resolvelib.resolvers.ResolverException): rslvr.resolve([Requirement("submodlib")]) @@ -1033,3 +1033,68 @@ def test_pep592_support_constraint_mismatch() -> None: def test_extract_filename_from_url(url, filename) -> None: result = resolver.extract_filename_from_url(url) assert result == filename + + +def test_custom_resolver_error_message_missing_tag() -> None: + """Test that error message indicates custom resolver when tag doesn't exist. + + This reproduces issue #858 where the error message mentions PyPI and sdists + even when using a custom resolver like GitHubTagProvider. + """ + with requests_mock.Mocker() as r: + # Mock GitHub API to return empty tags (simulating missing tag) + r.get( + "https://api.github.com:443/repos/test-org/test-repo/tags", + json=[], # Empty tags list - tag doesn't exist + ) + + provider = resolver.GitHubTagProvider(organization="test-org", repo="test-repo") + + with pytest.raises(resolvelib.resolvers.ResolverException) as exc_info: + resolver.resolve_from_provider(provider, Requirement("test-package==1.0.0")) + + error_message = str(exc_info.value) + assert ( + "GitHub" in error_message + or "test-org/test-repo" in error_message + or "custom resolver" in error_message.lower() + ), ( + f"Error message should indicate custom resolver was used (GitHub tag resolver), " + f"but got: {error_message}" + ) + # Should NOT mention PyPI when using GitHub resolver + assert "pypi.org" not in error_message.lower(), ( + f"Error message incorrectly mentions PyPI when using GitHub resolver: {error_message}" + ) + + +def test_custom_resolver_error_message_via_resolve() -> None: + """Test error message when using resolve() function with custom resolver override.""" + + def custom_resolver_provider(*args, **kwargs): + """Custom resolver that returns GitHubTagProvider.""" + return resolver.GitHubTagProvider(organization="test-org", repo="test-repo") + + with requests_mock.Mocker() as r: + # Mock GitHub API to return empty tags + r.get( + "https://api.github.com:443/repos/test-org/test-repo/tags", + json=[], + ) + + provider = custom_resolver_provider() + + with pytest.raises(resolvelib.resolvers.ResolverException) as exc_info: + resolver.resolve_from_provider(provider, Requirement("test-package==1.0.0")) + + error_message = str(exc_info.value) + # After fix for issue #858, the error message should indicate that a GitHub resolver was used + assert ( + "GitHub" in error_message + or "test-org/test-repo" in error_message + or "custom resolver" in error_message.lower() + ), f"Error message should indicate GitHub resolver was used: {error_message}" + # Should NOT mention PyPI when using GitHub resolver + assert "pypi.org" not in error_message.lower(), ( + f"Error message incorrectly mentions PyPI when using GitHub resolver: {error_message}" + )