unify isolated build error generation

abn · abn · commit ca4ceb187ba5 · 2024-11-27T20:45:44.000+01:00
diff --git a/src/poetry/inspection/info.py b/src/poetry/inspection/info.py
@@ -4,7 +4,6 @@
 import functools
 import glob
 import logging
-import subprocess
 import tempfile
 
 from pathlib import Path
@@ -13,7 +12,6 @@
 
 import pkginfo
 
-from build import BuildBackendException
 from poetry.core.constraints.version import Version
 from poetry.core.factory import Factory
 from poetry.core.packages.dependency import Dependency
@@ -25,6 +23,7 @@
 from poetry.core.version.requirements import InvalidRequirementError
 
 from poetry.utils.helpers import extractall
+from poetry.utils.isolated_build import IsolatedBuildBackendError
 from poetry.utils.isolated_build import isolated_builder
 
 
@@ -541,35 +540,8 @@ def get_pep517_metadata(path: Path) -> PackageInfo:
                 builder.metadata_path(dest)
 
             info = PackageInfo.from_metadata_directory(dest)
-        except BuildBackendException as e:
-            logger.debug("PEP517 build failed: %s", e)
-
-            if isinstance(e.exception, subprocess.CalledProcessError):
-                inner_traceback = (
-                    e.exception.output.decode()
-                    if type(e.exception.output) is bytes
-                    else e.exception.output
-                )
-                inner_reason = "\n    | ".join(
-                    ["", str(e.exception), "", *inner_traceback.split("\n")]
-                )
-                reasons = [
-                    f"<warning>{inner_reason}</warning>",
-                    (
-                        "<info>"
-                        "<options=bold>Note:</> This error originates from the build backend, and is likely not a "
-                        f"problem with poetry but with the package at {path}\n\n"
-                        "  (a) not supporting PEP 517 builds\n"
-                        "  (b) not specifying PEP 517 build requirements correctly; or\n"
-                        "  (c) the build requirement not being successfully installed in your system environment.\n\n"
-                        f'You can verify this by running <c1>pip wheel --no-cache-dir --use-pep517 "{path}"</c1>.'
-                        "</info>"
-                    ),
-                ]
-            else:
-                reasons = [str(e), "PEP517 build failed"]
-
-            raise PackageInfoError(path, *reasons) from None
+        except IsolatedBuildBackendError as e:
+            raise PackageInfoError(path, str(e)) from None
 
     if info:
         return info
diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py
@@ -5,12 +5,9 @@
 from pathlib import Path
 from typing import TYPE_CHECKING
 
-from build import BuildBackendException
 from poetry.core.utils.helpers import temporary_directory
 
-from poetry.utils._compat import decode
 from poetry.utils.helpers import extractall
-from poetry.utils.isolated_build import IsolatedBuildError
 from poetry.utils.isolated_build import isolated_builder
 
 
@@ -48,38 +45,19 @@ def prepare(
     def _prepare(
         self, directory: Path, destination: Path, *, editable: bool = False
     ) -> Path:
-        from subprocess import CalledProcessError
-
         distribution: DistributionType = "editable" if editable else "wheel"
-        error: Exception | None = None
-
-        try:
-            with isolated_builder(
-                source=directory,
-                distribution=distribution,
-                python_executable=self._env.python,
-                pool=self._pool,
-            ) as builder:
-                return Path(
-                    builder.build(
-                        distribution,
-                        destination.as_posix(),
-                    )
+        with isolated_builder(
+            source=directory,
+            distribution=distribution,
+            python_executable=self._env.python,
+            pool=self._pool,
+        ) as builder:
+            return Path(
+                builder.build(
+                    distribution,
+                    destination.as_posix(),
                 )
-        except BuildBackendException as e:
-            message_parts = [str(e)]
-
-            if isinstance(e.exception, CalledProcessError):
-                text = e.exception.stderr or e.exception.stdout
-                if text is not None:
-                    message_parts.append(decode(text))
-            else:
-                message_parts.append(str(e.exception))
-
-            error = IsolatedBuildError("\n\n".join(message_parts))
-
-        if error is not None:
-            raise error from None
+            )
 
     def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path:
         from poetry.core.packages.utils.link import Link
diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py
@@ -31,7 +31,7 @@
 from poetry.utils.helpers import get_highest_priority_hash_type
 from poetry.utils.helpers import pluralize
 from poetry.utils.helpers import remove_directory
-from poetry.utils.isolated_build import IsolatedBuildError
+from poetry.utils.isolated_build import IsolatedBuildBackendError
 from poetry.utils.isolated_build import IsolatedBuildInstallError
 from poetry.vcs.git import Git
 
@@ -300,10 +300,13 @@ def _execute_operation(self, operation: Operation) -> None:
                     io = self._sections.get(id(operation), self._io)
 
                 with self._lock:
-                    trace = ExceptionTrace(e)
-                    trace.render(io)
                     pkg = operation.package
-                    if isinstance(e, IsolatedBuildError):
+                    with_trace = True
+
+                    if isinstance(e, IsolatedBuildBackendError):
+                        # TODO: Revisit once upstream fix is available https://github.com/python-poetry/cleo/issues/454
+                        # we disable trace here explicitly to workaround incorrect context detection by crashtest
+                        with_trace = False
                         pip_command = "pip wheel --no-cache-dir --use-pep517"
                         if pkg.develop:
                             requirement = pkg.source_url
@@ -312,14 +315,9 @@ def _execute_operation(self, operation: Operation) -> None:
                             requirement = (
                                 pkg.to_dependency().to_pep_508().split(";")[0].strip()
                             )
-                        message = (
-                            "<info>"
-                            "Note: This error originates from the build backend,"
-                            " and is likely not a problem with poetry"
-                            f" but with {pkg.pretty_name} ({pkg.full_pretty_version})"
-                            " not supporting PEP 517 builds. You can verify this by"
-                            f" running '{pip_command} \"{requirement}\"'."
-                            "</info>"
+                        message = e.generate_message(
+                            source_string=f"{pkg.pretty_name} ({pkg.full_pretty_version})",
+                            build_command=f'{pip_command} "{requirement}"',
                         )
                     elif isinstance(e, IsolatedBuildInstallError):
                         message = (
@@ -338,6 +336,9 @@ def _execute_operation(self, operation: Operation) -> None:
                     else:
                         message = f"<error>Cannot install {pkg.pretty_name}.</error>"
 
+                    if with_trace:
+                        ExceptionTrace(e).render(io)
+
                     io.write_line("")
                     io.write_line(message)
                     io.write_line("")
diff --git a/src/poetry/utils/isolated_build.py b/src/poetry/utils/isolated_build.py
@@ -1,14 +1,17 @@
 from __future__ import annotations
 
 import os
+import subprocess
 
 from contextlib import contextmanager
 from contextlib import redirect_stdout
 from io import StringIO
 from typing import TYPE_CHECKING
 
+from build import BuildBackendException
 from build.env import IsolatedEnv as BaseIsolatedEnv
 
+from poetry.utils._compat import decode
 from poetry.utils.env import Env
 from poetry.utils.env import EnvManager
 from poetry.utils.env import ephemeral_environment
@@ -28,7 +31,47 @@
 class IsolatedBuildBaseError(Exception): ...
 
 
-class IsolatedBuildError(IsolatedBuildBaseError): ...
+class IsolatedBuildBackendError(IsolatedBuildBaseError):
+    def __init__(self, source: Path, exception: BuildBackendException) -> None:
+        super().__init__()
+        self.source = source
+        self.exception = exception
+
+    def generate_message(
+        self, source_string: str | None = None, build_command: str | None = None
+    ) -> str:
+        e = self.exception.exception
+        source_string = source_string or self.source.as_posix()
+        build_command = (
+            build_command
+            or f'pip wheel --no-cache-dir --use-pep517 "{self.source.as_posix()}"'
+        )
+
+        reasons = ["PEP517 build of a dependency failed", str(self.exception)]
+
+        if isinstance(e, subprocess.CalledProcessError):
+            inner_traceback = decode(e.stderr or e.stdout or e.output).strip()
+            inner_reason = "\n    | ".join(
+                ["", str(e), "", *inner_traceback.split("\n")]
+            ).lstrip("\n")
+            reasons.append(f"<warning>{inner_reason}</warning>")
+
+        reasons.append(
+            "<info>"
+            "<options=bold>Note:</> This error originates from the build backend, and is likely not a "
+            f"problem with poetry but one of the following issues with {source_string}\n\n"
+            "  - not supporting PEP 517 builds\n"
+            "  - not specifying PEP 517 build requirements correctly\n"
+            "  - the build requirements are incompatible with your operating system or Python version\n"
+            "  - the build requirements are missing system dependencies (eg: compilers, libraries, headers).\n\n"
+            f"You can verify this by running <c1>{build_command}</c1>."
+            "</info>"
+        )
+
+        return "\n\n".join(reasons)
+
+    def __str__(self) -> str:
+        return self.generate_message()
 
 
 class IsolatedBuildInstallError(IsolatedBuildBaseError):
@@ -140,18 +183,20 @@ def isolated_builder(
     ) as venv:
         env = IsolatedEnv(venv, pool)
         stdout = StringIO()
+        try:
+            builder = ProjectBuilder.from_isolated_env(
+                env, source, runner=quiet_subprocess_runner
+            )
 
-        builder = ProjectBuilder.from_isolated_env(
-            env, source, runner=quiet_subprocess_runner
-        )
-
-        with redirect_stdout(stdout):
-            env.install(builder.build_system_requires)
+            with redirect_stdout(stdout):
+                env.install(builder.build_system_requires)
 
-            # we repeat the build system requirements to avoid poetry installer from removing them
-            env.install(
-                builder.build_system_requires
-                | builder.get_requires_for_build(distribution)
-            )
+                # we repeat the build system requirements to avoid poetry installer from removing them
+                env.install(
+                    builder.build_system_requires
+                    | builder.get_requires_for_build(distribution)
+                )
 
-            yield builder
+                yield builder
+        except BuildBackendException as e:
+            raise IsolatedBuildBackendError(source, e) from None
diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py
@@ -1292,18 +1292,6 @@ def test_build_backend_errors_are_reported_correctly_if_caused_by_subprocess(
     assert return_code == 1
 
     package_url = directory_package.source_url
-    expected_start = f"""
-Package operations: 1 install, 0 updates, 0 removals
-
-  - Installing {package_name} ({package_version} {package_url})
-
-  IsolatedBuildError
-
-  hide the original error
-  \
-
-  original error
-"""
 
     assert directory_package.source_url is not None
     if editable:
@@ -1313,16 +1301,41 @@ def test_build_backend_errors_are_reported_correctly_if_caused_by_subprocess(
     else:
         pip_command = "pip wheel --no-cache-dir --use-pep517"
         requirement = f"{package_name} @ {path_to_url(directory_package.source_url)}"
-    expected_end = f"""
-Note: This error originates from the build backend, and is likely not a problem with \
-poetry but with {package_name} ({package_version} {package_url}) not supporting \
-PEP 517 builds. You can verify this by running '{pip_command} "{requirement}"'.
 
+    expected_source_string = f"{package_name} ({package_version} {package_url})"
+    expected_pip_command = f'{pip_command} "{requirement}"'
+
+    expected_output = f"""
+Package operations: 1 install, 0 updates, 0 removals
+
+  - Installing {package_name} ({package_version} {package_url})
+
+PEP517 build of a dependency failed
+
+hide the original error
 """
 
-    output = io.fetch_output()
-    assert output.startswith(expected_start)
-    assert output.endswith(expected_end)
+    if isinstance(exception, CalledProcessError):
+        expected_output += (
+            "\n    | Command '['pip']' returned non-zero exit status 1."
+            "\n    | "
+            "\n    | original error"
+            "\n"
+        )
+
+    expected_output += f"""
+Note: This error originates from the build backend, and is likely not a problem with poetry but one of the following issues with {expected_source_string}
+
+  - not supporting PEP 517 builds
+  - not specifying PEP 517 build requirements correctly
+  - the build requirements are incompatible with your operating system or Python version
+  - the build requirements are missing system dependencies (eg: compilers, libraries, headers).
+
+You can verify this by running {expected_pip_command}.
+
+"""
+
+    assert io.fetch_output() == expected_output
 
 
 @pytest.mark.parametrize("encoding", ["utf-8", "latin-1"])
