From 8784fb419a857a797397aa2e1d025f7ce0f52060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Sun, 29 Dec 2019 16:44:06 +0100 Subject: [PATCH 01/18] Make build_wheels return successes too In preparation for moving the unpacking out of WheelBuilder.build --- src/pip/_internal/commands/install.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 0f39b90d8..638c644f0 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -52,7 +52,7 @@ if MYPY_CHECK_RUNNING: from pip._internal.models.format_control import FormatControl from pip._internal.req.req_install import InstallRequirement - from pip._internal.wheel_builder import BinaryAllowedPredicate + from pip._internal.wheel_builder import BinaryAllowedPredicate, BuildResult logger = logging.getLogger(__name__) @@ -79,15 +79,17 @@ def build_wheels( global_options, # type: List[str] check_binary_allowed, # type: BinaryAllowedPredicate ): - # type: (...) -> List[InstallRequirement] + # type: (...) -> BuildResult """ Build wheels for requirements, depending on whether wheel is installed. + + Return failures only for PEP 517 requirements. """ # 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_build_successes, build_failures = builder.build( pep517_requirements, should_unpack=True, wheel_cache=wheel_cache, @@ -100,7 +102,7 @@ def build_wheels( # We don't care about failures building legacy # requirements, as we'll fall through to a direct # install for those. - builder.build( + legacy_build_successes, _ = builder.build( legacy_requirements, should_unpack=True, wheel_cache=wheel_cache, @@ -109,7 +111,7 @@ def build_wheels( check_binary_allowed=check_binary_allowed, ) - return build_failures + return pep517_build_successes + legacy_build_successes, build_failures def get_check_binary_allowed(format_control): @@ -409,7 +411,7 @@ class InstallCommand(RequirementCommand): legacy_requirements.append(req) wheel_builder = WheelBuilder(preparer) - build_failures = build_wheels( + _, build_failures = build_wheels( builder=wheel_builder, pep517_requirements=pep517_requirements, legacy_requirements=legacy_requirements, From 8601bb5d0678ad2a1a1fe91e4b0e3f6c6e028d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Sun, 29 Dec 2019 17:54:18 +0100 Subject: [PATCH 02/18] Make build_wheels return all failures Check for PEP 517 build failures by filtering failures. --- src/pip/_internal/commands/install.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 638c644f0..4b5836f48 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -82,14 +82,12 @@ def build_wheels( # type: (...) -> BuildResult """ Build wheels for requirements, depending on whether wheel is installed. - - Return failures only for PEP 517 requirements. """ # We don't build wheels for legacy requirements if wheel is not installed. should_build_legacy = is_wheel_installed() # Always build PEP 517 requirements - pep517_build_successes, build_failures = builder.build( + pep517_build_successes, pep517_build_failures = builder.build( pep517_requirements, should_unpack=True, wheel_cache=wheel_cache, @@ -102,7 +100,7 @@ def build_wheels( # We don't care about failures building legacy # requirements, as we'll fall through to a direct # install for those. - legacy_build_successes, _ = builder.build( + legacy_build_successes, legacy_build_failures = builder.build( legacy_requirements, should_unpack=True, wheel_cache=wheel_cache, @@ -111,7 +109,10 @@ def build_wheels( check_binary_allowed=check_binary_allowed, ) - return pep517_build_successes + legacy_build_successes, build_failures + return ( + pep517_build_successes + legacy_build_successes, + pep517_build_failures + legacy_build_failures, + ) def get_check_binary_allowed(format_control): @@ -423,11 +424,14 @@ class InstallCommand(RequirementCommand): # If we're using PEP 517, we cannot do a direct install # so we fail here. - if build_failures: + 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 From 10ff58d7beb318aa3931f16bd23d90b345a5ad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Sun, 29 Dec 2019 18:07:49 +0100 Subject: [PATCH 03/18] Simplify install by calling build once We filter what to build beforehand so we can call build once. We then filter failures to detect PEP 517 failures. --- src/pip/_internal/commands/install.py | 28 +++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 4b5836f48..2cbdf3a64 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -402,20 +402,21 @@ 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) + + if is_wheel_installed(): + reqs_to_build = list(requirement_set.requirements.values()) + else: + # We don't build wheels for legacy requirements + # if wheel is not installed. + reqs_to_build = [ + r for r in requirement_set.requirements.values() + if r.use_pep517 + ] wheel_builder = WheelBuilder(preparer) - _, build_failures = build_wheels( - builder=wheel_builder, - pep517_requirements=pep517_requirements, - legacy_requirements=legacy_requirements, + _, build_failures = wheel_builder.build( + reqs_to_build, + should_unpack=True, wheel_cache=wheel_cache, build_options=[], global_options=[], @@ -424,6 +425,9 @@ class InstallCommand(RequirementCommand): # If we're using PEP 517, we cannot do a direct install # so we fail here. + # 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 ] From 4bdca1a09ad93b2dc18a860c8f64558185e2d243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Sun, 29 Dec 2019 18:10:42 +0100 Subject: [PATCH 04/18] Remove now useless build_wheels function --- src/pip/_internal/commands/install.py | 47 +------------------ tests/unit/test_command_install.py | 65 +-------------------------- 2 files changed, 2 insertions(+), 110 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 2cbdf3a64..221d3caf9 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -52,7 +52,7 @@ if MYPY_CHECK_RUNNING: from pip._internal.models.format_control import FormatControl from pip._internal.req.req_install import InstallRequirement - from pip._internal.wheel_builder import BinaryAllowedPredicate, BuildResult + from pip._internal.wheel_builder import BinaryAllowedPredicate logger = logging.getLogger(__name__) @@ -70,51 +70,6 @@ def is_wheel_installed(): 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: (...) -> BuildResult - """ - 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 - pep517_build_successes, pep517_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. - legacy_build_successes, legacy_build_failures = 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 ( - pep517_build_successes + legacy_build_successes, - pep517_build_failures + legacy_build_failures, - ) - - def get_check_binary_allowed(format_control): # type: (FormatControl) -> BinaryAllowedPredicate def check_binary_allowed(req): diff --git a/tests/unit/test_command_install.py b/tests/unit/test_command_install.py index 36b3ca73f..aee03b77a 100644 --- a/tests/unit/test_command_install.py +++ b/tests/unit/test_command_install.py @@ -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') From 3de4765ec7fbd35c56a0f6bae9f761bc70ed4483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 10:04:30 +0100 Subject: [PATCH 05/18] Add should_build function for wheel and install commands --- src/pip/_internal/wheel_builder.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index 9015795b9..d80f982fc 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -82,6 +82,25 @@ def should_build( return True +def should_build_for_wheel_command( + req, # type: InstallRequirement +): + # type: (...) -> Optional[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: (...) -> Optional[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 From e45005f4bbf69110e49534113e331c0364f5c847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 10:08:26 +0100 Subject: [PATCH 06/18] Filter requirements to build beforehand in wheel command One more step so build() becomes only concerned with building. --- src/pip/_internal/commands/wheel.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index e376440b7..13a4a9152 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -18,7 +18,10 @@ 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 ( + WheelBuilder, + should_build_for_wheel_command, +) if MYPY_CHECK_RUNNING: from optparse import Values @@ -160,10 +163,15 @@ 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(), + reqs_to_build, should_unpack=False, wheel_cache=wheel_cache, build_options=options.build_options or [], From 3ae13f7d28f24d63d826197f7b57c67a80ae1be8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Thu, 26 Dec 2019 18:19:06 +0100 Subject: [PATCH 07/18] Move is_wheel_installed to utils --- src/pip/_internal/commands/install.py | 13 +------------ src/pip/_internal/utils/misc.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 221d3caf9..52198bb25 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -38,6 +38,7 @@ from pip._internal.utils.filesystem import test_writable_dir from pip._internal.utils.misc import ( ensure_dir, get_installed_version, + is_wheel_installed, protect_pip_from_modification_on_windows, write_output, ) @@ -58,18 +59,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 get_check_binary_allowed(format_control): # type: (FormatControl) -> BinaryAllowedPredicate def check_binary_allowed(req): diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index a53ce0dc9..19a96290e 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -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 From 8ca8e9bf61f88ac2b7a45a025b04a5387b3a8d5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Thu, 26 Dec 2019 18:24:46 +0100 Subject: [PATCH 08/18] Extend should_build scope wrt legacy requirements We don't build legacy requirements when wheel is not installed because we'll fallback to a legacy install in such case. --- src/pip/_internal/wheel_builder.py | 9 ++++++++- tests/unit/test_wheel_builder.py | 28 +++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index d80f982fc..423d3b726 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -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 @@ -69,6 +69,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 diff --git a/tests/unit/test_wheel_builder.py b/tests/unit/test_wheel_builder.py index 9d2c80303..87250d0b4 100644 --- a/tests/unit/test_wheel_builder.py +++ b/tests/unit/test_wheel_builder.py @@ -1,7 +1,7 @@ import logging import pytest -from mock import Mock +from mock import Mock, 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,6 +47,7 @@ class ReqMock: self.link = link self.constraint = constraint self.source_dir = source_dir + self.use_pep517 = use_pep517 @pytest.mark.parametrize( @@ -83,6 +85,30 @@ def test_should_build(req, need_wheel, disallow_binaries, expected): 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( + legacy_req, + need_wheel=False, + 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( + legacy_req, + need_wheel=False, + check_binary_allowed=lambda req: True, + ) + assert should_build + + @pytest.mark.parametrize( "req, disallow_binaries, expected", [ From 212ee12474bc88deffb90c300074e05c215d37d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 10:20:12 +0100 Subject: [PATCH 09/18] Filter requirements to build beforehand in install command One more step so build() becomes only concerned with building. --- src/pip/_internal/commands/install.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 52198bb25..34f6fc7f9 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -38,14 +38,16 @@ from pip._internal.utils.filesystem import test_writable_dir from pip._internal.utils.misc import ( ensure_dir, get_installed_version, - is_wheel_installed, protect_pip_from_modification_on_windows, write_output, ) 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 ( + WheelBuilder, + should_build_for_install_command, +) if MYPY_CHECK_RUNNING: from optparse import Values @@ -347,15 +349,12 @@ class InstallCommand(RequirementCommand): finder.format_control ) - if is_wheel_installed(): - reqs_to_build = list(requirement_set.requirements.values()) - else: - # We don't build wheels for legacy requirements - # if wheel is not installed. - reqs_to_build = [ - r for r in requirement_set.requirements.values() - if r.use_pep517 - ] + reqs_to_build = [ + r for r in requirement_set.requirements.values() + if should_build_for_install_command( + r, check_binary_allowed + ) + ] wheel_builder = WheelBuilder(preparer) _, build_failures = wheel_builder.build( From 870106b9bb45afde8c69e5cfcd8339de304560b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 10:24:41 +0100 Subject: [PATCH 10/18] Simplify _collect_buildset Since filtering of what to build has been done beforehand, there is no need to filter again here anymore. --- src/pip/_internal/wheel_builder.py | 10 +--------- tests/unit/test_wheel_builder.py | 22 +--------------------- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index 423d3b726..d336f948c 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -152,22 +152,15 @@ def _collect_buildset( requirements, # type: Iterable[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, + """Return the list of InstallRequirement, with the persistent or temporary cache directory where the built wheel needs 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) @@ -312,7 +305,6 @@ class WheelBuilder(object): requirements, wheel_cache=wheel_cache, check_binary_allowed=check_binary_allowed, - need_wheel=not should_unpack, ) if not buildset: return [], [] diff --git a/tests/unit/test_wheel_builder.py b/tests/unit/test_wheel_builder.py index 87250d0b4..14d40111d 100644 --- a/tests/unit/test_wheel_builder.py +++ b/tests/unit/test_wheel_builder.py @@ -1,7 +1,7 @@ import logging import pytest -from mock import Mock, patch +from mock import patch from pip._internal import wheel_builder from pip._internal.models.link import Link @@ -207,23 +207,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 == [] From 6e7d0e5a05c12712f48a858e6e961febd3616ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 10:29:34 +0100 Subject: [PATCH 11/18] _collect_buildset becomes _get_cache_dir The only purpose of _collect_buildset is now to compute the cache directory to use for a given requirements. This is better computed one by one in the build loop. --- src/pip/_internal/wheel_builder.py | 44 +++++++++++++----------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index d336f948c..7cb46e84a 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -148,28 +148,24 @@ def should_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 ): - # type: (...) -> List[Tuple[InstallRequirement, str]] - """Return the list of InstallRequirement, - 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 ( - 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(_): @@ -301,12 +297,7 @@ class WheelBuilder(object): # Binaries allowed by default. check_binary_allowed = _always_true - buildset = _collect_buildset( - requirements, - wheel_cache=wheel_cache, - check_binary_allowed=check_binary_allowed, - ) - if not buildset: + if not requirements: return [], [] # TODO by @pradyunsg @@ -315,12 +306,15 @@ class WheelBuilder(object): # Build the wheels. logger.info( 'Building wheels for collected packages: %s', - ', '.join([req.name for (req, _) in buildset]), + ', '.join(req.name for req in requirements), ) with indent_log(): build_successes, build_failures = [], [] - for req, cache_dir in buildset: + 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 ) From 25521f29b5945b646fad76f8741f1e967a55d871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 10:38:58 +0100 Subject: [PATCH 12/18] should_build always returns a boolean --- src/pip/_internal/wheel_builder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index 7cb46e84a..0c676455f 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -53,7 +53,7 @@ def should_build( 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 @@ -92,7 +92,7 @@ def should_build( def should_build_for_wheel_command( req, # type: InstallRequirement ): - # type: (...) -> Optional[bool] + # type: (...) -> bool return should_build( req, need_wheel=True, check_binary_allowed=_always_true ) @@ -102,7 +102,7 @@ def should_build_for_install_command( req, # type: InstallRequirement check_binary_allowed, # type: BinaryAllowedPredicate ): - # type: (...) -> Optional[bool] + # type: (...) -> bool return should_build( req, need_wheel=False, check_binary_allowed=check_binary_allowed ) From 68179ea0ded8a8ca466518dbaf064d8ca434f19d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Mon, 30 Dec 2019 19:31:41 +0100 Subject: [PATCH 13/18] Add .trivial newsfile --- news/7527.trivial | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 news/7527.trivial diff --git a/news/7527.trivial b/news/7527.trivial new file mode 100644 index 000000000..e69de29bb From c0aca1212376e20bb1c022a3a33aabf89d489304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Tue, 31 Dec 2019 05:22:59 +0100 Subject: [PATCH 14/18] Clarify should_cache a bit --- src/pip/_internal/wheel_builder.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index 0c676455f..5899da2f0 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -118,17 +118,16 @@ def should_cache( 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) @@ -137,14 +136,11 @@ 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 From 9729273ca82e97b4c90719be44b094f1cddb2915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Wed, 1 Jan 2020 12:42:22 +0100 Subject: [PATCH 15/18] Make should_build and should_cache "private" --- src/pip/_internal/wheel_builder.py | 12 ++++++------ tests/unit/test_wheel_builder.py | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index 5899da2f0..c29f17bee 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -48,7 +48,7 @@ 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 @@ -93,7 +93,7 @@ def should_build_for_wheel_command( req, # type: InstallRequirement ): # type: (...) -> bool - return should_build( + return _should_build( req, need_wheel=True, check_binary_allowed=_always_true ) @@ -103,19 +103,19 @@ def should_build_for_install_command( check_binary_allowed, # type: BinaryAllowedPredicate ): # type: (...) -> bool - return should_build( + return _should_build( req, need_wheel=False, check_binary_allowed=check_binary_allowed ) -def should_cache( +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_for_install_command( @@ -156,7 +156,7 @@ def _get_cache_dir( cache_available = bool(wheel_cache.cache_dir) if ( cache_available and - should_cache(req, check_binary_allowed) + _should_cache(req, check_binary_allowed) ): cache_dir = wheel_cache.get_path_for_link(req.link) else: diff --git a/tests/unit/test_wheel_builder.py b/tests/unit/test_wheel_builder.py index 14d40111d..fd788fbc0 100644 --- a/tests/unit/test_wheel_builder.py +++ b/tests/unit/test_wheel_builder.py @@ -77,7 +77,7 @@ class ReqMock: ], ) def test_should_build(req, need_wheel, disallow_binaries, expected): - should_build = wheel_builder.should_build( + should_build = wheel_builder._should_build( req, need_wheel, check_binary_allowed=lambda req: not disallow_binaries, @@ -89,7 +89,7 @@ def test_should_build(req, need_wheel, disallow_binaries, expected): 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( + should_build = wheel_builder._should_build( legacy_req, need_wheel=False, check_binary_allowed=lambda req: True, @@ -101,7 +101,7 @@ def test_should_build_legacy_wheel_not_installed(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( + should_build = wheel_builder._should_build( legacy_req, need_wheel=False, check_binary_allowed=lambda req: True, @@ -130,10 +130,10 @@ 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( + if not wheel_builder._should_build( req, need_wheel=False, check_binary_allowed=check_binary_allowed ): # never cache if pip install (need_wheel=False) would not have built) @@ -150,14 +150,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, ) From f04e6ab7b5a8a47a5d109663d01c57b2684082e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Wed, 1 Jan 2020 15:53:46 +0100 Subject: [PATCH 16/18] Split should_build test Separately test should_build_for_wheel_command and should_buid_for_install_command. The tests are simpler and this open the door for reasoning about both functions independently. --- tests/unit/test_wheel_builder.py | 61 +++++++++++++++++--------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/tests/unit/test_wheel_builder.py b/tests/unit/test_wheel_builder.py index fd788fbc0..7d70e8dcb 100644 --- a/tests/unit/test_wheel_builder.py +++ b/tests/unit/test_wheel_builder.py @@ -51,47 +51,51 @@ class ReqMock: @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( + should_build = wheel_builder.should_build_for_install_command( legacy_req, - need_wheel=False, check_binary_allowed=lambda req: True, ) assert not should_build @@ -101,9 +105,8 @@ def test_should_build_legacy_wheel_not_installed(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( + should_build = wheel_builder.should_build_for_install_command( legacy_req, - need_wheel=False, check_binary_allowed=lambda req: True, ) assert should_build @@ -133,10 +136,10 @@ def test_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 From 66e010980a11cfff417a29a339cae35dde2576ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Fri, 3 Jan 2020 12:46:01 +0100 Subject: [PATCH 17/18] Remove WheelBuilder build is now a function --- src/pip/_internal/commands/install.py | 8 +- src/pip/_internal/commands/wheel.py | 8 +- src/pip/_internal/wheel_builder.py | 126 ++++++++++++-------------- 3 files changed, 60 insertions(+), 82 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 34f6fc7f9..7784d86e5 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -44,10 +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, - should_build_for_install_command, -) +from pip._internal.wheel_builder import build, should_build_for_install_command if MYPY_CHECK_RUNNING: from optparse import Values @@ -356,8 +353,7 @@ class InstallCommand(RequirementCommand): ) ] - wheel_builder = WheelBuilder(preparer) - _, build_failures = wheel_builder.build( + _, build_failures = build( reqs_to_build, should_unpack=True, wheel_cache=wheel_cache, diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 13a4a9152..849f5842c 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -18,10 +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, - should_build_for_wheel_command, -) +from pip._internal.wheel_builder import build, should_build_for_wheel_command if MYPY_CHECK_RUNNING: from optparse import Values @@ -169,8 +166,7 @@ class WheelCommand(RequirementCommand): ] # build wheels - wb = WheelBuilder(preparer) - build_successes, build_failures = wb.build( + build_successes, build_failures = build( reqs_to_build, should_unpack=False, wheel_cache=wheel_cache, diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index c29f17bee..e644014e0 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -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] @@ -261,78 +258,67 @@ def _clean_one_legacy(req, global_options): return False -class WheelBuilder(object): - """Build wheels from a RequirementSet.""" +def build( + 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. - def __init__( - self, - preparer, # type: RequirementPreparer - ): - # type: (...) -> None - self.preparer = preparer + :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 - 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 + # TODO by @pradyunsg + # Should break up this method into 2 separate methods. - if not requirements: - return [], [] + # Build the wheels. + logger.info( + 'Building wheels for collected packages: %s', + ', '.join(req.name for req in requirements), + ) - # TODO by @pradyunsg - # Should break up this method into 2 separate methods. + 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) - # Build the wheels. + # notify success/failure + if build_successes: logger.info( - 'Building wheels for collected packages: %s', - ', '.join(req.name for req in requirements), + 'Successfully built %s', + ' '.join([req.name for req in build_successes]), ) - - 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) - - # 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 From 5aa3b3d2f02dc7e66f3b09a7c1e0ad7565fe466b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Fri, 3 Jan 2020 12:49:08 +0100 Subject: [PATCH 18/18] Remove unused build() argument --- src/pip/_internal/commands/install.py | 1 - src/pip/_internal/commands/wheel.py | 1 - src/pip/_internal/wheel_builder.py | 7 ------- 3 files changed, 9 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 7784d86e5..82f94213e 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -355,7 +355,6 @@ class InstallCommand(RequirementCommand): _, build_failures = build( reqs_to_build, - should_unpack=True, wheel_cache=wheel_cache, build_options=[], global_options=[], diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 849f5842c..eb44bcee4 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -168,7 +168,6 @@ class WheelCommand(RequirementCommand): # build wheels build_successes, build_failures = build( reqs_to_build, - should_unpack=False, wheel_cache=wheel_cache, build_options=options.build_options or [], global_options=options.global_options or [], diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index e644014e0..5940e4ad9 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -260,7 +260,6 @@ def _clean_one_legacy(req, global_options): def build( requirements, # type: Iterable[InstallRequirement] - should_unpack, # type: bool wheel_cache, # type: WheelCache build_options, # type: List[str] global_options, # type: List[str] @@ -269,9 +268,6 @@ def build( # 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 succeeded to build and the list of InstallRequirement that failed to build. """ @@ -282,9 +278,6 @@ def build( if not requirements: return [], [] - # TODO by @pradyunsg - # Should break up this method into 2 separate methods. - # Build the wheels. logger.info( 'Building wheels for collected packages: %s',