From 50e1d2d129213e286fd4321197d0c9821a034d77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Tj=C3=A4der?= Date: Sun, 5 Feb 2023 01:50:29 +0100 Subject: [PATCH] Upgrade to clap v4, use log and env_logger --- xtask/Cargo.toml | 4 +- xtask/src/command.rs | 131 +++++++++++----- xtask/src/main.rs | 346 +++++++++++++++++++++++++++++++++---------- 3 files changed, 362 insertions(+), 119 deletions(-) diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index fa4f7d748e..ba0e134b2f 100644 --- a/xtask/Cargo.toml +++ b/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" diff --git a/xtask/src/command.rs b/xtask/src/command.rs index 6be14633c6..ed73b2d0ea 100644 --- a/xtask/src/command.rs +++ b/xtask/src/command.rs @@ -1,8 +1,11 @@ -use crate::Sizearguments; -use crate::{RunResult, TestRunError}; +use crate::{debug, RunResult, Sizearguments, 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![ - "+nightly", - self.name(), - "--example", - example, - "--target", - target, - "--features", - "test-critical-section", - ]; + 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_name) = features { - args.extend_from_slice(&["--features", feature_name]); + if let Some(feature) = features { + 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![ - "+nightly", - self.name(), - "--examples", - "--target", - target, - "--features", - "test-critical-section", - ]; + 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_name) = features { - args.extend_from_slice(&["--features", feature_name]); + if let Some(feature) = features { + 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![ - "+nightly", - self.name(), - "--example", - example, - "--target", - target, - "--features", - "test-critical-section", - ]; + 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_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 { 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 { /// 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: expected_output_file.clone(), + File::open(expected_output_file).map_err(|_| TestRunError::FileError { + 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 { diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 2d259dca04..47f3980668 100644 --- a/xtask/src/main.rs +++ b/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)] -struct Options { +const DEFAULT_FEATURES: Option<&str> = Some("test-critical-section"); + +#[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 - /// - /// * thumbv7m-none-eabi - #[structopt(short, long)] + /// thumbv6m-none-eabi + /// thumbv7m-none-eabi + #[arg(short, long)] target: Option, - /// 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` - #[structopt(short, long)] + /// If omitted all examples are run + /// + /// Example: `cargo xtask --example complex,spawn,init` + /// would include complex, spawn and init + #[arg(short, long, group = "example_group")] example: Option, - /// 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, + + /// 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, +} + +#[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` - #[structopt(short, long)] - size: bool, + /// Example: `cargo xtask size -- -A` + Size(Size), + + /// 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, } -#[derive(Clone, Debug, PartialEq, StructOpt)] +#[derive(Clone, Debug, PartialEq, Parser)] pub enum Sizearguments { - // `external_subcommand` tells structopt to put - // all the extra arguments into this Vec - #[structopt(external_subcommand)] + /// All remaining flags and options + #[command(external_subcommand)] Other(Vec), } @@ -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 = [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 target = &opts.target; - let check_size = opts.size; - let size_arguments = &opts.sizearguments; - let example = opts.example; + let env_logger_default_level = match cli.verbose { + 0 => Env::default().default_filter_or("error"), + 1 => Env::default().default_filter_or("info"), + 2 => Env::default().default_filter_or("debug"), + _ => 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 { - None => { - for t in targets { - println!("Testing all targets: {targets:?}"); - run_test(t, &examples, check_size, size_arguments)?; + init_build_dir()?; + #[allow(clippy::if_same_then_else)] + let cargoarg = if log_enabled!(Level::Trace) { + Some("-vv") + } 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) => { - if targets.contains(&target.as_str()) { - run_test(target, &examples, check_size, size_arguments)?; - } else { - eprintln!( - "The target you specified is not available. Available targets are:\ - \n{targets:?}\n", - ); - process::exit(1); + Some(Commands::Qemu { + overwrite_expected: overwrite, + }) => { + debug!("Testing on target(s): {targets:?}"); + for t in &targets { + info!("Testing for target: {t:?}"); + run_test(&cargoarg, t, &examples, overwrite)?; } } + 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, - size_arguments: &Option, + overwrite: bool, ) -> 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 { - arm_example(&CargoCommand::Size { - example, - target, - features: None, - mode: BuildMode::Release, - arguments: size_arguments.clone(), - })?; - } + + Ok(()) +} + +fn build_and_check_size( + cargoarg: &Option<&str>, + target: &str, + examples: &[String], + size_arguments: &Option, +) -> 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 { - run_successful(&cargo_run_result, expected_output_file)?; + // Create a file for the expected output if it does not exist or mismatches + 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 - let cargo_run_result = run_command(command)?; - println!("{}", cargo_run_result.output); + // cargo build --examples + let cargo_build_result = run_command(command)?; + 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)?; - println!("{}", cargo_run_result.output); + // cargo size + let cargo_size_result = run_command(command)?; + if !cargo_size_result.output.is_empty() { + info!("{}", cargo_size_result.output); + } Ok(()) } }