fix(errors): reformat plugin build error output (#1012)

In case of Python plugin build errors, print error messages to stderr instead of stdout so they can be captured and properly displayed to the user. Don't start printing from the last script execution trace line because messages printed to stderr appear before the traced exit line. Fixes #1000 Signed-off-by: Claudio Matsuoka [email protected]
canonical · Feb 14, 2025 · f47837a · f47837a
1 parent bddb753
commit f47837a
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 45 deletions.
diff --git a/craft_parts/errors.py b/craft_parts/errors.py
@@ -513,27 +513,28 @@ def __init__(
     def details(self) -> str | None:
         """Further details on the error.
 
-        Discards all trace lines that come before the last-executed script line
+        Displays the last three trace lines from the error output.
         """
-        with contextlib.closing(StringIO()) as details_io:
-            if self.stderr is None:
-                return None
-
-            stderr = self.stderr.decode("utf-8", errors="replace")
-            stderr_lines = stderr.split("\n")
-            # Find the final command captured in the logs
-            last_command = None
-            for idx, line in enumerate(reversed(stderr_lines)):
-                if line.startswith("+"):
-                    last_command = len(stderr_lines) - idx - 1
+        if self.stderr is None:
+            return None
+
+        stderr = self.stderr.decode("utf-8", errors="replace")
+        stderr_lines = list(filter(lambda x: x, stderr.split("\n")))
+
+        # Find the third trace output line
+        anchor_line = 0
+        traced_lines_to_display = 3
+        count = 0
+        for idx, line in enumerate(reversed(stderr_lines)):
+            if line.startswith("+"):
+                count += 1
+                if count > traced_lines_to_display:
+                    anchor_line = -idx
                     break
-            else:
-                # Fallback to printing the whole log
-                last_command = 0
 
-            for line in stderr_lines[last_command:]:
-                if line:
-                    details_io.write(f"\n:: {line}")
+        with contextlib.closing(StringIO()) as details_io:
+            for line in stderr_lines[anchor_line:]:
+                details_io.write(f"\n:: {line}")
 
             return details_io.getvalue()
 

diff --git a/craft_parts/plugins/base.py b/craft_parts/plugins/base.py
@@ -1,6 +1,6 @@
 # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
 #
-# Copyright 2020-2024 Canonical Ltd.
+# Copyright 2020-2025 Canonical Ltd.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -164,7 +164,7 @@ def _get_find_python_interpreter_commands(self) -> list[str]:
             # look for python3.10
             basename=$(basename $(readlink -f ${{PARTS_PYTHON_VENV_INTERP_PATH}}))
             echo Looking for a Python interpreter called \\"${{basename}}\\" in the payload...
-            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null)
+            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null || true)
 
             if [ -n "$payload_python" ]; then
                 # We found a provisioned interpreter, use it.
@@ -179,12 +179,12 @@ def _get_find_python_interpreter_commands(self) -> list[str]:
                 fi
             else
                 # Otherwise use what _get_system_python_interpreter() told us.
-                echo "Python interpreter not found in payload."
+                echo "Python interpreter not found in payload." >&2
                 symlink_target="{python_interpreter}"
             fi
 
             if [ -z "$symlink_target" ]; then
-                echo "No suitable Python interpreter found, giving up."
+                echo "No suitable Python interpreter found, giving up." >&2
                 exit 1
             fi
 

diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst
@@ -2,6 +2,13 @@
 Changelog
 *********
 
+X.Y.Z ()
+------------------
+
+Bug fixes:
+
+- Fix handling and propagation of Python plugin error messages.
+
 2.6.1 (2025-02-12)
 ------------------
 
@@ -11,7 +18,6 @@ Bug fixes:
 - Fix Jdeps parameter ordering in the 
   :ref:`jlink plugin<craft_parts_jlink_plugin>`.
 
-
 2.3.1 (2025-02-07)
 ------------------
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -75,7 +75,7 @@ dev = [  # TODO: Remove this once we've switched CI to uv
     # Testing
     "hypothesis",
     "jsonschema",
-    "pytest-check",
+    "pytest-check<2.5.0",
     "pytest-subprocess",
     "requests-mock",
 

diff --git a/tests/integration/executor/test_errors.py b/tests/integration/executor/test_errors.py
@@ -74,6 +74,7 @@ def test_plugin_build_errors(new_dir, partitions):
         """\
             Failed to run the build script for part 'foo'.
 
+            :: + go mod download all
             :: + go install -p 1 ./...
             :: # example.com/hello
             :: ./hello.go:9:9: undefined: fmt.Printfs

diff --git a/tests/integration/plugins/test_poetry.py b/tests/integration/plugins/test_poetry.py
@@ -241,21 +241,39 @@ def _get_system_python_interpreter(self) -> str | None:
     )
     actions = lf.plan(Step.PRIME)
 
+    expected_error_text = textwrap.dedent(
+        """\
+        + '[' -z '' ']'
+        + echo 'No suitable Python interpreter found, giving up.'
+        No suitable Python interpreter found, giving up.
+        + exit 1
+        """
+    )
+
     out = pathlib.Path("out.txt")
-    with out.open(mode="w") as outfile, pytest.raises(errors.PluginBuildError):
+    err = pathlib.Path("err.txt")
+    with (
+        out.open(mode="w") as outfile,
+        err.open(mode="w") as errfile,
+        pytest.raises(errors.PluginBuildError) as exc_info,
+    ):
         with lf.action_executor() as ctx:
-            ctx.execute(actions, stdout=outfile)
+            ctx.execute(actions, stdout=outfile, stderr=errfile)
+
+    assert exc_info.value.stderr is not None
+    assert expected_error_text in exc_info.value.stderr.decode()
 
     output = out.read_text()
     expected_text = textwrap.dedent(
         f"""\
         Looking for a Python interpreter called "{real_basename}" in the payload...
-        Python interpreter not found in payload.
-        No suitable Python interpreter found, giving up.
         """
     )
     assert expected_text in output
 
+    output = err.read_text()
+    assert expected_error_text in output
+
 
 def test_find_payload_python_good_version(new_dir, partitions, parts_dict, poetry_part):
     """Test that the build succeeds if a payload interpreter is needed, and it's

diff --git a/tests/integration/plugins/test_python.py b/tests/integration/plugins/test_python.py
@@ -321,21 +321,39 @@ def _get_system_python_interpreter(self) -> str | None:
     )
     actions = lf.plan(Step.PRIME)
 
+    expected_error_text = textwrap.dedent(
+        """\
+        + '[' -z '' ']'
+        + echo 'No suitable Python interpreter found, giving up.'
+        No suitable Python interpreter found, giving up.
+        + exit 1
+        """
+    )
+
     out = Path("out.txt")
-    with out.open(mode="w") as outfile, pytest.raises(errors.PluginBuildError):
+    err = Path("err.txt")
+    with (
+        out.open(mode="w") as outfile,
+        err.open(mode="w") as errfile,
+        pytest.raises(errors.PluginBuildError) as exc_info,
+    ):
         with lf.action_executor() as ctx:
-            ctx.execute(actions, stdout=outfile)
+            ctx.execute(actions, stdout=outfile, stderr=errfile)
+
+    assert exc_info.value.stderr is not None
+    assert expected_error_text in exc_info.value.stderr.decode()
 
     output = out.read_text()
     expected_text = textwrap.dedent(
         f"""\
         Looking for a Python interpreter called "{real_basename}" in the payload...
-        Python interpreter not found in payload.
-        No suitable Python interpreter found, giving up.
         """
     )
     assert expected_text in output
 
+    output = err.read_text()
+    assert expected_error_text in output
+
 
 def test_find_payload_python_good_version(new_dir, partitions):
     """Test that the build succeeds if a payload interpreter is needed, and it's

diff --git a/tests/unit/plugins/test_base.py b/tests/unit/plugins/test_base.py
@@ -1,6 +1,6 @@
 # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
 #
-# Copyright 2021-2023 Canonical Ltd.
+# Copyright 2021-2025 Canonical Ltd.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -146,7 +146,7 @@ def test_python_get_find_python_interpreter_commands(
             # look for python3.10
             basename=$(basename $(readlink -f ${{PARTS_PYTHON_VENV_INTERP_PATH}}))
             echo Looking for a Python interpreter called \\"${{basename}}\\" in the payload...
-            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null)
+            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null || true)
 
             if [ -n "$payload_python" ]; then
                 # We found a provisioned interpreter, use it.
@@ -161,12 +161,12 @@ def test_python_get_find_python_interpreter_commands(
                 fi
             else
                 # Otherwise use what _get_system_python_interpreter() told us.
-                echo "Python interpreter not found in payload."
+                echo "Python interpreter not found in payload." >&2
                 symlink_target="$(readlink -f "$(which "${{PARTS_PYTHON_INTERPRETER}}")")"
             fi
 
             if [ -z "$symlink_target" ]; then
-                echo "No suitable Python interpreter found, giving up."
+                echo "No suitable Python interpreter found, giving up." >&2
                 exit 1
             fi
 
@@ -238,7 +238,7 @@ def test_python_get_build_commands(new_dir, python_plugin: FooPythonPlugin):
             # look for python3.10
             basename=$(basename $(readlink -f ${{PARTS_PYTHON_VENV_INTERP_PATH}}))
             echo Looking for a Python interpreter called \\"${{basename}}\\" in the payload...
-            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null)
+            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null || true)
 
             if [ -n "$payload_python" ]; then
                 # We found a provisioned interpreter, use it.
@@ -253,12 +253,12 @@ def test_python_get_build_commands(new_dir, python_plugin: FooPythonPlugin):
                 fi
             else
                 # Otherwise use what _get_system_python_interpreter() told us.
-                echo "Python interpreter not found in payload."
+                echo "Python interpreter not found in payload." >&2
                 symlink_target="$(readlink -f "$(which "${{PARTS_PYTHON_INTERPRETER}}")")"
             fi
 
             if [ -z "$symlink_target" ]; then
-                echo "No suitable Python interpreter found, giving up."
+                echo "No suitable Python interpreter found, giving up." >&2
                 exit 1
             fi
 

diff --git a/tests/unit/plugins/test_base_python.py b/tests/unit/plugins/test_base_python.py
@@ -1,6 +1,6 @@
 # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
 #
-# Copyright 2024 Canonical Ltd.
+# Copyright 2025 Canonical Ltd.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -72,7 +72,7 @@ def get_python_build_commands(
             # look for python3.10
             basename=$(basename $(readlink -f ${{PARTS_PYTHON_VENV_INTERP_PATH}}))
             echo Looking for a Python interpreter called \\"${{basename}}\\" in the payload...
-            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null)
+            payload_python=$(find "$install_dir" "$stage_dir" -type f -executable -name "${{basename}}" -print -quit 2>/dev/null || true)
 
             if [ -n "$payload_python" ]; then
                 # We found a provisioned interpreter, use it.
@@ -87,12 +87,12 @@ def get_python_build_commands(
                 fi
             else
                 # Otherwise use what _get_system_python_interpreter() told us.
-                echo "Python interpreter not found in payload."
+                echo "Python interpreter not found in payload." >&2
                 symlink_target="$(readlink -f "$(which "${{PARTS_PYTHON_INTERPRETER}}")")"
             fi
 
             if [ -z "$symlink_target" ]; then
-                echo "No suitable Python interpreter found, giving up."
+                echo "No suitable Python interpreter found, giving up." >&2
                 exit 1
             fi
 

diff --git a/tests/unit/utils/test_process.py b/tests/unit/utils/test_process.py
@@ -33,7 +33,7 @@
 
 @pytest.mark.parametrize(("out", "err"), _RUN_TEST_CASES)
 def test_run(out, err):
-    result = process.run(["/usr/bin/sh", "-c", f"echo {out};sleep 0;echo {err} >&2"])
+    result = process.run(["/usr/bin/sh", "-c", f"echo {out};sleep 0.1;echo {err} >&2"])
     assert result.returncode == 0
     assert result.stdout == (out + "\n").encode()
     assert result.stderr == (err + "\n").encode()
@@ -82,7 +82,7 @@ def read(conn: socket.socket, _mask: int) -> None:
         [
             "/usr/bin/sh",
             "-c",
-            f"echo {out};sleep 0;echo {err} >&2; echo -n {out}|socat - UNIX-CONNECT:{new_dir}/test.socket",
+            f"echo {out};sleep 0.1;echo {err} >&2; echo -n {out}|socat - UNIX-CONNECT:{new_dir}/test.socket",
         ],
         selector=selector,
     )