Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

info: fix metadata build error propagation #9870

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/poetry/inspection/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,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
Expand All @@ -24,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


Expand Down Expand Up @@ -540,9 +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)
raise PackageInfoError(path, e, "PEP517 build failed")
except IsolatedBuildBackendError as e:
raise PackageInfoError(path, str(e)) from None

if info:
return info
Expand Down
44 changes: 11 additions & 33 deletions src/poetry/installation/chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down
25 changes: 13 additions & 12 deletions src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
abn marked this conversation as resolved.
Show resolved Hide resolved
with_trace = False
pip_command = "pip wheel --no-cache-dir --use-pep517"
if pkg.develop:
requirement = pkg.source_url
Expand All @@ -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 = (
Expand All @@ -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("")
Expand Down
71 changes: 58 additions & 13 deletions src/poetry/utils/isolated_build.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
14 changes: 12 additions & 2 deletions tests/inspection/test_info.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import shutil
import uuid

from subprocess import CalledProcessError
from typing import TYPE_CHECKING
Expand Down Expand Up @@ -331,15 +332,24 @@ def test_info_setup_complex(demo_setup_complex: Path) -> None:
def test_info_setup_complex_pep517_error(
mocker: MockerFixture, demo_setup_complex: Path
) -> None:
output = uuid.uuid4().hex
mocker.patch(
"build.ProjectBuilder.from_isolated_env",
autospec=True,
side_effect=BuildBackendException(CalledProcessError(1, "mock", output="mock")),
side_effect=BuildBackendException(CalledProcessError(1, "mock", output=output)),
)

with pytest.raises(PackageInfoError):
with pytest.raises(PackageInfoError) as exc:
PackageInfo.from_directory(demo_setup_complex)

text = str(exc.value)
assert "Command 'mock' returned non-zero exit status 1." in text
assert output in text
assert (
"This error originates from the build backend, and is likely not a problem with poetry"
in text
)


def test_info_setup_complex_pep517_legacy(
demo_setup_complex_pep517_legacy: Path,
Expand Down
51 changes: 32 additions & 19 deletions tests/installation/test_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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"])
Expand Down