Move should_build filtering to wheel and install commands (#7527)

This commit is contained in:
Pradyun Gedam 2020-01-06 14:25:40 +00:00 committed by GitHub
commit 5e1455561e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 198 additions and 302 deletions

0
news/7527.trivial Normal file
View File

View File

@ -44,7 +44,7 @@ from pip._internal.utils.misc import (
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.virtualenv import virtualenv_no_global
from pip._internal.wheel_builder import WheelBuilder
from pip._internal.wheel_builder import build, should_build_for_install_command
if MYPY_CHECK_RUNNING:
from optparse import Values
@ -58,60 +58,6 @@ if MYPY_CHECK_RUNNING:
logger = logging.getLogger(__name__)
def is_wheel_installed():
"""
Return whether the wheel package is installed.
"""
try:
import wheel # noqa: F401
except ImportError:
return False
return True
def build_wheels(
builder, # type: WheelBuilder
pep517_requirements, # type: List[InstallRequirement]
legacy_requirements, # type: List[InstallRequirement]
wheel_cache, # type: WheelCache
build_options, # type: List[str]
global_options, # type: List[str]
check_binary_allowed, # type: BinaryAllowedPredicate
):
# type: (...) -> List[InstallRequirement]
"""
Build wheels for requirements, depending on whether wheel is installed.
"""
# We don't build wheels for legacy requirements if wheel is not installed.
should_build_legacy = is_wheel_installed()
# Always build PEP 517 requirements
_, build_failures = builder.build(
pep517_requirements,
should_unpack=True,
wheel_cache=wheel_cache,
build_options=build_options,
global_options=global_options,
check_binary_allowed=check_binary_allowed,
)
if should_build_legacy:
# We don't care about failures building legacy
# requirements, as we'll fall through to a direct
# install for those.
builder.build(
legacy_requirements,
should_unpack=True,
wheel_cache=wheel_cache,
build_options=build_options,
global_options=global_options,
check_binary_allowed=check_binary_allowed,
)
return build_failures
def get_check_binary_allowed(format_control):
# type: (FormatControl) -> BinaryAllowedPredicate
def check_binary_allowed(req):
@ -399,20 +345,16 @@ class InstallCommand(RequirementCommand):
check_binary_allowed = get_check_binary_allowed(
finder.format_control
)
# Consider legacy and PEP517-using requirements separately
legacy_requirements = []
pep517_requirements = []
for req in requirement_set.requirements.values():
if req.use_pep517:
pep517_requirements.append(req)
else:
legacy_requirements.append(req)
wheel_builder = WheelBuilder(preparer)
build_failures = build_wheels(
builder=wheel_builder,
pep517_requirements=pep517_requirements,
legacy_requirements=legacy_requirements,
reqs_to_build = [
r for r in requirement_set.requirements.values()
if should_build_for_install_command(
r, check_binary_allowed
)
]
_, build_failures = build(
reqs_to_build,
wheel_cache=wheel_cache,
build_options=[],
global_options=[],
@ -421,11 +363,17 @@ class InstallCommand(RequirementCommand):
# If we're using PEP 517, we cannot do a direct install
# so we fail here.
if build_failures:
# We don't care about failures building legacy
# requirements, as we'll fall through to a direct
# install for those.
pep517_build_failures = [
r for r in build_failures if r.use_pep517
]
if pep517_build_failures:
raise InstallationError(
"Could not build wheels for {} which use"
" PEP 517 and cannot be installed directly".format(
", ".join(r.name for r in build_failures)))
", ".join(r.name for r in pep517_build_failures)))
to_install = resolver.get_installation_order(
requirement_set

View File

@ -18,7 +18,7 @@ from pip._internal.req.req_tracker import get_requirement_tracker
from pip._internal.utils.misc import ensure_dir, normalize_path
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.wheel_builder import WheelBuilder
from pip._internal.wheel_builder import build, should_build_for_wheel_command
if MYPY_CHECK_RUNNING:
from optparse import Values
@ -160,11 +160,14 @@ class WheelCommand(RequirementCommand):
resolver.resolve(requirement_set)
reqs_to_build = [
r for r in requirement_set.requirements.values()
if should_build_for_wheel_command(r)
]
# build wheels
wb = WheelBuilder(preparer)
build_successes, build_failures = wb.build(
requirement_set.requirements.values(),
should_unpack=False,
build_successes, build_failures = build(
reqs_to_build,
wheel_cache=wheel_cache,
build_options=options.build_options or [],
global_options=options.global_options or [],

View File

@ -878,3 +878,15 @@ def hash_file(path, blocksize=1 << 20):
length += len(block)
h.update(block)
return h, length
def is_wheel_installed():
"""
Return whether the wheel package is installed.
"""
try:
import wheel # noqa: F401
except ImportError:
return False
return True

View File

@ -13,7 +13,7 @@ from pip._internal.models.link import Link
from pip._internal.operations.build.wheel import build_wheel_pep517
from pip._internal.operations.build.wheel_legacy import build_wheel_legacy
from pip._internal.utils.logging import indent_log
from pip._internal.utils.misc import ensure_dir, hash_file
from pip._internal.utils.misc import ensure_dir, hash_file, is_wheel_installed
from pip._internal.utils.setuptools_build import make_setuptools_clean_args
from pip._internal.utils.subprocess import call_subprocess
from pip._internal.utils.temp_dir import TempDirectory
@ -27,9 +27,6 @@ if MYPY_CHECK_RUNNING:
)
from pip._internal.cache import WheelCache
from pip._internal.operations.prepare import (
RequirementPreparer
)
from pip._internal.req.req_install import InstallRequirement
BinaryAllowedPredicate = Callable[[InstallRequirement], bool]
@ -48,12 +45,12 @@ def _contains_egg_info(
return bool(_egg_info_re.search(s))
def should_build(
def _should_build(
req, # type: InstallRequirement
need_wheel, # type: bool
check_binary_allowed, # type: BinaryAllowedPredicate
):
# type: (...) -> Optional[bool]
# type: (...) -> bool
"""Return whether an InstallRequirement should be built into a wheel."""
if req.constraint:
# never build requirements that are merely constraints
@ -69,6 +66,13 @@ def should_build(
# i.e. pip wheel, not pip install
return True
# From this point, this concerns the pip install command only
# (need_wheel=False).
if not req.use_pep517 and not is_wheel_installed():
# we don't build legacy requirements if wheel is not installed
return False
if req.editable or not req.source_dir:
return False
@ -82,27 +86,45 @@ def should_build(
return True
def should_cache(
def should_build_for_wheel_command(
req, # type: InstallRequirement
):
# type: (...) -> bool
return _should_build(
req, need_wheel=True, check_binary_allowed=_always_true
)
def should_build_for_install_command(
req, # type: InstallRequirement
check_binary_allowed, # type: BinaryAllowedPredicate
):
# type: (...) -> bool
return _should_build(
req, need_wheel=False, check_binary_allowed=check_binary_allowed
)
def _should_cache(
req, # type: InstallRequirement
check_binary_allowed, # type: BinaryAllowedPredicate
):
# type: (...) -> Optional[bool]
"""
Return whether a built InstallRequirement can be stored in the persistent
wheel cache, assuming the wheel cache is available, and should_build()
wheel cache, assuming the wheel cache is available, and _should_build()
has determined a wheel needs to be built.
"""
if not should_build(
req, need_wheel=False, check_binary_allowed=check_binary_allowed
if not should_build_for_install_command(
req, check_binary_allowed=check_binary_allowed
):
# never cache if pip install (need_wheel=False) would not have built
# never cache if pip install would not have built
# (editable mode, etc)
return False
if req.link and req.link.is_vcs:
# VCS checkout. Build wheel just for this run
# unless it points to an immutable commit hash in which
# case it can be cached.
# VCS checkout. Do not cache
# unless it points to an immutable commit hash.
assert not req.editable
assert req.source_dir
vcs_backend = vcs.get_backend_for_scheme(req.link.scheme)
@ -111,46 +133,32 @@ def should_cache(
return True
return False
link = req.link
base, ext = link.splitext()
base, ext = req.link.splitext()
if _contains_egg_info(base):
return True
# Otherwise, build the wheel just for this run using the ephemeral
# cache since we are either in the case of e.g. a local directory, or
# no cache directory is available to use.
# Otherwise, do not cache.
return False
def _collect_buildset(
requirements, # type: Iterable[InstallRequirement]
def _get_cache_dir(
req, # type: InstallRequirement
wheel_cache, # type: WheelCache
check_binary_allowed, # type: BinaryAllowedPredicate
need_wheel, # type: bool
):
# type: (...) -> List[Tuple[InstallRequirement, str]]
"""Return the list of InstallRequirement that need to be built,
with the persistent or temporary cache directory where the built
wheel needs to be stored.
# type: (...) -> str
"""Return the persistent or temporary cache directory where the built
wheel need to be stored.
"""
buildset = []
cache_available = bool(wheel_cache.cache_dir)
for req in requirements:
if not should_build(
req,
need_wheel=need_wheel,
check_binary_allowed=check_binary_allowed,
):
continue
if (
cache_available and
should_cache(req, check_binary_allowed)
):
cache_dir = wheel_cache.get_path_for_link(req.link)
else:
cache_dir = wheel_cache.get_ephem_path_for_link(req.link)
buildset.append((req, cache_dir))
return buildset
if (
cache_available and
_should_cache(req, check_binary_allowed)
):
cache_dir = wheel_cache.get_path_for_link(req.link)
else:
cache_dir = wheel_cache.get_ephem_path_for_link(req.link)
return cache_dir
def _always_true(_):
@ -250,81 +258,60 @@ def _clean_one_legacy(req, global_options):
return False
class WheelBuilder(object):
"""Build wheels from a RequirementSet."""
def build(
requirements, # type: Iterable[InstallRequirement]
wheel_cache, # type: WheelCache
build_options, # type: List[str]
global_options, # type: List[str]
check_binary_allowed=None, # type: Optional[BinaryAllowedPredicate]
):
# type: (...) -> BuildResult
"""Build wheels.
def __init__(
self,
preparer, # type: RequirementPreparer
):
# type: (...) -> None
self.preparer = preparer
:return: The list of InstallRequirement that succeeded to build and
the list of InstallRequirement that failed to build.
"""
if check_binary_allowed is None:
# Binaries allowed by default.
check_binary_allowed = _always_true
def build(
self,
requirements, # type: Iterable[InstallRequirement]
should_unpack, # type: bool
wheel_cache, # type: WheelCache
build_options, # type: List[str]
global_options, # type: List[str]
check_binary_allowed=None, # type: Optional[BinaryAllowedPredicate]
):
# type: (...) -> BuildResult
"""Build wheels.
if not requirements:
return [], []
:param should_unpack: If True, after building the wheel, unpack it
and replace the sdist with the unpacked version in preparation
for installation.
:return: The list of InstallRequirement that succeeded to build and
the list of InstallRequirement that failed to build.
"""
if check_binary_allowed is None:
# Binaries allowed by default.
check_binary_allowed = _always_true
# Build the wheels.
logger.info(
'Building wheels for collected packages: %s',
', '.join(req.name for req in requirements),
)
buildset = _collect_buildset(
requirements,
wheel_cache=wheel_cache,
check_binary_allowed=check_binary_allowed,
need_wheel=not should_unpack,
)
if not buildset:
return [], []
with indent_log():
build_successes, build_failures = [], []
for req in requirements:
cache_dir = _get_cache_dir(
req, wheel_cache, check_binary_allowed
)
wheel_file = _build_one(
req, cache_dir, build_options, global_options
)
if wheel_file:
# Update the link for this.
req.link = Link(path_to_url(wheel_file))
req.local_file_path = req.link.file_path
assert req.link.is_wheel
build_successes.append(req)
else:
build_failures.append(req)
# TODO by @pradyunsg
# Should break up this method into 2 separate methods.
# Build the wheels.
# notify success/failure
if build_successes:
logger.info(
'Building wheels for collected packages: %s',
', '.join([req.name for (req, _) in buildset]),
'Successfully built %s',
' '.join([req.name for req in build_successes]),
)
with indent_log():
build_successes, build_failures = [], []
for req, cache_dir in buildset:
wheel_file = _build_one(
req, cache_dir, build_options, global_options
)
if wheel_file:
# Update the link for this.
req.link = Link(path_to_url(wheel_file))
req.local_file_path = req.link.file_path
assert req.link.is_wheel
build_successes.append(req)
else:
build_failures.append(req)
# notify success/failure
if build_successes:
logger.info(
'Successfully built %s',
' '.join([req.name for req in build_successes]),
)
if build_failures:
logger.info(
'Failed to build %s',
' '.join([req.name for req in build_failures]),
)
# Return a list of requirements that failed to build
return build_successes, build_failures
if build_failures:
logger.info(
'Failed to build %s',
' '.join([req.name for req in build_failures]),
)
# Return a list of requirements that failed to build
return build_successes, build_failures

View File

@ -1,11 +1,10 @@
import errno
import pytest
from mock import Mock, patch
from mock import patch
from pip._vendor.packaging.requirements import Requirement
from pip._internal.commands.install import (
build_wheels,
create_env_error_message,
decide_user_install,
warn_deprecated_install_options,
@ -14,68 +13,6 @@ from pip._internal.req.req_install import InstallRequirement
from pip._internal.req.req_set import RequirementSet
class TestWheelCache:
def check_build_wheels(
self,
pep517_requirements,
legacy_requirements,
):
"""
Return: (mock_calls, return_value).
"""
built_reqs = []
def build(reqs, **kwargs):
# Fail the first requirement.
built_reqs.append(reqs)
return ([], [reqs[0]])
builder = Mock()
builder.build.side_effect = build
build_failures = build_wheels(
builder=builder,
pep517_requirements=pep517_requirements,
legacy_requirements=legacy_requirements,
wheel_cache=Mock(cache_dir=None),
build_options=[],
global_options=[],
check_binary_allowed=None,
)
return (built_reqs, build_failures)
@patch('pip._internal.commands.install.is_wheel_installed')
def test_build_wheels__wheel_installed(self, is_wheel_installed):
is_wheel_installed.return_value = True
built_reqs, build_failures = self.check_build_wheels(
pep517_requirements=['a', 'b'],
legacy_requirements=['c', 'd'],
)
# Legacy requirements were built.
assert built_reqs == [['a', 'b'], ['c', 'd']]
# Legacy build failures are not included in the return value.
assert build_failures == ['a']
@patch('pip._internal.commands.install.is_wheel_installed')
def test_build_wheels__wheel_not_installed(self, is_wheel_installed):
is_wheel_installed.return_value = False
built_reqs, build_failures = self.check_build_wheels(
pep517_requirements=['a', 'b'],
legacy_requirements=['c', 'd'],
)
# Legacy requirements were not built.
assert built_reqs == [['a', 'b']]
assert build_failures == ['a']
class TestDecideUserInstall:
@patch('site.ENABLE_USER_SITE', True)
@patch('pip._internal.commands.install.site_packages_writable')

View File

@ -1,7 +1,7 @@
import logging
import pytest
from mock import Mock
from mock import patch
from pip._internal import wheel_builder
from pip._internal.models.link import Link
@ -39,6 +39,7 @@ class ReqMock:
link=None,
constraint=False,
source_dir="/tmp/pip-install-123/pendulum",
use_pep517=True,
):
self.name = name
self.is_wheel = is_wheel
@ -46,43 +47,71 @@ class ReqMock:
self.link = link
self.constraint = constraint
self.source_dir = source_dir
self.use_pep517 = use_pep517
@pytest.mark.parametrize(
"req, need_wheel, disallow_binaries, expected",
"req, disallow_binaries, expected",
[
# pip wheel (need_wheel=True)
(ReqMock(), True, False, True),
(ReqMock(), True, True, True),
(ReqMock(constraint=True), True, False, False),
(ReqMock(is_wheel=True), True, False, False),
(ReqMock(editable=True), True, False, True),
(ReqMock(source_dir=None), True, False, True),
(ReqMock(link=Link("git+https://g.c/org/repo")), True, False, True),
(ReqMock(link=Link("git+https://g.c/org/repo")), True, True, True),
# pip install (need_wheel=False)
(ReqMock(), False, False, True),
(ReqMock(), False, True, False),
(ReqMock(constraint=True), False, False, False),
(ReqMock(is_wheel=True), False, False, False),
(ReqMock(editable=True), False, False, False),
(ReqMock(source_dir=None), False, False, False),
(ReqMock(), False, True),
(ReqMock(), True, False),
(ReqMock(constraint=True), False, False),
(ReqMock(is_wheel=True), False, False),
(ReqMock(editable=True), False, False),
(ReqMock(source_dir=None), False, False),
# By default (i.e. when binaries are allowed), VCS requirements
# should be built in install mode.
(ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True),
(ReqMock(link=Link("git+https://g.c/org/repo")), False, True),
# Disallowing binaries, however, should cause them not to be built.
(ReqMock(link=Link("git+https://g.c/org/repo")), False, True, False),
(ReqMock(link=Link("git+https://g.c/org/repo")), True, False),
],
)
def test_should_build(req, need_wheel, disallow_binaries, expected):
should_build = wheel_builder.should_build(
def test_should_build_for_install_command(req, disallow_binaries, expected):
should_build = wheel_builder.should_build_for_install_command(
req,
need_wheel,
check_binary_allowed=lambda req: not disallow_binaries,
)
assert should_build is expected
@pytest.mark.parametrize(
"req, expected",
[
(ReqMock(), True),
(ReqMock(constraint=True), False),
(ReqMock(is_wheel=True), False),
(ReqMock(editable=True), True),
(ReqMock(source_dir=None), True),
(ReqMock(link=Link("git+https://g.c/org/repo")), True),
],
)
def test_should_build_for_wheel_command(req, expected):
should_build = wheel_builder.should_build_for_wheel_command(req)
assert should_build is expected
@patch("pip._internal.wheel_builder.is_wheel_installed")
def test_should_build_legacy_wheel_not_installed(is_wheel_installed):
is_wheel_installed.return_value = False
legacy_req = ReqMock(use_pep517=False)
should_build = wheel_builder.should_build_for_install_command(
legacy_req,
check_binary_allowed=lambda req: True,
)
assert not should_build
@patch("pip._internal.wheel_builder.is_wheel_installed")
def test_should_build_legacy_wheel_installed(is_wheel_installed):
is_wheel_installed.return_value = True
legacy_req = ReqMock(use_pep517=False)
should_build = wheel_builder.should_build_for_install_command(
legacy_req,
check_binary_allowed=lambda req: True,
)
assert should_build
@pytest.mark.parametrize(
"req, disallow_binaries, expected",
[
@ -104,13 +133,13 @@ def test_should_cache(
def check_binary_allowed(req):
return not disallow_binaries
should_cache = wheel_builder.should_cache(
should_cache = wheel_builder._should_cache(
req, check_binary_allowed
)
if not wheel_builder.should_build(
req, need_wheel=False, check_binary_allowed=check_binary_allowed
if not wheel_builder.should_build_for_install_command(
req, check_binary_allowed=check_binary_allowed
):
# never cache if pip install (need_wheel=False) would not have built)
# never cache if pip install would not have built)
assert not should_cache
assert should_cache is expected
@ -124,14 +153,14 @@ def test_should_cache_git_sha(script, tmpdir):
# a link referencing a sha should be cached
url = "git+https://g.c/o/r@" + commit + "#egg=mypkg"
req = ReqMock(link=Link(url), source_dir=repo_path)
assert wheel_builder.should_cache(
assert wheel_builder._should_cache(
req, check_binary_allowed=lambda r: True,
)
# a link not referencing a sha should not be cached
url = "git+https://g.c/o/r@master#egg=mypkg"
req = ReqMock(link=Link(url), source_dir=repo_path)
assert not wheel_builder.should_cache(
assert not wheel_builder._should_cache(
req, check_binary_allowed=lambda r: True,
)
@ -181,23 +210,3 @@ def test_format_command_result__empty_output(caplog, log_level):
"Command arguments: arg1 arg2",
'Command output: None',
]
class TestWheelBuilder(object):
def test_skip_building_wheels(self, caplog):
wb = wheel_builder.WheelBuilder(preparer=Mock())
wb._build_one = mock_build_one = Mock()
wheel_req = Mock(is_wheel=True, editable=False, constraint=False)
with caplog.at_level(logging.INFO):
wb.build(
[wheel_req],
should_unpack=False,
wheel_cache=Mock(cache_dir=None),
build_options=[],
global_options=[],
)
assert "due to already being wheel" in caplog.text
assert mock_build_one.mock_calls == []