Merge pull request #8054 from deveshks/correct-package-name-while-install

Canonicalize req name while doing pre-install package search

Author: chrahunt

news/5021.bugfix

@@ -0,0 +1 @@
+Use canonical package names while looking up already installed packages.

src/pip/_internal/commands/search.py

@@ -19,7 +19,7 @@
 from pip._internal.network.xmlrpc import PipXmlrpcTransport
 from pip._internal.utils.compat import get_terminal_size
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.misc import write_output
+from pip._internal.utils.misc import get_distribution, write_output
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
 if MYPY_CHECK_RUNNING:
@@ -139,7 +139,7 @@ def print_results(hits, name_column_width=None, terminal_width=None):
         try:
             write_output(line)
             if name in installed_packages:
-                dist = pkg_resources.get_distribution(name)
+                dist = get_distribution(name)
                 with indent_log():
                     if dist.version == latest:
                         write_output('INSTALLED: %s (latest)', dist.version)

src/pip/_internal/req/req_install.py

@@ -41,6 +41,7 @@
     display_path,
     dist_in_site_packages,
     dist_in_usersite,
+    get_distribution,
     get_installed_version,
     hide_url,
     redact_auth_from_url,
@@ -434,12 +435,17 @@ def check_if_exists(self, use_user_site):
         # 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:
             return
         except pkg_resources.VersionConflict:
-            existing_dist = pkg_resources.get_distribution(
+            existing_dist = get_distribution(
                 self.req.name
             )
             if use_user_site:
@@ -679,13 +685,11 @@ def uninstall(self, auto_confirm=False, verbose=False):
 
         """
         assert self.req
-        try:
-            dist = pkg_resources.get_distribution(self.req.name)
-        except pkg_resources.DistributionNotFound:
+        dist = get_distribution(self.req.name)
+        if not dist:
             logger.warning("Skipping %s as it is not installed.", self.name)
             return None
-        else:
-            logger.info('Found existing installation: %s', dist)
+        logger.info('Found existing installation: %s', dist)
 
         uninstalled_pathset = UninstallPathSet.from_dist(dist)
         uninstalled_pathset.remove(auto_confirm, verbose)

src/pip/_internal/self_outdated_check.py

@@ -7,7 +7,6 @@
 import os.path
 import sys
 
-from pip._vendor import pkg_resources
 from pip._vendor.packaging import version as packaging_version
 from pip._vendor.six import ensure_binary
 
@@ -19,7 +18,11 @@
     check_path_owner,
     replace,
 )
-from pip._internal.utils.misc import ensure_dir, get_installed_version
+from pip._internal.utils.misc import (
+    ensure_dir,
+    get_distribution,
+    get_installed_version,
+)
 from pip._internal.utils.packaging import get_installer
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
@@ -110,11 +113,10 @@ def was_installed_by_pip(pkg):
     This is used not to display the upgrade message when pip is in fact
     installed by system package manager, such as dnf on Fedora.
     """
-    try:
-        dist = pkg_resources.get_distribution(pkg)
-        return "pip" == get_installer(dist)
-    except pkg_resources.DistributionNotFound:
+    dist = get_distribution(pkg)
+    if not dist:
         return False
+    return "pip" == get_installer(dist)
 
 
 def pip_self_version_check(session, options):

src/pip/_internal/utils/misc.py

@@ -18,6 +18,7 @@
 from collections import deque
 
 from pip._vendor import pkg_resources
+from pip._vendor.packaging.utils import canonicalize_name
 # NOTE: retrying is not annotated in typeshed as on 2017-07-17, which is
 #       why we ignore the type on this import.
 from pip._vendor.retrying import retry  # type: ignore
@@ -480,6 +481,40 @@ def user_test(d):
             ]
 
 
+def search_distribution(req_name):
+
+    # Canonicalize the name before searching in the list of
+    # installed distributions and also while creating the package
+    # dictionary to get the Distribution object
+    req_name = canonicalize_name(req_name)
+    packages = get_installed_distributions(skip=())
+    pkg_dict = {canonicalize_name(p.key): p for p in packages}
+    return pkg_dict.get(req_name)
+
+
+def get_distribution(req_name):
+    """Given a requirement name, return the installed Distribution object"""
+
+    # Search the distribution by looking through the working set
+    dist = search_distribution(req_name)
+
+    # If distribution could not be found, call working_set.require
+    # to update the working set, and try to find the distribution
+    # again.
+    # This might happen for e.g. when you install a package
+    # twice, once using setup.py develop and again using setup.py install.
+    # Now when run pip uninstall twice, the package gets removed
+    # from the working set in the first uninstall, so we have to populate
+    # the working set again so that pip knows about it and the packages
+    # gets picked up and is successfully uninstalled the second time too.
+    if not dist:
+        try:
+            pkg_resources.working_set.require(req_name)
+        except pkg_resources.DistributionNotFound:
+            return None
+    return search_distribution(req_name)
+
+
 def egg_link_path(dist):
     # type: (Distribution) -> Optional[str]
     """

tests/functional/test_install.py

@@ -1882,3 +1882,23 @@ def test_install_skip_work_dir_pkg(script, data):
 
     assert 'Requirement already satisfied: simple' not in result.stdout
     assert 'Successfully installed simple' in result.stdout
+
+
+@pytest.mark.parametrize('package_name', ('simple-package', 'simple_package',
+                                          'simple.package'))
+def test_install_verify_package_name_normalization(script, package_name):
+
+    """
+    Test that install of a package again using a name which
+    normalizes to the original package name, is a no-op
+    since the package is already installed
+    """
+    pkg_path = create_test_package_with_setup(
+        script, name='simple-package', version='1.0')
+    result = script.pip('install', '-e', '.',
+                        expect_stderr=True, cwd=pkg_path)
+    assert 'Successfully installed simple-package' in result.stdout
+
+    result = script.pip('install', package_name)
+    assert 'Requirement already satisfied: {}'.format(
+        package_name) in result.stdout

tests/functional/test_search.py

@@ -168,7 +168,8 @@ def test_latest_prerelease_install_message(caplog, monkeypatch):
 
     dist = pretend.stub(version="1.0.0")
     get_dist = pretend.call_recorder(lambda x: dist)
-    monkeypatch.setattr("pip._vendor.pkg_resources.get_distribution", get_dist)
+    monkeypatch.setattr("pip._internal.commands.search.get_distribution",
+                        get_dist)
     with caplog.at_level(logging.INFO):
         print_results(hits)
 

tests/unit/test_self_check_outdated.py

@@ -6,7 +6,6 @@
 import freezegun
 import pretend
 import pytest
-from pip._vendor import pkg_resources
 
 from pip._internal import self_outdated_check
 from pip._internal.models.candidate import InstallationCandidate
@@ -98,7 +97,7 @@ def test_pip_self_version_check(monkeypatch, stored_time, installed_ver,
                         pretend.call_recorder(lambda *a, **kw: None))
     monkeypatch.setattr(logger, 'debug',
                         pretend.call_recorder(lambda s, exc_info=None: None))
-    monkeypatch.setattr(pkg_resources, 'get_distribution',
+    monkeypatch.setattr(self_outdated_check, 'get_distribution',
                         lambda name: MockDistribution(installer))
 
     fake_state = pretend.stub(