From 71f506e71ed2790126a78abc97c6c2d2feff7a02 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 29 Mar 2019 06:59:42 -0700 Subject: [PATCH] Require --no-use-pep517 if using editable mode with pyproject.toml. --- news/6370.bugfix | 3 + 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 +++++++++++++++++++++--- 6 files changed, 202 insertions(+), 62 deletions(-) create mode 100644 news/6370.bugfix diff --git a/news/6370.bugfix b/news/6370.bugfix new file mode 100644 index 00000000000..eff3d91ab33 --- /dev/null +++ b/news/6370.bugfix @@ -0,0 +1,3 @@ +Fix the handling of editable mode during installs when ``pyproject.toml`` is +present but PEP 517 doesn't require the source tree to be treated as +``pyproject.toml``-style. diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 077a985ae5b..bc7f86eed56 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -136,7 +136,11 @@ 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 and build_isolation + + should_isolate = ( + (self.req.use_pep517 or self.req.pyproject_requires) and + build_isolation + ) if should_isolate: # Isolate in a BuildEnvironment and install the build-time @@ -163,10 +167,12 @@ def prep_for_dist(self, finder, build_isolation): " and ".join(map(repr, sorted(missing))) ) - # 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) + 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) 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 8a873a230c6..13a8f35a95d 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -12,6 +12,8 @@ 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 @@ -64,6 +66,37 @@ 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 @@ -72,7 +105,7 @@ def resolve_pyproject_toml( editable, # type: bool req_name, # type: str ): - # type: (...) -> Optional[Tuple[List[str], str, List[str]]] + # type: (...) -> Tuple[Optional[List[str]], Optional[Pep517Data]] """ Return how a pyproject.toml file's contents should be interpreted. @@ -86,6 +119,13 @@ 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 @@ -126,19 +166,34 @@ 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. + # 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). 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 None + return (requires, None) if build_system is None: # Either the user has a pyproject.toml with no build-system @@ -149,8 +204,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__", } @@ -160,30 +215,6 @@ 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: @@ -202,7 +233,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( @@ -212,7 +243,7 @@ def load_pyproject_toml( setup_py, # type: str req_name # type: str ): - # type: (...) -> Optional[Tuple[List[str], str, List[str]]] + # type: (...) -> Tuple[Optional[List[str]], Optional[Pep517Data]] """Load the pyproject.toml file. Parameters: @@ -224,13 +255,13 @@ def load_pyproject_toml( 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 + 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: ( - 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 f595e9b2a41..111ad0c69e2 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. """ - pep517_data = load_pyproject_toml( + requires, pep517_data = load_pyproject_toml( self.use_pep517, self.editable, self.pyproject_toml, @@ -493,13 +493,14 @@ def load_pyproject_toml(self): str(self) ) - if pep517_data is None: - self.use_pep517 = False - else: - self.use_pep517 = True - requires, backend, check = pep517_data + use_pep517 = bool(pep517_data) + + self.use_pep517 = use_pep517 + self.pyproject_requires = requires + + if use_pep517: + 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 b0a5f06e149..a79a973c40a 100644 --- a/tests/functional/test_install_reqs.py +++ b/tests/functional/test_install_reqs.py @@ -302,8 +302,12 @@ 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-index', '-f', data.find_links, '-e', to_install) + 'install', '--no-use-pep517', '--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 6b07bafc370..d539d7b20b3 100644 --- a/tests/unit/test_pep517.py +++ b/tests/unit/test_pep517.py @@ -1,3 +1,5 @@ +from contextlib import contextmanager + import pytest from pip._internal.exceptions import InstallationError @@ -5,8 +7,19 @@ from pip._internal.req import InstallRequirement -@pytest.mark.parametrize('editable', [False, True]) -def test_resolve_pyproject_toml__pep_517_optional(editable): +@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(): """ Test resolve_pyproject_toml() when has_pyproject=True but the source tree isn't pyproject.toml-style per PEP 517. @@ -16,17 +29,57 @@ def test_resolve_pyproject_toml__pep_517_optional(editable): has_pyproject=True, has_setup=True, use_pep517=None, - editable=editable, + editable=False, 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. @@ -46,7 +99,12 @@ def test_resolve_pyproject_toml__editable_and_pep_517_required( Test that passing editable=True raises an error if PEP 517 processing is required. """ - with pytest.raises(InstallationError) as excinfo: + expected_start = ( + "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, @@ -55,13 +113,50 @@ def test_resolve_pyproject_toml__editable_and_pep_517_required( 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" + + +@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:' ) - assert expected_err in msg, 'full message: {}'.format(msg) + 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', + ) @pytest.mark.parametrize(('source', 'expected'), [