Skip to content

fix(resolver): indicate resolver type in error messages #859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(resolver): indicate resolver type in error messages #859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 75 additions & 28 deletions src/fromager/resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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,
Expand Down
69 changes: 67 additions & 2 deletions tests/test_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")])


Expand Down Expand Up @@ -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")])


Expand Down Expand Up @@ -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}"
)
Loading