From f66c1f7639bee37d1874c8c9e663da7cdbaf49cf Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Wed, 6 Mar 2019 06:35:06 -0800 Subject: [PATCH 1/2] Refactor out read_pyproject_toml() and resolve_pyproject_toml(). --- src/pip/_internal/pyproject.py | 109 +++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 32 deletions(-) diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index 43efbed42be..639e6a87e29 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, Tuple, Optional, List + from typing import Any, Dict, List, Optional, Tuple def _is_list_of_str(obj): @@ -32,42 +32,44 @@ def make_pyproject_path(setup_py_dir): return path -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. +def read_pyproject_toml(path): + # type: (str) -> Optional[Dict[str, str]] + """ + Read a project's 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) + :param path: The path to the 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 - ) + :return: The "build_system" value specified in the project's + pyproject.toml file. """ - has_pyproject = os.path.isfile(pyproject_toml) - has_setup = os.path.isfile(setup_py) + with io.open(path, encoding="utf-8") as f: + pp_toml = pytoml.load(f) + build_system = pp_toml.get("build-system") + + return build_system - 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 @@ -169,3 +171,46 @@ def load_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 cc2d299f76b52ad8ecf2040040ce33e5057e17eb Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Tue, 12 Mar 2019 02:49:01 -0700 Subject: [PATCH 2/2] Error out if installing a pyproject.toml-style (PEP 517) project in editable mode. --- news/6314.feature | 2 + src/pip/_internal/pyproject.py | 37 ++++++++++++++++- src/pip/_internal/req/req_install.py | 1 + tests/unit/test_pep517.py | 60 ++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 news/6314.feature diff --git a/news/6314.feature b/news/6314.feature new file mode 100644 index 00000000000..f0dbcf8216e --- /dev/null +++ b/news/6314.feature @@ -0,0 +1,2 @@ +Error out with an informative message if one tries to install a +``pyproject.toml``-style (PEP 517) source tree using ``--editable`` mode. diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index 639e6a87e29..8a873a230c6 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -49,11 +49,27 @@ 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, str]] + build_system, # type: Optional[Dict[str, Any]] 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]]] @@ -67,6 +83,7 @@ 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). """ @@ -82,6 +99,10 @@ 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: @@ -92,7 +113,18 @@ 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, @@ -175,6 +207,7 @@ 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 @@ -185,6 +218,7 @@ 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 @@ -212,5 +246,6 @@ 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 ddca56861ae..79a6df054bb 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -484,6 +484,7 @@ 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 e531639c086..6b07bafc370 100644 --- a/tests/unit/test_pep517.py +++ b/tests/unit/test_pep517.py @@ -1,9 +1,69 @@ 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),