From 1fd5098b2480eee5be4a0033b43cb02605bc0fe1 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 30 Jul 2020 21:58:05 +0800 Subject: [PATCH] Canonicalize name in check_if_exists The previous implementation uses pkg_resources.get_distribution(), which does not canonicalize the package name correctly, and fails when combined with pip's own get_distribution(), which does canonicalize names. This makes InstallRequirement.check_if_exists() only use pip's own canonicalization logic so different package name forms are matched as expected. --- news/8645.bugfix | 2 ++ src/pip/_internal/req/req_install.py | 28 +++++++------------- tests/functional/test_install_upgrade.py | 33 ++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 news/8645.bugfix diff --git a/news/8645.bugfix b/news/8645.bugfix new file mode 100644 index 000000000..a388d24e4 --- /dev/null +++ b/news/8645.bugfix @@ -0,0 +1,2 @@ +Correctly find already-installed distributions with dot (``.``) in the name +and uninstall them when needed. diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 644930a15..4759f4af6 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -429,25 +429,13 @@ class InstallRequirement(object): """ if self.req is None: return - # get_distribution() will resolve the entire list of requirements - # anyway, and we've already determined that we need the requirement - # in question, so strip the marker so that we don't try to - # evaluate it. - no_marker = Requirement(str(self.req)) - no_marker.marker = None - - # pkg_resources uses the canonical name to look up packages, but - # the name passed passed to get_distribution is not canonicalized - # so we have to explicitly convert it to a canonical name - no_marker.name = canonicalize_name(no_marker.name) - try: - self.satisfied_by = pkg_resources.get_distribution(str(no_marker)) - except pkg_resources.DistributionNotFound: + existing_dist = get_distribution(self.req.name) + if not existing_dist: return - except pkg_resources.VersionConflict: - existing_dist = get_distribution( - self.req.name - ) + + existing_version = existing_dist.parsed_version + if not self.req.specifier.contains(existing_version, prereleases=True): + self.satisfied_by = None if use_user_site: if dist_in_usersite(existing_dist): self.should_reinstall = True @@ -461,11 +449,13 @@ class InstallRequirement(object): else: self.should_reinstall = True else: - if self.editable and self.satisfied_by: + if self.editable: self.should_reinstall = True # when installing editables, nothing pre-existing should ever # satisfy self.satisfied_by = None + else: + self.satisfied_by = existing_dist # Things valid for wheels @property diff --git a/tests/functional/test_install_upgrade.py b/tests/functional/test_install_upgrade.py index e45bf3148..02e221101 100644 --- a/tests/functional/test_install_upgrade.py +++ b/tests/functional/test_install_upgrade.py @@ -1,3 +1,4 @@ +import itertools import os import sys import textwrap @@ -7,6 +8,7 @@ import pytest from tests.lib import pyversion # noqa: F401 from tests.lib import assert_all_changes from tests.lib.local_repos import local_checkout +from tests.lib.wheel import make_wheel @pytest.mark.network @@ -439,3 +441,34 @@ class TestUpgradeDistributeToSetuptools(object): cwd=pip_src, expect_stderr=True, ) + + +@pytest.mark.parametrize("req1, req2", list(itertools.product( + ["foo.bar", "foo_bar", "foo-bar"], ["foo.bar", "foo_bar", "foo-bar"], +))) +def test_install_find_existing_package_canonicalize(script, req1, req2): + """Ensure an already-installed dist is found no matter how the dist name + was normalized on installation. (pypa/pip#8645) + """ + # Create and install a package that's not available in the later stage. + req_container = script.scratch_path.joinpath("foo-bar") + req_container.mkdir() + req_path = make_wheel("foo_bar", "1.0").save_to_dir(req_container) + script.pip("install", "--no-index", req_path) + + # Depend on the previously installed, but now unavailable package. + pkg_container = script.scratch_path.joinpath("pkg") + pkg_container.mkdir() + make_wheel( + "pkg", + "1.0", + metadata_updates={"Requires-Dist": req2}, + ).save_to_dir(pkg_container) + + # Ensure the previously installed package can be correctly used to match + # the dependency. + result = script.pip( + "install", "--no-index", "--find-links", pkg_container, "pkg", + ) + satisfied_message = "Requirement already satisfied: {}".format(req2) + assert satisfied_message in result.stdout, str(result)