Merge pull request #5457 from pradyunsg/check/only-check-requirements-modified

Restrict install time dependency warnings to directly-dependant packages

news/5196.bugfix

@@ -0,0 +1,4 @@
+Restrict install time dependency warnings to directly-dependant packages
+
+Warning about the entire package set has resulted in users getting confused as
+to why pip is printing these warnings.

news/5457.bugfix

@@ -0,0 +1,4 @@
+Restrict install time dependency warnings to directly-dependant packages
+
+Warning about the entire package set has resulted in users getting confused as
+to why pip is printing these warnings.

src/pip/_internal/operations/check.py

@@ -12,7 +12,9 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
 if MYPY_CHECK_RUNNING:
     from pip._internal.req.req_install import InstallRequirement  # noqa: F401
-    from typing import Any, Dict, Iterator, Set, Tuple, List  # noqa: F401
+    from typing import (  # noqa: F401
+        Any, Callable, Dict, Iterator, Optional, Set, Tuple, List
+    )
 
     # Shorthands
     PackageSet = Dict[str, 'PackageDetails']
@@ -33,17 +35,25 @@ def create_package_set_from_installed(**kwargs):
     # Default to using all packages installed on the system
     if kwargs == {}:
         kwargs = {"local_only": False, "skip": ()}
-    retval = {}
+
+    package_set = {}
     for dist in get_installed_distributions(**kwargs):
         name = canonicalize_name(dist.project_name)
-        retval[name] = PackageDetails(dist.version, dist.requires())
-    return retval
+        package_set[name] = PackageDetails(dist.version, dist.requires())
+    return package_set
 
 
-def check_package_set(package_set):
-    # type: (PackageSet) -> CheckResult
+def check_package_set(package_set, should_ignore=None):
+    # type: (PackageSet, Optional[Callable[[str], bool]]) -> CheckResult
     """Check if a package set is consistent
+
+    If should_ignore is passed, it should be a callable that takes a
+    package name and returns a boolean.
     """
+    if should_ignore is None:
+        def should_ignore(name):
+            return False
+
     missing = dict()
     conflicting = dict()
 
@@ -52,6 +62,9 @@ def check_package_set(package_set):
         missing_deps = set()  # type: Set[Missing]
         conflicting_deps = set()  # type: Set[Conflicting]
 
+        if should_ignore(package_name):
+            continue
+
         for req in package_set[package_name].requires:
             name = canonicalize_name(req.project_name)  # type: str
 
@@ -69,13 +82,10 @@ def check_package_set(package_set):
             if not req.specifier.contains(version, prereleases=True):
                 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)
+            missing[package_name] = sorted(missing_deps, key=str)
         if conflicting_deps:
-            conflicting[package_name] = sorted(conflicting_deps, key=str_key)
+            conflicting[package_name] = sorted(conflicting_deps, key=str)
 
     return missing, conflicting
 
@@ -86,21 +96,54 @@ def check_install_conflicts(to_install):
     installing given requirements
     """
     # Start from the current state
-    state = create_package_set_from_installed()
-    _simulate_installation_of(to_install, state)
-    return state, check_package_set(state)
+    package_set = create_package_set_from_installed()
+    # Install packages
+    would_be_installed = _simulate_installation_of(to_install, package_set)
+
+    # Only warn about directly-dependent packages; create a whitelist of them
+    whitelist = _create_whitelist(would_be_installed, package_set)
+
+    return (
+        package_set,
+        check_package_set(
+            package_set, should_ignore=lambda name: name not in whitelist
+        )
+    )
 
 
 # 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
+def _simulate_installation_of(to_install, package_set):
+    # type: (List[InstallRequirement], PackageSet) -> Set[str]
     """Computes the version of packages after installing to_install.
     """
 
+    # Keep track of packages that were installed
+    installed = set()
+
     # 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)
         name = canonicalize_name(dist.key)
-        state[name] = PackageDetails(dist.version, dist.requires())
+        package_set[name] = PackageDetails(dist.version, dist.requires())
+
+        installed.add(name)
+
+    return installed
+
+
+def _create_whitelist(would_be_installed, package_set):
+    # type: (Set[str], PackageSet) -> Set[str]
+    packages_affected = set(would_be_installed)
+
+    for package_name in package_set:
+        if package_name in packages_affected:
+            continue
+
+        for req in package_set[package_name].requires:
+            if canonicalize_name(req.name) in packages_affected:
+                packages_affected.add(package_name)
+                break
+
+    return packages_affected

tests/functional/test_install_check.py

@@ -2,58 +2,102 @@ from tests.lib import create_test_package_with_setup
 
 
 def matches_expected_lines(string, expected_lines):
-    def predicate(line):
-        return line and not line.startswith('DEPRECATION')
-
-    output_lines = set(filter(predicate, string.splitlines()))
-    # Match regardless of order
-    return set(output_lines) == set(expected_lines)
+    return set(string.splitlines()) == set(expected_lines)
 
 
-def test_check_install_warnings(script):
+def test_check_install_canonicalization(script):
     pkga_path = create_test_package_with_setup(
         script,
-        name='pkga',
+        name='pkgA',
         version='1.0',
-        install_requires=['normal-missing', 'special.missing'],
+        install_requires=['normal-missing', 'SPECIAL.missing'],
     )
-    # Let's install pkga without its dependency
-    result = script.pip('install', '--no-index', pkga_path, '--no-deps')
-    assert "Successfully installed pkga-1.0" in result.stdout, str(result)
-
-    # Install the first missing dependency. Only an error for the
-    # second dependency should remain.
     normal_path = create_test_package_with_setup(
         script,
         name='normal-missing', version='0.1',
     )
+    special_path = create_test_package_with_setup(
+        script,
+        name='SPECIAL.missing', version='0.1',
+    )
+
+    # Let's install pkgA without its dependency
+    result = script.pip('install', '--no-index', pkga_path, '--no-deps')
+    assert "Successfully installed pkgA-1.0" in result.stdout, str(result)
+
+    # Install the first missing dependency. Only an error for the
+    # second dependency should remain.
     result = script.pip(
         'install', '--no-index', normal_path, '--quiet', expect_error=True
     )
-    expected_lines = (
-        "pkga 1.0 requires special.missing, which is not installed.",
-    )
+    expected_lines = [
+        "pkga 1.0 requires SPECIAL.missing, which is not installed.",
+    ]
     assert matches_expected_lines(result.stderr, expected_lines)
     assert result.returncode == 0
 
     # Install the second missing package and expect that there is no warning
     # during the installation. This is special as the package name requires
     # name normalization (as in https://github.com/pypa/pip/issues/5134)
-    missing_path = create_test_package_with_setup(
-        script,
-        name='special.missing', version='0.1',
-    )
     result = script.pip(
-        'install', '--no-index', missing_path, '--quiet',
+        'install', '--no-index', special_path, '--quiet',
     )
-    assert matches_expected_lines(result.stdout, [])
     assert matches_expected_lines(result.stderr, [])
     assert result.returncode == 0
 
     # Double check that all errors are resolved in the end
     result = script.pip('check')
-    expected_lines = (
+    expected_lines = [
         "No broken requirements found.",
-    )
+    ]
     assert matches_expected_lines(result.stdout, expected_lines)
     assert result.returncode == 0
+
+
+def test_check_install_does_not_warn_for_out_of_graph_issues(script):
+    pkg_broken_path = create_test_package_with_setup(
+        script,
+        name='broken',
+        version='1.0',
+        install_requires=['missing', 'conflict < 1.0'],
+    )
+    pkg_unrelated_path = create_test_package_with_setup(
+        script,
+        name='unrelated',
+        version='1.0',
+    )
+    pkg_conflict_path = create_test_package_with_setup(
+        script,
+        name='conflict',
+        version='1.0',
+    )
+
+    # Install a package without it's dependencies
+    result = script.pip('install', '--no-index', pkg_broken_path, '--no-deps')
+    assert matches_expected_lines(result.stderr, [])
+
+    # Install conflict package
+    result = script.pip(
+        'install', '--no-index', pkg_conflict_path, expect_error=True,
+    )
+    assert matches_expected_lines(result.stderr, [
+        "broken 1.0 requires missing, which is not installed.",
+        (
+            "broken 1.0 has requirement conflict<1.0, but "
+            "you'll have conflict 1.0 which is incompatible."
+        ),
+    ])
+
+    # Install unrelated package
+    result = script.pip(
+        'install', '--no-index', pkg_unrelated_path, '--quiet',
+    )
+    # should not warn about broken's deps when installing unrelated package
+    assert matches_expected_lines(result.stderr, [])
+
+    result = script.pip('check', expect_error=True)
+    expected_lines = [
+        "broken 1.0 requires missing, which is not installed.",
+        "broken 1.0 has requirement conflict<1.0, but you have conflict 1.0.",
+    ]
+    assert matches_expected_lines(result.stdout, expected_lines)