diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index bc7f86eed..077a985ae 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -136,11 +136,7 @@ class IsSDist(DistAbstraction): # 2. Set up the build environment self.req.load_pyproject_toml() - - should_isolate = ( - (self.req.use_pep517 or self.req.pyproject_requires) and - build_isolation - ) + should_isolate = self.req.use_pep517 and build_isolation if should_isolate: # Isolate in a BuildEnvironment and install the build-time @@ -167,12 +163,10 @@ class IsSDist(DistAbstraction): " and ".join(map(repr, sorted(missing))) ) - if self.req.use_pep517: - # If we're using PEP 517, then install any extra build - # dependencies that the backend requested. This must be - # done in a second pass, as the pyproject.toml dependencies - # must be installed before we can call the backend. - self.install_backend_dependencies(finder=finder) + # Install any extra build dependencies that the backend requests. + # This must be done in a second pass, as the pyproject.toml + # dependencies must be installed before we can call the backend. + self.install_backend_dependencies(finder=finder) self.req.prepare_metadata() self.req.assert_source_matches_version() diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index 13a8f35a9..8a873a230 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -12,8 +12,6 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: from typing import Any, Dict, List, Optional, Tuple - Pep517Data = Tuple[str, List[str]] - def _is_list_of_str(obj): # type: (Any) -> bool @@ -66,37 +64,6 @@ def make_editable_error(req_name, reason): return InstallationError(message) -def get_build_system_requires(build_system, req_name): - if build_system is None: - return None - - # Ensure that the build-system section in pyproject.toml conforms - # to PEP 518. - error_template = ( - "{package} has a pyproject.toml file that does not comply " - "with PEP 518: {reason}" - ) - - # Specifying the build-system table but not the requires key is invalid - if "requires" not in build_system: - raise InstallationError( - error_template.format(package=req_name, reason=( - "it has a 'build-system' table but not " - "'build-system.requires' which is mandatory in the table" - )) - ) - - # Error out if requires is not a list of strings - requires = build_system["requires"] - if not _is_list_of_str(requires): - raise InstallationError(error_template.format( - package=req_name, - reason="'build-system.requires' is not a list of strings.", - )) - - return requires - - def resolve_pyproject_toml( build_system, # type: Optional[Dict[str, Any]] has_pyproject, # type: bool @@ -105,7 +72,7 @@ def resolve_pyproject_toml( editable, # type: bool req_name, # type: str ): - # type: (...) -> Tuple[Optional[List[str]], Optional[Pep517Data]] + # type: (...) -> Optional[Tuple[List[str], str, List[str]]] """ Return how a pyproject.toml file's contents should be interpreted. @@ -119,13 +86,6 @@ def resolve_pyproject_toml( :param editable: whether editable mode was requested for the requirement. :param req_name: the name of the requirement we're processing (for error reporting). - - :return: a tuple (requires, pep517_data), where `requires` is the list - of build requirements from pyproject.toml (or else None). The value - `pep517_data` is None if `use_pep517` is False. Otherwise, it is the - tuple (backend, check), where `backend` is the name of the PEP 517 - backend and `check` is the list of requirements we should check are - installed after setting up the build environment. """ # The following cases must use PEP 517 # We check for use_pep517 being non-None and falsey because that means @@ -166,34 +126,19 @@ def resolve_pyproject_toml( req_name, 'PEP 517 processing was explicitly requested' ) - # If we haven't worked out whether to use PEP 517 yet, and the user - # hasn't explicitly stated a preference, we do so if the project has - # a pyproject.toml file (provided editable mode wasn't requested). + # If we haven't worked out whether to use PEP 517 yet, + # and the user hasn't explicitly stated a preference, + # we do so if the project has a pyproject.toml file. elif use_pep517 is None: - if has_pyproject and editable: - message = ( - 'Error installing {!r}: editable mode is not supported for ' - 'pyproject.toml-style projects. pip is processing this ' - 'project as pyproject.toml-style because it has a ' - 'pyproject.toml file. Since the project has a setup.py and ' - 'the pyproject.toml has no "build-backend" key for the ' - '"build_system" value, you may pass --no-use-pep517 to opt ' - 'out of pyproject.toml-style processing. ' - 'See PEP 517 for details on pyproject.toml-style projects.' - ).format(req_name) - raise InstallationError(message) - use_pep517 = has_pyproject # At this point, we know whether we're going to use PEP 517. assert use_pep517 is not None - requires = get_build_system_requires(build_system, req_name=req_name) - # If we're using the legacy code path, there is nothing further # for us to do here. if not use_pep517: - return (requires, None) + return None if build_system is None: # Either the user has a pyproject.toml with no build-system @@ -204,8 +149,8 @@ def resolve_pyproject_toml( # traditional direct setup.py execution, and require wheel and # a version of setuptools that supports that backend. - requires = ["setuptools>=40.8.0", "wheel"] build_system = { + "requires": ["setuptools>=40.8.0", "wheel"], "build-backend": "setuptools.build_meta:__legacy__", } @@ -215,6 +160,30 @@ def resolve_pyproject_toml( # specified a backend, though. assert build_system is not None + # Ensure that the build-system section in pyproject.toml conforms + # to PEP 518. + error_template = ( + "{package} has a pyproject.toml file that does not comply " + "with PEP 518: {reason}" + ) + + # Specifying the build-system table but not the requires key is invalid + if "requires" not in build_system: + raise InstallationError( + error_template.format(package=req_name, reason=( + "it has a 'build-system' table but not " + "'build-system.requires' which is mandatory in the table" + )) + ) + + # Error out if requires is not a list of strings + requires = build_system["requires"] + if not _is_list_of_str(requires): + raise InstallationError(error_template.format( + package=req_name, + reason="'build-system.requires' is not a list of strings.", + )) + backend = build_system.get("build-backend") check = [] # type: List[str] if backend is None: @@ -233,7 +202,7 @@ def resolve_pyproject_toml( backend = "setuptools.build_meta:__legacy__" check = ["setuptools>=40.8.0", "wheel"] - return (requires, (backend, check)) + return (requires, backend, check) def load_pyproject_toml( @@ -243,7 +212,7 @@ def load_pyproject_toml( setup_py, # type: str req_name # type: str ): - # type: (...) -> Tuple[Optional[List[str]], Optional[Pep517Data]] + # type: (...) -> Optional[Tuple[List[str], str, List[str]]] """Load the pyproject.toml file. Parameters: @@ -255,13 +224,13 @@ def load_pyproject_toml( req_name - The name of the requirement we're processing (for error reporting) - Returns: (requires, pep_517_data) - requires: requirements from pyproject.toml (can be None). - pep_517_data: None if we should use the legacy code path, otherwise: + Returns: + None if we should use the legacy code path, otherwise a tuple ( + requirements from pyproject.toml, name of PEP 517 backend, - requirements we should check are installed after setting up - the build environment + requirements we should check are installed after setting + up the build environment ) """ has_pyproject = os.path.isfile(pyproject_toml) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 111ad0c69..f595e9b2a 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -485,7 +485,7 @@ class InstallRequirement(object): use_pep517 attribute can be used to determine whether we should follow the PEP 517 or legacy (setup.py) code path. """ - requires, pep517_data = load_pyproject_toml( + pep517_data = load_pyproject_toml( self.use_pep517, self.editable, self.pyproject_toml, @@ -493,14 +493,13 @@ class InstallRequirement(object): str(self) ) - use_pep517 = bool(pep517_data) - - self.use_pep517 = use_pep517 - self.pyproject_requires = requires - - if use_pep517: - backend, check = pep517_data + if pep517_data is None: + self.use_pep517 = False + else: + self.use_pep517 = True + requires, backend, check = pep517_data self.requirements_to_check = check + self.pyproject_requires = requires self.pep517_backend = Pep517HookCaller(self.setup_py_dir, backend) # Use a custom function to call subprocesses diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py index a79a973c4..b0a5f06e1 100644 --- a/tests/functional/test_install_reqs.py +++ b/tests/functional/test_install_reqs.py @@ -302,12 +302,8 @@ def test_constraints_local_editable_install_pep518(script, data): to_install = data.src.join("pep518-3.0") script.pip('download', 'setuptools', 'wheel', '-d', data.packages) - # --no-use-pep517 has to be passed since a pyproject.toml file is - # present but PEP 517 doesn't support editable mode. script.pip( - 'install', '--no-use-pep517', '--no-index', '-f', data.find_links, - '-e', to_install, - ) + 'install', '--no-index', '-f', data.find_links, '-e', to_install) def test_constraints_local_install_causes_error(script, data): diff --git a/tests/unit/test_pep517.py b/tests/unit/test_pep517.py index d539d7b20..6b07bafc3 100644 --- a/tests/unit/test_pep517.py +++ b/tests/unit/test_pep517.py @@ -1,5 +1,3 @@ -from contextlib import contextmanager - import pytest from pip._internal.exceptions import InstallationError @@ -7,19 +5,8 @@ from pip._internal.pyproject import resolve_pyproject_toml from pip._internal.req import InstallRequirement -@contextmanager -def assert_error_startswith(exc_type, expected_start, expected_substr): - with pytest.raises(InstallationError) as excinfo: - yield - - err_args = excinfo.value.args - assert len(err_args) == 1 - msg = err_args[0] - assert msg.startswith(expected_start), 'full message: {}'.format(msg) - assert expected_substr in msg, 'full message: {}'.format(msg) - - -def test_resolve_pyproject_toml__pep_517_optional(): +@pytest.mark.parametrize('editable', [False, True]) +def test_resolve_pyproject_toml__pep_517_optional(editable): """ Test resolve_pyproject_toml() when has_pyproject=True but the source tree isn't pyproject.toml-style per PEP 517. @@ -29,57 +16,17 @@ def test_resolve_pyproject_toml__pep_517_optional(): has_pyproject=True, has_setup=True, use_pep517=None, - editable=False, + editable=editable, req_name='my-package', ) expected = ( ['setuptools>=40.8.0', 'wheel'], - ('setuptools.build_meta:__legacy__', []), + 'setuptools.build_meta:__legacy__', + [], ) assert actual == expected -def test_resolve_pyproject_toml__editable_with_build_system_requires(): - """ - Test a case of editable=True when build-system.requires are returned. - """ - actual = resolve_pyproject_toml( - build_system={'requires': ['package-a', 'package=b']}, - has_pyproject=True, - has_setup=True, - use_pep517=False, - editable=True, - req_name='my-package', - ) - expected = (['package-a', 'package=b'], None) - assert actual == expected - - -def test_resolve_pyproject_toml__editable_without_use_pep517_false(): - """ - Test passing editable=True when PEP 517 processing is optional, but - use_pep517=False hasn't been passed. - """ - expected_start = ( - "Error installing 'my-package': editable mode is not supported" - ) - expected_substr = ( - 'you may pass --no-use-pep517 to opt out of pyproject.toml-style ' - 'processing' - ) - with assert_error_startswith( - InstallationError, expected_start, expected_substr=expected_substr, - ): - resolve_pyproject_toml( - build_system=None, - has_pyproject=True, - has_setup=True, - use_pep517=None, - editable=True, - req_name='my-package', - ) - - @pytest.mark.parametrize( 'has_pyproject, has_setup, use_pep517, build_system, expected_err', [ # Test pyproject.toml with no setup.py. @@ -99,12 +46,7 @@ def test_resolve_pyproject_toml__editable_and_pep_517_required( Test that passing editable=True raises an error if PEP 517 processing is required. """ - expected_start = ( - "Error installing 'my-package': editable mode is not supported" - ) - with assert_error_startswith( - InstallationError, expected_start, expected_substr=expected_err, - ): + with pytest.raises(InstallationError) as excinfo: resolve_pyproject_toml( build_system=build_system, has_pyproject=has_pyproject, @@ -113,50 +55,13 @@ def test_resolve_pyproject_toml__editable_and_pep_517_required( editable=True, req_name='my-package', ) - - -@pytest.mark.parametrize( - 'has_pyproject, has_setup, use_pep517, editable, build_system, ' - 'expected_err', [ - # Test editable=False with no build-system.requires. - (True, False, None, False, {}, - "it has a 'build-system' table but not 'build-system.requires'"), - # Test editable=True with no build-system.requires (we also - # need to pass use_pep517=False). - (True, True, False, True, {}, - "it has a 'build-system' table but not 'build-system.requires'"), - # Test editable=False with a bad build-system.requires value. - (True, False, None, False, {'requires': 'foo'}, - "'build-system.requires' is not a list of strings"), - # Test editable=True with a bad build-system.requires value (we also - # need to pass use_pep517=False). - (True, True, False, True, {'requires': 'foo'}, - "'build-system.requires' is not a list of strings"), - ] -) -def test_resolve_pyproject_toml__bad_build_system_section( - has_pyproject, has_setup, use_pep517, editable, build_system, - expected_err, -): - """ - Test a pyproject.toml build-system section that doesn't conform to - PEP 518. - """ - expected_start = ( - 'my-package has a pyproject.toml file that does not comply with ' - 'PEP 518:' + err_args = excinfo.value.args + assert len(err_args) == 1 + msg = err_args[0] + assert msg.startswith( + "Error installing 'my-package': editable mode is not supported" ) - with assert_error_startswith( - InstallationError, expected_start, expected_substr=expected_err, - ): - resolve_pyproject_toml( - build_system=build_system, - has_pyproject=has_pyproject, - has_setup=has_setup, - use_pep517=use_pep517, - editable=editable, - req_name='my-package', - ) + assert expected_err in msg, 'full message: {}'.format(msg) @pytest.mark.parametrize(('source', 'expected'), [