From 6db0df928cc6811d46e64a0d772da4d8309ff8c2 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Fri, 28 Feb 2020 17:42:13 +0800 Subject: [PATCH] Move wheel cache out of InstallRequirment --- src/pip/_internal/cli/req_command.py | 8 +--- src/pip/_internal/commands/download.py | 8 +--- src/pip/_internal/commands/install.py | 2 +- src/pip/_internal/commands/wheel.py | 5 +-- src/pip/_internal/exceptions.py | 2 +- src/pip/_internal/operations/freeze.py | 5 +-- src/pip/_internal/req/constructors.py | 12 +----- src/pip/_internal/req/req_install.py | 31 --------------- .../_internal/resolution/legacy/resolver.py | 39 ++++++++++++++++--- .../resolution/resolvelib/candidates.py | 1 - .../resolution/resolvelib/resolver.py | 2 + tests/unit/test_req.py | 6 +-- 12 files changed, 46 insertions(+), 75 deletions(-) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 9a98335b4fc..104b033281f 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -255,7 +255,6 @@ def make_resolver( make_install_req = partial( install_req_from_req_string, isolated=options.isolated_mode, - wheel_cache=wheel_cache, use_pep517=use_pep517, ) # The long import name and duplicated invocation is needed to convince @@ -266,6 +265,7 @@ def make_resolver( return pip._internal.resolution.resolvelib.resolver.Resolver( preparer=preparer, finder=finder, + wheel_cache=wheel_cache, make_install_req=make_install_req, use_user_site=use_user_site, ignore_dependencies=options.ignore_dependencies, @@ -279,6 +279,7 @@ def make_resolver( return pip._internal.resolution.legacy.resolver.Resolver( preparer=preparer, finder=finder, + wheel_cache=wheel_cache, make_install_req=make_install_req, use_user_site=use_user_site, ignore_dependencies=options.ignore_dependencies, @@ -295,7 +296,6 @@ def get_requirements( options, # type: Values finder, # type: PackageFinder session, # type: PipSession - wheel_cache, # type: Optional[WheelCache] check_supported_wheels=True, # type: bool ): # type: (...) -> List[InstallRequirement] @@ -313,7 +313,6 @@ def get_requirements( req_to_add = install_req_from_parsed_requirement( parsed_req, isolated=options.isolated_mode, - wheel_cache=wheel_cache, ) req_to_add.is_direct = True requirement_set.add_requirement(req_to_add) @@ -322,7 +321,6 @@ def get_requirements( req_to_add = install_req_from_line( req, None, isolated=options.isolated_mode, use_pep517=options.use_pep517, - wheel_cache=wheel_cache ) req_to_add.is_direct = True requirement_set.add_requirement(req_to_add) @@ -332,7 +330,6 @@ def get_requirements( req, isolated=options.isolated_mode, use_pep517=options.use_pep517, - wheel_cache=wheel_cache ) req_to_add.is_direct = True requirement_set.add_requirement(req_to_add) @@ -345,7 +342,6 @@ def get_requirements( req_to_add = install_req_from_parsed_requirement( parsed_req, isolated=options.isolated_mode, - wheel_cache=wheel_cache, use_pep517=options.use_pep517 ) req_to_add.is_direct = True diff --git a/src/pip/_internal/commands/download.py b/src/pip/_internal/commands/download.py index 5b3b10da64f..c829550633e 100644 --- a/src/pip/_internal/commands/download.py +++ b/src/pip/_internal/commands/download.py @@ -107,13 +107,7 @@ def run(self, options, args): globally_managed=True, ) - reqs = self.get_requirements( - args, - options, - finder, - session, - None - ) + reqs = self.get_requirements(args, options, finder, session) preparer = self.make_requirement_preparer( temp_build_dir=directory, diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index df9681f8c1c..70bda2e2a92 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -299,7 +299,7 @@ def run(self, options, args): try: reqs = self.get_requirements( args, options, finder, session, - wheel_cache, check_supported_wheels=not options.target_dir, + check_supported_wheels=not options.target_dir, ) warn_deprecated_install_options( diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index b60d13e10d6..48f3bfa29ca 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -133,10 +133,7 @@ def run(self, options, args): globally_managed=True, ) - reqs = self.get_requirements( - args, options, finder, session, - wheel_cache - ) + reqs = self.get_requirements(args, options, finder, session) preparer = self.make_requirement_preparer( temp_build_dir=directory, diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 115f7eb0c6b..8ac85485e17 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -147,7 +147,7 @@ def body(self): triggering requirement. :param req: The InstallRequirement that provoked this error, with - populate_link() having already been called + its link already populated by the resolver's _populate_link(). """ return ' {}'.format(self._requirement_name()) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 5575d70d06f..841133d01b7 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -118,15 +118,12 @@ def freeze( else: line = line[len('--editable'):].strip().lstrip('=') line_req = install_req_from_editable( - line, - isolated=isolated, - wheel_cache=wheel_cache, + line, isolated=isolated, ) else: line_req = install_req_from_line( COMMENT_RE.sub('', line).strip(), isolated=isolated, - wheel_cache=wheel_cache, ) if not line_req.name: diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 195b0d0be78..edc6c36f999 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -36,7 +36,6 @@ from typing import ( Any, Dict, Optional, Set, Tuple, Union, ) - from pip._internal.cache import WheelCache from pip._internal.req.req_file import ParsedRequirement @@ -223,7 +222,6 @@ def install_req_from_editable( use_pep517=None, # type: Optional[bool] isolated=False, # type: bool options=None, # type: Optional[Dict[str, Any]] - wheel_cache=None, # type: Optional[WheelCache] constraint=False # type: bool ): # type: (...) -> InstallRequirement @@ -242,7 +240,6 @@ def install_req_from_editable( install_options=options.get("install_options", []) if options else [], global_options=options.get("global_options", []) if options else [], hash_options=options.get("hashes", {}) if options else {}, - wheel_cache=wheel_cache, extras=parts.extras, ) @@ -387,7 +384,6 @@ def install_req_from_line( use_pep517=None, # type: Optional[bool] isolated=False, # type: bool options=None, # type: Optional[Dict[str, Any]] - wheel_cache=None, # type: Optional[WheelCache] constraint=False, # type: bool line_source=None, # type: Optional[str] ): @@ -406,7 +402,6 @@ def install_req_from_line( install_options=options.get("install_options", []) if options else [], global_options=options.get("global_options", []) if options else [], hash_options=options.get("hashes", {}) if options else {}, - wheel_cache=wheel_cache, constraint=constraint, extras=parts.extras, ) @@ -416,7 +411,6 @@ def install_req_from_req_string( req_string, # type: str comes_from=None, # type: Optional[InstallRequirement] isolated=False, # type: bool - wheel_cache=None, # type: Optional[WheelCache] use_pep517=None # type: Optional[bool] ): # type: (...) -> InstallRequirement @@ -439,15 +433,13 @@ def install_req_from_req_string( ) return InstallRequirement( - req, comes_from, isolated=isolated, wheel_cache=wheel_cache, - use_pep517=use_pep517 + req, comes_from, isolated=isolated, use_pep517=use_pep517 ) def install_req_from_parsed_requirement( parsed_req, # type: ParsedRequirement isolated=False, # type: bool - wheel_cache=None, # type: Optional[WheelCache] use_pep517=None # type: Optional[bool] ): # type: (...) -> InstallRequirement @@ -458,7 +450,6 @@ def install_req_from_parsed_requirement( use_pep517=use_pep517, constraint=parsed_req.constraint, isolated=isolated, - wheel_cache=wheel_cache ) else: @@ -468,7 +459,6 @@ def install_req_from_parsed_requirement( use_pep517=use_pep517, isolated=isolated, options=parsed_req.options, - wheel_cache=wheel_cache, constraint=parsed_req.constraint, line_source=parsed_req.line_source, ) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index f5e0197f2b4..7da6640502f 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -30,7 +30,6 @@ from pip._internal.operations.install.wheel import install_wheel from pip._internal.pyproject import load_pyproject_toml, make_pyproject_path from pip._internal.req.req_uninstall import UninstallPathSet -from pip._internal.utils import compatibility_tags from pip._internal.utils.deprecation import deprecated from pip._internal.utils.hashes import Hashes from pip._internal.utils.logging import indent_log @@ -55,8 +54,6 @@ Any, Dict, Iterable, List, Optional, Sequence, Union, ) from pip._internal.build_env import BuildEnvironment - from pip._internal.cache import WheelCache - from pip._internal.index.package_finder import PackageFinder from pip._vendor.pkg_resources import Distribution from pip._vendor.packaging.specifiers import SpecifierSet from pip._vendor.packaging.markers import Marker @@ -111,7 +108,6 @@ def __init__( install_options=None, # type: Optional[List[str]] global_options=None, # type: Optional[List[str]] hash_options=None, # type: Optional[Dict[str, List[str]]] - wheel_cache=None, # type: Optional[WheelCache] constraint=False, # type: bool extras=() # type: Iterable[str] ): @@ -126,7 +122,6 @@ def __init__( self.source_dir = os.path.normpath(os.path.abspath(source_dir)) self.editable = editable - self._wheel_cache = wheel_cache if link is None and req and req.url: # PEP 508 URL requirement link = Link(req.url) @@ -241,32 +236,6 @@ def format_debug(self): state=", ".join(state), ) - def populate_link(self, finder, upgrade, require_hashes): - # type: (PackageFinder, bool, bool) -> None - """Ensure that if a link can be found for this, that it is found. - - Note that self.link may still be None - if Upgrade is False and the - requirement is already installed. - - If require_hashes is True, don't use the wheel cache, because cached - wheels, always built locally, have different hashes than the files - downloaded from the index server and thus throw false hash mismatches. - Furthermore, cached wheels at present have undeterministic contents due - to file modification times. - """ - if self.link is None: - self.link = finder.find_requirement(self, upgrade) - if self._wheel_cache is not None and not require_hashes: - old_link = self.link - supported_tags = compatibility_tags.get_supported() - self.link = self._wheel_cache.get( - link=self.link, - package_name=self.name, - supported_tags=supported_tags, - ) - if old_link != self.link: - logger.debug('Using cached wheel link: %s', self.link) - # Things that are valid for all kinds of requirements? @property def name(self): diff --git a/src/pip/_internal/resolution/legacy/resolver.py b/src/pip/_internal/resolution/legacy/resolver.py index d6800352614..a2f7895cee4 100644 --- a/src/pip/_internal/resolution/legacy/resolver.py +++ b/src/pip/_internal/resolution/legacy/resolver.py @@ -30,6 +30,7 @@ ) from pip._internal.req.req_set import RequirementSet from pip._internal.resolution.base import BaseResolver +from pip._internal.utils.compatibility_tags import get_supported from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import dist_in_usersite, normalize_version_info from pip._internal.utils.packaging import ( @@ -42,6 +43,7 @@ from typing import DefaultDict, List, Optional, Set, Tuple from pip._vendor import pkg_resources + from pip._internal.cache import WheelCache from pip._internal.distributions import AbstractDistribution from pip._internal.index.package_finder import PackageFinder from pip._internal.operations.prepare import RequirementPreparer @@ -112,6 +114,7 @@ def __init__( self, preparer, # type: RequirementPreparer finder, # type: PackageFinder + wheel_cache, # type: Optional[WheelCache] make_install_req, # type: InstallRequirementProvider use_user_site, # type: bool ignore_dependencies, # type: bool @@ -134,6 +137,7 @@ def __init__( self.preparer = preparer self.finder = finder + self.wheel_cache = wheel_cache self.upgrade_strategy = upgrade_strategy self.force_reinstall = force_reinstall @@ -166,7 +170,7 @@ def resolve(self, root_reqs, check_supported_wheels): # Actually prepare the files, and collect any exceptions. Most hash # exceptions cannot be checked ahead of time, because - # req.populate_link() needs to be called before we can make decisions + # _populate_link() needs to be called before we can make decisions # based on link type. discovered_reqs = [] # type: List[InstallRequirement] hash_errors = HashErrors() @@ -256,6 +260,34 @@ def _check_skip_installed(self, req_to_install): self._set_req_to_reinstall(req_to_install) return None + def _populate_link(self, req): + # type: (InstallRequirement) -> None + """Ensure that if a link can be found for this, that it is found. + + Note that req.link may still be None - if Upgrade is False and the + requirement is already installed. + + If preparer.require_hashes is True, don't use the wheel cache, because + cached wheels, always built locally, have different hashes than the + files downloaded from the index server and thus throw false hash + mismatches. Furthermore, cached wheels at present have undeterministic + contents due to file modification times. + """ + upgrade = self._is_upgrade_allowed(req) + if req.link is None: + req.link = self.finder.find_requirement(req, upgrade) + + if self.wheel_cache is None or self.preparer.require_hashes: + return + cached_link = self.wheel_cache.get( + link=req.link, + package_name=req.name, + supported_tags=get_supported(), + ) + if req.link != cached_link: + logger.debug('Using cached wheel link: %s', cached_link) + req.link = cached_link + def _get_abstract_dist_for(self, req): # type: (InstallRequirement) -> AbstractDistribution """Takes a InstallRequirement and returns a single AbstractDist \ @@ -274,11 +306,8 @@ def _get_abstract_dist_for(self, req): req, skip_reason ) - upgrade_allowed = self._is_upgrade_allowed(req) - # We eagerly populate the link, since that's our "legacy" behavior. - require_hashes = self.preparer.require_hashes - req.populate_link(self.finder, upgrade_allowed, require_hashes) + self._populate_link(req) abstract_dist = self.preparer.prepare_linked_requirement(req) # NOTE diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 2af6e25f18b..b589649c5f2 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -30,7 +30,6 @@ def make_install_req_from_link(link, parent): comes_from=parent.comes_from, use_pep517=parent.use_pep517, isolated=parent.isolated, - wheel_cache=parent._wheel_cache, constraint=parent.constraint, options=dict( install_options=parent.install_options, diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index 6e769f19ee5..1f332d4ed79 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -16,6 +16,7 @@ from pip._vendor.resolvelib.resolvers import Result + from pip._internal.cache import WheelCache from pip._internal.index.package_finder import PackageFinder from pip._internal.operations.prepare import RequirementPreparer from pip._internal.req.req_install import InstallRequirement @@ -27,6 +28,7 @@ def __init__( self, preparer, # type: RequirementPreparer finder, # type: PackageFinder + wheel_cache, # type: Optional[WheelCache] make_install_req, # type: InstallRequirementProvider use_user_site, # type: bool ignore_dependencies, # type: bool diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 530c3735c5e..d6129d51e1f 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -74,7 +74,6 @@ def _basic_resolver(self, finder, require_hashes=False): make_install_req = partial( install_req_from_req_string, isolated=False, - wheel_cache=None, use_pep517=None, ) @@ -95,6 +94,7 @@ def _basic_resolver(self, finder, require_hashes=False): preparer=preparer, make_install_req=make_install_req, finder=finder, + wheel_cache=None, use_user_site=False, upgrade_strategy="to-satisfy-only", ignore_dependencies=False, ignore_installed=False, ignore_requires_python=False, force_reinstall=False, @@ -176,9 +176,7 @@ def test_missing_hash_with_require_hashes_in_reqs_file(self, data, tmpdir): command = create_command('install') with requirements_file('--require-hashes', tmpdir) as reqs_file: options, args = command.parse_args(['-r', reqs_file]) - command.get_requirements( - args, options, finder, session, wheel_cache=None, - ) + command.get_requirements(args, options, finder, session) assert options.require_hashes def test_unsupported_hashes(self, data):