Upgrade to clap v4, use log and env_logger

xtask/Cargo.toml

@@ -7,4 +7,6 @@ publish = false
 [dependencies]
 anyhow = "1.0.43"
 os_pipe = "1.1.2"
-structopt = "0.3.22"
+clap = { version = "4", features = ["derive"] }
+env_logger = "0.10.0"
+log = "0.4.17"

xtask/src/command.rs

@@ -1,8 +1,11 @@
-use crate::Sizearguments;
+use crate::{debug, RunResult, Sizearguments, TestRunError};
-use crate::{RunResult, 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)]
@@ -14,17 +17,32 @@ pub enum BuildMode {
 #[derive(Debug)]
 pub enum CargoCommand<'a> {
     Run {
+        cargoarg: &'a Option<&'a str>,
+        example: &'a str,
+        target: &'a str,
+        features: Option<&'a str>,
+        mode: BuildMode,
+    },
+    Build {
+        cargoarg: &'a Option<&'a str>,
         example: &'a str,
         target: &'a str,
         features: Option<&'a str>,
         mode: BuildMode,
     },
     BuildAll {
+        cargoarg: &'a Option<&'a str>,
         target: &'a str,
         features: Option<&'a str>,
         mode: BuildMode,
     },
+    CheckAll {
+        cargoarg: &'a Option<&'a str>,
+        target: &'a str,
+        features: Option<&'a str>,
+    },
     Size {
+        cargoarg: &'a Option<&'a str>,
         example: &'a str,
         target: &'a str,
         features: Option<&'a str>,
@@ -37,32 +55,30 @@ impl<'a> CargoCommand<'a> {
     fn name(&self) -> &str {
         match self {
             CargoCommand::Run { .. } => "run",
+            CargoCommand::Build { .. } => "build",
             CargoCommand::Size { .. } => "size",
             CargoCommand::BuildAll { .. } => "build",
+            CargoCommand::CheckAll { .. } => "check",
         }
     }
 
     pub fn args(&self) -> Vec<&str> {
         match self {
             CargoCommand::Run {
+                cargoarg,
                 example,
                 target,
                 features,
                 mode,
             } => {
-                let mut args = vec![
+                let mut args = vec!["+nightly"];
-                    "+nightly",
+                if let Some(cargoarg) = cargoarg {
-                    self.name(),
+                    args.extend_from_slice(&[cargoarg]);
-                    "--example",
+                }
-                    example,
+                args.extend_from_slice(&[self.name(), "--example", example, "--target", target]);
-                    "--target",
-                    target,
-                    "--features",
-                    "test-critical-section",
-                ];
 
-                if let Some(feature_name) = features {
+                if let Some(feature) = features {
-                    args.extend_from_slice(&["--features", feature_name]);
+                    args.extend_from_slice(&["--features", feature]);
                 }
                 if let Some(flag) = mode.to_flag() {
                     args.push(flag);
@@ -70,22 +86,56 @@ impl<'a> CargoCommand<'a> {
                 args
             }
             CargoCommand::BuildAll {
+                cargoarg,
                 target,
                 features,
                 mode,
             } => {
-                let mut args = vec![
+                let mut args = vec!["+nightly"];
-                    "+nightly",
+                if let Some(cargoarg) = cargoarg {
-                    self.name(),
+                    args.extend_from_slice(&[cargoarg]);
-                    "--examples",
+                }
-                    "--target",
+                args.extend_from_slice(&[self.name(), "--examples", "--target", target]);
-                    target,
-                    "--features",
-                    "test-critical-section",
-                ];
 
-                if let Some(feature_name) = features {
+                if let Some(feature) = features {
-                    args.extend_from_slice(&["--features", feature_name]);
+                    args.extend_from_slice(&["--features", feature]);
+                }
+                if let Some(flag) = mode.to_flag() {
+                    args.push(flag);
+                }
+                args
+            }
+            CargoCommand::CheckAll {
+                cargoarg,
+                target,
+                features,
+            } => {
+                let mut args = vec!["+nightly"];
+                if let Some(cargoarg) = cargoarg {
+                    args.extend_from_slice(&[cargoarg]);
+                }
+                args.extend_from_slice(&[self.name(), "--examples", "--target", target]);
+
+                if let Some(feature) = features {
+                    args.extend_from_slice(&["--features", feature]);
+                }
+                args
+            }
+            CargoCommand::Build {
+                cargoarg,
+                example,
+                target,
+                features,
+                mode,
+            } => {
+                let mut args = vec!["+nightly"];
+                if let Some(cargoarg) = cargoarg {
+                    args.extend_from_slice(&[cargoarg]);
+                }
+                args.extend_from_slice(&[self.name(), "--example", example, "--target", target]);
+
+                if let Some(feature) = features {
+                    args.extend_from_slice(&["--features", feature]);
                 }
                 if let Some(flag) = mode.to_flag() {
                     args.push(flag);
@@ -93,22 +143,19 @@ impl<'a> CargoCommand<'a> {
                 args
             }
             CargoCommand::Size {
+                cargoarg,
                 example,
                 target,
                 features,
                 mode,
                 arguments,
             } => {
-                let mut args = vec![
+                let mut args = vec!["+nightly"];
-                    "+nightly",
+                if let Some(cargoarg) = cargoarg {
-                    self.name(),
+                    args.extend_from_slice(&[cargoarg]);
-                    "--example",
+                }
-                    example,
+                args.extend_from_slice(&[self.name(), "--example", example, "--target", target]);
-                    "--target",
+
-                    target,
-                    "--features",
-                    "test-critical-section",
-                ];
                 if let Some(feature_name) = features {
                     args.extend_from_slice(&["--features", feature_name]);
                 }
@@ -155,11 +202,13 @@ impl fmt::Display for BuildMode {
 
 pub fn run_command(command: &CargoCommand) -> anyhow::Result<RunResult> {
     let (mut reader, writer) = pipe()?;
-    println!("👟 {} {}", command.command(), command.args().join(" "));
+    debug!("👟 {} {}", command.command(), command.args().join(" "));
 
     let mut handle = Command::new(command.command())
         .args(command.args())
         .stdout(writer)
+        // Throw away stderr, TODO
+        .stderr(Stdio::null())
         .spawn()?;
 
     // retrieve output and clean up
@@ -176,16 +225,16 @@ pub fn run_command(command: &CargoCommand) -> anyhow::Result<RunResult> {
 /// Check if `run` was successful.
 /// returns Ok in case the run went as expected,
 /// Err otherwise
-pub fn run_successful(run: &RunResult, expected_output_file: String) -> Result<(), TestRunError> {
+pub fn run_successful(run: &RunResult, expected_output_file: &str) -> Result<(), TestRunError> {
     let mut file_handle =
-        File::open(expected_output_file.clone()).map_err(|_| TestRunError::FileError {
+        File::open(expected_output_file).map_err(|_| TestRunError::FileError {
-            file: expected_output_file.clone(),
+            file: expected_output_file.to_owned(),
         })?;
     let mut expected_output = String::new();
     file_handle
         .read_to_string(&mut expected_output)
         .map_err(|_| TestRunError::FileError {
-            file: expected_output_file.clone(),
+            file: expected_output_file.to_owned(),
         })?;
 
     if expected_output != run.output {

xtask/src/main.rs

@@ -2,16 +2,21 @@ mod build;
 mod command;
 
 use anyhow::bail;
+use clap::{Parser, Subcommand};
 use core::fmt;
 use std::{
     error::Error,
     ffi::OsString,
+    fs::File,
+    io::prelude::*,
     path::{Path, PathBuf},
     process,
     process::ExitStatus,
     str,
 };
-use structopt::StructOpt;
+
+use env_logger::Env;
+use log::{debug, error, info, log_enabled, trace, Level};
 
 use crate::{
     build::init_build_dir,
@@ -21,41 +26,88 @@ use crate::{
 const ARMV6M: &str = "thumbv6m-none-eabi";
 const ARMV7M: &str = "thumbv7m-none-eabi";
 
-#[derive(Debug, StructOpt)]
+const DEFAULT_FEATURES: Option<&str> = Some("test-critical-section");
-struct Options {
+
+#[derive(Parser)]
+#[command(author, version, about, long_about = None)]
+/// RTIC xtask powered testing toolbox
+struct Cli {
     /// For which ARM target to build: v7 or v6
     ///
+    /// Defaults to all targets if omitted.
     /// The permissible targets are:
-    /// * all
     ///
-    /// * thumbv6m-none-eabi
+    /// thumbv6m-none-eabi
-    ///
+    /// thumbv7m-none-eabi
-    /// * thumbv7m-none-eabi
+    #[arg(short, long)]
-    #[structopt(short, long)]
     target: Option<String>,
-    /// Example to run, by default all examples are run
+
+    /// List of comma separated examples to run, all others are excluded
     ///
-    /// Example: `cargo xtask --example complex`
+    /// If omitted all examples are run
-    #[structopt(short, long)]
+    ///
+    /// Example: `cargo xtask --example complex,spawn,init`
+    /// would include complex, spawn and init
+    #[arg(short, long, group = "example_group")]
     example: Option<String>,
-    /// Enables also running `cargo size` on the selected examples
+
+    /// List of comma separated examples to exclude, all others are included
+    ///
+    /// If omitted all examples are run
+    ///
+    /// Example: `cargo xtask --excludeexample complex,spawn,init`
+    /// would exclude complex, spawn and init
+    #[arg(long, group = "example_group")]
+    exampleexclude: Option<String>,
+
+    /// Enable more verbose output, repeat up to `-vvv` for even more
+    #[arg(short, long, action = clap::ArgAction::Count)]
+    verbose: u8,
+
+    /// Subcommand picking which kind of operation
+    ///
+    /// If omitted run all tests
+    #[command(subcommand)]
+    command: Option<Commands>,
+}
+
+#[derive(Debug, Subcommand)]
+enum Commands {
+    /// Run `cargo size` on selected or all examples
     ///
     /// To pass options to `cargo size`, add `--` and then the following
     /// arguments will be passed on
     ///
-    /// Example: `cargo xtask -s -- -A`
+    /// Example: `cargo xtask size -- -A`
-    #[structopt(short, long)]
+    Size(Size),
-    size: bool,
+
+    /// Run examples in QEMU and compare against expected output
+    ///
+    /// Example runtime output is matched against `rtic/ci/expected/`
+    Qemu {
+        /// If expected output is missing or mismatching, recreate the file
+        ///
+        /// This overwrites only missing or mismatching
+        #[arg(long)]
+        overwrite_expected: bool,
+    },
+    /// Build all examples
+    Build,
+    /// Check all examples
+    Check,
+}
+
+#[derive(Debug, Parser)]
+struct Size {
     /// Options to pass to `cargo size`
-    #[structopt(subcommand)]
+    #[command(subcommand)]
     sizearguments: Option<Sizearguments>,
 }
 
-#[derive(Clone, Debug, PartialEq, StructOpt)]
+#[derive(Clone, Debug, PartialEq, Parser)]
 pub enum Sizearguments {
-    // `external_subcommand` tells structopt to put
+    /// All remaining flags and options
-    // all the extra arguments into this Vec
+    #[command(external_subcommand)]
-    #[structopt(external_subcommand)]
     Other(Vec<String>),
 }
 
@@ -85,7 +137,7 @@ impl fmt::Display for TestRunError {
                 write!(f, "{got}")
             }
             TestRunError::FileError { file } => {
-                write!(f, "File error on: {file}")
+                write!(f, "File error on: {file}\nSee flag overwrite.")
             }
             TestRunError::CommandError(e) => {
                 write!(
@@ -114,7 +166,7 @@ fn main() -> anyhow::Result<()> {
         bail!("xtasks can only be executed from the root of the `rtic` repository");
     }
 
-    let targets = [ARMV7M, ARMV6M];
+    let mut targets: Vec<String> = [ARMV7M.to_owned(), ARMV6M.to_owned()].to_vec();
 
     let mut examples: Vec<_> = std::fs::read_dir("./rtic/examples")?
         .filter_map(|p| p.ok())
@@ -123,92 +175,196 @@ fn main() -> anyhow::Result<()> {
         .map(|path| path.file_stem().unwrap().to_str().unwrap().to_string())
         .collect();
 
-    println!("examples: {examples:?}");
+    let cli = Cli::parse();
 
-    let opts = Options::from_args();
+    let env_logger_default_level = match cli.verbose {
-    let target = &opts.target;
+        0 => Env::default().default_filter_or("error"),
-    let check_size = opts.size;
+        1 => Env::default().default_filter_or("info"),
-    let size_arguments = &opts.sizearguments;
+        2 => Env::default().default_filter_or("debug"),
-    let example = opts.example;
+        _ => Env::default().default_filter_or("trace"),
+    };
+    env_logger::Builder::from_env(env_logger_default_level)
+        .format_module_path(false)
+        .format_timestamp(None)
+        .init();
+
+    trace!("default logging level: {0}", cli.verbose);
+    trace!("examples: {examples:?}");
+
+    let target = cli.target;
+    let example = cli.example;
 
     if let Some(example) = example {
         if examples.contains(&example) {
-            println!("\nTesting example: {example}");
+            info!("Testing example: {example}");
             // If we managed to filter, set the examples to test to only this one
             examples = vec![example]
         } else {
-            eprintln!(
+            error!(
                 "\nThe example you specified is not available. Available examples are:\
                     \n{examples:#?}\n\
-             By default all examples are tested.",
+             By default if example flag is emitted, all examples are tested.",
+            );
+            process::exit(1);
+        }
+    }
+    if let Some(target) = target {
+        if targets.contains(&target) {
+            debug!("\nTesting target: {target}");
+            // If we managed to filter, set the targets to test to only this one
+            targets = vec![target]
+        } else {
+            error!(
+                "\nThe target you specified is not available. Available targets are:\
+                    \n{targets:#?}\n\
+             By default all targets are tested.",
             );
             process::exit(1);
         }
     }
-    init_build_dir()?;
 
-    match target {
+    init_build_dir()?;
-        None => {
+    #[allow(clippy::if_same_then_else)]
-            for t in targets {
+    let cargoarg = if log_enabled!(Level::Trace) {
-                println!("Testing all targets: {targets:?}");
+        Some("-vv")
-                run_test(t, &examples, check_size, size_arguments)?;
+    } else if log_enabled!(Level::Debug) {
+        Some("-v")
+    } else if log_enabled!(Level::Info) {
+        None
+    } else if log_enabled!(Level::Warn) || log_enabled!(Level::Error) {
+        Some("--quiet")
+    } else {
+        // Off case
+        Some("--quiet")
+    };
+
+    match cli.command {
+        Some(Commands::Size(arguments)) => {
+            debug!("Measuring on target(s): {targets:?}");
+            for t in &targets {
+                info!("Measuring for target: {t:?}");
+                build_and_check_size(&cargoarg, t, &examples, &arguments.sizearguments)?;
             }
         }
-        Some(target) => {
+        Some(Commands::Qemu {
-            if targets.contains(&target.as_str()) {
+            overwrite_expected: overwrite,
-                run_test(target, &examples, check_size, size_arguments)?;
+        }) => {
-            } else {
+            debug!("Testing on target(s): {targets:?}");
-                eprintln!(
+            for t in &targets {
-                    "The target you specified is not available. Available targets are:\
+                info!("Testing for target: {t:?}");
-                            \n{targets:?}\n",
+                run_test(&cargoarg, t, &examples, overwrite)?;
-                );
-                process::exit(1);
             }
         }
+        Some(Commands::Build) => {
+            debug!("Building for target(s): {targets:?}");
+            for t in &targets {
+                info!("Building for target: {t:?}");
+                build_all(&cargoarg, t)?;
+            }
+        }
+        Some(Commands::Check) => {
+            debug!("Checking on target(s): {targets:?}");
+            for t in &targets {
+                info!("Checking on target: {t:?}");
+                check_all(&cargoarg, t)?;
+            }
+        }
+        None => {
+            todo!();
+        }
     }
 
     Ok(())
 }
 
+fn build_all(cargoarg: &Option<&str>, target: &str) -> anyhow::Result<()> {
+    arm_example(
+        &CargoCommand::BuildAll {
+            cargoarg,
+            target,
+            features: DEFAULT_FEATURES,
+            mode: BuildMode::Release,
+        },
+        false,
+    )?;
+    Ok(())
+}
+
+fn check_all(cargoarg: &Option<&str>, target: &str) -> anyhow::Result<()> {
+    arm_example(
+        &CargoCommand::CheckAll {
+            cargoarg,
+            target,
+            features: DEFAULT_FEATURES,
+        },
+        false,
+    )?;
+    Ok(())
+}
+
 fn run_test(
+    cargoarg: &Option<&str>,
     target: &str,
     examples: &[String],
-    check_size: bool,
+    overwrite: bool,
-    size_arguments: &Option<Sizearguments>,
 ) -> anyhow::Result<()> {
-    arm_example(&CargoCommand::BuildAll {
-        target,
-        features: None,
-        mode: BuildMode::Release,
-    })?;
-
     for example in examples {
-        let cmd = CargoCommand::Run {
+        let cmd = CargoCommand::Build {
+            cargoarg: &Some("--quiet"),
             example,
             target,
-            features: None,
+            features: DEFAULT_FEATURES,
+            mode: BuildMode::Release,
+        };
+        arm_example(&cmd, false)?;
+
+        let cmd = CargoCommand::Run {
+            cargoarg,
+            example,
+            target,
+            features: DEFAULT_FEATURES,
             mode: BuildMode::Release,
         };
 
-        arm_example(&cmd)?;
+        arm_example(&cmd, overwrite)?;
     }
-    if check_size {
+
-        for example in examples {
+    Ok(())
-            arm_example(&CargoCommand::Size {
+}
-                example,
+
-                target,
+fn build_and_check_size(
-                features: None,
+    cargoarg: &Option<&str>,
-                mode: BuildMode::Release,
+    target: &str,
-                arguments: size_arguments.clone(),
+    examples: &[String],
-            })?;
+    size_arguments: &Option<Sizearguments>,
-        }
+) -> anyhow::Result<()> {
+    for example in examples {
+        // Make sure the requested example(s) are built
+        let cmd = CargoCommand::Build {
+            cargoarg: &Some("--quiet"),
+            example,
+            target,
+            features: DEFAULT_FEATURES,
+            mode: BuildMode::Release,
+        };
+        arm_example(&cmd, false)?;
+
+        let cmd = CargoCommand::Size {
+            cargoarg,
+            example,
+            target,
+            features: DEFAULT_FEATURES,
+            mode: BuildMode::Release,
+            arguments: size_arguments.clone(),
+        };
+        arm_example(&cmd, false)?;
     }
 
     Ok(())
 }
 
 // run example binary `example`
-fn arm_example(command: &CargoCommand) -> anyhow::Result<()> {
+fn arm_example(command: &CargoCommand, overwrite: bool) -> anyhow::Result<()> {
     match *command {
         CargoCommand::Run { example, .. } => {
             let run_file = format!("{example}.run");
@@ -219,26 +375,62 @@ fn arm_example(command: &CargoCommand) -> anyhow::Result<()> {
                 .into_string()
                 .map_err(TestRunError::PathConversionError)?;
 
-            // command is either build or run
+            // cargo run <..>
             let cargo_run_result = run_command(command)?;
-            println!("{}", cargo_run_result.output);
+            info!("{}", cargo_run_result.output);
 
-            if let CargoCommand::Run { .. } = &command {
+            // Create a file for the expected output if it does not exist or mismatches
-                run_successful(&cargo_run_result, expected_output_file)?;
+            if overwrite {
+                let result = run_successful(&cargo_run_result, &expected_output_file);
+                if let Err(e) = result {
+                    // FileError means the file did not exist or was unreadable
+                    error!("Error: {e}");
+                    let mut file_handle = File::create(&expected_output_file).map_err(|_| {
+                        TestRunError::FileError {
+                            file: expected_output_file.clone(),
+                        }
+                    })?;
+                    info!("Creating/updating file: {expected_output_file}");
+                    file_handle.write_all(cargo_run_result.output.as_bytes())?;
+                };
+            } else {
+                run_successful(&cargo_run_result, &expected_output_file)?;
+            }
+            Ok(())
+        }
+        CargoCommand::Build { .. } => {
+            // cargo run <..>
+            let cargo_build_result = run_command(command)?;
+            if !cargo_build_result.output.is_empty() {
+                info!("{}", cargo_build_result.output);
             }
 
             Ok(())
         }
         CargoCommand::BuildAll { .. } => {
-            // command is either build or run
+            // cargo build --examples
-            let cargo_run_result = run_command(command)?;
+            let cargo_build_result = run_command(command)?;
-            println!("{}", cargo_run_result.output);
+            if !cargo_build_result.output.is_empty() {
+                info!("{}", cargo_build_result.output);
+            }
+
+            Ok(())
+        }
+        CargoCommand::CheckAll { .. } => {
+            // cargo check --examples
+            let cargo_check_result = run_command(command)?;
+            if !cargo_check_result.output.is_empty() {
+                info!("{}", cargo_check_result.output);
+            }
 
             Ok(())
         }
         CargoCommand::Size { .. } => {
-            let cargo_run_result = run_command(command)?;
+            // cargo size
-            println!("{}", cargo_run_result.output);
+            let cargo_size_result = run_command(command)?;
+            if !cargo_size_result.output.is_empty() {
+                info!("{}", cargo_size_result.output);
+            }
             Ok(())
         }
     }