diff --git a/news/7517.feature b/news/7517.feature new file mode 100644 index 00000000000..089fbc38781 --- /dev/null +++ b/news/7517.feature @@ -0,0 +1,4 @@ +The build step of ``pip wheel`` now builds all wheels to a cache first, +then copies them to the wheel directory all at once. +Before, it built them to a temporary direcory and moved +them to the wheel directory one by one. diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index c7dcf28df8a..747d243537d 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -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, ) diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 8b3ddd0dccf..d3fe3430886 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -7,6 +7,7 @@ import logging import os +import shutil from pip._internal.cache import WheelCache from pip._internal.cli import cmdoptions @@ -14,6 +15,7 @@ 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 @@ -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) + 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" diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index 627cba30496..200e70fc986 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -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__) @@ -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 @@ -413,23 +412,15 @@ 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. + :return: The list of InstallRequirement that succeeded to build and + 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, @@ -437,7 +428,7 @@ def build( need_wheel=not should_unpack, ) if not buildset: - return [] + return [], [] # TODO by @pradyunsg # Should break up this method into 2 separate methods. @@ -449,7 +440,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: @@ -478,32 +469,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 diff --git a/tests/unit/test_command_install.py b/tests/unit/test_command_install.py index 7f7154c94f1..9d862792bba 100644 --- a/tests/unit/test_command_install.py +++ b/tests/unit/test_command_install.py @@ -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