Merge pull request #5000 from pradyunsg/resolver/warn-after-resolution

Warn about broken dependencies during pip install

news/5000.feature

+pip install now prints an error message when it installs an incompatible version of a dependency.

src/pip/_internal/commands/check.py

 import logging
 
 from pip._internal.basecommand import Command
-from pip._internal.operations.check import check_requirements
+from pip._internal.operations.check import (
+    check_package_set, create_package_set_from_installed,
+)
 from pip._internal.utils.misc import get_installed_distributions
 
 logger = logging.getLogger(__name__)
@@ -15,25 +17,26 @@ class CheckCommand(Command):
     summary = 'Verify installed packages have compatible dependencies.'
 
     def run(self, options, args):
-        dists = get_installed_distributions(local_only=False, skip=())
-        missing_reqs_dict, incompatible_reqs_dict = check_requirements(dists)
+        package_set = create_package_set_from_installed()
+        missing, conflicting = check_package_set(package_set)
 
-        for dist in dists:
-            for requirement in missing_reqs_dict.get(dist.key, []):
+        for project_name in missing:
+            version = package_set[project_name].version
+            for dependency in missing[project_name]:
                 logger.info(
                     "%s %s requires %s, which is not installed.",
-                    dist.project_name, dist.version, requirement.project_name,
+                    project_name, version, dependency[0],
                 )
 
-            for requirement, actual in incompatible_reqs_dict.get(
-                    dist.key, []):
+        for project_name in conflicting:
+            version = package_set[project_name].version
+            for dep_name, dep_version, req in conflicting[project_name]:
                 logger.info(
                     "%s %s has requirement %s, but you have %s %s.",
-                    dist.project_name, dist.version, requirement,
-                    actual.project_name, actual.version,
+                    project_name, version, req, dep_name, dep_version,
                 )
 
-        if missing_reqs_dict or incompatible_reqs_dict:
+        if missing or conflicting:
             return 1
         else:
             logger.info("No broken requirements found.")

src/pip/_internal/commands/install.py

@@ -14,6 +14,7 @@ from pip._internal.exceptions import (
     CommandError, InstallationError, PreviousBuildDirError,
 )
 from pip._internal.locations import distutils_scheme, virtualenv_no_global
+from pip._internal.operations.check import check_install_conflicts
 from pip._internal.operations.prepare import RequirementPreparer
 from pip._internal.req import RequirementSet, install_given_reqs
 from pip._internal.resolve import Resolver
@@ -172,6 +173,13 @@ class InstallCommand(RequirementCommand):
             default=True,
             help="Do not warn when installing scripts outside PATH",
         )
+        cmd_opts.add_option(
+            "--no-warn-conflicts",
+            action="store_false",
+            dest="warn_about_conflicts",
+            default=True,
+            help="Do not warn about broken dependencies",
+        )
 
         cmd_opts.add_option(cmdoptions.no_binary())
         cmd_opts.add_option(cmdoptions.only_binary())
@@ -300,6 +308,15 @@ class InstallCommand(RequirementCommand):
                     to_install = resolver.get_installation_order(
                         requirement_set
                     )
+
+                    # Consistency Checking of the package set we're installing.
+                    should_warn_about_conflicts = (
+                        not options.ignore_dependencies and
+                        options.warn_about_conflicts
+                    )
+                    if should_warn_about_conflicts:
+                        self._warn_about_conflicts(to_install)
+
                     installed = install_given_reqs(
                         to_install,
                         install_options,
@@ -426,6 +443,28 @@ class InstallCommand(RequirementCommand):
                         target_item_dir
                     )
 
+    def _warn_about_conflicts(self, to_install):
+        package_set, _dep_info = check_install_conflicts(to_install)
+        missing, conflicting = _dep_info
+
+        # NOTE: There is some duplication here from pip check
+        for project_name in missing:
+            version = package_set[project_name][0]
+            for dependency in missing[project_name]:
+                logger.critical(
+                    "%s %s requires %s, which is not installed.",
+                    project_name, version, dependency[1],
+                )
+
+        for project_name in conflicting:
+            version = package_set[project_name][0]
+            for dep_name, dep_version, req in conflicting[project_name]:
+                logger.critical(
+                    "%s %s has requirement %s, but you'll have %s %s which is "
+                    "incompatible.",
+                    project_name, version, req, dep_name, dep_version,
+                )
+
 
 def get_lib_location_guesses(*args, **kwargs):
     scheme = distutils_scheme('', *args, **kwargs)

src/pip/_internal/operations/check.py

+"""Validation of dependencies of packages
+"""
 
+from collections import namedtuple
 
-def check_requirements(installed_dists):
-    missing_reqs_dict = {}
-    incompatible_reqs_dict = {}
+from pip._vendor.packaging.utils import canonicalize_name
 
-    for dist in installed_dists:
-        missing_reqs = list(get_missing_reqs(dist, installed_dists))
-        if missing_reqs:
-            missing_reqs_dict[dist.key] = missing_reqs
+from pip._internal.operations.prepare import make_abstract_dist
 
-        incompatible_reqs = list(get_incompatible_reqs(dist, installed_dists))
-        if incompatible_reqs:
-            incompatible_reqs_dict[dist.key] = incompatible_reqs
+from pip._internal.utils.misc import get_installed_distributions
+from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
-    return (missing_reqs_dict, incompatible_reqs_dict)
+if MYPY_CHECK_RUNNING:
+    from pip._internal.req.req_install import InstallRequirement
+    from typing import Any, Dict, Iterator, Set, Tuple, List
 
+    # Shorthands
+    PackageSet = Dict[str, 'PackageDetails']
+    Missing = Tuple[str, Any]
+    Conflicting = Tuple[str, str, Any]
 
-def get_missing_reqs(dist, installed_dists):
-    """Return all of the requirements of `dist` that aren't present in
-    `installed_dists`.
+    MissingDict = Dict[str, List[Missing]]
+    ConflictingDict = Dict[str, List[Conflicting]]
+    CheckResult = Tuple[MissingDict, ConflictingDict]
 
+PackageDetails = namedtuple('PackageDetails', ['version', 'requires'])
+
+
+def create_package_set_from_installed(**kwargs):
+    # type: (**Any) -> PackageSet
+    """Converts a list of distributions into a PackageSet.
+    """
+    retval = {}
+    for dist in get_installed_distributions(**kwargs):
+        name = canonicalize_name(dist.project_name)
+        retval[name] = PackageDetails(dist.version, dist.requires())
+    return retval
+
+
+def check_package_set(package_set):
+    # type: (PackageSet) -> CheckResult
+    """Check if a package set is consistent
     """
-    installed_names = {d.project_name.lower() for d in installed_dists}
-    missing_requirements = set()
+    missing = dict()
+    conflicting = dict()
+
+    for package_name in package_set:
+        # Info about dependencies of package_name
+        missing_deps = set()  # type: Set[Missing]
+        conflicting_deps = set()  # type: Set[Conflicting]
 
-    for requirement in dist.requires():
-        if requirement.project_name.lower() not in installed_names:
-            missing_requirements.add(requirement)
-            yield requirement
+        for req in package_set[package_name].requires:
+            name = canonicalize_name(req.project_name)  # type: str
 
+            # Check if it's missing
+            if name not in package_set:
+                missed = True
+                if req.marker is not None:
+                    missed = req.marker.evaluate()
+                if missed:
+                    missing_deps.add((name, req))
+                continue
 
-def get_incompatible_reqs(dist, installed_dists):
-    """Return all of the requirements of `dist` that are present in
-    `installed_dists`, but have incompatible versions.
+            # Check if there's a conflict
+            version = package_set[name].version  # type: str
+            if version not in req.specifier:
+                conflicting_deps.add((name, version, req))
 
+        def str_key(x):
+            return str(x)
+
+        if missing_deps:
+            missing[package_name] = sorted(missing_deps, key=str_key)
+        if conflicting_deps:
+            conflicting[package_name] = sorted(conflicting_deps, key=str_key)
+
+    return missing, conflicting
+
+
+def check_install_conflicts(to_install):
+    # type: (List[InstallRequirement]) -> Tuple[PackageSet, CheckResult]
+    """For checking if the dependency graph would be consistent after \
+    installing given requirements
     """
-    installed_dists_by_name = {}
-    for installed_dist in installed_dists:
-        installed_dists_by_name[installed_dist.project_name] = installed_dist
+    # Start from the current state
+    state = create_package_set_from_installed()
+    _simulate_installation_of(to_install, state)
+    return state, check_package_set(state)
+
 
-    for requirement in dist.requires():
-        present_dist = installed_dists_by_name.get(requirement.project_name)
+# NOTE from @pradyunsg
+# This required a minor update in dependency link handling logic over at
+# operations.prepare.IsSDist.dist() to get it working
+def _simulate_installation_of(to_install, state):
+    # type: (List[InstallRequirement], PackageSet) -> None
+    """Computes the version of packages after installing to_install.
+    """
 
-        if present_dist and present_dist not in requirement:
-            yield (requirement, present_dist)
+    # Modify it as installing requirement_set would (assuming no errors)
+    for inst_req in to_install:
+        dist = make_abstract_dist(inst_req).dist(finder=None)
+        state[dist.key] = PackageDetails(dist.version, dist.requires())

src/pip/_internal/operations/prepare.py

@@ -114,7 +114,7 @@ class IsSDist(DistAbstraction):
     def dist(self, finder):
         dist = self.req.get_dist()
         # FIXME: shouldn't be globally added.
-        if dist.has_metadata('dependency_links.txt'):
+        if finder and dist.has_metadata('dependency_links.txt'):
             finder.add_dependency_links(
                 dist.get_metadata_lines('dependency_links.txt')
             )

src/pip/_internal/resolve.py

@@ -192,7 +192,6 @@ class Resolver(object):
         if req.editable:
             return self.preparer.prepare_editable_requirement(
                 req, self.require_hashes, self.use_user_site, self.finder,
-
             )
 
         # satisfied_by is only evaluated by calling _check_skip_installed,

tests/functional/test_check.py

@@ -18,6 +18,7 @@ def test_basic_check_clean(script):
         "No broken requirements found.",
     )
     assert matches_expected_lines(result.stdout, expected_lines)
+    assert result.returncode == 0
 
 
 def test_basic_check_missing_dependency(script):
@@ -55,7 +56,9 @@ def test_basic_check_broken_dependency(script):
         name='broken', version='0.1',
     )
     # Let's install broken==0.1
-    res = script.pip('install', '--no-index', broken_path)
+    res = script.pip(
+        'install', '--no-index', broken_path, '--no-warn-conflicts',
+    )
     assert "Successfully installed broken-0.1" in res.stdout, str(res)
 
     result = script.pip('check', expect_error=True)
@@ -96,25 +99,97 @@ def test_basic_check_broken_dependency_and_missing_dependency(script):
     assert result.returncode == 1
 
 
-def test_check_complex_names(script):
-    # Check that uppercase letters and '-' are dealt with
-    # Setup two small projects
-    pkga_path = create_test_package_with_setup(
+def test_check_complicated_name_missing(script):
+    package_a_path = create_test_package_with_setup(
         script,
-        name='pkga', version='1.0', install_requires=['Complex_Name==0.1'],
+        name='package_A', version='1.0',
+        install_requires=['Dependency-B>=1.0'],
+    )
+
+    # Without dependency
+    result = script.pip('install', '--no-index', package_a_path, '--no-deps')
+    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+
+    result = script.pip('check', expect_error=True)
+    expected_lines = (
+        "package-a 1.0 requires dependency-b, which is not installed.",
     )
+    assert matches_expected_lines(result.stdout, expected_lines)
+    assert result.returncode == 1
 
-    complex_path = create_test_package_with_setup(
+
+def test_check_complicated_name_broken(script):
+    package_a_path = create_test_package_with_setup(
+        script,
+        name='package_A', version='1.0',
+        install_requires=['Dependency-B>=1.0'],
+    )
+    dependency_b_path_incompatible = create_test_package_with_setup(
         script,
-        name='Complex-Name', version='0.1',
+        name='dependency-b', version='0.1',
     )
 
-    res = script.pip('install', '--no-index', complex_path)
-    assert "Successfully installed Complex-Name-0.1" in res.stdout, str(res)
+    # With broken dependency
+    result = script.pip('install', '--no-index', package_a_path, '--no-deps')
+    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
 
-    res = script.pip('install', '--no-index', pkga_path, '--no-deps')
-    assert "Successfully installed pkga-1.0" in res.stdout, str(res)
+    result = script.pip(
+        'install', '--no-index', dependency_b_path_incompatible, '--no-deps',
+    )
+    assert "Successfully installed dependency-b-0.1" in result.stdout
+
+    result = script.pip('check', expect_error=True)
+    expected_lines = (
+        "package-a 1.0 has requirement Dependency-B>=1.0, but you have "
+        "dependency-b 0.1.",
+    )
+    assert matches_expected_lines(result.stdout, expected_lines)
+    assert result.returncode == 1
+
+
+def test_check_complicated_name_clean(script):
+    package_a_path = create_test_package_with_setup(
+        script,
+        name='package_A', version='1.0',
+        install_requires=['Dependency-B>=1.0'],
+    )
+    dependency_b_path = create_test_package_with_setup(
+        script,
+        name='dependency-b', version='1.0',
+    )
+
+    result = script.pip('install', '--no-index', package_a_path, '--no-deps')
+    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+
+    result = script.pip(
+        'install', '--no-index', dependency_b_path, '--no-deps',
+    )
+    assert "Successfully installed dependency-b-1.0" in result.stdout
 
-    # Check that Complex_Name is correctly dealt with
-    res = script.pip('check')
-    assert "No broken requirements found." in res.stdout, str(res)
+    result = script.pip('check', expect_error=True)
+    expected_lines = (
+        "No broken requirements found.",
+    )
+    assert matches_expected_lines(result.stdout, expected_lines)
+    assert result.returncode == 0
+
+
+def test_check_considers_conditional_reqs(script):
+    package_a_path = create_test_package_with_setup(
+        script,
+        name='package_A', version='1.0',
+        install_requires=[
+            "Dependency-B>=1.0; python_version != '2.7'",
+            "Dependency-B>=2.0; python_version == '2.7'",
+        ],
+    )
+
+    result = script.pip('install', '--no-index', package_a_path, '--no-deps')
+    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+
+    result = script.pip('check', expect_error=True)
+    expected_lines = (
+        "package-a 1.0 requires dependency-b, which is not installed.",
+    )
+    assert matches_expected_lines(result.stdout, expected_lines)
+    assert result.returncode == 1

tests/functional/test_install.py

@@ -1259,3 +1259,47 @@ def test_installed_files_recorded_in_deterministic_order(script, data):
         p for p in Path(installed_files_path).read_text().split('\n') if p
     ]
     assert installed_files_lines == sorted(installed_files_lines)
+
+
+def test_install_conflict_results_in_warning(script, data):
+    pkgA_path = create_test_package_with_setup(
+        script,
+        name='pkgA', version='1.0', install_requires=['pkgb == 1.0'],
+    )
+    pkgB_path = create_test_package_with_setup(
+        script,
+        name='pkgB', version='2.0',
+    )
+
+    # Install pkgA without its dependency
+    result1 = script.pip('install', '--no-index', pkgA_path, '--no-deps')
+    assert "Successfully installed pkgA-1.0" in result1.stdout, str(result1)
+
+    # Then install an incorrect version of the dependency
+    result2 = script.pip(
+        'install', '--no-index', pkgB_path,
+        expect_stderr=True,
+    )
+    assert "pkga 1.0 has requirement pkgb==1.0" in result2.stderr, str(result2)
+    assert "Successfully installed pkgB-2.0" in result2.stdout, str(result2)
+
+
+def test_install_conflict_warning_can_be_suppressed(script, data):
+    pkgA_path = create_test_package_with_setup(
+        script,
+        name='pkgA', version='1.0', install_requires=['pkgb == 1.0'],
+    )
+    pkgB_path = create_test_package_with_setup(
+        script,
+        name='pkgB', version='2.0',
+    )
+
+    # Install pkgA without its dependency
+    result1 = script.pip('install', '--no-index', pkgA_path, '--no-deps')
+    assert "Successfully installed pkgA-1.0" in result1.stdout, str(result1)
+
+    # Then install an incorrect version of the dependency; suppressing warning
+    result2 = script.pip(
+        'install', '--no-index', pkgB_path, '--no-warn-conflicts'
+    )
+    assert "Successfully installed pkgB-2.0" in result2.stdout, str(result2)