Merge PR #10577 from sbidoul/fix-pep660-metadata-preparation-fallback

Author: uranusjr

news/10531.bugfix.rst

@@ -0,0 +1,2 @@
+Always refuse installing or building projects that have no ``pyproject.toml`` nor
+``setup.py``.

news/10573.bugfix.rst

@@ -0,0 +1,5 @@
+When installing projects with a ``pyproject.toml`` in editable mode, and the build
+backend does not support :pep:`660`, prepare metadata using
+``prepare_metadata_for_build_wheel`` instead of ``setup.py egg_info``. Also, refuse
+installing projects that only have a ``setup.cfg`` and no ``setup.py`` nor
+``pyproject.toml``. These restore the pre-21.3 behaviour.

news/pep517.vendor.rst

@@ -0,0 +1 @@
+Upgrade pep517 to 0.12.0

src/pip/_internal/distributions/sdist.py

@@ -32,21 +32,22 @@ def prepare_distribution_metadata(
         # Set up the build isolation, if this requirement should be isolated
         should_isolate = self.req.use_pep517 and build_isolation
         if should_isolate:
-            self._setup_isolation(finder)
+            # Setup an isolated environment and install the build backend static
+            # requirements in it.
+            self._prepare_build_backend(finder)
+            # Check that if the requirement is editable, it either supports PEP 660 or
+            # has a setup.py or a setup.cfg. This cannot be done earlier because we need
+            # to setup the build backend to verify it supports build_editable, nor can
+            # it be done later, because we want to avoid installing build requirements
+            # needlessly. Doing it here also works around setuptools generating
+            # UNKNOWN.egg-info when running get_requires_for_build_wheel on a directory
+            # without setup.py nor setup.cfg.
+            self.req.isolated_editable_sanity_check()
+            # Install the dynamic build requirements.
+            self._install_build_reqs(finder)
 
         self.req.prepare_metadata()
 
-    def _setup_isolation(self, finder: PackageFinder) -> None:
-        self._prepare_build_backend(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.
-        if self.req.editable and self.req.permit_editable_wheels:
-            build_reqs = self._get_build_requires_editable()
-        else:
-            build_reqs = self._get_build_requires_wheel()
-        self._install_build_reqs(finder, build_reqs)
-
     def _prepare_build_backend(self, finder: PackageFinder) -> None:
         # Isolate in a BuildEnvironment and install the build-time
         # requirements.
@@ -91,8 +92,19 @@ def _get_build_requires_editable(self) -> Iterable[str]:
             with backend.subprocess_runner(runner):
                 return backend.get_requires_for_build_editable()
 
-    def _install_build_reqs(self, finder: PackageFinder, reqs: Iterable[str]) -> None:
-        conflicting, missing = self.req.build_env.check_requirements(reqs)
+    def _install_build_reqs(self, finder: PackageFinder) -> None:
+        # 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.
+        if (
+            self.req.editable
+            and self.req.permit_editable_wheels
+            and self.req.supports_pyproject_editable()
+        ):
+            build_reqs = self._get_build_requires_editable()
+        else:
+            build_reqs = self._get_build_requires_wheel()
+        conflicting, missing = self.req.build_env.check_requirements(build_reqs)
         if conflicting:
             self._raise_conflicts("the backend dependencies", conflicting)
         self.req.build_env.install_requirements(

src/pip/_internal/operations/build/metadata.py

@@ -23,9 +23,7 @@ def generate_metadata(build_env: BuildEnvironment, backend: Pep517HookCaller) ->
         # Note that Pep517HookCaller implements a fallback for
         # prepare_metadata_for_build_wheel, so we don't have to
         # consider the possibility that this hook doesn't exist.
-        runner = runner_with_spinner_message(
-            "Preparing wheel metadata (pyproject.toml)"
-        )
+        runner = runner_with_spinner_message("Preparing metadata (pyproject.toml)")
         with backend.subprocess_runner(runner):
             distinfo_dir = backend.prepare_metadata_for_build_wheel(metadata_dir)
 

src/pip/_internal/operations/prepare.py

@@ -348,6 +348,7 @@ def _ensure_link_req_src_dir(
         # installation.
         # FIXME: this won't upgrade when there's an existing
         # package unpacked in `req.source_dir`
+        # TODO: this check is now probably dead code
         if is_installable_dir(req.source_dir):
             raise PreviousBuildDirError(
                 "pip can't proceed with requirements '{}' due to a"

src/pip/_internal/pyproject.py

@@ -48,6 +48,12 @@ def load_pyproject_toml(
     has_pyproject = os.path.isfile(pyproject_toml)
     has_setup = os.path.isfile(setup_py)
 
+    if not has_pyproject and not has_setup:
+        raise InstallationError(
+            f"{req_name} does not appear to be a Python project: "
+            f"neither 'setup.py' nor 'pyproject.toml' found."
+        )
+
     if has_pyproject:
         with open(pyproject_toml, encoding="utf-8") as f:
             pp_toml = tomli.load(f)

src/pip/_internal/req/constructors.py

@@ -235,6 +235,8 @@ def _get_url_from_path(path: str, name: str) -> Optional[str]:
     if _looks_like_path(name) and os.path.isdir(path):
         if is_installable_dir(path):
             return path_to_url(path)
+        # TODO: The is_installable_dir test here might not be necessary
+        #       now that it is done in load_pyproject_toml too.
         raise InstallationError(
             f"Directory {name!r} is not installable. Neither 'setup.py' "
             "nor 'pyproject.toml' found."

src/pip/_internal/req/req_install.py

@@ -1,6 +1,7 @@
 # The following comment should be removed at some point in the future.
 # mypy: strict-optional=False
 
+import functools
 import logging
 import os
 import shutil
@@ -16,7 +17,7 @@
 from pip._vendor.packaging.utils import canonicalize_name
 from pip._vendor.packaging.version import Version
 from pip._vendor.packaging.version import parse as parse_version
-from pip._vendor.pep517.wrappers import HookMissing, Pep517HookCaller
+from pip._vendor.pep517.wrappers import Pep517HookCaller
 from pip._vendor.pkg_resources import Distribution
 
 from pip._internal.build_env import BuildEnvironment, NoOpBuildEnvironment
@@ -53,6 +54,7 @@
     redact_auth_from_url,
 )
 from pip._internal.utils.packaging import get_metadata
+from pip._internal.utils.subprocess import runner_with_spinner_message
 from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds
 from pip._internal.utils.virtualenv import running_under_virtualenv
 from pip._internal.vcs import vcs
@@ -196,11 +198,6 @@ def __init__(
         # but after loading this flag should be treated as read only.
         self.use_pep517 = use_pep517
 
-        # supports_pyproject_editable will be set to True or False when we try
-        # to prepare editable metadata or build an editable wheel. None means
-        # "we don't know yet".
-        self.supports_pyproject_editable: Optional[bool] = None
-
         # This requirement needs more preparation before it can be built
         self.needs_more_preparation = False
 
@@ -247,6 +244,18 @@ def name(self) -> Optional[str]:
             return None
         return pkg_resources.safe_name(self.req.name)
 
+    @functools.lru_cache()  # use cached_property in python 3.8+
+    def supports_pyproject_editable(self) -> bool:
+        if not self.use_pep517:
+            return False
+        assert self.pep517_backend
+        with self.build_env:
+            runner = runner_with_spinner_message(
+                "Checking if build backend supports build_editable"
+            )
+            with self.pep517_backend.subprocess_runner(runner):
+                return "build_editable" in self.pep517_backend._supported_features()
+
     @property
     def specifier(self) -> SpecifierSet:
         return self.req.specifier
@@ -503,93 +512,59 @@ def load_pyproject_toml(self) -> None:
             backend_path=backend_path,
         )
 
-    def _generate_editable_metadata(self) -> str:
-        """Invokes metadata generator functions, with the required arguments."""
-        if self.use_pep517:
-            assert self.pep517_backend is not None
-            try:
-                metadata_directory = generate_editable_metadata(
-                    build_env=self.build_env,
-                    backend=self.pep517_backend,
-                )
-            except HookMissing as e:
-                self.supports_pyproject_editable = False
-                if not os.path.exists(self.setup_py_path) and not os.path.exists(
-                    self.setup_cfg_path
-                ):
-                    raise InstallationError(
-                        f"Project {self} has a 'pyproject.toml' and its build "
-                        f"backend is missing the {e} hook. Since it does not "
-                        f"have a 'setup.py' nor a 'setup.cfg', "
-                        f"it cannot be installed in editable mode. "
-                        f"Consider using a build backend that supports PEP 660."
-                    )
-                # At this point we have determined that the build_editable hook
-                # is missing, and there is a setup.py or setup.cfg
-                # so we fallback to the legacy metadata generation
-                logger.info(
-                    "Build backend does not support editables, "
-                    "falling back to setup.py egg_info."
-                )
-            else:
-                self.supports_pyproject_editable = True
-                return metadata_directory
-        elif not os.path.exists(self.setup_py_path) and not os.path.exists(
-            self.setup_cfg_path
-        ):
-            raise InstallationError(
-                f"File 'setup.py' or 'setup.cfg' not found "
-                f"for legacy project {self}. "
-                f"It cannot be installed in editable mode."
-            )
-
-        return generate_metadata_legacy(
-            build_env=self.build_env,
-            setup_py_path=self.setup_py_path,
-            source_dir=self.unpacked_source_directory,
-            isolated=self.isolated,
-            details=self.name or f"from {self.link}",
-        )
+    def isolated_editable_sanity_check(self) -> None:
+        """Check that an editable requirement if valid for use with PEP 517/518.
 
-    def _generate_metadata(self) -> str:
-        """Invokes metadata generator functions, with the required arguments."""
-        if self.use_pep517:
-            assert self.pep517_backend is not None
-            try:
-                return generate_metadata(
-                    build_env=self.build_env,
-                    backend=self.pep517_backend,
-                )
-            except HookMissing as e:
-                raise InstallationError(
-                    f"Project {self} has a pyproject.toml but its build "
-                    f"backend is missing the required {e} hook."
-                )
-        elif not os.path.exists(self.setup_py_path):
+        This verifies that an editable that has a pyproject.toml either supports PEP 660
+        or as a setup.py or a setup.cfg
+        """
+        if (
+            self.editable
+            and self.use_pep517
+            and not self.supports_pyproject_editable()
+            and not os.path.isfile(self.setup_py_path)
+            and not os.path.isfile(self.setup_cfg_path)
+        ):
             raise InstallationError(
-                f"File 'setup.py' not found for legacy project {self}."
+                f"Project {self} has a 'pyproject.toml' and its build "
+                f"backend is missing the 'build_editable' hook. Since it does not "
+                f"have a 'setup.py' nor a 'setup.cfg', "
+                f"it cannot be installed in editable mode. "
+                f"Consider using a build backend that supports PEP 660."
             )
 
-        return generate_metadata_legacy(
-            build_env=self.build_env,
-            setup_py_path=self.setup_py_path,
-            source_dir=self.unpacked_source_directory,
-            isolated=self.isolated,
-            details=self.name or f"from {self.link}",
-        )
-
     def prepare_metadata(self) -> None:
         """Ensure that project metadata is available.
 
-        Under PEP 517, call the backend hook to prepare the metadata.
+        Under PEP 517 and PEP 660, call the backend hook to prepare the metadata.
         Under legacy processing, call setup.py egg-info.
         """
         assert self.source_dir
 
-        if self.editable and self.permit_editable_wheels:
-            self.metadata_directory = self._generate_editable_metadata()
+        if self.use_pep517:
+            assert self.pep517_backend is not None
+            if (
+                self.editable
+                and self.permit_editable_wheels
+                and self.supports_pyproject_editable()
+            ):
+                self.metadata_directory = generate_editable_metadata(
+                    build_env=self.build_env,
+                    backend=self.pep517_backend,
+                )
+            else:
+                self.metadata_directory = generate_metadata(
+                    build_env=self.build_env,
+                    backend=self.pep517_backend,
+                )
         else:
-            self.metadata_directory = self._generate_metadata()
+            self.metadata_directory = generate_metadata_legacy(
+                build_env=self.build_env,
+                setup_py_path=self.setup_py_path,
+                source_dir=self.unpacked_source_directory,
+                isolated=self.isolated,
+                details=self.name or f"from {self.link}",
+            )
 
         # Act on the newly generated metadata, based on the name and version.
         if not self.name:

src/pip/_internal/wheel_builder.py

@@ -71,10 +71,8 @@ def _should_build(
         return False
 
     if req.editable:
-        if req.use_pep517 and req.supports_pyproject_editable is not False:
-            return True
-        # we don't build legacy editable requirements
-        return False
+        # we only build PEP 660 editable requirements
+        return req.supports_pyproject_editable()
 
     if req.use_pep517:
         return True