From 7a8e6d5f9ee7c137144c5f2f6cfe13023e6ef1bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Eustace?= Date: Sun, 18 Sep 2022 19:41:11 +0200 Subject: [PATCH] Improve error reporting for build backend errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- src/poetry/installation/chef.py | 35 ++++++++++---- src/poetry/installation/executor.py | 14 ++++++ tests/installation/test_executor.py | 72 +++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py index 866cc5344d4..2a4c02c7451 100644 --- a/src/poetry/installation/chef.py +++ b/src/poetry/installation/chef.py @@ -110,6 +110,8 @@ def prepare( def _prepare( self, directory: Path, destination: Path, *, editable: bool = False ) -> Path: + from subprocess import CalledProcessError + with ephemeral_environment(self._env.python) as venv: env = IsolatedEnv(venv, self._config) builder = ProjectBuilder( @@ -119,21 +121,38 @@ def _prepare( runner=quiet_subprocess_runner, ) env.install(builder.build_system_requires) - env.install( - builder.build_system_requires | builder.get_requires_for_build("wheel") - ) stdout = StringIO() - with redirect_stdout(stdout): - try: - return Path( + error: Exception | None = None + try: + with redirect_stdout(stdout): + env.install( + builder.build_system_requires + | builder.get_requires_for_build("wheel") + ) + path = Path( builder.build( "wheel" if not editable else "editable", destination.as_posix(), ) ) - except BuildBackendException as e: - raise ChefBuildError(str(e)) + except BuildBackendException as e: + message_parts = [str(e)] + if isinstance(e.exception, CalledProcessError) and ( + e.exception.stdout is not None or e.exception.stderr is not None + ): + message_parts.append( + e.exception.stderr.decode() + if e.exception.stderr is not None + else e.exception.stdout.decode() + ) + + error = ChefBuildError("\n\n".join(message_parts)) + + if error is not None: + raise error from None + + return path 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 index d3c2574f8aa..764f3dd6aab 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -18,6 +18,7 @@ from poetry.core.packages.utils.link import Link from poetry.installation.chef import Chef +from poetry.installation.chef import ChefBuildError from poetry.installation.chooser import Chooser from poetry.installation.operations import Install from poetry.installation.operations import Uninstall @@ -295,6 +296,19 @@ def _execute_operation(self, operation: Operation) -> None: with self._lock: trace = ExceptionTrace(e) trace.render(io) + if isinstance(e, ChefBuildError): + pkg = operation.package + requirement = pkg.to_dependency().to_pep_508() + io.write_line("") + io.write_line( + "" + "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 wheel --use-pep517 \"{requirement}\"'." + "" + ) io.write_line("") finally: with self._lock: diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index 51a987f662e..c6277182f32 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -7,6 +7,7 @@ import tempfile from pathlib import Path +from subprocess import CalledProcessError from typing import TYPE_CHECKING from typing import Any from typing import Callable @@ -14,12 +15,15 @@ import pytest +from build import BuildBackendException +from build import ProjectBuilder from cleo.formatters.style import Style from cleo.io.buffered_io import BufferedIO from cleo.io.outputs.output import Verbosity from poetry.core.packages.package import Package from poetry.core.packages.utils.link import Link +from poetry.factory import Factory from poetry.installation.chef import Chef as BaseChef from poetry.installation.executor import Executor from poetry.installation.operations import Install @@ -903,3 +907,71 @@ def test_executor_fallback_on_poetry_create_error_without_wheel_installer( assert mock_pip_install.call_count == 1 assert mock_pip_install.call_args[1].get("upgrade") is True assert mock_pip_install.call_args[1].get("editable") is False + + +@pytest.mark.parametrize("failing_method", ["build", "get_requires_for_build"]) +def test_build_backend_errors_are_reported_correctly_if_caused_by_subprocess( + failing_method: str, + mocker: MockerFixture, + config: Config, + pool: RepositoryPool, + io: BufferedIO, + tmp_dir: str, + mock_file_downloads: None, + env: MockEnv, +): + mocker.patch.object(Factory, "create_pool", return_value=pool) + + error = BuildBackendException( + CalledProcessError(1, ["pip"], output=b"Error on stdout") + ) + mocker.patch.object(ProjectBuilder, failing_method, side_effect=error) + io.set_verbosity(Verbosity.NORMAL) + + executor = Executor(env, pool, config, io) + + package_name = "simple-project" + package_version = "1.2.3" + directory_package = Package( + package_name, + package_version, + source_type="directory", + source_url=Path(__file__) + .parent.parent.joinpath("fixtures/simple_project") + .resolve() + .as_posix(), + ) + + return_code = executor.execute( + [ + Install(directory_package), + ] + ) + + 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}) + + ChefBuildError + + Backend operation failed: CalledProcessError(1, ['pip']) + \ + + Error on stdout +""" + + requirement = directory_package.to_dependency().to_pep_508() + 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 wheel --use-pep517 "{requirement}"'. + +""" + + output = io.fetch_output() + assert output.startswith(expected_start) + assert output.endswith(expected_end)