Improve legibility of build failure errors (#7854)

## Summary We now... - Only show `stdout` and/or `stderr` if they're non-empty. - Use a colorful header for the start of each section - Highlight the step that failed ## Test Plan ![Screenshot 2024-10-01 at 8 18 42 PM](https://github.com/user-attachments/assets/c3dadf6d-11be-468d-940c-a0a29c4a88f0)
2025-10-17 05:47:45 +00:00 · 2024-10-03 13:52:53 +01:00 · 2024-10-03 13:52:53 +01:00 · 005bb235f0
commit 005bb235f0
parent d5c5a29490
7 changed files with 176 additions and 119 deletions
--- a/crates/uv-build-frontend/src/error.rs
+++ b/crates/uv-build-frontend/src/error.rs
@ -1,12 +1,12 @@
-use crate::PythonRunnerOutput;
-use itertools::Itertools;
-use regex::Regex;
 use std::env;
 use std::fmt::{Display, Formatter};
 use std::io;
 use std::path::PathBuf;
 use std::process::ExitStatus;
 use std::sync::LazyLock;
+
+use owo_colors::OwoColorize;
+use regex::Regex;
 use thiserror::Error;
 use tracing::error;
 use uv_configuration::BuildOutput;
@ -14,6 +14,8 @@ use uv_fs::Simplified;
 use uv_pep440::Version;
 use uv_pep508::PackageName;

+use crate::PythonRunnerOutput;
+
 /// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory`
 static MISSING_HEADER_RE_GCC: LazyLock<Regex> = LazyLock::new(|| {
    Regex::new(
@ -71,35 +73,10 @@ pub enum Error {
    Virtualenv(#[from] uv_virtualenv::Error),
    #[error("Failed to run `{0}`")]
    CommandFailed(PathBuf, #[source] io::Error),
-    #[error("{message} ({exit_code})\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
-    BuildBackendOutput {
-        message: String,
-        exit_code: ExitStatus,
-        stdout: String,
-        stderr: String,
-    },
-    /// Nudge the user towards installing the missing dev library
-    #[error("{message} ({exit_code})\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
-    MissingHeaderOutput {
-        message: String,
-        exit_code: ExitStatus,
-        stdout: String,
-        stderr: String,
-        #[source]
-        missing_header_cause: MissingHeaderCause,
-    },
-    #[error("{message} ({exit_code})")]
-    BuildBackend {
-        message: String,
-        exit_code: ExitStatus,
-    },
-    #[error("{message} ({exit_code})")]
-    MissingHeader {
-        message: String,
-        exit_code: ExitStatus,
-        #[source]
-        missing_header_cause: MissingHeaderCause,
-    },
+    #[error(transparent)]
+    BuildBackend(#[from] BuildBackendError),
+    #[error(transparent)]
+    MissingHeader(#[from] MissingHeaderError),
    #[error("Failed to build PATH for build script")]
    BuildScriptPath(#[source] env::JoinPathsError),
 }
@ -202,6 +179,72 @@ impl Display for MissingHeaderCause {
    }
 }

+#[derive(Debug, Error)]
+pub struct BuildBackendError {
+    message: String,
+    exit_code: ExitStatus,
+    stdout: Vec<String>,
+    stderr: Vec<String>,
+}
+
+impl Display for BuildBackendError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{} ({})", self.message, self.exit_code)?;
+
+        let mut non_empty = false;
+
+        if self.stdout.iter().any(|line| !line.trim().is_empty()) {
+            write!(f, "\n\n{}\n{}", "[stdout]".red(), self.stdout.join("\n"))?;
+            non_empty = true;
+        }
+
+        if self.stderr.iter().any(|line| !line.trim().is_empty()) {
+            write!(f, "\n\n{}\n{}", "[stderr]".red(), self.stderr.join("\n"))?;
+            non_empty = true;
+        }
+
+        if non_empty {
+            writeln!(f)?;
+        }
+
+        Ok(())
+    }
+}
+
+#[derive(Debug, Error)]
+pub struct MissingHeaderError {
+    message: String,
+    exit_code: ExitStatus,
+    stdout: Vec<String>,
+    stderr: Vec<String>,
+    #[source]
+    cause: MissingHeaderCause,
+}
+
+impl Display for MissingHeaderError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{} ({})", self.message, self.exit_code)?;
+
+        let mut non_empty = false;
+
+        if self.stdout.iter().any(|line| !line.trim().is_empty()) {
+            write!(f, "\n\n{}\n{}", "[stdout]".red(), self.stdout.join("\n"))?;
+            non_empty = true;
+        }
+
+        if self.stderr.iter().any(|line| !line.trim().is_empty()) {
+            write!(f, "\n\n{}\n{}", "[stderr]".red(), self.stderr.join("\n"))?;
+            non_empty = true;
+        }
+
+        if non_empty {
+            writeln!(f)?;
+        }
+
+        Ok(())
+    }
+}
+
 impl Error {
    /// Construct an [`Error`] from the output of a failed command.
    pub(crate) fn from_command_output(
@ -241,42 +284,48 @@ impl Error {

        if let Some(missing_library) = missing_library {
            return match level {
-                BuildOutput::Stderr | BuildOutput::Quiet => Self::MissingHeader {
+                BuildOutput::Stderr | BuildOutput::Quiet => {
+                    Self::MissingHeader(MissingHeaderError {
+                        message,
+                        exit_code: output.status,
+                        stdout: vec![],
+                        stderr: vec![],
+                        cause: MissingHeaderCause {
+                            missing_library,
+                            package_name: name.cloned(),
+                            package_version: version.cloned(),
+                            version_id: version_id.map(ToString::to_string),
+                        },
+                    })
+                }
+                BuildOutput::Debug => Self::MissingHeader(MissingHeaderError {
                    message,
                    exit_code: output.status,
-                    missing_header_cause: MissingHeaderCause {
+                    stdout: output.stdout.clone(),
+                    stderr: output.stderr.clone(),
+                    cause: MissingHeaderCause {
                        missing_library,
                        package_name: name.cloned(),
                        package_version: version.cloned(),
                        version_id: version_id.map(ToString::to_string),
                    },
-                },
-                BuildOutput::Debug => Self::MissingHeaderOutput {
-                    message,
-                    exit_code: output.status,
-                    stdout: output.stdout.iter().join("\n"),
-                    stderr: output.stderr.iter().join("\n"),
-                    missing_header_cause: MissingHeaderCause {
-                        missing_library,
-                        package_name: name.cloned(),
-                        package_version: version.cloned(),
-                        version_id: version_id.map(ToString::to_string),
-                    },
-                },
+                }),
            };
        }

        match level {
-            BuildOutput::Stderr | BuildOutput::Quiet => Self::BuildBackend {
+            BuildOutput::Stderr | BuildOutput::Quiet => Self::BuildBackend(BuildBackendError {
                message,
                exit_code: output.status,
-            },
-            BuildOutput::Debug => Self::BuildBackendOutput {
+                stdout: vec![],
+                stderr: vec![],
+            }),
+            BuildOutput::Debug => Self::BuildBackend(BuildBackendError {
                message,
                exit_code: output.status,
-                stdout: output.stdout.iter().join("\n"),
-                stderr: output.stderr.iter().join("\n"),
-            },
+                stdout: output.stdout.clone(),
+                stderr: output.stderr.clone(),
+            }),
        }
    }
 }
@ -325,18 +374,22 @@ mod test {
            None,
            Some("pygraphviz-1.11"),
        );
-        assert!(matches!(err, Error::MissingHeaderOutput { .. }));
+
+        assert!(matches!(err, Error::MissingHeader { .. }));
        // Unix uses exit status, Windows uses exit code.
        let formatted = err.to_string().replace("exit status: ", "exit code: ");
+        let formatted = anstream::adapter::strip_str(&formatted);
        insta::assert_snapshot!(formatted, @r###"
        Failed building wheel through setup.py (exit code: 0)
-        --- stdout:
+
+        [stdout]
        running bdist_wheel
        running build
        [...]
        creating build/temp.linux-x86_64-cpython-39/pygraphviz
        gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -DOPENSSL_NO_SSL3 -fPIC -DSWIG_PYTHON_STRICT_BYTE_CHAR -I/tmp/.tmpy6vVes/.venv/include -I/home/konsti/.pyenv/versions/3.9.18/include/python3.9 -c pygraphviz/graphviz_wrap.c -o build/temp.linux-x86_64-cpython-39/pygraphviz/graphviz_wrap.o
-        --- stderr:
+
+        [stderr]
        warning: no files found matching '*.png' under directory 'doc'
        warning: no files found matching '*.txt' under directory 'doc'
        [...]
@ -346,7 +399,6 @@ mod test {
              |          ^~~~~~~~~~~~~~~~~~~
        compilation terminated.
        error: command '/usr/bin/gcc' failed with exit code 1
-        ---
        "###);
        insta::assert_snapshot!(
            std::error::Error::source(&err).unwrap(),
@ -380,20 +432,19 @@ mod test {
            None,
            Some("pygraphviz-1.11"),
        );
-        assert!(matches!(err, Error::MissingHeaderOutput { .. }));
+        assert!(matches!(err, Error::MissingHeader { .. }));
        // Unix uses exit status, Windows uses exit code.
        let formatted = err.to_string().replace("exit status: ", "exit code: ");
+        let formatted = anstream::adapter::strip_str(&formatted);
        insta::assert_snapshot!(formatted, @r###"
        Failed building wheel through setup.py (exit code: 0)
-        --- stdout:

-        --- stderr:
+        [stderr]
        1099 |     n = strlen(p);
             |         ^~~~~~~~~
        /usr/bin/ld: cannot find -lncurses: No such file or directory
        collect2: error: ld returned 1 exit status
        error: command '/usr/bin/x86_64-linux-gnu-gcc' failed with exit code 1
-        ---
        "###);
        insta::assert_snapshot!(
            std::error::Error::source(&err).unwrap(),
@ -428,21 +479,20 @@ mod test {
            None,
            Some("pygraphviz-1.11"),
        );
-        assert!(matches!(err, Error::MissingHeaderOutput { .. }));
+        assert!(matches!(err, Error::MissingHeader { .. }));
        // Unix uses exit status, Windows uses exit code.
        let formatted = err.to_string().replace("exit status: ", "exit code: ");
+        let formatted = anstream::adapter::strip_str(&formatted);
        insta::assert_snapshot!(formatted, @r###"
        Failed building wheel through setup.py (exit code: 0)
-        --- stdout:

-        --- stderr:
+        [stderr]
        usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
           or: setup.py --help [cmd1 cmd2 ...]
           or: setup.py --help-commands
           or: setup.py cmd --help

        error: invalid command 'bdist_wheel'
-        ---
        "###);
        insta::assert_snapshot!(
            std::error::Error::source(&err).unwrap(),
@ -474,17 +524,16 @@ mod test {
            Some(&Version::new([1, 11])),
            Some("pygraphviz-1.11"),
        );
-        assert!(matches!(err, Error::MissingHeaderOutput { .. }));
+        assert!(matches!(err, Error::MissingHeader { .. }));
        // Unix uses exit status, Windows uses exit code.
        let formatted = err.to_string().replace("exit status: ", "exit code: ");
+        let formatted = anstream::adapter::strip_str(&formatted);
        insta::assert_snapshot!(formatted, @r###"
        Failed building wheel through setup.py (exit code: 0)
-        --- stdout:

-        --- stderr:
+        [stderr]
        import distutils.core
        ModuleNotFoundError: No module named 'distutils'
-        ---
        "###);
        insta::assert_snapshot!(
            std::error::Error::source(&err).unwrap(),
--- a/crates/uv-build-frontend/src/lib.rs
+++ b/crates/uv-build-frontend/src/lib.rs
@ -7,6 +7,7 @@ mod error;
 use fs_err as fs;
 use indoc::formatdoc;
 use itertools::Itertools;
+use owo_colors::OwoColorize;
 use rustc_hash::FxHashMap;
 use serde::de::{value, IntoDeserializer, SeqAccess, Visitor};
 use serde::{de, Deserialize, Deserializer};
@ -564,7 +565,10 @@ impl SourceBuild {
            .await?;
        if !output.status.success() {
            return Err(Error::from_command_output(
-                format!("Build backend failed to determine metadata through `prepare_metadata_for_build_{}`", self.build_kind),
+                format!(
+                    "Build backend failed to determine metadata through `{}`",
+                    format!("prepare_metadata_for_build_{}", self.build_kind).green()
+                ),
                &output,
                self.level,
                self.package_name.as_ref(),
@ -694,8 +698,9 @@ impl SourceBuild {
        if !output.status.success() {
            return Err(Error::from_command_output(
                format!(
-                    "Build backend failed to build {} through `build_{}()`",
-                    self.build_kind, self.build_kind,
+                    "Build backend failed to build {} through `{}`",
+                    self.build_kind,
+                    format!("build_{}", self.build_kind).green(),
                ),
                &output,
                self.level,
@ -709,8 +714,8 @@ impl SourceBuild {
        if !output_dir.join(&distribution_filename).is_file() {
            return Err(Error::from_command_output(
                format!(
-                    "Build backend failed to produce {} through `build_{}()`: `{distribution_filename}` not found",
-                    self.build_kind, self.build_kind,
+                    "Build backend failed to produce {} through `{}`: `{distribution_filename}` not found",
+                    self.build_kind, format!("build_{}", self.build_kind).green(),
                ),
                &output,
                self.level,
@ -802,7 +807,10 @@ async fn create_pep517_build_environment(
        .await?;
    if !output.status.success() {
        return Err(Error::from_command_output(
-            format!("Build backend failed to determine requirements with `build_{build_kind}()`"),
+            format!(
+                "Build backend failed to determine requirements with `{}`",
+                format!("build_{build_kind}()").green()
+            ),
            &output,
            level,
            package_name,
@ -815,7 +823,8 @@ async fn create_pep517_build_environment(
    let contents = fs_err::read(&outfile).map_err(|err| {
        Error::from_command_output(
            format!(
-                "Build backend failed to read requirements from `get_requires_for_build_{build_kind}`: {err}"
+                "Build backend failed to read requirements from `{}`: {err}",
+                format!("get_requires_for_build_{build_kind}").green(),
            ),
            &output,
            level,
@ -826,18 +835,21 @@ async fn create_pep517_build_environment(
    })?;

    // Deserialize the requirements from the output file.
-    let extra_requires: Vec<uv_pep508::Requirement<VerbatimParsedUrl>> = serde_json::from_slice::<Vec<uv_pep508::Requirement<VerbatimParsedUrl>>>(&contents).map_err(|err| {
-        Error::from_command_output(
-            format!(
-                "Build backend failed to return requirements from `get_requires_for_build_{build_kind}`: {err}"
-            ),
-            &output,
-            level,
-            package_name,
-            package_version,
-            version_id,
-        )
-    })?;
+    let extra_requires: Vec<uv_pep508::Requirement<VerbatimParsedUrl>> =
+        serde_json::from_slice::<Vec<uv_pep508::Requirement<VerbatimParsedUrl>>>(&contents)
+            .map_err(|err| {
+                Error::from_command_output(
+                    format!(
+                        "Build backend failed to return requirements from `{}`: {err}",
+                        format!("get_requires_for_build_{build_kind}").green(),
+                    ),
+                    &output,
+                    level,
+                    package_name,
+                    package_version,
+                    version_id,
+                )
+            })?;
    let extra_requires: Vec<_> = extra_requires.into_iter().map(Requirement::from).collect();

    // Some packages (such as tqdm 4.66.1) list only extra requires that have already been part of