Skip to content
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.

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

Improve the message for "Resolution Conflict" errors #8394

Merged
merged 6 commits into from Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/pip/_internal/resolution/resolvelib/base.py
Expand Up @@ -37,6 +37,10 @@ def get_candidate_lookup(self):
# type: () -> CandidateLookup
raise NotImplementedError("Subclass should override")

def format_for_error(self):
# type: () -> str
raise NotImplementedError("Subclass should override")


class Candidate(object):
@property
Expand All @@ -61,3 +65,7 @@ def iter_dependencies(self):
def get_install_requirement(self):
# type: () -> Optional[InstallRequirement]
raise NotImplementedError("Override in subclass")

def format_for_error(self):
# type: () -> str
raise NotImplementedError("Subclass should override")
23 changes: 23 additions & 0 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Expand Up @@ -162,6 +162,14 @@ def version(self):
self._version = self.dist.parsed_version
return self._version

def format_for_error(self):
# type: () -> str
return "{} {} (from {})".format(
self.name,
self.version,
self.link.file_path if self.link.is_file else self.link
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this look for cached paths?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try it tomorrow, but from my recollection "annoyingly long" ;-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the original link w/ (cached) in the name in that case?

OK to do this in a follow up, if you file an issue to aggregate all the call sites that print req.link.

)

def _prepare_abstract_distribution(self):
# type: () -> AbstractDistribution
raise NotImplementedError("Override in subclass")
Expand Down Expand Up @@ -336,6 +344,10 @@ def version(self):
# type: () -> _BaseVersion
return self.dist.parsed_version

def format_for_error(self):
# type: () -> str
return "{} {} (Installed)".format(self.name, self.version)

def iter_dependencies(self):
# type: () -> Iterable[Optional[Requirement]]
for r in self.dist.requires():
Expand Down Expand Up @@ -413,6 +425,13 @@ def version(self):
# type: () -> _BaseVersion
return self.base.version

def format_for_error(self):
# type: () -> str
return "{} [{}]".format(
self.base.format_for_error(),
", ".join(sorted(self.extras))
)

@property
def is_installed(self):
# type: () -> _BaseVersion
Expand Down Expand Up @@ -479,6 +498,10 @@ def version(self):
# type: () -> _BaseVersion
return self._version

def format_for_error(self):
# type: () -> str
return "Python {}".format(self.version)

def iter_dependencies(self):
# type: () -> Iterable[Optional[Requirement]]
return ()
Expand Down
95 changes: 93 additions & 2 deletions src/pip/_internal/resolution/resolvelib/factory.py
Expand Up @@ -5,6 +5,7 @@
from pip._vendor.packaging.utils import canonicalize_name

from pip._internal.exceptions import (
DistributionNotFound,
InstallationError,
UnsupportedPythonVersion,
)
Expand Down Expand Up @@ -343,11 +344,101 @@ def _report_requires_python_error(
return UnsupportedPythonVersion(message)

def get_installation_error(self, e):
# type: (ResolutionImpossible) -> Optional[InstallationError]
# type: (ResolutionImpossible) -> InstallationError

assert e.causes, "Installation error reported with no cause"

# If one of the things we can't solve is "we need Python X.Y",
# that is what we report.
for cause in e.causes:
if isinstance(cause.requirement, RequiresPythonRequirement):
return self._report_requires_python_error(
cause.requirement,
cause.parent,
)
return None

# Otherwise, we have a set of causes which can't all be satisfied
# at once.

# The simplest case is when we have *one* cause that can't be
# satisfied. We just report that case.
if len(e.causes) == 1:
req, parent = e.causes[0]
logger.critical(
"Could not find a version that satisfies " +
"the requirement " +
str(req) +
("" if parent is None else " (from {})".format(
parent.name
))
)
return DistributionNotFound(
'No matching distribution found for {}'.format(req)
)

# OK, we now have a list of requirements that can't all be
# satisfied at once.

# A couple of formatting helpers
def text_join(parts):
# type: (List[str]) -> str
if len(parts) == 1:
return parts[0]

return ", ".join(parts[:-1]) + " and " + parts[-1]

def readable_form(cand):
# type: (Candidate) -> str
return "{} {}".format(cand.name, cand.version)

triggers = []
for req, parent in e.causes:
if parent is None:
# This is a root requirement, so we can report it directly
trigger = req.format_for_error()
else:
ireq = parent.get_install_requirement()
if ireq and ireq.comes_from:
trigger = "{}".format(
ireq.comes_from.name
)
else:
trigger = "{} {}".format(
parent.name,
parent.version
)
triggers.append(trigger)

if triggers:
info = text_join(triggers)
else:
info = "the requested packages"

msg = "Cannot install {} because these package versions " \
"have conflicting dependencies.".format(info)
logger.critical(msg)
msg = "\nThe conflict is caused by:"
for req, parent in e.causes:
msg = msg + "\n "
if parent:
msg = msg + "{} {} depends on ".format(
parent.name,
parent.version
)
else:
msg = msg + "The user requested "
msg = msg + req.format_for_error()

msg = msg + "\n\n" + \
"To fix this you could try to:\n" + \
"1. loosen the range of package versions you've specified\n" + \
"2. remove package versions to allow pip attempt to solve " + \
"the dependency conflict\n"

logger.info(msg)

return DistributionNotFound(
"ResolutionImpossible For help visit: "
"https://pip.pypa.io/en/stable/user_guide/"
"#dependency-conflicts-resolution-impossible"
)
23 changes: 23 additions & 0 deletions src/pip/_internal/resolution/resolvelib/requirements.py
Expand Up @@ -30,6 +30,10 @@ def name(self):
# No need to canonicalise - the candidate did this
return self.candidate.name

def format_for_error(self):
# type: () -> str
return self.candidate.format_for_error()

def get_candidate_lookup(self):
# type: () -> CandidateLookup
return self.candidate, None
Expand Down Expand Up @@ -63,6 +67,21 @@ def name(self):
canonical_name = canonicalize_name(self._ireq.req.name)
return format_name(canonical_name, self._extras)

def format_for_error(self):
# type: () -> str

# Convert comma-separated specifiers into "A, B, ..., F and G"
# This makes the specifier a bit more "human readable", without
# risking a change in meaning. (Hopefully! Not all edge cases have
# been checked)
parts = [s.strip() for s in str(self).split(",")]
if len(parts) == 0:
return ""
elif len(parts) == 1:
return parts[0]

return ", ".join(parts[:-1]) + " and " + parts[-1]

def get_candidate_lookup(self):
# type: () -> CandidateLookup
return None, self._ireq
Expand Down Expand Up @@ -99,6 +118,10 @@ def name(self):
# type: () -> str
return self._candidate.name

def format_for_error(self):
# type: () -> str
return "Python " + str(self.specifier)

def get_candidate_lookup(self):
# type: () -> CandidateLookup
if self.specifier.contains(self._candidate.version, prereleases=True):
Expand Down
20 changes: 1 addition & 19 deletions src/pip/_internal/resolution/resolvelib/resolver.py
Expand Up @@ -6,7 +6,7 @@
from pip._vendor.resolvelib import BaseReporter, ResolutionImpossible
from pip._vendor.resolvelib import Resolver as RLResolver

from pip._internal.exceptions import DistributionNotFound, InstallationError
from pip._internal.exceptions import InstallationError
from pip._internal.req.req_set import RequirementSet
from pip._internal.resolution.base import BaseResolver
from pip._internal.resolution.resolvelib.provider import PipProvider
Expand Down Expand Up @@ -142,24 +142,6 @@ def resolve(self, root_reqs, check_supported_wheels):

except ResolutionImpossible as e:
error = self.factory.get_installation_error(e)
if not error:
# TODO: This needs fixing, we need to look at the
# factory.get_installation_error infrastructure, as that
# doesn't really allow for the logger.critical calls I'm
# using here.
for req, parent in e.causes:
logger.critical(
"Could not find a version that satisfies " +
"the requirement " +
str(req) +
("" if parent is None else " (from {})".format(
parent.name
))
)
raise DistributionNotFound(
"No matching distribution found for " +
", ".join([r.name for r, _ in e.causes])
)
six.raise_from(error, e)

req_set = RequirementSet(check_supported_wheels=check_supported_wheels)
Expand Down
18 changes: 15 additions & 3 deletions tests/functional/test_new_resolver.py
Expand Up @@ -282,9 +282,7 @@ def test_new_resolver_no_dist_message(script):

assert "Could not find a version that satisfies the requirement B" \
in result.stderr, str(result)
# TODO: This reports the canonical name of the project. But the current
# resolver reports the originally specified name (i.e. uppercase B)
assert "No matching distribution found for b" in result.stderr, str(result)
assert "No matching distribution found for B" in result.stderr, str(result)


def test_new_resolver_installs_editable(script):
Expand Down Expand Up @@ -927,3 +925,17 @@ def test_new_resolver_upgrade_same_version(script):
"pkg",
)
assert_installed(script, pkg="2")


def test_new_resolver_local_and_req(script):
source_dir = create_test_package_with_setup(
script,
name="pkg",
version="0.1.0",
)
script.pip(
"install", "--unstable-feature=resolver",
"--no-cache-dir", "--no-index",
source_dir, "pkg!=0.1.0",
expect_error=True,
)