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 fast-dep handling to RequirementPreparer #8685

Merged
merged 18 commits into from Aug 4, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/pip/_internal/cli/req_command.py
Expand Up @@ -218,6 +218,19 @@ def make_requirement_preparer(
temp_build_dir_path = temp_build_dir.path
assert temp_build_dir_path is not None

if '2020-resolver' in options.features_enabled:
lazy_wheel = 'fast-deps' in options.features_enabled
if lazy_wheel:
logger.warning(
'pip is using lazily downloaded wheels using HTTP '
'range requests to obtain dependency information. '
'This experimental feature is enabled through '
'--use-feature=fast-deps and it is not ready for '
'production.'
)
else:
lazy_wheel = False

return RequirementPreparer(
build_dir=temp_build_dir_path,
src_dir=options.src_dir,
Expand All @@ -229,6 +242,7 @@ def make_requirement_preparer(
finder=finder,
require_hashes=options.require_hashes,
use_user_site=use_user_site,
lazy_wheel=lazy_wheel,
)

@staticmethod
Expand Down Expand Up @@ -259,6 +273,7 @@ def make_resolver(
# "Resolver" class being redefined.
if '2020-resolver' in options.features_enabled:
import pip._internal.resolution.resolvelib.resolver

return pip._internal.resolution.resolvelib.resolver.Resolver(
preparer=preparer,
finder=finder,
Expand All @@ -271,7 +286,6 @@ def make_resolver(
force_reinstall=force_reinstall,
upgrade_strategy=upgrade_strategy,
py_version_info=py_version_info,
lazy_wheel='fast-deps' in options.features_enabled,
)
import pip._internal.resolution.legacy.resolver
return pip._internal.resolution.legacy.resolver.Resolver(
Expand Down
47 changes: 47 additions & 0 deletions src/pip/_internal/operations/prepare.py
Expand Up @@ -9,6 +9,8 @@
import os
import shutil

from pip._vendor.contextlib2 import suppress
from pip._vendor.packaging.utils import canonicalize_name
from pip._vendor.six import PY2

from pip._internal.distributions import (
Expand All @@ -24,6 +26,11 @@
PreviousBuildDirError,
VcsHashUnsupported,
)
from pip._internal.models.wheel import Wheel
from pip._internal.network.lazy_wheel import (
HTTPRangeRequestUnsupported,
dist_from_wheel_url,
)
from pip._internal.utils.filesystem import copy2_fixed
from pip._internal.utils.hashes import MissingHashes
from pip._internal.utils.logging import indent_log
Expand Down Expand Up @@ -329,6 +336,7 @@ def __init__(
finder, # type: PackageFinder
require_hashes, # type: bool
use_user_site, # type: bool
lazy_wheel, # type: bool
):
# type: (...) -> None
super(RequirementPreparer, self).__init__()
Expand Down Expand Up @@ -362,6 +370,9 @@ def __init__(
# Should install in user site-packages?
self.use_user_site = use_user_site

# Should wheels be downloaded lazily?
self.use_lazy_wheel = lazy_wheel

@property
def _download_should_save(self):
# type: () -> bool
Expand Down Expand Up @@ -448,12 +459,48 @@ def _get_linked_req_hashes(self, req):
# showing the user what the hash should be.
return req.hashes(trust_internet=False) or MissingHashes()

def _fetch_metadata(preparer, link):
Copy link
Member

@pradyunsg pradyunsg Aug 4, 2020

Choose a reason for hiding this comment

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

This is probably something we can do in a follow-up, but we should add logging here, to denote which branch we're going through, and introduce early returns to make the flow cleaner.

if not preparer.use_lazy_wheel:
    return None

if preparer.require_hashes:
    logger.debug(...)
    return None

if link.is_file or not link.is_wheel:
    logger.debug(...)
    return None

<all the logic, currently inside if block>

Copy link
Member

Choose a reason for hiding this comment

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

Let's do this in a follow up. @McSinyx wanna pick this up? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I'm glad to.

# type: (Link) -> Optional[Distribution]
"""Fetch metadata, using lazy wheel if possible."""
use_lazy_wheel = preparer.use_lazy_wheel
remote_wheel = link.is_wheel and not link.is_file
if use_lazy_wheel and remote_wheel and not preparer.require_hashes:
wheel = Wheel(link.filename)
name = canonicalize_name(wheel.name)
# If HTTPRangeRequestUnsupported is raised, fallback silently.
with indent_log(), suppress(HTTPRangeRequestUnsupported):
logger.info(
'Obtaining dependency information from %s %s',
name, wheel.version,
)
url = link.url.split('#', 1)[0]
session = preparer.downloader._session
return dist_from_wheel_url(name, url, session)
return None

def prepare_linked_requirement(self, req, parallel_builds=False):
# type: (InstallRequirement, bool) -> Distribution
"""Prepare a requirement to be obtained from req.link."""
assert req.link
link = req.link
self._log_preparing_link(req)
wheel_dist = self._fetch_metadata(link)
if wheel_dist is not None:
req.needs_more_preparation = True
return wheel_dist
return self._prepare_linked_requirement(req, parallel_builds)

def prepare_linked_requirement_more(self, req, parallel_builds=False):
# type: (InstallRequirement, bool) -> None
"""Prepare a linked requirement more, if needed."""
if not req.needs_more_preparation:
return
self._prepare_linked_requirement(req, parallel_builds)

def _prepare_linked_requirement(self, req, parallel_builds):
# type: (InstallRequirement, bool) -> Distribution
assert req.link
link = req.link
if link.is_wheel and self.wheel_download_dir:
# Download wheels to a dedicated dir when doing `pip wheel`.
download_dir = self.wheel_download_dir
Expand Down
3 changes: 3 additions & 0 deletions src/pip/_internal/req/req_install.py
Expand Up @@ -213,6 +213,9 @@ def __init__(
# but after loading this flag should be treated as read only.
self.use_pep517 = use_pep517

# This requirement needs more preparation before it can be built
self.needs_more_preparation = False

def __str__(self):
# type: () -> str
if self.req:
Expand Down
61 changes: 23 additions & 38 deletions src/pip/_internal/resolution/resolvelib/candidates.py
@@ -1,22 +1,17 @@
import logging
import sys

from pip._vendor.contextlib2 import suppress
from pip._vendor.packaging.specifiers import InvalidSpecifier, SpecifierSet
from pip._vendor.packaging.utils import canonicalize_name
from pip._vendor.packaging.version import Version

from pip._internal.exceptions import HashError, MetadataInconsistent
from pip._internal.network.lazy_wheel import (
HTTPRangeRequestUnsupported,
dist_from_wheel_url,
)
from pip._internal.models.wheel import Wheel
from pip._internal.req.constructors import (
install_req_from_editable,
install_req_from_line,
)
from pip._internal.req.req_install import InstallRequirement
from pip._internal.utils.logging import indent_log
from pip._internal.utils.misc import dist_is_editable, normalize_version_info
from pip._internal.utils.packaging import get_requires_python
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
Expand Down Expand Up @@ -147,7 +142,6 @@ def __init__(
self._name = name
self._version = version
self._dist = None # type: Optional[Distribution]
self._prepared = False

def __repr__(self):
# type: () -> str
Expand Down Expand Up @@ -203,12 +197,11 @@ def _prepare_distribution(self):
# type: () -> Distribution
raise NotImplementedError("Override in subclass")

def _check_metadata_consistency(self):
# type: () -> None
def _check_metadata_consistency(self, dist):
# type: (Distribution) -> None
"""Check for consistency of project name and version of dist."""
# TODO: (Longer term) Rather than abort, reject this candidate
# and backtrack. This would need resolvelib support.
dist = self._dist # type: Distribution
name = canonicalize_name(dist.project_name)
if self._name is not None and self._name != name:
raise MetadataInconsistent(self._ireq, "name", dist.project_name)
Expand All @@ -218,45 +211,23 @@ def _check_metadata_consistency(self):

def _prepare(self):
# type: () -> None
if self._prepared:
if self._dist is not None:
return
try:
self._dist = self._prepare_distribution()
dist = self._prepare_distribution()
except HashError as e:
e.req = self._ireq
raise

assert self._dist is not None, "Distribution already installed"
self._check_metadata_consistency()
self._prepared = True

def _fetch_metadata(self):
# type: () -> None
"""Fetch metadata, using lazy wheel if possible."""
preparer = self._factory.preparer
use_lazy_wheel = self._factory.use_lazy_wheel
remote_wheel = self._link.is_wheel and not self._link.is_file
if use_lazy_wheel and remote_wheel and not preparer.require_hashes:
assert self._name is not None
logger.info('Collecting %s', self._ireq.req or self._ireq)
# If HTTPRangeRequestUnsupported is raised, fallback silently.
with indent_log(), suppress(HTTPRangeRequestUnsupported):
logger.info(
'Obtaining dependency information from %s %s',
self._name, self._version,
)
url = self._link.url.split('#', 1)[0]
session = preparer.downloader._session
self._dist = dist_from_wheel_url(self._name, url, session)
self._check_metadata_consistency()
if self._dist is None:
self._prepare()
assert dist is not None, "Distribution already installed"
self._check_metadata_consistency(dist)
self._dist = dist

@property
def dist(self):
# type: () -> Distribution
if self._dist is None:
self._fetch_metadata()
self._prepare()
return self._dist

def _get_requires_python_specifier(self):
Expand Down Expand Up @@ -307,6 +278,20 @@ def __init__(
logger.debug("Using cached wheel link: %s", cache_entry.link)
link = cache_entry.link
ireq = make_install_req_from_link(link, template)
assert ireq.link == link
if ireq.link.is_wheel and not ireq.link.is_file:
wheel = Wheel(ireq.link.filename)
wheel_name = canonicalize_name(wheel.name)
assert name == wheel_name, (
"{!r} != {!r} for wheel".format(name, wheel_name)
)
# Version may not be present for PEP 508 direct URLs
if version is not None:
assert str(version) == wheel.version, (
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
"{!r} != {!r} for wheel {}".format(
version, wheel.version, name
)
)

if (cache_entry is not None and
cache_entry.persistent and
Expand Down
2 changes: 0 additions & 2 deletions src/pip/_internal/resolution/resolvelib/factory.py
Expand Up @@ -82,7 +82,6 @@ def __init__(
ignore_installed, # type: bool
ignore_requires_python, # type: bool
py_version_info=None, # type: Optional[Tuple[int, ...]]
lazy_wheel=False, # type: bool
):
# type: (...) -> None
self._finder = finder
Expand All @@ -93,7 +92,6 @@ def __init__(
self._use_user_site = use_user_site
self._force_reinstall = force_reinstall
self._ignore_requires_python = ignore_requires_python
self.use_lazy_wheel = lazy_wheel

self._link_candidate_cache = {} # type: Cache[LinkCandidate]
self._editable_candidate_cache = {} # type: Cache[EditableCandidate]
Expand Down
13 changes: 3 additions & 10 deletions src/pip/_internal/resolution/resolvelib/resolver.py
Expand Up @@ -49,17 +49,8 @@ def __init__(
force_reinstall, # type: bool
upgrade_strategy, # type: str
py_version_info=None, # type: Optional[Tuple[int, ...]]
lazy_wheel=False, # type: bool
):
super(Resolver, self).__init__()
if lazy_wheel:
logger.warning(
'pip is using lazily downloaded wheels using HTTP '
'range requests to obtain dependency information. '
'This experimental feature is enabled through '
'--use-feature=fast-deps and it is not ready for production.'
)

assert upgrade_strategy in self._allowed_strategies

self.factory = Factory(
Expand All @@ -72,7 +63,6 @@ def __init__(
ignore_installed=ignore_installed,
ignore_requires_python=ignore_requires_python,
py_version_info=py_version_info,
lazy_wheel=lazy_wheel,
)
self.ignore_dependencies = ignore_dependencies
self.upgrade_strategy = upgrade_strategy
Expand Down Expand Up @@ -169,6 +159,9 @@ def resolve(self, root_reqs, check_supported_wheels):

req_set.add_named_requirement(ireq)

for actual_req in req_set.all_requirements:
self.factory.preparer.prepare_linked_requirement_more(actual_req)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, this is fantastic!


return req_set

def get_installation_order(self, req_set):
Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_req.py
Expand Up @@ -89,6 +89,7 @@ def _basic_resolver(self, finder, require_hashes=False):
finder=finder,
require_hashes=require_hashes,
use_user_site=False,
lazy_wheel=False,
)
yield Resolver(
preparer=preparer,
Expand Down