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 final wheel builder copy operation to wheel command #7517

Merged
merged 5 commits into from
Dec 29, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def build_wheels(
should_build_legacy = is_wheel_installed()

# Always build PEP 517 requirements
build_failures = builder.build(
_, build_failures = builder.build(
pep517_requirements,
should_unpack=True,
)
Expand Down
17 changes: 16 additions & 1 deletion src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@

import logging
import os
import shutil

from pip._internal.cache import WheelCache
from pip._internal.cli import cmdoptions
from pip._internal.cli.req_command import RequirementCommand
from pip._internal.exceptions import CommandError, PreviousBuildDirError
from pip._internal.req import RequirementSet
from pip._internal.req.req_tracker import get_requirement_tracker
from pip._internal.utils.misc import ensure_dir
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.wheel_builder import WheelBuilder
Expand Down Expand Up @@ -161,10 +163,23 @@ def run(self, options, args):
build_options=options.build_options or [],
global_options=options.global_options or [],
)
build_failures = wb.build(
build_successes, build_failures = wb.build(
requirement_set.requirements.values(),
should_unpack=False,
)
for req in build_successes:
assert req.link and req.link.is_wheel
assert req.local_file_path
# copy from cache to target directory
try:
ensure_dir(options.wheel_dir)
chrahunt marked this conversation as resolved.
Show resolved Hide resolved
shutil.copy(req.local_file_path, options.wheel_dir)
except OSError as e:
logger.warning(
"Building wheel for %s failed: %s",
req.name, e,
)
build_failures.append(req)
if len(build_failures) != 0:
raise CommandError(
"Failed to build one or more wheels"
Expand Down
44 changes: 11 additions & 33 deletions src/pip/_internal/wheel_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from pip._vendor.pep517.wrappers import Pep517HookCaller

BinaryAllowedPredicate = Callable[[InstallRequirement], bool]
BuildResult = Tuple[List[InstallRequirement], List[InstallRequirement]]

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -318,8 +319,6 @@ def __init__(
self.preparer = preparer
self.wheel_cache = wheel_cache

self._wheel_dir = preparer.wheel_download_dir

self.build_options = build_options or []
self.global_options = global_options or []
self.check_binary_allowed = check_binary_allowed
Expand Down Expand Up @@ -413,31 +412,22 @@ def build(
requirements, # type: Iterable[InstallRequirement]
should_unpack, # type: bool
):
# type: (...) -> List[InstallRequirement]
# type: (...) -> BuildResult
"""Build wheels.

: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 failed to build.
"""
# pip install uses should_unpack=True.
# pip install never provides a _wheel_dir.
# pip wheel uses should_unpack=False.
# pip wheel always provides a _wheel_dir (via the preparer).
assert (
(should_unpack and not self._wheel_dir) or
(not should_unpack and self._wheel_dir)
)

buildset = _collect_buildset(
requirements,
wheel_cache=self.wheel_cache,
check_binary_allowed=self.check_binary_allowed,
need_wheel=not should_unpack,
)
if not buildset:
return []
return [], []

# TODO by @pradyunsg
# Should break up this method into 2 separate methods.
Expand All @@ -449,7 +439,7 @@ def build(
)

with indent_log():
build_success, build_failure = [], []
build_successes, build_failures = [], []
for req, cache_dir in buildset:
wheel_file = self._build_one(req, cache_dir)
if wheel_file:
Expand Down Expand Up @@ -478,32 +468,20 @@ def build(
)
# extract the wheel into the dir
unpack_file(req.link.file_path, req.source_dir)
else:
# copy from cache to target directory
try:
ensure_dir(self._wheel_dir)
shutil.copy(wheel_file, self._wheel_dir)
except OSError as e:
logger.warning(
"Building wheel for %s failed: %s",
req.name, e,
)
build_failure.append(req)
continue
build_success.append(req)
build_successes.append(req)
else:
build_failure.append(req)
build_failures.append(req)

# notify success/failure
if build_success:
if build_successes:
logger.info(
'Successfully built %s',
' '.join([req.name for req in build_success]),
' '.join([req.name for req in build_successes]),
)
if build_failure:
if build_failures:
logger.info(
'Failed to build %s',
' '.join([req.name for req in build_failure]),
' '.join([req.name for req in build_failures]),
)
# Return a list of requirements that failed to build
return build_failure
return build_successes, build_failures
2 changes: 1 addition & 1 deletion tests/unit/test_command_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def check_build_wheels(
"""
def build(reqs, **kwargs):
# Fail the first requirement.
return [reqs[0]]
return ([], [reqs[0]])

builder = Mock()
builder.build.side_effect = build
Expand Down