Skip to content

Commit

Permalink
Move wheel cache out of InstallRequirment
Browse files Browse the repository at this point in the history
  • Loading branch information
uranusjr committed Feb 28, 2020
1 parent 5b442d5 commit d998eaf
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 73 deletions.
7 changes: 1 addition & 6 deletions src/pip/_internal/cli/req_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ def make_resolver(
make_install_req = partial(
install_req_from_req_string,
isolated=options.isolated_mode,
wheel_cache=wheel_cache,
use_pep517=use_pep517,
)
return 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,
Expand All @@ -277,7 +277,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]
Expand All @@ -295,7 +294,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)
Expand All @@ -304,7 +302,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)
Expand All @@ -314,7 +311,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)
Expand All @@ -327,7 +323,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
Expand Down
8 changes: 1 addition & 7 deletions src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,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(
Expand Down
5 changes: 1 addition & 4 deletions src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 1 addition & 4 deletions src/pip/_internal/operations/freeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 1 addition & 11 deletions src/pip/_internal/req/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand All @@ -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,
)

Expand Down Expand Up @@ -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]
):
Expand All @@ -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,
)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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,
)
Expand Down
31 changes: 0 additions & 31 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pip._vendor.packaging.version import parse as parse_version
from pip._vendor.pep517.wrappers import Pep517HookCaller

from pip._internal import pep425tags
from pip._internal.build_env import NoOpBuildEnvironment
from pip._internal.exceptions import InstallationError
from pip._internal.locations import get_scheme
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
):
Expand All @@ -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)
Expand Down Expand Up @@ -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 = pep425tags.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):
Expand Down
39 changes: 34 additions & 5 deletions src/pip/_internal/resolution/legacy/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from pip._vendor.packaging import specifiers

from pip._internal import pep425tags
from pip._internal.exceptions import (
BestVersionAlreadyInstalled,
DistributionNotFound,
Expand All @@ -41,6 +42,7 @@
from typing import Callable, 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
Expand Down Expand Up @@ -113,6 +115,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
Expand All @@ -135,6 +138,7 @@ def __init__(

self.preparer = preparer
self.finder = finder
self.wheel_cache = wheel_cache

self.upgrade_strategy = upgrade_strategy
self.force_reinstall = force_reinstall
Expand Down Expand Up @@ -167,7 +171,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()
Expand Down Expand Up @@ -257,6 +261,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 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=pep425tags.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 \
Expand All @@ -275,11 +307,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
Expand Down
6 changes: 2 additions & 4 deletions tests/unit/test_req.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand All @@ -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,
Expand Down Expand Up @@ -175,9 +175,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):
Expand Down

0 comments on commit d998eaf

Please sign in to comment.