Merge pull request #8394 from pfmoore/nr_conflict_message

Improve the message for "Resolution Conflict" errors

src/pip/_internal/resolution/resolvelib/base.py

@@ -38,6 +38,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
@@ -67,3 +71,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")

src/pip/_internal/resolution/resolvelib/candidates.py

@@ -182,6 +182,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
+        )
+
     def _prepare_abstract_distribution(self):
         # type: () -> AbstractDistribution
         raise NotImplementedError("Override in subclass")
@@ -360,6 +368,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():
@@ -437,6 +449,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
@@ -509,6 +528,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 ()

src/pip/_internal/resolution/resolvelib/factory.py

@@ -5,6 +5,7 @@
 from pip._vendor.packaging.utils import canonicalize_name
 
 from pip._internal.exceptions import (
+    DistributionNotFound,
     InstallationError,
     UnsupportedPythonVersion,
     UnsupportedWheel,
@@ -352,11 +353,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"
+        )

src/pip/_internal/resolution/resolvelib/requirements.py

@@ -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
@@ -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
@@ -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):

src/pip/_internal/resolution/resolvelib/resolver.py

@@ -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
@@ -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)

tests/functional/test_new_resolver.py

@@ -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):
@@ -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,
+    )