Merge pull request #6331 from cjerdonek/issue-6314-editable-with-pep517

Show a nice error if editable mode is attempted with a pyproject.toml source tree

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.

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,61 @@ 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")
 
-    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
+    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]]
+    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]]]
+    """
+    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 editable: whether editable mode was requested for the requirement.
+    :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
@@ -80,6 +99,10 @@ def load_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:
@@ -90,7 +113,18 @@ def load_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,
@@ -169,3 +203,49 @@ def load_pyproject_toml(
         check = ["setuptools>=40.8.0", "wheel"]
 
     return (requires, backend, check)
+
+
+def load_pyproject_toml(
+    use_pep517,  # type: Optional[bool]
+    editable,  # type: 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.
+        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
+                   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,
+        editable=editable,
+        req_name=req_name,
+    )

src/pip/_internal/req/req_install.py

@@ -487,6 +487,7 @@ def load_pyproject_toml(self):
         """
         pep517_data = load_pyproject_toml(
             self.use_pep517,
+            self.editable,
             self.pyproject_toml,
             self.setup_py,
             str(self)

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),