Skip to content

Commit

Permalink
Check Ninja exit status and report errors (#2058)
Browse files Browse the repository at this point in the history
Concretely, this improves behavior in a few ways:
* better error messages when Ninja fails
* don't attempt to print stuff to stdout when Ninja fails
* clean up .fud2 temp directory even when Ninja fails
  • Loading branch information
sampsyo authored May 27, 2024
1 parent b789140 commit d8d5a45
Showing 1 changed file with 28 additions and 15 deletions.
43 changes: 28 additions & 15 deletions fud2/fud-core/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,44 @@ use crate::utils::relative_path;
use camino::{Utf8Path, Utf8PathBuf};
use std::collections::{HashMap, HashSet};
use std::io::Write;
use std::process::Command;
use std::process::{Command, ExitStatus};

/// An error that arises while emitting the Ninja file.
/// An error that arises while emitting the Ninja file or executing Ninja.
#[derive(Debug)]
pub enum EmitError {
pub enum RunError {
/// An IO error when writing the Ninja file.
Io(std::io::Error),

/// A required configuration key was missing.
MissingConfig(String),

/// The Ninja process exited with nonzero status.
NinjaFailed(ExitStatus),
}

impl From<std::io::Error> for EmitError {
impl From<std::io::Error> for RunError {
fn from(e: std::io::Error) -> Self {
Self::Io(e)
}
}

impl std::fmt::Display for EmitError {
impl std::fmt::Display for RunError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match &self {
EmitError::Io(e) => write!(f, "{}", e),
EmitError::MissingConfig(s) => {
RunError::Io(e) => write!(f, "{}", e),
RunError::MissingConfig(s) => {
write!(f, "missing required config key: {}", s)
}
RunError::NinjaFailed(c) => {
write!(f, "ninja exited with {}", c)
}
}
}
}

impl std::error::Error for EmitError {}
impl std::error::Error for RunError {}

pub type EmitResult = std::result::Result<(), EmitError>;
pub type EmitResult = std::result::Result<(), RunError>;

/// Code to emit a Ninja `build` command.
pub trait EmitBuild {
Expand Down Expand Up @@ -214,10 +223,10 @@ impl<'a> Run<'a> {
// When we're printing to stdout, suppress Ninja's output by default.
cmd.stdout(std::process::Stdio::null());
}
cmd.status()?;
let status = cmd.status()?;

// Emit stdout.
if self.plan.stdout {
// Emit stdout, only when Ninja succeeded.
if status.success() && self.plan.stdout {
let stdout_file =
std::fs::File::open(self.plan.workdir.join(self.plan.end()))?;
std::io::copy(
Expand All @@ -231,7 +240,11 @@ impl<'a> Run<'a> {
std::fs::remove_dir_all(dir)?;
}

Ok(())
if status.success() {
Ok(())
} else {
Err(RunError::NinjaFailed(status))
}
}

pub fn emit<T: Write + 'a>(&self, out: T) -> EmitResult {
Expand Down Expand Up @@ -300,10 +313,10 @@ impl<'a> Emitter<'a> {
}

/// Fetch a configuration value, or panic if it's missing.
pub fn config_val(&self, key: &str) -> Result<String, EmitError> {
pub fn config_val(&self, key: &str) -> Result<String, RunError> {
self.config_data
.extract_inner::<String>(key)
.map_err(|_| EmitError::MissingConfig(key.to_string()))
.map_err(|_| RunError::MissingConfig(key.to_string()))
}

/// Fetch a configuration value, using a default if it's missing.
Expand Down

0 comments on commit d8d5a45

Please sign in to comment.