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

Move wheel cache out of InstallRequirement #7801

Merged
merged 4 commits into from
Mar 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 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 @@ -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,
Expand All @@ -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,
Expand All @@ -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]
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
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
2 changes: 0 additions & 2 deletions src/pip/_internal/operations/freeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,11 @@ def freeze(
line_req = install_req_from_editable(
line,
isolated=isolated,
wheel_cache=wheel_cache,
)
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 @@ -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
Expand All @@ -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 = 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
40 changes: 35 additions & 5 deletions src/pip/_internal/resolution/legacy/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
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,35 @@ def _check_skip_installed(self, req_to_install):
self._set_req_to_reinstall(req_to_install)
return None

def _populate_link(self, req):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, nice.

My brain is too fixed at thinking of populate_link as a method of InstallRequirement to have thought of doing this.

# 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 the requirement is already
installed and not needed to be upgraded based on the return value of
_is_upgrade_allowed().

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 \
Expand All @@ -274,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
1 change: 0 additions & 1 deletion src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/pip/_internal/resolution/resolvelib/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/unit/resolution_resolvelib/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ def resolver(preparer, finder):
resolver = Resolver(
preparer=preparer,
finder=finder,
wheel_cache=None,
make_install_req=mock.Mock(),
use_user_site="not-used",
ignore_dependencies="not-used",
Expand Down
Loading