diff --git a/news/5000.feature b/news/5000.feature new file mode 100644 index 0000000000000000000000000000000000000000..5c6b43b9ec29c688f924b4b13a526df30da25819 --- /dev/null +++ b/news/5000.feature @@ -0,0 +1 @@ +pip install now prints an error message when it installs an incompatible version of a dependency. diff --git a/src/pip/_internal/commands/check.py b/src/pip/_internal/commands/check.py index 945af705acba80475ef7921e899975a071035394..88db5101e7a4ac8f1df1681cc79358fd11b20869 100644 --- a/src/pip/_internal/commands/check.py +++ b/src/pip/_internal/commands/check.py @@ -1,7 +1,9 @@ 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.") diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 606478173a7b6bba936517f64b5a620d3deee60d..62b59715c06d87f2f731735c9dad4ba3f2fe3f1b 100644 --- a/src/pip/_internal/commands/install.py +++ b/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) diff --git a/src/pip/_internal/operations/check.py b/src/pip/_internal/operations/check.py index 3855f34edbf4465262d99f04d9bab75b6fc48a19..91a329850d1b3ccad5f5280529f83fc4b2f023a5 100644 --- a/src/pip/_internal/operations/check.py +++ b/src/pip/_internal/operations/check.py @@ -1,46 +1,102 @@ +"""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()) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 2f3dc5081b93aadc6d9a7ed35ab7c5595172c7ac..2c4ff94cb1c19d082fd3a3d67c0be38943bb4572 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/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') ) diff --git a/src/pip/_internal/resolve.py b/src/pip/_internal/resolve.py index 06abd3c0e7cc3022534ea3c7245fc0bd39738064..3200fca8ad74fa8a003efd632c3e613eecb247ad 100644 --- a/src/pip/_internal/resolve.py +++ b/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, diff --git a/tests/functional/test_check.py b/tests/functional/test_check.py index 854363aa5d357da15cd2f3f653917e403b4c70f8..ade1080672b33034dbc377eb8f132d695d164e52 100644 --- a/tests/functional/test_check.py +++ b/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 diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index b944db7768f7a86debc07745b9b48edb9eae98e9..87db792d066a89d13591887d636ff9971c38e76b 100644 --- a/tests/functional/test_install.py +++ b/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)