From 1f27d9659ec8b1342be798e93edb44d54a387ee4 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 26 Apr 2019 20:18:27 -0700 Subject: [PATCH 1/6] Revert "Fix "build_system value" to "build-system table" in pyproject.toml messages." This reverts commit ee80bf98ffeac98f07b216625df81fd90d761e77. --- news/6434.feature | 2 +- src/pip/_internal/pyproject.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/news/6434.feature b/news/6434.feature index 5a9caf8bf4b..958d75165c1 100644 --- a/news/6434.feature +++ b/news/6434.feature @@ -3,5 +3,5 @@ project in editable mode, even when `PEP 517 `_ mandates ``pyproject.toml``-style processing (i.e. when the project has a ``pyproject.toml`` file as well as a ``"build-backend"`` key for the -``[build-system]`` table). Since this option conflicts with the PEP 517 spec, +``"build_system"`` value). Since this option conflicts with the PEP 517 spec, this mode of operation is officially unsupported. diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index b012cb67e2b..2e76c399a88 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -115,7 +115,7 @@ def resolve_pyproject_toml( :param build_system: the "build_system" value specified in a project's pyproject.toml file, or None if the project either doesn't have the - file or does but the file doesn't have a [build-system] table. + file or does but the file doesn't have a "build_system" value. :param has_pyproject: whether the project has a pyproject.toml file. :param has_setup: whether the project has a setup.py file. :param use_pep517: whether the user requested PEP 517 processing. None @@ -160,7 +160,7 @@ def resolve_pyproject_toml( 'for pyproject.toml-style projects. ' 'This project is pyproject.toml-style because it has a ' 'pyproject.toml file and a "build-backend" key for the ' - '[build-system] table, but editable mode is undefined ' + '"build_system" value, but editable mode is undefined ' 'for pyproject.toml-style projects. ' 'Since the project has a setup.py, you may pass ' '--no-use-pep517 to opt out of pyproject.toml-style ' @@ -177,7 +177,7 @@ def resolve_pyproject_toml( 'for pyproject.toml-style projects: ' 'this project is pyproject.toml-style because it has a ' 'pyproject.toml file and a "build-backend" key for the ' - '[build-system] table, but editable mode is undefined ' + '"build_system" value, but editable mode is undefined ' 'for pyproject.toml-style projects. ' 'See PEP 517 for details on pyproject.toml-style projects.' ).format(req_name) @@ -204,7 +204,7 @@ def resolve_pyproject_toml( '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] table, you may pass --no-use-pep517 to opt ' + '"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) From 0453f79b9ed3c26bef4d103feb86902b79c2c1d2 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 26 Apr 2019 20:18:43 -0700 Subject: [PATCH 2/6] Revert "Allow --no-use-pep517 to be used with editable mode in more cases." This reverts commit f06976994851090ca3715387231594dd99b626be. --- news/6434.feature | 7 ----- src/pip/_internal/pyproject.py | 55 +++++++++------------------------- tests/unit/test_pep517.py | 52 ++------------------------------ 3 files changed, 17 insertions(+), 97 deletions(-) delete mode 100644 news/6434.feature diff --git a/news/6434.feature b/news/6434.feature deleted file mode 100644 index 958d75165c1..00000000000 --- a/news/6434.feature +++ /dev/null @@ -1,7 +0,0 @@ -Allow ``--no-use-pep517`` to be used as a work-around when installing a -project in editable mode, even when `PEP 517 -`_ mandates -``pyproject.toml``-style processing (i.e. when the project has a -``pyproject.toml`` file as well as a ``"build-backend"`` key for the -``"build_system"`` value). Since this option conflicts with the PEP 517 spec, -this mode of operation is officially unsupported. diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index 2e76c399a88..13a8f35a95d 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -1,7 +1,6 @@ from __future__ import absolute_import import io -import logging import os import sys @@ -16,9 +15,6 @@ Pep517Data = Tuple[str, List[str]] -logger = logging.getLogger(__name__) - - def _is_list_of_str(obj): # type: (Any) -> bool return ( @@ -137,11 +133,7 @@ def resolve_pyproject_toml( # opposed to False can occur when the value is provided via an # environment variable or config file option (due to the quirk of # strtobool() returning an integer in pip's configuration code). - if editable and use_pep517: - raise make_editable_error( - req_name, 'PEP 517 processing was explicitly requested' - ) - elif has_pyproject and not has_setup: + if has_pyproject and not has_setup: if use_pep517 is not None and not use_pep517: raise InstallationError( "Disabling PEP 517 processing is invalid: " @@ -153,36 +145,7 @@ def resolve_pyproject_toml( ) use_pep517 = True elif build_system and "build-backend" in build_system: - if editable: - if use_pep517 is None: - message = ( - 'Error installing {!r}: editable mode is not supported ' - 'for pyproject.toml-style projects. ' - 'This project is pyproject.toml-style because it has a ' - 'pyproject.toml file and a "build-backend" key for the ' - '"build_system" value, but editable mode is undefined ' - 'for pyproject.toml-style projects. ' - 'Since the project has a setup.py, you may pass ' - '--no-use-pep517 to opt out of pyproject.toml-style ' - 'processing. However, this is an unsupported combination. ' - 'See PEP 517 for details on pyproject.toml-style projects.' - ).format(req_name) - raise InstallationError(message) - - # The case of `editable and use_pep517` being true was already - # handled above. - assert not use_pep517 - message = ( - 'Installing {!r} in editable mode, which is not supported ' - 'for pyproject.toml-style projects: ' - 'this project is pyproject.toml-style because it has a ' - 'pyproject.toml file and a "build-backend" key for the ' - '"build_system" value, but editable mode is undefined ' - 'for pyproject.toml-style projects. ' - 'See PEP 517 for details on pyproject.toml-style projects.' - ).format(req_name) - logger.warning(message) - elif use_pep517 is not None and not use_pep517: + if use_pep517 is not None and not use_pep517: raise InstallationError( "Disabling PEP 517 processing is invalid: " "project specifies a build backend of {} " @@ -190,8 +153,18 @@ def resolve_pyproject_toml( build_system["build-backend"] ) ) - else: - use_pep517 = True + if editable: + reason = ( + 'it has a pyproject.toml file with a "build-backend" key ' + 'in the "build_system" value' + ) + raise make_editable_error(req_name, reason) + use_pep517 = True + elif use_pep517: + if editable: + raise make_editable_error( + 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 diff --git a/tests/unit/test_pep517.py b/tests/unit/test_pep517.py index 83787d56c20..d539d7b20b3 100644 --- a/tests/unit/test_pep517.py +++ b/tests/unit/test_pep517.py @@ -84,6 +84,9 @@ def test_resolve_pyproject_toml__editable_without_use_pep517_false(): 'has_pyproject, has_setup, use_pep517, build_system, expected_err', [ # Test pyproject.toml with no setup.py. (True, False, None, None, 'has a pyproject.toml file and no setup.py'), + # Test "build-backend" present. + (True, True, None, {'build-backend': 'foo'}, + 'has a pyproject.toml file with a "build-backend" key'), # Test explicitly requesting PEP 517 processing. (True, True, True, None, 'PEP 517 processing was explicitly requested'), @@ -112,55 +115,6 @@ def test_resolve_pyproject_toml__editable_and_pep_517_required( ) -def test_resolve_pyproject_toml__editable_build_backend_use_pep517_none(): - """ - Test editable=True with "build-backend" and use_pep517=None. - """ - expected_start = ( - "Error installing 'my-package': editable mode is not supported " - ) - expected_substr = 'you may pass --no-use-pep517 to opt out' - - with assert_error_startswith( - InstallationError, expected_start, expected_substr=expected_substr, - ): - resolve_pyproject_toml( - build_system={'requires': ['my-package'], 'build-backend': 'foo'}, - has_pyproject=True, - has_setup=True, - use_pep517=None, - editable=True, - req_name='my-package', - ) - - -def test_resolve_pyproject_toml__editable_build_backend_use_pep517_false( - caplog -): - """ - Test editable=True with "build-backend" and use_pep517=False. - """ - resolve_pyproject_toml( - build_system={'requires': ['my-package'], 'build-backend': 'foo'}, - has_pyproject=True, - has_setup=True, - use_pep517=False, - editable=True, - req_name='my-package', - ) - - records = caplog.records - assert len(records) == 1 - record = records[0] - assert record.levelname == 'WARNING' - expected_start = ( - "Installing 'my-package' in editable mode, which is not supported " - ) - assert record.message.startswith(expected_start), ( - 'full message: {}'.format(record.message) - ) - - @pytest.mark.parametrize( 'has_pyproject, has_setup, use_pep517, editable, build_system, ' 'expected_err', [ From 24688ee8e8fb3ec11c3424ac105609dd9d2388ca Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 26 Apr 2019 20:19:05 -0700 Subject: [PATCH 3/6] Revert "Require --no-use-pep517 if using editable mode with pyproject.toml." This reverts commit 71f506e71ed2790126a78abc97c6c2d2feff7a02. --- src/pip/_internal/operations/prepare.py | 16 +--- src/pip/_internal/pyproject.py | 105 ++++++++------------- src/pip/_internal/req/req_install.py | 15 ++- tests/functional/test_install_reqs.py | 6 +- tests/unit/test_pep517.py | 119 +++--------------------- 5 files changed, 62 insertions(+), 199 deletions(-) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index bc7f86eed56..077a985ae5b 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -136,11 +136,7 @@ def prep_for_dist(self, finder, build_isolation): # 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 @@ def prep_for_dist(self, finder, build_isolation): " 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 13a8f35a95d..8a873a230c6 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -12,8 +12,6 @@ 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 111ad0c69e2..f595e9b2a41 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -485,7 +485,7 @@ def load_pyproject_toml(self): 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 @@ def load_pyproject_toml(self): 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 a79a973c40a..b0a5f06e149 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 d539d7b20b3..6b07bafc370 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.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__', []), - ) - 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', + 'setuptools.build_meta:__legacy__', + [], ) - 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'), [ From fb2533a94557fb14f7f811ed2dad298be097e794 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 26 Apr 2019 20:19:24 -0700 Subject: [PATCH 4/6] Revert "Error out if installing a pyproject.toml-style (PEP 517) project in editable mode." This reverts commit cc2d299f76b52ad8ecf2040040ce33e5057e17eb. --- src/pip/_internal/pyproject.py | 37 +---------------- src/pip/_internal/req/req_install.py | 1 - tests/unit/test_pep517.py | 60 ---------------------------- 3 files changed, 1 insertion(+), 97 deletions(-) diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index 8a873a230c6..639e6a87e29 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -49,27 +49,11 @@ def read_pyproject_toml(path): return build_system -def make_editable_error(req_name, reason): - """ - :param req_name: the name of the requirement. - :param reason: the reason the requirement is being processed as - pyproject.toml-style. - """ - message = ( - 'Error installing {!r}: editable mode is not supported for ' - 'pyproject.toml-style projects. This project is being processed ' - 'as pyproject.toml-style because {}. ' - 'See PEP 517 for the relevant specification.' - ).format(req_name, reason) - return InstallationError(message) - - def resolve_pyproject_toml( - build_system, # type: Optional[Dict[str, Any]] + build_system, # type: Optional[Dict[str, str]] has_pyproject, # type: bool has_setup, # type: bool use_pep517, # type: Optional[bool] - editable, # type: bool req_name, # type: str ): # type: (...) -> Optional[Tuple[List[str], str, List[str]]] @@ -83,7 +67,6 @@ def resolve_pyproject_toml( :param has_setup: whether the project has a setup.py file. :param use_pep517: whether the user requested PEP 517 processing. None means the user didn't explicitly specify. - :param editable: whether editable mode was requested for the requirement. :param req_name: the name of the requirement we're processing (for error reporting). """ @@ -99,10 +82,6 @@ def resolve_pyproject_toml( "Disabling PEP 517 processing is invalid: " "project does not have a setup.py" ) - if editable: - raise make_editable_error( - req_name, 'it has a pyproject.toml file and no setup.py' - ) use_pep517 = True elif build_system and "build-backend" in build_system: if use_pep517 is not None and not use_pep517: @@ -113,18 +92,7 @@ def resolve_pyproject_toml( build_system["build-backend"] ) ) - if editable: - reason = ( - 'it has a pyproject.toml file with a "build-backend" key ' - 'in the "build_system" value' - ) - raise make_editable_error(req_name, reason) use_pep517 = True - elif use_pep517: - if editable: - raise make_editable_error( - 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, @@ -207,7 +175,6 @@ def resolve_pyproject_toml( def load_pyproject_toml( use_pep517, # type: Optional[bool] - editable, # type: bool pyproject_toml, # type: str setup_py, # type: str req_name # type: str @@ -218,7 +185,6 @@ def load_pyproject_toml( Parameters: use_pep517 - Has the user requested PEP 517 processing? None means the user hasn't explicitly specified. - editable - Whether editable mode was requested for the requirement. pyproject_toml - Location of the project's pyproject.toml file setup_py - Location of the project's setup.py file req_name - The name of the requirement we're processing (for @@ -246,6 +212,5 @@ def load_pyproject_toml( has_pyproject=has_pyproject, has_setup=has_setup, use_pep517=use_pep517, - editable=editable, req_name=req_name, ) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index f595e9b2a41..25a692e1a9a 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -487,7 +487,6 @@ def load_pyproject_toml(self): """ pep517_data = load_pyproject_toml( self.use_pep517, - self.editable, self.pyproject_toml, self.setup_py, str(self) diff --git a/tests/unit/test_pep517.py b/tests/unit/test_pep517.py index 6b07bafc370..e531639c086 100644 --- a/tests/unit/test_pep517.py +++ b/tests/unit/test_pep517.py @@ -1,69 +1,9 @@ import pytest from pip._internal.exceptions import InstallationError -from pip._internal.pyproject import resolve_pyproject_toml from pip._internal.req import InstallRequirement -@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. - """ - actual = resolve_pyproject_toml( - build_system=None, - has_pyproject=True, - has_setup=True, - use_pep517=None, - editable=editable, - req_name='my-package', - ) - expected = ( - ['setuptools>=40.8.0', 'wheel'], - 'setuptools.build_meta:__legacy__', - [], - ) - assert actual == expected - - -@pytest.mark.parametrize( - 'has_pyproject, has_setup, use_pep517, build_system, expected_err', [ - # Test pyproject.toml with no setup.py. - (True, False, None, None, 'has a pyproject.toml file and no setup.py'), - # Test "build-backend" present. - (True, True, None, {'build-backend': 'foo'}, - 'has a pyproject.toml file with a "build-backend" key'), - # Test explicitly requesting PEP 517 processing. - (True, True, True, None, - 'PEP 517 processing was explicitly requested'), - ] -) -def test_resolve_pyproject_toml__editable_and_pep_517_required( - has_pyproject, has_setup, use_pep517, build_system, expected_err, -): - """ - Test that passing editable=True raises an error if PEP 517 processing - is required. - """ - with pytest.raises(InstallationError) as excinfo: - resolve_pyproject_toml( - build_system=build_system, - has_pyproject=has_pyproject, - has_setup=has_setup, - use_pep517=use_pep517, - editable=True, - req_name='my-package', - ) - 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" - ) - assert expected_err in msg, 'full message: {}'.format(msg) - - @pytest.mark.parametrize(('source', 'expected'), [ ("pep517_setup_and_pyproject", True), ("pep517_setup_only", False), From 285a23fb8f3a7a91454e8ab4aad44895084dde61 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 26 Apr 2019 20:21:04 -0700 Subject: [PATCH 5/6] Revert "Refactor out read_pyproject_toml() and resolve_pyproject_toml()." This reverts commit f66c1f7639bee37d1874c8c9e663da7cdbaf49cf. --- src/pip/_internal/pyproject.py | 109 ++++++++++----------------------- 1 file changed, 32 insertions(+), 77 deletions(-) diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index 639e6a87e29..43efbed42be 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -10,7 +10,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Any, Dict, List, Optional, Tuple + from typing import Any, Tuple, Optional, List def _is_list_of_str(obj): @@ -32,44 +32,42 @@ def make_pyproject_path(setup_py_dir): return path -def read_pyproject_toml(path): - # type: (str) -> Optional[Dict[str, str]] - """ - Read a project's pyproject.toml file. +def load_pyproject_toml( + use_pep517, # type: Optional[bool] + pyproject_toml, # type: str + setup_py, # type: str + req_name # type: str +): + # type: (...) -> Optional[Tuple[List[str], str, List[str]]] + """Load the pyproject.toml file. - :param path: The path to the pyproject.toml file. + Parameters: + use_pep517 - Has the user requested PEP 517 processing? None + means the user hasn't explicitly specified. + pyproject_toml - Location of the project's pyproject.toml file + setup_py - Location of the project's setup.py file + req_name - The name of the requirement we're processing (for + error reporting) - :return: The "build_system" value specified in the project's - pyproject.toml file. + 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 + ) """ - with io.open(path, encoding="utf-8") as f: - pp_toml = pytoml.load(f) - build_system = pp_toml.get("build-system") - - return build_system + has_pyproject = os.path.isfile(pyproject_toml) + has_setup = os.path.isfile(setup_py) + if has_pyproject: + with io.open(pyproject_toml, encoding="utf-8") as f: + pp_toml = pytoml.load(f) + build_system = pp_toml.get("build-system") + else: + build_system = None -def resolve_pyproject_toml( - build_system, # type: Optional[Dict[str, str]] - has_pyproject, # type: bool - has_setup, # type: bool - use_pep517, # type: Optional[bool] - req_name, # type: str -): - # type: (...) -> Optional[Tuple[List[str], str, List[str]]] - """ - Return how a pyproject.toml file's contents should be interpreted. - - :param build_system: the "build_system" value specified in a project's - pyproject.toml file, or None if the project either doesn't have the - file or does but the file doesn't have a "build_system" value. - :param has_pyproject: whether the project has a pyproject.toml file. - :param has_setup: whether the project has a setup.py file. - :param use_pep517: whether the user requested PEP 517 processing. None - means the user didn't explicitly specify. - :param req_name: the name of the requirement we're processing (for - error reporting). - """ # The following cases must use PEP 517 # We check for use_pep517 being non-None and falsey because that means # the user explicitly requested --no-use-pep517. The value 0 as @@ -171,46 +169,3 @@ def resolve_pyproject_toml( check = ["setuptools>=40.8.0", "wheel"] return (requires, backend, check) - - -def load_pyproject_toml( - use_pep517, # type: Optional[bool] - pyproject_toml, # type: str - setup_py, # type: str - req_name # type: str -): - # type: (...) -> Optional[Tuple[List[str], str, List[str]]] - """Load the pyproject.toml file. - - Parameters: - use_pep517 - Has the user requested PEP 517 processing? None - means the user hasn't explicitly specified. - pyproject_toml - Location of the project's pyproject.toml file - setup_py - Location of the project's setup.py file - req_name - The name of the requirement we're processing (for - error reporting) - - 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 - ) - """ - has_pyproject = os.path.isfile(pyproject_toml) - has_setup = os.path.isfile(setup_py) - - if has_pyproject: - build_system = read_pyproject_toml(pyproject_toml) - else: - build_system = None - - return resolve_pyproject_toml( - build_system=build_system, - has_pyproject=has_pyproject, - has_setup=has_setup, - use_pep517=use_pep517, - req_name=req_name, - ) From 44643b8e28f8ecdc24e42b00d4b7b1a481dd3d94 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 26 Apr 2019 20:29:44 -0700 Subject: [PATCH 6/6] Restore pyproject.toml handling to its version 19.0.3 state. --- news/6434.feature | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 news/6434.feature diff --git a/news/6434.feature b/news/6434.feature new file mode 100644 index 00000000000..12d4d316d03 --- /dev/null +++ b/news/6434.feature @@ -0,0 +1,2 @@ +Restore ``pyproject.toml`` handling to how it was with pip 19.0.3 to prevent +the need to add ``--no-use-pep517`` when installing in editable mode.