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 Mar 30, 2020
1 parent 019637c commit a8b88f0
Show file tree
Hide file tree
Showing 10 changed files with 43 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,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
Expand All @@ -279,6 +278,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,
Expand All @@ -295,7 +295,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 @@ -313,7 +312,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 @@ -322,7 +320,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 @@ -332,7 +329,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 @@ -345,7 +341,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 @@ -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(
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
2 changes: 1 addition & 1 deletion src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
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
30 changes: 0 additions & 30 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,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 +109,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 +123,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 +237,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):
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 @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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=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 @@ -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
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 @@ -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):
Expand Down

0 comments on commit a8b88f0

Please sign in to comment.