xtask: Clippy, optionally get cargo size output

4 files changed, 115 insertions, 43 deletions

diff --git a/rtic/xtask/Cargo.toml b/rtic/xtask/Cargo.toml
index 6053b91..fa4f7d7 100644
--- a/rtic/xtask/Cargo.toml
+++ b/rtic/xtask/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "xtask"
-version = "0.1.0"
-edition = "2018"
+version = "0.0.0"
+edition = "2021"
 publish = false
 
 [dependencies]
diff --git a/rtic/xtask/src/build.rs b/rtic/xtask/src/build.rs
index 148a9fd..a11b4e0 100644
--- a/rtic/xtask/src/build.rs
+++ b/rtic/xtask/src/build.rs
@@ -2,7 +2,7 @@ use std::{fs, path::Path};
 
 const HEX_BUILD_ROOT: &str = "ci/builds";
 
-/// make sure we're starting with a clean,but existing slate
+/// Make sure we're starting with a clean, but existing slate
 pub fn init_build_dir() -> anyhow::Result<()> {
     if Path::new(HEX_BUILD_ROOT).exists() {
         fs::remove_dir_all(HEX_BUILD_ROOT)
diff --git a/rtic/xtask/src/command.rs b/rtic/xtask/src/command.rs
index 4e90369..6be1463 100644
--- a/rtic/xtask/src/command.rs
+++ b/rtic/xtask/src/command.rs
@@ -1,3 +1,4 @@
+use crate::Sizearguments;
 use crate::{RunResult, TestRunError};
 use core::fmt;
 use os_pipe::pipe;
@@ -23,17 +24,20 @@ pub enum CargoCommand<'a> {
         features: Option<&'a str>,
         mode: BuildMode,
     },
-    // Size {
-    //     example_paths: Vec<&'a Path>,
-    // },
-    // Clean,
+    Size {
+        example: &'a str,
+        target: &'a str,
+        features: Option<&'a str>,
+        mode: BuildMode,
+        arguments: Option<Sizearguments>,
+    },
 }
 
 impl<'a> CargoCommand<'a> {
     fn name(&self) -> &str {
         match self {
             CargoCommand::Run { .. } => "run",
-            // CargoCommand::Size { example_paths: _ } => "rust-size",
+            CargoCommand::Size { .. } => "size",
             CargoCommand::BuildAll { .. } => "build",
         }
     }
@@ -87,24 +91,49 @@ impl<'a> CargoCommand<'a> {
                     args.push(flag);
                 }
                 args
-            } // CargoCommand::Size { example_paths } => {
-              //     example_paths.iter().map(|p| p.to_str().unwrap()).collect()
-              // }
+            }
+            CargoCommand::Size {
+                example,
+                target,
+                features,
+                mode,
+                arguments,
+            } => {
+                let mut args = vec![
+                    "+nightly",
+                    self.name(),
+                    "--example",
+                    example,
+                    "--target",
+                    target,
+                    "--features",
+                    "test-critical-section",
+                ];
+                if let Some(feature_name) = features {
+                    args.extend_from_slice(&["--features", feature_name]);
+                }
+                if let Some(flag) = mode.to_flag() {
+                    args.push(flag);
+                }
+                if let Some(Sizearguments::Other(arguments)) = arguments {
+                    // Arguments to cargo size must be passed after "--"
+                    args.extend_from_slice(&["--"]);
+                    for arg in arguments {
+                        args.extend_from_slice(&[arg.as_str()]);
+                    }
+                }
+                args
+            }
         }
     }
 
     pub fn command(&self) -> &str {
-        match self {
-            // we need to cheat a little here:
-            // `cargo size` can't be ran on multiple files, so we're using `rust-size` instead –
-            // which isn't a command that starts wizh `cargo`. So we're sneakily swapping them out :)
-            // CargoCommand::Size { .. } => "rust-size",
-            _ => "cargo",
-        }
+        "cargo"
     }
 }
 
 impl BuildMode {
+    #[allow(clippy::wrong_self_convention)]
     pub fn to_flag(&self) -> Option<&str> {
         match self {
             BuildMode::Release => Some("--release"),
@@ -120,7 +149,7 @@ impl fmt::Display for BuildMode {
             BuildMode::Debug => "debug",
         };
 
-        write!(f, "{}", cmd)
+        write!(f, "{cmd}")
     }
 }
 
diff --git a/rtic/xtask/src/main.rs b/rtic/xtask/src/main.rs
index 7eada91..dfe3194 100644
--- a/rtic/xtask/src/main.rs
+++ b/rtic/xtask/src/main.rs
@@ -23,8 +23,34 @@ const ARMV7M: &str = "thumbv7m-none-eabi";
 
 #[derive(Debug, StructOpt)]
 struct Options {
+    /// For which ARM target to build: v7 or v6
+    ///
+    /// The permissible targets are:
+    ///
+    /// * thumbv6m-none-eabi
+    ///
+    /// * thumbv7m-none-eabi
     #[structopt(short, long)]
     target: String,
+    /// Enables also running `cargo size` on the selected examples
+    ///
+    /// To pass options to `cargo size`, add `--` and then the following
+    /// arguments will be passed on
+    ///
+    /// Example: `cargo xtask --target thumbv7m-none-eabi -s -- -A`
+    #[structopt(short, long)]
+    size: bool,
+    /// Options to pass to `cargo size`
+    #[structopt(subcommand)]
+    sizearguments: Option<Sizearguments>,
+}
+
+#[derive(Clone, Debug, PartialEq, StructOpt)]
+pub enum Sizearguments {
+    // `external_subcommand` tells structopt to put
+    // all the extra arguments into this Vec
+    #[structopt(external_subcommand)]
+    Other(Vec<String>),
 }
 
 #[derive(Debug, Clone)]
@@ -46,16 +72,14 @@ impl fmt::Display for TestRunError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             TestRunError::FileCmpError { expected, got } => {
-                writeln!(f, "Differing output in files.")?;
-                writeln!(f, "")?;
+                writeln!(f, "Differing output in files.\n")?;
                 writeln!(f, "Expected:")?;
-                writeln!(f, "{}", expected)?;
-                writeln!(f, "")?;
+                writeln!(f, "{expected}\n")?;
                 writeln!(f, "Got:")?;
-                write!(f, "{}", got)
+                write!(f, "{got}")
             }
             TestRunError::FileError { file } => {
-                write!(f, "File error on: {}", file)
+                write!(f, "File error on: {file}")
             }
             TestRunError::CommandError(e) => {
                 write!(
@@ -65,7 +89,7 @@ impl fmt::Display for TestRunError {
                 )
             }
             TestRunError::PathConversionError(p) => {
-                write!(f, "Can't convert path from `OsString` to `String`: {:?}", p)
+                write!(f, "Can't convert path from `OsString` to `String`: {p:?}")
             }
             TestRunError::IncompatibleCommand => {
                 write!(f, "Can't run that command in this context")
@@ -80,8 +104,8 @@ fn main() -> anyhow::Result<()> {
     // if there's an `xtask` folder, we're *probably* at the root of this repo (we can't just
     // check the name of `env::current_dir()` because people might clone it into a different name)
     let probably_running_from_repo_root = Path::new("./xtask").exists();
-    if probably_running_from_repo_root == false {
-        bail!("xtasks can only be executed from the root of the `cortex-m-rtic` repository");
+    if !probably_running_from_repo_root {
+        bail!("xtasks can only be executed from the root of the `rtic` repository");
     }
 
     let targets = [ARMV7M, ARMV6M];
@@ -97,21 +121,22 @@ fn main() -> anyhow::Result<()> {
 
     let opts = Options::from_args();
     let target = &opts.target;
+    let check_size = opts.size;
+    let size_arguments = &opts.sizearguments;
 
     init_build_dir()?;
 
     if target == "all" {
         for t in targets {
-            run_test(t, &examples)?;
+            run_test(t, &examples, check_size, size_arguments)?;
         }
     } else if targets.contains(&target.as_str()) {
-        run_test(&target, &examples)?;
+        run_test(target, &examples, check_size, size_arguments)?;
     } else {
         eprintln!(
             "The target you specified is not available. Available targets are:\
-                    \n{:?}\n\
+                    \n{targets:?}\n\
                     as well as `all` (testing on all of the above)",
-            targets
         );
         process::exit(1);
     }
@@ -119,7 +144,12 @@ fn main() -> anyhow::Result<()> {
     Ok(())
 }
 
-fn run_test(target: &str, examples: &[String]) -> anyhow::Result<()> {
+fn run_test(
+    target: &str,
+    examples: &[String],
+    check_size: bool,
+    size_arguments: &Option<Sizearguments>,
+) -> anyhow::Result<()> {
     arm_example(&CargoCommand::BuildAll {
         target,
         features: None,
@@ -136,6 +166,17 @@ fn run_test(target: &str, examples: &[String]) -> anyhow::Result<()> {
 
         arm_example(&cmd)?;
     }
+    if check_size {
+        for example in examples {
+            arm_example(&CargoCommand::Size {
+                example,
+                target,
+                features: None,
+                mode: BuildMode::Release,
+                arguments: size_arguments.clone(),
+            })?;
+        }
+    }
 
     Ok(())
 }
@@ -144,33 +185,35 @@ fn run_test(target: &str, examples: &[String]) -> anyhow::Result<()> {
 fn arm_example(command: &CargoCommand) -> anyhow::Result<()> {
     match *command {
         CargoCommand::Run { example, .. } => {
-            let run_file = format!("{}.run", example);
+            let run_file = format!("{example}.run");
             let expected_output_file = ["ci", "expected", &run_file]
                 .iter()
                 .collect::<PathBuf>()
                 .into_os_string()
                 .into_string()
-                .map_err(|e| TestRunError::PathConversionError(e))?;
+                .map_err(TestRunError::PathConversionError)?;
 
             // command is either build or run
-            let cargo_run_result = run_command(&command)?;
+            let cargo_run_result = run_command(command)?;
             println!("{}", cargo_run_result.output);
 
-            match &command {
-                CargoCommand::Run { .. } => {
-                    run_successful(&cargo_run_result, expected_output_file)?;
-                }
-                _ => (),
+            if let CargoCommand::Run { .. } = &command {
+                run_successful(&cargo_run_result, expected_output_file)?;
             }
 
             Ok(())
         }
         CargoCommand::BuildAll { .. } => {
             // command is either build or run
-            let cargo_run_result = run_command(&command)?;
+            let cargo_run_result = run_command(command)?;
             println!("{}", cargo_run_result.output);
 
             Ok(())
-        } // _ => Err(anyhow::Error::new(TestRunError::IncompatibleCommand)),
+        }
+        CargoCommand::Size { .. } => {
+            let cargo_run_result = run_command(command)?;
+            println!("{}", cargo_run_result.output);
+            Ok(())
+        }
     }
 }