Merge #734

734: QoL + fixes for `xtasks` r=korken89 a=datdenkikniet

Preferably merge this before #732

Draft till I get around to fixing "no package" -> "all packages" on `xtask build` (and probably others)

Co-authored-by: datdenkikniet <jcdra1@gmail.com>

1 files changed, 346 insertions, 32 deletions

diff --git a/xtask/src/command.rs b/xtask/src/command.rs
index 6e91a52..b62724a 100644
--- a/xtask/src/command.rs
+++ b/xtask/src/command.rs
@@ -1,7 +1,15 @@
-use crate::{debug, ExtraArguments, Package, RunResult, TestRunError};
+use log::{error, info, Level};
+
+use crate::{
+    argument_parsing::Globals, cargo_commands::FinalRunResult, ExtraArguments, Package, RunResult,
+    Target, TestRunError,
+};
 use core::fmt;
-use os_pipe::pipe;
-use std::{fs::File, io::Read, process::Command};
+use std::{
+    fs::File,
+    io::Read,
+    process::{Command, Stdio},
+};
 
 #[allow(dead_code)]
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -10,6 +18,21 @@ pub enum BuildMode {
     Debug,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum OutputMode {
+    PipedAndCollected,
+    Inherited,
+}
+
+impl From<OutputMode> for Stdio {
+    fn from(value: OutputMode) -> Self {
+        match value {
+            OutputMode::PipedAndCollected => Stdio::piped(),
+            OutputMode::Inherited => Stdio::inherit(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub enum CargoCommand<'a> {
     // For future embedded-ci
@@ -17,49 +40,49 @@ pub enum CargoCommand<'a> {
     Run {
         cargoarg: &'a Option<&'a str>,
         example: &'a str,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
     },
     Qemu {
         cargoarg: &'a Option<&'a str>,
         example: &'a str,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
     },
     ExampleBuild {
         cargoarg: &'a Option<&'a str>,
         example: &'a str,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
     },
     ExampleCheck {
         cargoarg: &'a Option<&'a str>,
         example: &'a str,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
     },
     Build {
         cargoarg: &'a Option<&'a str>,
         package: Option<Package>,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
     },
     Check {
         cargoarg: &'a Option<&'a str>,
         package: Option<Package>,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
     },
     Clippy {
         cargoarg: &'a Option<&'a str>,
         package: Option<Package>,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
     },
     Format {
@@ -83,14 +106,216 @@ pub enum CargoCommand<'a> {
     ExampleSize {
         cargoarg: &'a Option<&'a str>,
         example: &'a str,
-        target: &'a str,
+        target: Target<'a>,
         features: Option<String>,
         mode: BuildMode,
         arguments: Option<ExtraArguments>,
     },
 }
 
+impl core::fmt::Display for CargoCommand<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let p = |p: &Option<Package>| {
+            if let Some(package) = p {
+                format!("package {package}")
+            } else {
+                format!("default package")
+            }
+        };
+
+        let feat = |f: &Option<String>| {
+            if let Some(features) = f {
+                format!("\"{features}\"")
+            } else {
+                format!("no features")
+            }
+        };
+
+        let carg = |f: &&Option<&str>| {
+            if let Some(cargoarg) = f {
+                format!("{cargoarg}")
+            } else {
+                format!("no cargo args")
+            }
+        };
+
+        let details = |target: &Target,
+                       mode: &BuildMode,
+                       features: &Option<String>,
+                       cargoarg: &&Option<&str>| {
+            let feat = feat(features);
+            let carg = carg(cargoarg);
+            if cargoarg.is_some() {
+                format!("({target}, {mode}, {feat}, {carg})")
+            } else {
+                format!("({target}, {mode}, {feat})")
+            }
+        };
+
+        match self {
+            CargoCommand::Run {
+                cargoarg,
+                example,
+                target,
+                features,
+                mode,
+            } => write!(
+                f,
+                "Run example {example} {}",
+                details(target, mode, features, cargoarg)
+            ),
+            CargoCommand::Qemu {
+                cargoarg,
+                example,
+                target,
+                features,
+                mode,
+            } => write!(
+                f,
+                "Run example {example} in QEMU {}",
+                details(target, mode, features, cargoarg)
+            ),
+            CargoCommand::ExampleBuild {
+                cargoarg,
+                example,
+                target,
+                features,
+                mode,
+            } => write!(
+                f,
+                "Build example {example} {}",
+                details(target, mode, features, cargoarg)
+            ),
+            CargoCommand::ExampleCheck {
+                cargoarg,
+                example,
+                target,
+                features,
+                mode,
+            } => write!(
+                f,
+                "Check example {example} {}",
+                details(target, mode, features, cargoarg)
+            ),
+            CargoCommand::Build {
+                cargoarg,
+                package,
+                target,
+                features,
+                mode,
+            } => {
+                let package = p(package);
+                write!(
+                    f,
+                    "Build {package} {}",
+                    details(target, mode, features, cargoarg)
+                )
+            }
+            CargoCommand::Check {
+                cargoarg,
+                package,
+                target,
+                features,
+                mode,
+            } => {
+                let package = p(package);
+                write!(
+                    f,
+                    "Check {package} {}",
+                    details(target, mode, features, cargoarg)
+                )
+            }
+            CargoCommand::Clippy {
+                cargoarg,
+                package,
+                target,
+                features,
+            } => {
+                let package = p(package);
+                let features = feat(features);
+                let carg = carg(cargoarg);
+                if cargoarg.is_some() {
+                    write!(f, "Clippy {package} ({target}, {features}, {carg})")
+                } else {
+                    write!(f, "Clippy {package} ({target}, {features})")
+                }
+            }
+            CargoCommand::Format {
+                cargoarg,
+                package,
+                check_only,
+            } => {
+                let package = p(package);
+                let carg = carg(cargoarg);
+
+                let carg = if cargoarg.is_some() {
+                    format!("(cargo args: {carg})")
+                } else {
+                    format!("")
+                };
+
+                if *check_only {
+                    write!(f, "Check format for {package} {carg}")
+                } else {
+                    write!(f, "Format {package} {carg}")
+                }
+            }
+            CargoCommand::Doc {
+                cargoarg,
+                features,
+                arguments,
+            } => {
+                let feat = feat(features);
+                let carg = carg(cargoarg);
+                let arguments = arguments
+                    .clone()
+                    .map(|a| format!("{a}"))
+                    .unwrap_or_else(|| "no extra arguments".into());
+                if cargoarg.is_some() {
+                    write!(f, "Document ({feat}, {carg}, {arguments})")
+                } else {
+                    write!(f, "Document ({feat}, {arguments})")
+                }
+            }
+            CargoCommand::Test {
+                package,
+                features,
+                test,
+            } => {
+                let p = p(package);
+                let test = test
+                    .clone()
+                    .map(|t| format!("test {t}"))
+                    .unwrap_or("all tests".into());
+                let feat = feat(features);
+                write!(f, "Run {test} in {p} (features: {feat})")
+            }
+            CargoCommand::Book { arguments: _ } => write!(f, "Build the book"),
+            CargoCommand::ExampleSize {
+                cargoarg,
+                example,
+                target,
+                features,
+                mode,
+                arguments: _,
+            } => {
+                write!(
+                    f,
+                    "Compute size of example {example} {}",
+                    details(target, mode, features, cargoarg)
+                )
+            }
+        }
+    }
+}
+
 impl<'a> CargoCommand<'a> {
+    pub fn as_cmd_string(&self) -> String {
+        let executable = self.executable();
+        let args = self.args().join(" ");
+        format!("{executable} {args}")
+    }
+
     fn command(&self) -> &str {
         match self {
             CargoCommand::Run { .. } | CargoCommand::Qemu { .. } => "run",
@@ -135,7 +360,13 @@ impl<'a> CargoCommand<'a> {
                 if let Some(cargoarg) = cargoarg {
                     args.extend_from_slice(&[cargoarg]);
                 }
-                args.extend_from_slice(&[self.command(), "--example", example, "--target", target]);
+                args.extend_from_slice(&[
+                    self.command(),
+                    "--example",
+                    example,
+                    "--target",
+                    target.triple(),
+                ]);
 
                 if let Some(feature) = features {
                     args.extend_from_slice(&["--features", feature]);
@@ -156,7 +387,13 @@ impl<'a> CargoCommand<'a> {
                 if let Some(cargoarg) = cargoarg {
                     args.extend_from_slice(&[cargoarg]);
                 }
-                args.extend_from_slice(&[self.command(), "--example", example, "--target", target]);
+                args.extend_from_slice(&[
+                    self.command(),
+                    "--example",
+                    example,
+                    "--target",
+                    target.triple(),
+                ]);
 
                 if let Some(feature) = features {
                     args.extend_from_slice(&["--features", feature]);
@@ -178,7 +415,7 @@ impl<'a> CargoCommand<'a> {
                     args.extend_from_slice(&[cargoarg]);
                 }
 
-                args.extend_from_slice(&[self.command(), "--target", target]);
+                args.extend_from_slice(&[self.command(), "--target", target.triple()]);
 
                 if let Some(package) = package {
                     args.extend_from_slice(&["--package", package.name()]);
@@ -326,7 +563,13 @@ impl<'a> CargoCommand<'a> {
                 if let Some(cargoarg) = cargoarg {
                     args.extend_from_slice(&[cargoarg]);
                 }
-                args.extend_from_slice(&[self.command(), "--example", example, "--target", target]);
+                args.extend_from_slice(&[
+                    self.command(),
+                    "--example",
+                    example,
+                    "--target",
+                    target.triple(),
+                ]);
 
                 if let Some(feature) = features {
                     args.extend_from_slice(&["--features", feature]);
@@ -347,7 +590,13 @@ impl<'a> CargoCommand<'a> {
                 if let Some(cargoarg) = cargoarg {
                     args.extend_from_slice(&[cargoarg]);
                 }
-                args.extend_from_slice(&[self.command(), "--example", example, "--target", target]);
+                args.extend_from_slice(&[
+                    self.command(),
+                    "--example",
+                    example,
+                    "--target",
+                    target.triple(),
+                ]);
 
                 if let Some(feature) = features {
                     args.extend_from_slice(&["--features", feature]);
@@ -369,7 +618,13 @@ impl<'a> CargoCommand<'a> {
                 if let Some(cargoarg) = cargoarg {
                     args.extend_from_slice(&[cargoarg]);
                 }
-                args.extend_from_slice(&[self.command(), "--example", example, "--target", target]);
+                args.extend_from_slice(&[
+                    self.command(),
+                    "--example",
+                    example,
+                    "--target",
+                    target.triple(),
+                ]);
 
                 if let Some(feature_name) = features {
                     args.extend_from_slice(&["--features", feature_name]);
@@ -411,24 +666,18 @@ impl fmt::Display for BuildMode {
     }
 }
 
-pub fn run_command(command: &CargoCommand) -> anyhow::Result<RunResult> {
-    let (mut reader, writer) = pipe()?;
-    let (mut error_reader, error_writer) = pipe()?;
-    debug!("👟 {} {}", command.executable(), command.args().join(" "));
+pub fn run_command(command: &CargoCommand, stderr_mode: OutputMode) -> anyhow::Result<RunResult> {
+    log::info!("👟 {command}");
 
-    let mut handle = Command::new(command.executable())
+    let result = Command::new(command.executable())
         .args(command.args())
-        .stdout(writer)
-        .stderr(error_writer)
-        .spawn()?;
-
-    // retrieve output and clean up
-    let mut stdout = String::new();
-    reader.read_to_string(&mut stdout)?;
-    let exit_status = handle.wait()?;
+        .stdout(Stdio::piped())
+        .stderr(stderr_mode)
+        .output()?;
 
-    let mut stderr = String::new();
-    error_reader.read_to_string(&mut stderr)?;
+    let exit_status = result.status;
+    let stderr = String::from_utf8(result.stderr).unwrap_or("Not displayable".into());
+    let stdout = String::from_utf8(result.stdout).unwrap_or("Not displayable".into());
 
     Ok(RunResult {
         exit_status,
@@ -463,3 +712,68 @@ pub fn run_successful(run: &RunResult, expected_output_file: &str) -> Result<(),
         Ok(())
     }
 }
+
+pub fn handle_results(globals: &Globals, results: Vec<FinalRunResult>) -> anyhow::Result<()> {
+    let errors = results.iter().filter_map(|r| {
+        if let FinalRunResult::Failed(c, r) = r {
+            Some((c, r))
+        } else {
+            None
+        }
+    });
+
+    let successes = results.iter().filter_map(|r| {
+        if let FinalRunResult::Success(c, r) = r {
+            Some((c, r))
+        } else {
+            None
+        }
+    });
+
+    let log_stdout_stderr = |level: Level| {
+        move |(command, result): (&CargoCommand, &RunResult)| {
+            let stdout = &result.stdout;
+            let stderr = &result.stderr;
+            if !stdout.is_empty() && !stderr.is_empty() {
+                log::log!(
+                    level,
+                    "Output for \"{command}\"\nStdout:\n{stdout}\nStderr:\n{stderr}"
+                );
+            } else if !stdout.is_empty() {
+                log::log!(
+                    level,
+                    "Output for \"{command}\":\nStdout:\n{}",
+                    stdout.trim_end()
+                );
+            } else if !stderr.is_empty() {
+                log::log!(
+                    level,
+                    "Output for \"{command}\"\nStderr:\n{}",
+                    stderr.trim_end()
+                );
+            }
+        }
+    };
+
+    successes.clone().for_each(log_stdout_stderr(Level::Debug));
+    errors.clone().for_each(log_stdout_stderr(Level::Error));
+
+    successes.for_each(|(cmd, _)| {
+        if globals.verbose > 0 {
+            info!("✅ Success: {cmd}\n    {}", cmd.as_cmd_string());
+        } else {
+            info!("✅ Success: {cmd}");
+        }
+    });
+
+    errors.clone().for_each(|(cmd, _)| {
+        error!("❌ Failed: {cmd}\n    {}", cmd.as_cmd_string());
+    });
+
+    let ecount = errors.count();
+    if ecount != 0 {
+        Err(anyhow::anyhow!("{ecount} commands failed."))
+    } else {
+        Ok(())
+    }
+}