From d7069d0e612c3a37286ea4d3cef635786d3b36c2 Mon Sep 17 00:00:00 2001 From: Filipe Rodrigues Date: Thu, 22 Aug 2024 11:57:01 +0100 Subject: [PATCH] Removed ability to use command output as argument. It wasn't used and complicated the code. Ideally in the future we'll have a better mechanism to capture a command's stdout into a "variable" to then pass as as argument. --- src/ast.rs | 22 ---------------- src/build.rs | 64 +++++++++-------------------------------------- src/error.rs | 17 ------------- src/expand.rs | 4 --- src/rules/rule.rs | 21 ---------------- 5 files changed, 12 insertions(+), 116 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index ee3606e..7b96690 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -387,26 +387,4 @@ pub enum CommandArg<'a> { /// Expression #[serde(borrow)] Expr(Expr<'a>), - - /// Command - #[serde(borrow)] - Command(Command<'a>), - - /// Command full - CommandFull { - /// Strip the command if failed - strip_on_fail: bool, - - /// Command - #[serde(borrow)] - cmd: Command<'a>, - }, - - /// Strip on fail - StripOnFail { - /// Command - #[serde(rename = "strip_on_fail")] - #[serde(borrow)] - cmd: Command<'a>, - }, } diff --git a/src/build.rs b/src/build.rs index 7b75a55..13b3ac3 100644 --- a/src/build.rs +++ b/src/build.rs @@ -26,13 +26,7 @@ use { dashmap::DashMap, futures::{stream::FuturesUnordered, StreamExt, TryFutureExt}, itertools::Itertools, - std::{ - borrow::Cow, - collections::HashMap, - ffi::{OsStr, OsString}, - ops::Try, - time::SystemTime, - }, + std::{borrow::Cow, collections::HashMap, ffi::OsStr, ops::Try, time::SystemTime}, tokio::{fs, process, sync::Semaphore}, }; @@ -674,46 +668,24 @@ impl<'s> Builder<'s> { }; for cmd in &rule.exec.cmds { - // Note: We don't care about the stdout here and don't capture it anyway. - let _: Vec = self.exec_cmd(rule.name, cmd, false).await?; + self.exec_cmd(rule.name, cmd).await?; } Ok(()) } - /// Executes a command, returning it's stdout + /// Executes a command #[expect(unused_results, reason = "Due to the builder pattern of `Command`")] - #[expect(clippy::only_used_in_recursion, reason = "It might be used in the future")] + #[expect(clippy::unused_self, reason = "It might be used in the future")] #[async_recursion::async_recursion] - async fn exec_cmd( - &self, - rule_name: &str, - cmd: &Command>, - capture_stdout: bool, - ) -> Result, AppError> { + async fn exec_cmd(&self, rule_name: &str, cmd: &Command>) -> Result<(), AppError> { // Process all arguments - // Note: When recursing, always capture stdout let args = cmd .args .iter() .map(move |arg| async move { match arg { CommandArg::Expr(arg) => Ok(Some(Cow::Borrowed(OsStr::new(&**arg)))), - CommandArg::Command { strip_on_fail, cmd } => { - let res = self.exec_cmd(rule_name, cmd, true).await; - match (res, strip_on_fail) { - (Ok(arg), _) => { - let arg = String::from_utf8(arg).map_err(AppError::command_output_non_utf8(cmd))?; - let arg = OsString::from(arg); - Ok(Some(Cow::Owned(arg))) - }, - (Err(err), true) => { - tracing::debug!(?arg, err=%err.pretty(), "Stripping argument from failure"); - Ok(None) - }, - (Err(err), false) => Err(err), - } - }, } }) .enumerate() @@ -741,35 +713,23 @@ impl<'s> Builder<'s> { os_cmd.current_dir(&**cwd); } - // If we capture pipe stdout, do it - if capture_stdout { - #[expect( - clippy::absolute_paths, - reason = "We're already using a `process` (`tokio::process`)" - )] - os_cmd.stdout(std::process::Stdio::piped()); - } - // Then spawn it and measure tracing::debug!(target: "zbuild_exec", "{} {}", program.to_string_lossy(), args.iter().map(|arg| arg.to_string_lossy()).join(" ") ); - let (duration, stdout) = util::try_measure_async(async { - let output = os_cmd - .spawn() - .map_err(AppError::spawn_command(cmd))? - .wait_with_output() + let (duration, ()) = util::try_measure_async(async { + os_cmd + .status() .await - .map_err(AppError::wait_command(cmd))?; - - output.status.exit_ok().map_err(AppError::command_failed(cmd))?; - Ok(output.stdout) + .map_err(AppError::spawn_command(cmd))? + .exit_ok() + .map_err(AppError::command_failed(cmd)) }) .await?; tracing::trace!(target: "zbuild_exec", ?rule_name, ?program, ?args, ?duration, "Execution duration"); - Ok(stdout) + Ok(()) } /// Finds a rule for `file` diff --git a/src/error.rs b/src/error.rs index 72b685c..db7ebdd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -310,22 +310,6 @@ decl_error! { cmd: String, }, - /// Wait for command - #[from_fn( - fn wait_command(source: io::Error)( - cmd: &Command => self::cmd_to_string(cmd) - ) + '_ - )] - #[source(Some(source))] - #[fmt("Unable to wait for {cmd}")] - WaitCommand { - /// Underlying error - source: io::Error, - - /// Command formatted - cmd: String, - }, - /// Command failed #[from_fn( fn command_failed(source: ExitStatusError)( @@ -607,7 +591,6 @@ fn cmd_to_string(cmd: &Command) -> String { .iter() .map(|arg| match arg { rules::CommandArg::Expr(expr) => format!("\"{expr}\""), - rules::CommandArg::Command { cmd, .. } => self::cmd_to_string(cmd), }) .join(" "); format!("[{inner}]") diff --git a/src/expand.rs b/src/expand.rs index 5081a22..3e0d7e6 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -238,10 +238,6 @@ impl<'s> Expander<'s> { .map(|arg| { let arg = match *arg { CommandArg::Expr(ref expr) => CommandArg::Expr(self.expand_expr_string(expr, visitor)?), - CommandArg::Command { strip_on_fail, ref cmd } => CommandArg::Command { - strip_on_fail, - cmd: self.expand_cmd(cmd, visitor)?, - }, }; Ok(arg) }) diff --git a/src/rules/rule.rs b/src/rules/rule.rs index 93f2b38..00f7d90 100644 --- a/src/rules/rule.rs +++ b/src/rules/rule.rs @@ -97,15 +97,6 @@ impl<'s> Command> { pub enum CommandArg { /// Expression Expr(T), - - /// Command - Command { - /// Strip the command if failed - strip_on_fail: bool, - - /// Command - cmd: Command, - }, } impl<'s> CommandArg> { @@ -113,18 +104,6 @@ impl<'s> CommandArg> { pub fn new(arg: ast::CommandArg<'s>) -> Self { match arg { ast::CommandArg::Expr(expr) => Self::Expr(Expr::new(expr)), - ast::CommandArg::Command(cmd) => Self::Command { - strip_on_fail: false, - cmd: Command::new(cmd), - }, - ast::CommandArg::CommandFull { strip_on_fail, cmd } => Self::Command { - strip_on_fail, - cmd: Command::new(cmd), - }, - ast::CommandArg::StripOnFail { cmd } => Self::Command { - strip_on_fail: true, - cmd: Command::new(cmd), - }, } } }