Repository: pypa/pip
Pull request #12637 (2024-04-18 01:35:25)

Avoid network/index related imports for pip uninstall & list (unless necessary)
This PR builds upon the work started in https://github.com/pypa/pip/pull/12566, speeding up `pip list` and `pip uninstall`. Towards https://github.com/pypa/pip/issues/4768.

pip uninstall and list currently depend on `req_install.py` which always imports the expensive network and index machinery. However, it's only in rare situations that these commands actually hit the network:

- ``pip list --outdated``
- ``pip list --uptodate``
- ``pip uninstall --requirement <url>``

This PR refactors `req_install.py` so these commands can avoid the expensive imports unless truly necessary.

The news entry is trivial as the one already added in #12566 is sufficient. 

ichard26: I'll note that `pip list` still eventually imports the network/index machinery as the pip version check is still enabled for this command, but I'll file a PR to change that soon:tm: (as per #11677)
ichard26: Hey @pradyunsg, I would like to add this PR in addition to #12646 to the 24.1 milestone. Together with #12566 (and the pyparsing removal scheduled via the packaging bump), it would be a solid responsiveness improvement overall. Feel free to reject this request if you would prefer to preserve the churn budget (or your time :slightly_smiling_face:) for something else.

pradyunsg: @ichard26 This has an outstanding merge conflict, presumably since we've actually had a bunch of activity and merges in the last few days.

Patch:
diff --git a/news/24ca5f01-e27e-41b1-9a9c-7e3a828e22d9.trivial.rst b/news/24ca5f01-e27e-41b1-9a9c-7e3a828e22d9.trivial.rst
new file mode 100644
index 000000000..1be253956
--- /dev/null
+++ b/news/24ca5f01-e27e-41b1-9a9c-7e3a828e22d9.trivial.rst
@@ -0,0 +1,10 @@
+pip uninstall and list currently depend on req_install.py which always imports
+the expensive network and index machinery. However, it's only in rare situations
+that these commands actually hit the network:
+
+- ``pip list --outdated``
+- ``pip list --uptodate``
+- ``pip uninstall --requirement <url>``
+
+This patch refactors req_install.py so these commands can avoid the expensive
+imports unless truly necessary.
diff --git a/src/pip/_internal/cli/index_command.py b/src/pip/_internal/cli/index_command.py
new file mode 100644
index 000000000..4ff7b2c3a
--- /dev/null
+++ b/src/pip/_internal/cli/index_command.py
@@ -0,0 +1,172 @@
+"""
+Contains command classes which may interact with an index / the network.
+
+Unlike its sister module, req_command, this module still uses lazy imports
+so commands which don't always hit the network (e.g. list w/o --outdated or
+--uptodate) don't need waste time importing PipSession and friends.
+"""
+
+import logging
+import os
+import sys
+from optparse import Values
+from typing import TYPE_CHECKING, List, Optional
+
+from pip._internal.cli.base_command import Command
+from pip._internal.cli.command_context import CommandContextMixIn
+from pip._internal.exceptions import CommandError
+
+if TYPE_CHECKING:
+    from ssl import SSLContext
+
+    from pip._internal.network.session import PipSession
+
+logger = logging.getLogger(__name__)
+
+
+def _create_truststore_ssl_context() -> Optional["SSLContext"]:
+    if sys.version_info < (3, 10):
+        raise CommandError("The truststore feature is only available for Python 3.10+")
+
+    try:
+        import ssl
+    except ImportError:
+        logger.warning("Disabling truststore since ssl support is missing")
+        return None
+
+    try:
+        from pip._vendor import truststore
+    except ImportError as e:
+        raise CommandError(f"The truststore feature is unavailable: {e}")
+
+    return truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
+
+
+class SessionCommandMixin(CommandContextMixIn):
+    """
+    A class mixin for command classes needing _build_session().
+    """
+
+    def __init__(self) -> None:
+        super().__init__()
+        self._session: Optional["PipSession"] = None
+
+    @classmethod
+    def _get_index_urls(cls, options: Values) -> Optional[List[str]]:
+        """Return a list of index urls from user-provided options."""
+        index_urls = []
+        if not getattr(options, "no_index", False):
+            url = getattr(options, "index_url", None)
+            if url:
+                index_urls.append(url)
+        urls = getattr(options, "extra_index_urls", None)
+        if urls:
+            index_urls.extend(urls)
+        # Return None rather than an empty list
+        return index_urls or None
+
+    def get_default_session(self, options: Values) -> "PipSession":
+        """Get a default-managed session."""
+        if self._session is None:
+            self._session = self.enter_context(self._build_session(options))
+            # there's no type annotation on requests.Session, so it's
+            # automatically ContextManager[Any] and self._session becomes Any,
+            # then https://github.com/python/mypy/issues/7696 kicks in
+            assert self._session is not None
+        return self._session
+
+    def _build_session(
+        self,
+        options: Values,
+        retries: Optional[int] = None,
+        timeout: Optional[int] = None,
+        fallback_to_certifi: bool = False,
+    ) -> "PipSession":
+        from pip._internal.network.session import PipSession
+
+        cache_dir = options.cache_dir
+        assert not cache_dir or os.path.isabs(cache_dir)
+
+        if "truststore" in options.features_enabled:
+            try:
+                ssl_context = _create_truststore_ssl_context()
+            except Exception:
+                if not fallback_to_certifi:
+                    raise
+                ssl_context = None
+        else:
+            ssl_context = None
+
+        session = PipSession(
+            cache=os.path.join(cache_dir, "http-v2") if cache_dir else None,
+            retries=retries if retries is not None else options.retries,
+            trusted_hosts=options.trusted_hosts,
+            index_urls=self._get_index_urls(options),
+            ssl_context=ssl_context,
+        )
+
+        # Handle custom ca-bundles from the user
+        if options.cert:
+            session.verify = options.cert
+
+        # Handle SSL client certificate
+        if options.client_cert:
+            session.cert = options.client_cert
+
+        # Handle timeouts
+        if options.timeout or timeout:
+            session.timeout = timeout if timeout is not None else options.timeout
+
+        # Handle configured proxies
+        if options.proxy:
+            session.proxies = {
+                "http": options.proxy,
+                "https": options.proxy,
+            }
+            session.trust_env = False
+
+        # Determine if we can prompt the user for authentication or not
+        session.auth.prompting = not options.no_input
+        session.auth.keyring_provider = options.keyring_provider
+
+        return session
+
+
+def _pip_self_version_check(session: "PipSession", options: Values) -> None:
+    from pip._internal.self_outdated_check import pip_self_version_check as check
+
+    check(session, options)
+
+
+class IndexGroupCommand(Command, SessionCommandMixin):
+    """
+    Abstract base class for commands with the index_group options.
+
+    This also corresponds to the commands that permit the pip version check.
+    """
+
+    def handle_pip_version_check(self, options: Values) -> None:
+        """
+        Do the pip version check if not disabled.
+
+        This overrides the default behavior of not doing the check.
+        """
+        # Make sure the index_group options are present.
+        assert hasattr(options, "no_index")
+
+        if options.disable_pip_version_check or options.no_index:
+            return
+
+        # Otherwise, check if we're using the latest version of pip available.
+        session = self._build_session(
+            options,
+            retries=0,
+            timeout=min(5, options.timeout),
+            # This is set to ensure the function does not fail when truststore is
+            # specified in use-feature but cannot be loaded. This usually raises a
+            # CommandError and shows a nice user-facing error, but this function is not
+            # called in that try-except block.
+            fallback_to_certifi=True,
+        )
+        with session:
+            _pip_self_version_check(session, options)
diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py
index 7a5580a84..92900f94f 100644
--- a/src/pip/_internal/cli/req_command.py
+++ b/src/pip/_internal/cli/req_command.py
@@ -1,21 +1,19 @@
-"""Contains the Command base classes that depend on PipSession.
+"""Contains the RequirementCommand base class.
 
-The classes in this module are in a separate module so the commands not
-needing download / PackageFinder capability don't unnecessarily import the
+This class is in a separate module so the commands that do not always
+need PackageFinder capability don't unnecessarily import the
 PackageFinder machinery and all its vendored dependencies, etc.
 """
 
 import logging
-import os
-import sys
 from functools import partial
 from optparse import Values
-from typing import TYPE_CHECKING, Any, List, Optional, Tuple
+from typing import Any, List, Optional, Tuple
 
 from pip._internal.cache import WheelCache
 from pip._internal.cli import cmdoptions
-from pip._internal.cli.base_command import Command
-from pip._internal.cli.command_context import CommandContextMixIn
+from pip._internal.cli.index_command import IndexGroupCommand
+from pip._internal.cli.index_command import SessionCommandMixin as SessionCommandMixin
 from pip._internal.exceptions import CommandError, PreviousBuildDirError
 from pip._internal.index.collector import LinkCollector
 from pip._internal.index.package_finder import PackageFinder
@@ -33,160 +31,15 @@ from pip._internal.req.constructors import (
 from pip._internal.req.req_file import parse_requirements
 from pip._internal.req.req_install import InstallRequirement
 from pip._internal.resolution.base import BaseResolver
-from pip._internal.self_outdated_check import pip_self_version_check
 from pip._internal.utils.temp_dir import (
     TempDirectory,
     TempDirectoryTypeRegistry,
     tempdir_kinds,
 )
-from pip._internal.utils.virtualenv import running_under_virtualenv
-
-if TYPE_CHECKING:
-    from ssl import SSLContext
 
 logger = logging.getLogger(__name__)
 
 
-def _create_truststore_ssl_context() -> Optional["SSLContext"]:
-    if sys.version_info < (3, 10):
-        raise CommandError("The truststore feature is only available for Python 3.10+")
-
-    try:
-        import ssl
-    except ImportError:
-        logger.warning("Disabling truststore since ssl support is missing")
-        return None
-
-    try:
-        from pip._vendor import truststore
-    except ImportError as e:
-        raise CommandError(f"The truststore feature is unavailable: {e}")
-
-    return truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
-
-
-class SessionCommandMixin(CommandContextMixIn):
-    """
-    A class mixin for command classes needing _build_session().
-    """
-
-    def __init__(self) -> None:
-        super().__init__()
-        self._session: Optional[PipSession] = None
-
-    @classmethod
-    def _get_index_urls(cls, options: Values) -> Optional[List[str]]:
-        """Return a list of index urls from user-provided options."""
-        index_urls = []
-        if not getattr(options, "no_index", False):
-            url = getattr(options, "index_url", None)
-            if url:
-                index_urls.append(url)
-        urls = getattr(options, "extra_index_urls", None)
-        if urls:
-            index_urls.extend(urls)
-        # Return None rather than an empty list
-        return index_urls or None
-
-    def get_default_session(self, options: Values) -> PipSession:
-        """Get a default-managed session."""
-        if self._session is None:
-            self._session = self.enter_context(self._build_session(options))
-            # there's no type annotation on requests.Session, so it's
-            # automatically ContextManager[Any] and self._session becomes Any,
-            # then https://github.com/python/mypy/issues/7696 kicks in
-            assert self._session is not None
-        return self._session
-
-    def _build_session(
-        self,
-        options: Values,
-        retries: Optional[int] = None,
-        timeout: Optional[int] = None,
-        fallback_to_certifi: bool = False,
-    ) -> PipSession:
-        cache_dir = options.cache_dir
-        assert not cache_dir or os.path.isabs(cache_dir)
-
-        if "truststore" in options.features_enabled:
-            try:
-                ssl_context = _create_truststore_ssl_context()
-            except Exception:
-                if not fallback_to_certifi:
-                    raise
-                ssl_context = None
-        else:
-            ssl_context = None
-
-        session = PipSession(
-            cache=os.path.join(cache_dir, "http-v2") if cache_dir else None,
-            retries=retries if retries is not None else options.retries,
-            trusted_hosts=options.trusted_hosts,
-            index_urls=self._get_index_urls(options),
-            ssl_context=ssl_context,
-        )
-
-        # Handle custom ca-bundles from the user
-        if options.cert:
-            session.verify = options.cert
-
-        # Handle SSL client certificate
-        if options.client_cert:
-            session.cert = options.client_cert
-
-        # Handle timeouts
-        if options.timeout or timeout:
-            session.timeout = timeout if timeout is not None else options.timeout
-
-        # Handle configured proxies
-        if options.proxy:
-            session.proxies = {
-                "http": options.proxy,
-                "https": options.proxy,
-            }
-            session.trust_env = False
-
-        # Determine if we can prompt the user for authentication or not
-        session.auth.prompting = not options.no_input
-        session.auth.keyring_provider = options.keyring_provider
-
-        return session
-
-
-class IndexGroupCommand(Command, SessionCommandMixin):
-    """
-    Abstract base class for commands with the index_group options.
-
-    This also corresponds to the commands that permit the pip version check.
-    """
-
-    def handle_pip_version_check(self, options: Values) -> None:
-        """
-        Do the pip version check if not disabled.
-
-        This overrides the default behavior of not doing the check.
-        """
-        # Make sure the index_group options are present.
-        assert hasattr(options, "no_index")
-
-        if options.disable_pip_version_check or options.no_index:
-            return
-
-        # Otherwise, check if we're using the latest version of pip available.
-        session = self._build_session(
-            options,
-            retries=0,
-            timeout=min(5, options.timeout),
-            # This is set to ensure the function does not fail when truststore is
-            # specified in use-feature but cannot be loaded. This usually raises a
-            # CommandError and shows a nice user-facing error, but this function is not
-            # called in that try-except block.
-            fallback_to_certifi=True,
-        )
-        with session:
-            pip_self_version_check(session, options)
-
-
 KEEPABLE_TEMPDIR_TYPES = [
     tempdir_kinds.BUILD_ENV,
     tempdir_kinds.EPHEM_WHEEL_CACHE,
@@ -194,39 +47,6 @@ KEEPABLE_TEMPDIR_TYPES = [
 ]
 
 
-def warn_if_run_as_root() -> None:
-    """Output a warning for sudo users on Unix.
-
-    In a virtual environment, sudo pip still writes to virtualenv.
-    On Windows, users may run pip as Administrator without issues.
-    This warning only applies to Unix root users outside of virtualenv.
-    """
-    if running_under_virtualenv():
-        return
-    if not hasattr(os, "getuid"):
-        return
-    # On Windows, there are no "system managed" Python packages. Installing as
-    # Administrator via pip is the correct way of updating system environments.
-    #
-    # We choose sys.platform over utils.compat.WINDOWS here to enable Mypy platform
-    # checks: https://mypy.readthedocs.io/en/stable/common_issues.html
-    if sys.platform == "win32" or sys.platform == "cygwin":
-        return
-
-    if os.getuid() != 0:
-        return
-
-    logger.warning(
-        "Running pip as the 'root' user can result in broken permissions and "
-        "conflicting behaviour with the system package manager, possibly "
-        "rendering your system unusable."
-        "It is recommended to use a virtual environment instead: "
-        "https://pip.pypa.io/warnings/venv. "
-        "Use the --root-user-action option if you know what you are doing and "
-        "want to suppress this warning."
-    )
-
-
 def with_cleanup(func: Any) -> Any:
     """Decorator for common logic related to managing temporary
     directories.
diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py
index a9e83f9d8..29276c243 100644
--- a/src/pip/_internal/commands/install.py
+++ b/src/pip/_internal/commands/install.py
@@ -14,7 +14,6 @@ from pip._internal.cli import cmdoptions
 from pip._internal.cli.cmdoptions import make_target_python
 from pip._internal.cli.req_command import (
     RequirementCommand,
-    warn_if_run_as_root,
     with_cleanup,
 )
 from pip._internal.cli.status_codes import ERROR, SUCCESS
@@ -37,6 +36,7 @@ from pip._internal.utils.misc import (
     ensure_dir,
     get_pip_version,
     protect_pip_from_modification_on_windows,
+    warn_if_run_as_root,
     write_output,
 )
 from pip._internal.utils.temp_dir import TempDirectory
diff --git a/src/pip/_internal/commands/list.py b/src/pip/_internal/commands/list.py
index cb1e313f4..82fc46a11 100644
--- a/src/pip/_internal/commands/list.py
+++ b/src/pip/_internal/commands/list.py
@@ -7,18 +7,17 @@ from pip._vendor.packaging.utils import canonicalize_name
 from pip._vendor.packaging.version import Version
 
 from pip._internal.cli import cmdoptions
-from pip._internal.cli.req_command import IndexGroupCommand
+from pip._internal.cli.index_command import IndexGroupCommand
 from pip._internal.cli.status_codes import SUCCESS
 from pip._internal.exceptions import CommandError
-from pip._internal.index.collector import LinkCollector
-from pip._internal.index.package_finder import PackageFinder
 from pip._internal.metadata import BaseDistribution, get_environment
 from pip._internal.models.selection_prefs import SelectionPreferences
-from pip._internal.network.session import PipSession
 from pip._internal.utils.compat import stdlib_pkgs
 from pip._internal.utils.misc import tabulate, write_output
 
 if TYPE_CHECKING:
+    from pip._internal.index.package_finder import PackageFinder
+    from pip._internal.network.session import PipSession
 
     class _DistWithLatestInfo(BaseDistribution):
         """Give the distribution object a couple of extra fields.
@@ -140,11 +139,15 @@ class ListCommand(IndexGroupCommand):
             super().handle_pip_version_check(options)
 
     def _build_package_finder(
-        self, options: Values, session: PipSession
-    ) -> PackageFinder:
+        self, options: Values, session: "PipSession"
+    ) -> "PackageFinder":
         """
         Create a package finder appropriate to this list command.
         """
+        # Lazy import the heavy index modules as most list invocations won't need 'em.
+        from pip._internal.index.collector import LinkCollector
+        from pip._internal.index.package_finder import PackageFinder
+
         link_collector = LinkCollector.create(session, options=options)
 
         # Pass allow_yanked=False to ignore yanked versions.
diff --git a/src/pip/_internal/commands/uninstall.py b/src/pip/_internal/commands/uninstall.py
index f198fc313..bc0edeac9 100644
--- a/src/pip/_internal/commands/uninstall.py
+++ b/src/pip/_internal/commands/uninstall.py
@@ -6,7 +6,7 @@ from pip._vendor.packaging.utils import canonicalize_name
 
 from pip._internal.cli import cmdoptions
 from pip._internal.cli.base_command import Command
-from pip._internal.cli.req_command import SessionCommandMixin, warn_if_run_as_root
+from pip._internal.cli.index_command import SessionCommandMixin
 from pip._internal.cli.status_codes import SUCCESS
 from pip._internal.exceptions import InstallationError
 from pip._internal.req import parse_requirements
@@ -17,6 +17,7 @@ from pip._internal.req.constructors import (
 from pip._internal.utils.misc import (
     check_externally_managed,
     protect_pip_from_modification_on_windows,
+    warn_if_run_as_root,
 )
 
 logger = logging.getLogger(__name__)
diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py
index 82b626132..48771c099 100644
--- a/src/pip/_internal/utils/misc.py
+++ b/src/pip/_internal/utils/misc.py
@@ -745,3 +745,36 @@ class ConfiguredBuildBackendHookCaller(BuildBackendHookCaller):
             config_settings=cs,
             _allow_fallback=_allow_fallback,
         )
+
+
+def warn_if_run_as_root() -> None:
+    """Output a warning for sudo users on Unix.
+
+    In a virtual environment, sudo pip still writes to virtualenv.
+    On Windows, users may run pip as Administrator without issues.
+    This warning only applies to Unix root users outside of virtualenv.
+    """
+    if running_under_virtualenv():
+        return
+    if not hasattr(os, "getuid"):
+        return
+    # On Windows, there are no "system managed" Python packages. Installing as
+    # Administrator via pip is the correct way of updating system environments.
+    #
+    # We choose sys.platform over utils.compat.WINDOWS here to enable Mypy platform
+    # checks: https://mypy.readthedocs.io/en/stable/common_issues.html
+    if sys.platform == "win32" or sys.platform == "cygwin":
+        return
+
+    if os.getuid() != 0:
+        return
+
+    logger.warning(
+        "Running pip as the 'root' user can result in broken permissions and "
+        "conflicting behaviour with the system package manager, possibly "
+        "rendering your system unusable."
+        "It is recommended to use a virtual environment instead: "
+        "https://pip.pypa.io/warnings/venv. "
+        "Use the --root-user-action option if you know what you are doing and "
+        "want to suppress this warning."
+    )


Test patch:
diff --git a/tests/functional/test_cli.py b/tests/functional/test_cli.py
index 3c1661521..e1ccf04ea 100644
--- a/tests/functional/test_cli.py
+++ b/tests/functional/test_cli.py
@@ -57,8 +57,7 @@ def test_entrypoints_work(entrypoint: str, script: PipTestEnvironment) -> None:
     sorted(
         set(commands_dict).symmetric_difference(
             # Exclude commands that are expected to use the network.
-            # TODO: uninstall and list should only import network modules as needed
-            {"install", "uninstall", "download", "search", "index", "wheel", "list"}
+            {"install", "download", "search", "index", "wheel"}
         )
     ),
 )
diff --git a/tests/unit/test_base_command.py b/tests/unit/test_base_command.py
index 44dae384a..80c93799d 100644
--- a/tests/unit/test_base_command.py
+++ b/tests/unit/test_base_command.py
@@ -93,7 +93,7 @@ class TestCommand:
         assert "Traceback (most recent call last):" in stderr
 
 
-@patch("pip._internal.cli.req_command.Command.handle_pip_version_check")
+@patch("pip._internal.cli.index_command.Command.handle_pip_version_check")
 def test_handle_pip_version_check_called(mock_handle_version_check: Mock) -> None:
     """
     Check that Command.handle_pip_version_check() is called.
diff --git a/tests/unit/test_commands.py b/tests/unit/test_commands.py
index 4f6366513..9d5aefec2 100644
--- a/tests/unit/test_commands.py
+++ b/tests/unit/test_commands.py
@@ -87,7 +87,7 @@ def test_index_group_commands() -> None:
         (True, True, False),
     ],
 )
-@mock.patch("pip._internal.cli.req_command.pip_self_version_check")
+@mock.patch("pip._internal.cli.index_command._pip_self_version_check")
 def test_index_group_handle_pip_version_check(
     mock_version_check: mock.Mock,
     command_name: str,
@@ -128,7 +128,7 @@ def test_requirement_commands() -> None:
 
 
 @pytest.mark.parametrize("flag", ["", "--outdated", "--uptodate"])
-@mock.patch("pip._internal.cli.req_command.pip_self_version_check")
+@mock.patch("pip._internal.cli.index_command._pip_self_version_check")
 @mock.patch.dict(os.environ, {"PIP_DISABLE_PIP_VERSION_CHECK": "no"})
 def test_list_pip_version_check(version_check_mock: mock.Mock, flag: str) -> None:
     """