Skip to content

Commit

Permalink
Append command location information into errors
Browse files Browse the repository at this point in the history
  • Loading branch information
tao-guo committed Nov 27, 2023
1 parent 0755742 commit 98114b3
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 31 deletions.
2 changes: 1 addition & 1 deletion macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ pub fn spawn_with_output(input: proc_macro::TokenStream) -> proc_macro::TokenStr
pub fn cmd_die(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
let msg = parse_msg(input.into());
quote!({
::cmd_lib::error!("FATAL: {}", #msg);
::cmd_lib::error!("FATAL: {} at {}:{}", #msg, file!(), line!());
std::process::exit(1)
})
.into()
Expand Down
3 changes: 2 additions & 1 deletion macros/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ impl<I: Iterator<Item = ParseArg>> Parser<I> {
}

fn parse_pipe(&mut self) -> TokenStream {
let mut ret = quote!(::cmd_lib::Cmd::default());
// TODO: get accurate line number once `proc_macro::Span::line()` API is stable
let mut ret = quote!(::cmd_lib::Cmd::default().with_location(file!(), line!()));
while let Some(arg) = self.iter.peek() {
match arg {
ParseArg::RedirectFd(fd1, fd2) => {
Expand Down
62 changes: 47 additions & 15 deletions src/child.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ impl FunChildren {
/// provided function.
pub fn wait_with_pipe(&mut self, f: &mut dyn FnMut(Box<dyn Read>)) -> CmdResult {
let child = self.children.pop().unwrap();
let stderr_thread = StderrThread::new(&child.cmd, child.stderr, false);
let stderr_thread =
StderrThread::new(&child.cmd, &child.file, child.line, child.stderr, false);
match child.handle {
CmdChildHandle::Proc(mut proc) => {
if let Some(stdout) = child.stdout {
Expand Down Expand Up @@ -145,6 +146,8 @@ impl FunChildren {
pub(crate) struct CmdChild {
handle: CmdChildHandle,
cmd: String,
file: String,
line: u32,
stdout: Option<PipeReader>,
stderr: Option<PipeReader>,
}
Expand All @@ -153,10 +156,14 @@ impl CmdChild {
pub(crate) fn new(
handle: CmdChildHandle,
cmd: String,
file: String,
line: u32,
stdout: Option<PipeReader>,
stderr: Option<PipeReader>,
) -> Self {
Self {
file,
line,
handle,
cmd,
stdout,
Expand All @@ -165,8 +172,9 @@ impl CmdChild {
}

fn wait(mut self, is_last: bool) -> CmdResult {
let _stderr_thread = StderrThread::new(&self.cmd, self.stderr.take(), false);
let res = self.handle.wait(&self.cmd);
let _stderr_thread =
StderrThread::new(&self.cmd, &self.file, self.line, self.stderr.take(), false);
let res = self.handle.wait(&self.cmd, &self.file, self.line);
if let Err(e) = res {
if is_last || process::pipefail_enabled() {
return Err(e);
Expand All @@ -184,7 +192,13 @@ impl CmdChild {
}

fn wait_with_all(mut self, capture: bool) -> (CmdResult, FunResult, FunResult) {
let mut stderr_thread = StderrThread::new(&self.cmd, self.stderr.take(), capture);
let mut stderr_thread = StderrThread::new(
&self.cmd,
&self.file,
self.line,
self.stderr.take(),
capture,
);
let stdout_output = {
if let Some(mut out) = self.stdout.take() {
let mut s = String::new();
Expand All @@ -202,7 +216,7 @@ impl CmdChild {
}
};
let stderr_output = stderr_thread.join();
let res = self.handle.wait(&self.cmd);
let res = self.handle.wait(&self.cmd, &self.file, self.line);
(res, stdout_output, stderr_output)
}

Expand All @@ -222,17 +236,19 @@ pub(crate) enum CmdChildHandle {
}

impl CmdChildHandle {
fn wait(self, cmd: &str) -> CmdResult {
fn wait(self, cmd: &str, file: &str, line: u32) -> CmdResult {
match self {
CmdChildHandle::Proc(mut proc) => {
let status = proc.wait();
match status {
Err(e) => return Err(process::new_cmd_io_error(&e, cmd)),
Err(e) => return Err(process::new_cmd_io_error(&e, cmd, file, line)),
Ok(status) => {
if !status.success() {
return Err(Self::status_to_io_error(
status,
&format!("Running [{cmd}] exited with error"),
file,
line,
));
}
}
Expand All @@ -243,13 +259,15 @@ impl CmdChildHandle {
match status {
Ok(result) => {
if let Err(e) = result {
return Err(process::new_cmd_io_error(&e, cmd));
return Err(process::new_cmd_io_error(&e, cmd, file, line));
}
}
Err(e) => {
return Err(Error::new(
ErrorKind::Other,
format!("Running [{cmd}] thread joined with error: {e:?}"),
format!(
"Running [{cmd}] thread joined with error: {e:?} at {file}:{line}"
),
))
}
}
Expand All @@ -259,11 +277,17 @@ impl CmdChildHandle {
Ok(())
}

fn status_to_io_error(status: ExitStatus, cmd: &str) -> Error {
fn status_to_io_error(status: ExitStatus, run_cmd: &str, file: &str, line: u32) -> Error {
if let Some(code) = status.code() {
Error::new(ErrorKind::Other, format!("{cmd}; status code: {code}"))
Error::new(
ErrorKind::Other,
format!("{run_cmd}; status code: {code} at {file}:{line}"),
)
} else {
Error::new(ErrorKind::Other, format!("{cmd}; terminated by {status}"))
Error::new(
ErrorKind::Other,
format!("{run_cmd}; terminated by {status} at {file}:{line}"),
)
}
}

Expand All @@ -288,10 +312,12 @@ impl CmdChildHandle {
struct StderrThread {
thread: Option<JoinHandle<String>>,
cmd: String,
file: String,
line: u32,
}

impl StderrThread {
fn new(cmd: &str, stderr: Option<PipeReader>, capture: bool) -> Self {
fn new(cmd: &str, file: &str, line: u32, stderr: Option<PipeReader>, capture: bool) -> Self {
if let Some(stderr) = stderr {
let thread = std::thread::spawn(move || {
let mut output = String::new();
Expand All @@ -312,11 +338,15 @@ impl StderrThread {
});
Self {
cmd: cmd.into(),
file: file.into(),
line,
thread: Some(thread),
}
} else {
Self {
cmd: cmd.into(),
file: file.into(),
line,
thread: None,
}
}
Expand All @@ -326,10 +356,12 @@ impl StderrThread {
if let Some(thread) = self.thread.take() {
match thread.join() {
Err(e) => {
let cmd = &self.cmd;
return Err(Error::new(
ErrorKind::Other,
format!("Running [{cmd}] stderr thread joined with error: {e:?}",),
format!(
"Running [{}] stderr thread joined with error: {:?} at {}:{}",
self.cmd, e, self.file, self.line
),
));
}
Ok(output) => return Ok(output),
Expand Down
62 changes: 48 additions & 14 deletions src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,16 @@ pub struct Cmds {
cmds: Vec<Option<Cmd>>,
full_cmds: String,
ignore_error: bool,
file: String,
line: u32,
}

impl Cmds {
pub fn pipe(mut self, cmd: Cmd) -> Self {
if !self.full_cmds.is_empty() {
if self.full_cmds.is_empty() {
self.file = cmd.file.clone();
self.line = cmd.line;
} else {
self.full_cmds += " | ";
}
self.full_cmds += &cmd.cmd_str();
Expand All @@ -177,17 +182,22 @@ impl Cmds {
// first command in the pipe
self.ignore_error = true;
} else {
warn!("Builtin {IGNORE_CMD:?} command at wrong position");
warn!(
"Builtin {IGNORE_CMD:?} command at wrong position ({}:{})",
self.file, self.line
);
}
}
self.cmds.push(Some(cmd));
self
}

fn spawn(&mut self, current_dir: &mut PathBuf, with_output: bool) -> Result<CmdChildren> {
let full_cmds = format!("[{}]", self.full_cmds);
let full_cmds = self.full_cmds.clone();
let file = self.file.clone();
let line = self.line;
if debug_enabled() {
debug!("Running {full_cmds} ...");
debug!("Running [{full_cmds}] at {file}:{line} ...");
}

// spawning all the sub-processes
Expand All @@ -199,17 +209,17 @@ impl Cmds {
if i != len - 1 {
// not the last, update redirects
let (pipe_reader, pipe_writer) =
os_pipe::pipe().map_err(|e| new_cmd_io_error(&e, &full_cmds))?;
os_pipe::pipe().map_err(|e| new_cmd_io_error(&e, &full_cmds, &file, line))?;
cmd.setup_redirects(&mut prev_pipe_in, Some(pipe_writer), with_output)
.map_err(|e| new_cmd_io_error(&e, &full_cmds))?;
.map_err(|e| new_cmd_io_error(&e, &full_cmds, &file, line))?;
prev_pipe_in = Some(pipe_reader);
} else {
cmd.setup_redirects(&mut prev_pipe_in, None, with_output)
.map_err(|e| new_cmd_io_error(&e, &full_cmds))?;
.map_err(|e| new_cmd_io_error(&e, &full_cmds, &file, line))?;
}
let child = cmd
.spawn(current_dir, with_output)
.map_err(|e| new_cmd_io_error(&e, &full_cmds))?;
.map_err(|e| new_cmd_io_error(&e, &full_cmds, &file, line))?;
children.push(child);
}

Expand Down Expand Up @@ -269,6 +279,8 @@ pub struct Cmd {
args: Vec<OsString>,
vars: HashMap<String, String>,
redirects: Vec<Redirect>,
file: String,
line: u32,

// for running
std_cmd: Option<Command>,
Expand All @@ -286,6 +298,8 @@ impl Default for Cmd {
args: vec![],
vars: HashMap::new(),
redirects: vec![],
file: "".into(),
line: 0,
std_cmd: None,
stdin_redirect: None,
stdout_redirect: None,
Expand All @@ -297,6 +311,12 @@ impl Default for Cmd {
}

impl Cmd {
pub fn with_location(mut self, file: &str, line: u32) -> Self {
self.file = file.into();
self.line = line;
self
}

pub fn add_arg<O>(mut self, arg: O) -> Self
where
O: AsRef<OsStr>,
Expand Down Expand Up @@ -369,10 +389,12 @@ impl Cmd {
fn spawn(mut self, current_dir: &mut PathBuf, with_output: bool) -> Result<CmdChild> {
let arg0 = self.arg0();
if arg0 == CD_CMD {
self.run_cd_cmd(current_dir)?;
self.run_cd_cmd(current_dir, &self.file, self.line)?;
Ok(CmdChild::new(
CmdChildHandle::SyncFn,
self.cmd_str(),
self.file,
self.line,
self.stdout_logging,
self.stderr_logging,
))
Expand Down Expand Up @@ -415,6 +437,8 @@ impl Cmd {
Ok(CmdChild::new(
CmdChildHandle::Thread(handle),
cmd_str,
self.file,
self.line,
self.stdout_logging,
self.stderr_logging,
))
Expand All @@ -423,6 +447,8 @@ impl Cmd {
Ok(CmdChild::new(
CmdChildHandle::SyncFn,
cmd_str,
self.file,
self.line,
self.stdout_logging,
self.stderr_logging,
))
Expand Down Expand Up @@ -455,23 +481,28 @@ impl Cmd {
Ok(CmdChild::new(
CmdChildHandle::Proc(child),
self.cmd_str(),
self.file,
self.line,
self.stdout_logging,
self.stderr_logging,
))
}
}

fn run_cd_cmd(&self, current_dir: &mut PathBuf) -> CmdResult {
fn run_cd_cmd(&self, current_dir: &mut PathBuf, file: &str, line: u32) -> CmdResult {
if self.args.len() == 1 {
return Err(Error::new(ErrorKind::Other, "{CD_CMD}: missing directory"));
return Err(Error::new(
ErrorKind::Other,
"{CD_CMD}: missing directory at {file}:{line}",
));
} else if self.args.len() > 2 {
let err_msg = format!("{CD_CMD}: too many arguments");
return Err(Error::new(ErrorKind::Other, err_msg));
}

let dir = current_dir.join(&self.args[1]);
if !dir.is_dir() {
let err_msg = format!("{CD_CMD}: No such file or directory");
let err_msg = format!("{CD_CMD}: No such file or directory at {file}:{line}");
return Err(Error::new(ErrorKind::Other, err_msg));
}

Expand Down Expand Up @@ -606,8 +637,11 @@ impl fmt::Display for CmdString {
}
}

pub(crate) fn new_cmd_io_error(e: &Error, command: &str) -> Error {
Error::new(e.kind(), format!("Running {command} failed: {e}"))
pub(crate) fn new_cmd_io_error(e: &Error, command: &str, file: &str, line: u32) -> Error {
Error::new(
e.kind(),
format!("Running [{command}] failed: {e} at {file}:{line}"),
)
}

#[cfg(test)]
Expand Down

0 comments on commit 98114b3

Please sign in to comment.