Merge pull request #8702 from uranusjr/get-distribution-looks-for-all

Author: pradyunsg

news/8695.bugfix

@@ -0,0 +1,3 @@
+Fix regression that distributions in system site-packages are not correctly
+found when a virtual environment is configured with ``system-site-packages``
+on.

src/pip/_internal/commands/search.py

@@ -140,6 +140,7 @@ def print_results(hits, name_column_width=None, terminal_width=None):
             write_output(line)
             if name in installed_packages:
                 dist = get_distribution(name)
+                assert dist is not None
                 with indent_log():
                     if dist.version == latest:
                         write_output('INSTALLED: %s (latest)', dist.version)

src/pip/_internal/utils/misc.py

@@ -483,22 +483,39 @@ def user_test(d):
             ]
 
 
-def search_distribution(req_name):
+def _search_distribution(req_name):
+    # type: (str) -> Optional[Distribution]
+    """Find a distribution matching the ``req_name`` in the environment.
 
+    This searches from *all* distributions available in the environment, to
+    match the behavior of ``pkg_resources.get_distribution()``.
+    """
     # 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=())
+    packages = get_installed_distributions(
+        local_only=False,
+        skip=(),
+        include_editables=True,
+        editables_only=False,
+        user_only=False,
+        paths=None,
+    )
     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"""
+    # type: (str) -> Optional[Distribution]
+    """Given a requirement name, return the installed Distribution object.
+
+    This searches from *all* distributions available in the environment, to
+    match the behavior of ``pkg_resources.get_distribution()``.
+    """
 
     # Search the distribution by looking through the working set
-    dist = search_distribution(req_name)
+    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
@@ -514,7 +531,7 @@ def get_distribution(req_name):
             pkg_resources.working_set.require(req_name)
         except pkg_resources.DistributionNotFound:
             return None
-    return search_distribution(req_name)
+    return _search_distribution(req_name)
 
 
 def egg_link_path(dist):

tests/unit/test_utils.py

@@ -5,6 +5,7 @@
 
 """
 import codecs
+import itertools
 import os
 import shutil
 import stat
@@ -34,6 +35,7 @@
     build_url_from_netloc,
     egg_link_path,
     format_size,
+    get_distribution,
     get_installed_distributions,
     get_prog,
     hide_url,
@@ -192,26 +194,30 @@ def test_noegglink_in_sitepkgs_venv_global(self):
 @patch('pip._internal.utils.misc.dist_in_usersite')
 @patch('pip._internal.utils.misc.dist_is_local')
 @patch('pip._internal.utils.misc.dist_is_editable')
-class Tests_get_installed_distributions:
-    """test util.get_installed_distributions"""
-
-    workingset = [
-        Mock(test_name="global"),
-        Mock(test_name="editable"),
-        Mock(test_name="normal"),
-        Mock(test_name="user"),
-    ]
-
-    workingset_stdlib = [
+class TestsGetDistributions(object):
+    """Test get_installed_distributions() and get_distribution().
+    """
+    class MockWorkingSet(list):
+        def require(self, name):
+            pass
+
+    workingset = MockWorkingSet((
+        Mock(test_name="global", key="global"),
+        Mock(test_name="editable", key="editable"),
+        Mock(test_name="normal", key="normal"),
+        Mock(test_name="user", key="user"),
+    ))
+
+    workingset_stdlib = MockWorkingSet((
         Mock(test_name='normal', key='argparse'),
         Mock(test_name='normal', key='wsgiref')
-    ]
+    ))
 
-    workingset_freeze = [
+    workingset_freeze = MockWorkingSet((
         Mock(test_name='normal', key='pip'),
         Mock(test_name='normal', key='setuptools'),
         Mock(test_name='normal', key='distribute')
-    ]
+    ))
 
     def dist_is_editable(self, dist):
         return dist.test_name == "editable"
@@ -287,6 +293,46 @@ def test_freeze_excludes(self, mock_dist_is_editable,
             skip=('setuptools', 'pip', 'distribute'))
         assert len(dists) == 0
 
+    @pytest.mark.parametrize(
+        "working_set, req_name",
+        itertools.chain(
+            itertools.product([workingset], (d.key for d in workingset)),
+            itertools.product(
+                [workingset_stdlib], (d.key for d in workingset_stdlib),
+            ),
+        ),
+    )
+    def test_get_distribution(
+        self,
+        mock_dist_is_editable,
+        mock_dist_is_local,
+        mock_dist_in_usersite,
+        working_set,
+        req_name,
+    ):
+        """Ensure get_distribution() finds all kinds of distributions.
+        """
+        mock_dist_is_editable.side_effect = self.dist_is_editable
+        mock_dist_is_local.side_effect = self.dist_is_local
+        mock_dist_in_usersite.side_effect = self.dist_in_usersite
+        with patch("pip._vendor.pkg_resources.working_set", working_set):
+            dist = get_distribution(req_name)
+        assert dist is not None
+        assert dist.key == req_name
+
+    @patch('pip._vendor.pkg_resources.working_set', workingset)
+    def test_get_distribution_nonexist(
+        self,
+        mock_dist_is_editable,
+        mock_dist_is_local,
+        mock_dist_in_usersite,
+    ):
+        mock_dist_is_editable.side_effect = self.dist_is_editable
+        mock_dist_is_local.side_effect = self.dist_is_local
+        mock_dist_in_usersite.side_effect = self.dist_in_usersite
+        dist = get_distribution("non-exist")
+        assert dist is None
+
 
 def test_rmtree_errorhandler_nonexistent_directory(tmpdir):
     """