Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid network/index related imports for commands that won't need 'em #12566

Merged
merged 9 commits into from
Apr 9, 2024
1 change: 1 addition & 0 deletions news/4768.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce startup time of commands (e.g. show, freeze) that do not access the network by 15-30%.
2 changes: 1 addition & 1 deletion src/pip/_internal/commands/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pip import __version__
from pip._internal.cli import cmdoptions
from pip._internal.cli.req_command import Command
from pip._internal.cli.base_command import Command
from pip._internal.cli.status_codes import SUCCESS
from pip._internal.metadata import BaseDistribution, get_environment
from pip._internal.utils.compat import stdlib_pkgs
Expand Down
8 changes: 5 additions & 3 deletions src/pip/_internal/distributions/base.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import abc
from typing import Optional
from typing import TYPE_CHECKING, Optional

from pip._internal.index.package_finder import PackageFinder
from pip._internal.metadata.base import BaseDistribution
from pip._internal.req import InstallRequirement

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder


class AbstractDistribution(metaclass=abc.ABCMeta):
"""A base class for handling installable artifacts.
Expand Down Expand Up @@ -44,7 +46,7 @@ def get_metadata_distribution(self) -> BaseDistribution:
@abc.abstractmethod
def prepare_distribution_metadata(
self,
finder: PackageFinder,
finder: "PackageFinder",
build_isolation: bool,
check_build_deps: bool,
) -> None:
Expand Down
12 changes: 7 additions & 5 deletions src/pip/_internal/distributions/sdist.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import logging
from typing import Iterable, Optional, Set, Tuple
from typing import TYPE_CHECKING, Iterable, Optional, Set, Tuple

from pip._internal.build_env import BuildEnvironment
from pip._internal.distributions.base import AbstractDistribution
from pip._internal.exceptions import InstallationError
from pip._internal.index.package_finder import PackageFinder
from pip._internal.metadata import BaseDistribution
from pip._internal.utils.subprocess import runner_with_spinner_message

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder

logger = logging.getLogger(__name__)


Expand All @@ -29,7 +31,7 @@ def get_metadata_distribution(self) -> BaseDistribution:

def prepare_distribution_metadata(
self,
finder: PackageFinder,
finder: "PackageFinder",
build_isolation: bool,
check_build_deps: bool,
) -> None:
Expand Down Expand Up @@ -66,7 +68,7 @@ def prepare_distribution_metadata(
self._raise_missing_reqs(missing)
self.req.prepare_metadata()

def _prepare_build_backend(self, finder: PackageFinder) -> None:
def _prepare_build_backend(self, finder: "PackageFinder") -> None:
# Isolate in a BuildEnvironment and install the build-time
# requirements.
pyproject_requires = self.req.pyproject_requires
Expand Down Expand Up @@ -110,7 +112,7 @@ def _get_build_requires_editable(self) -> Iterable[str]:
with backend.subprocess_runner(runner):
return backend.get_requires_for_build_editable()

def _install_build_reqs(self, finder: PackageFinder) -> None:
def _install_build_reqs(self, finder: "PackageFinder") -> None:
# Install any extra build dependencies that the backend requests.
# This must be done in a second pass, as the pyproject.toml
# dependencies must be installed before we can call the backend.
Expand Down
8 changes: 5 additions & 3 deletions src/pip/_internal/distributions/wheel.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
from typing import Optional
from typing import TYPE_CHECKING, Optional

from pip._vendor.packaging.utils import canonicalize_name

from pip._internal.distributions.base import AbstractDistribution
from pip._internal.index.package_finder import PackageFinder
from pip._internal.metadata import (
BaseDistribution,
FilesystemWheel,
get_wheel_distribution,
)

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder


class WheelDistribution(AbstractDistribution):
"""Represents a wheel distribution.
Expand All @@ -33,7 +35,7 @@ def get_metadata_distribution(self) -> BaseDistribution:

def prepare_distribution_metadata(
self,
finder: PackageFinder,
finder: "PackageFinder",
build_isolation: bool,
check_build_deps: bool,
) -> None:
Expand Down
7 changes: 4 additions & 3 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
from itertools import chain, groupby, repeat
from typing import TYPE_CHECKING, Dict, Iterator, List, Literal, Optional, Union

from pip._vendor.requests.models import Request, Response
from pip._vendor.rich.console import Console, ConsoleOptions, RenderResult
from pip._vendor.rich.markup import escape
from pip._vendor.rich.text import Text

if TYPE_CHECKING:
from hashlib import _Hash

from pip._vendor.requests.models import Request, Response

from pip._internal.metadata import BaseDistribution
from pip._internal.req.req_install import InstallRequirement

Expand Down Expand Up @@ -293,8 +294,8 @@ class NetworkConnectionError(PipError):
def __init__(
self,
error_msg: str,
response: Optional[Response] = None,
request: Optional[Request] = None,
response: Optional["Response"] = None,
request: Optional["Request"] = None,
) -> None:
"""
Initialize NetworkConnectionError with `request` and `response`
Expand Down
16 changes: 9 additions & 7 deletions src/pip/_internal/req/req_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@
from pip._internal.cli import cmdoptions
from pip._internal.exceptions import InstallationError, RequirementsFileParseError
from pip._internal.models.search_scope import SearchScope
from pip._internal.network.session import PipSession
from pip._internal.network.utils import raise_for_status
from pip._internal.utils.encoding import auto_decode
from pip._internal.utils.urls import get_url_scheme

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder
from pip._internal.network.session import PipSession

__all__ = ["parse_requirements"]

Expand Down Expand Up @@ -133,7 +132,7 @@ def __init__(

def parse_requirements(
filename: str,
session: PipSession,
session: "PipSession",
finder: Optional["PackageFinder"] = None,
options: Optional[optparse.Values] = None,
constraint: bool = False,
Expand Down Expand Up @@ -210,7 +209,7 @@ def handle_option_line(
lineno: int,
finder: Optional["PackageFinder"] = None,
options: Optional[optparse.Values] = None,
session: Optional[PipSession] = None,
session: Optional["PipSession"] = None,
) -> None:
if opts.hashes:
logger.warning(
Expand Down Expand Up @@ -278,7 +277,7 @@ def handle_line(
line: ParsedLine,
options: Optional[optparse.Values] = None,
finder: Optional["PackageFinder"] = None,
session: Optional[PipSession] = None,
session: Optional["PipSession"] = None,
) -> Optional[ParsedRequirement]:
"""Handle a single parsed requirements line; This can result in
creating/yielding requirements, or updating the finder.
Expand Down Expand Up @@ -321,7 +320,7 @@ def handle_line(
class RequirementsFileParser:
def __init__(
self,
session: PipSession,
session: "PipSession",
line_parser: LineParser,
) -> None:
self._session = session
Expand Down Expand Up @@ -526,7 +525,7 @@ def expand_env_variables(lines_enum: ReqFileLines) -> ReqFileLines:
yield line_number, line


def get_file_content(url: str, session: PipSession) -> Tuple[str, str]:
def get_file_content(url: str, session: "PipSession") -> Tuple[str, str]:
"""Gets the content of a file; it may be a filename, file: URL, or
http: URL. Returns (location, content). Content is unicode.
Respects # -*- coding: declarations on the retrieved files.
Expand All @@ -538,6 +537,9 @@ def get_file_content(url: str, session: PipSession) -> Tuple[str, str]:

# Pip has special support for file:// URLs (LocalFSAdapter).
if scheme in ["http", "https", "file"]:
# Delay importing heavy network modules until absolutely necessary.
from pip._internal.network.utils import raise_for_status

resp = session.get(url)
raise_for_status(resp)
return resp.url, resp.text
Expand Down
50 changes: 50 additions & 0 deletions tests/functional/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
"""Basic CLI functionality checks.
"""
import subprocess
import sys
from pathlib import Path
from textwrap import dedent

import pytest

from pip._internal.commands import commands_dict
from tests.lib import PipTestEnvironment


Expand Down Expand Up @@ -45,3 +49,49 @@ def test_entrypoints_work(entrypoint: str, script: PipTestEnvironment) -> None:
result2 = script.run("fake_pip", "-V", allow_stderr_warning=True)
assert result.stdout == result2.stdout
assert "old script wrapper" in result2.stderr


@pytest.mark.parametrize(
"command",
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"}
)
),
)
def test_no_network_imports(command: str, tmp_path: Path) -> None:
"""
Verify that commands that don't access the network do NOT import network code.

This helps to reduce the startup time of these commands.

Note: This won't catch lazy network imports, but it'll catch top-level
network imports which were accidently added (which is the most likely way
to regress anyway).
"""
file = tmp_path / f"imported_modules_for_{command}.txt"
code = f"""
import runpy
import sys

sys.argv[1:] = [{command!r}, "--help"]

try:
runpy.run_module("pip", alter_sys=True, run_name="__main__")
finally:
with open({str(file)!r}, "w") as f:
print(*sys.modules.keys(), sep="\\n", file=f)
"""
subprocess.run(
[sys.executable],
input=code,
encoding="utf-8",
check=True,
)
imported = file.read_text().splitlines()
assert not any("pip._internal.index" in mod for mod in imported)
assert not any("pip._internal.network" in mod for mod in imported)
assert not any("requests" in mod for mod in imported)
assert not any("urllib3" in mod for mod in imported)
Loading