Deny on warnings in xtasks

10 files changed, 120 insertions, 55 deletions

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 4fa2b807..ad0f5eda 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -25,9 +25,6 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v3
 
-      - name: Fail on warnings
-        run: find . -type f -name lib.rs -execdir sed -i 's,//deny_warnings_placeholder_for_ci,#![deny(warnings)],' {} +
-
       - name: Cache Dependencies
         uses: Swatinem/rust-cache@v2
 
@@ -62,13 +59,10 @@ jobs:
           rustup target add thumbv8m.base-none-eabi
           rustup target add thumbv8m.main-none-eabi
 
-      - name: Fail on warnings
-        run: find . -type f -name lib.rs -execdir sed -i 's,//deny_warnings_placeholder_for_ci,#![deny(warnings)],' {} +
-
       - name: Cache Dependencies
         uses: Swatinem/rust-cache@v2
 
-      - run: cargo xtask --backend ${{ matrix.backend }} check 
+      - run: cargo xtask --deny-warnings --backend ${{ matrix.backend }} check
 
   # Clippy
   clippy:
@@ -101,13 +95,10 @@ jobs:
       - name: Add Rust component clippy
         run: rustup component add clippy
 
-      - name: Fail on warnings
-        run: find . -type f -name lib.rs -execdir sed -i 's,//deny_warnings_placeholder_for_ci,#![deny(warnings)],' {} +
-
       - name: Cache Dependencies
         uses: Swatinem/rust-cache@v2
 
-      - run: cargo xtask --backend ${{ matrix.backend }} clippy
+      - run: cargo xtask --deny-warnings --backend ${{ matrix.backend }} clippy
 
   # Verify all examples, checks
   checkexamples:
@@ -219,12 +210,8 @@ jobs:
           sudo apt update
           sudo apt install -y qemu-system-arm
 
-      - name: Fail on warnings
-        working-directory: ./rtic
-        run: sed -i 's,//deny_warnings_placeholder_for_ci,#![deny(warnings)],' src/lib.rs
-
       - name: Run-pass tests
-        run: cargo xtask --backend ${{ matrix.backend }} qemu 
+        run: cargo xtask --deny-warnings --backend ${{ matrix.backend }} qemu
 
   # Run test suite
   tests:
@@ -260,11 +247,8 @@ jobs:
           rustup target add thumbv8m.base-none-eabi
           rustup target add thumbv8m.main-none-eabi
 
-      - name: Fail on warnings
-        run: find . -type f -name lib.rs -execdir sed -i 's,//deny_warnings_placeholder_for_ci,#![deny(warnings)],' {} +
-
       - name: Run cargo test
-        run: cargo xtask --backend ${{ matrix.backend }} test ${{ matrix.package }}
+        run: cargo xtask --deny-warnings --backend ${{ matrix.backend }} test ${{ matrix.package }}
 
   # Build documentation, check links
   docs:
diff --git a/rtic-common/src/lib.rs b/rtic-common/src/lib.rs
index 03d03063..e8f5af7e 100644
--- a/rtic-common/src/lib.rs
+++ b/rtic-common/src/lib.rs
@@ -2,7 +2,6 @@
 
 #![no_std]
 #![deny(missing_docs)]
-//deny_warnings_placeholder_for_ci
 
 #[cfg(test)]
 #[macro_use]
diff --git a/rtic-macros/src/lib.rs b/rtic-macros/src/lib.rs
index cd2a9245..e0043d91 100644
--- a/rtic-macros/src/lib.rs
+++ b/rtic-macros/src/lib.rs
@@ -2,7 +2,6 @@
     html_logo_url = "https://raw.githubusercontent.com/rtic-rs/rtic/master/book/en/src/RTIC.svg",
     html_favicon_url = "https://raw.githubusercontent.com/rtic-rs/rtic/master/book/en/src/RTIC.svg"
 )]
-//deny_warnings_placeholder_for_ci
 
 use proc_macro::TokenStream;
 use std::{env, fs, path::Path};
diff --git a/rtic-monotonics/src/lib.rs b/rtic-monotonics/src/lib.rs
index 04ce4e24..714200a6 100644
--- a/rtic-monotonics/src/lib.rs
+++ b/rtic-monotonics/src/lib.rs
@@ -2,7 +2,6 @@
 
 #![no_std]
 #![deny(missing_docs)]
-//deny_warnings_placeholder_for_ci
 #![allow(incomplete_features)]
 #![feature(async_fn_in_trait)]
 
diff --git a/rtic-sync/src/lib.rs b/rtic-sync/src/lib.rs
index ac062205..fd8b6c3a 100644
--- a/rtic-sync/src/lib.rs
+++ b/rtic-sync/src/lib.rs
@@ -2,7 +2,6 @@
 
 #![no_std]
 #![deny(missing_docs)]
-//deny_warnings_placeholder_for_ci
 
 pub mod arbiter;
 pub mod channel;
diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs
index 1ed1e9d6..7c0c7645 100644
--- a/rtic-time/src/lib.rs
+++ b/rtic-time/src/lib.rs
@@ -2,7 +2,6 @@
 
 #![no_std]
 #![deny(missing_docs)]
-//deny_warnings_placeholder_for_ci
 #![allow(incomplete_features)]
 #![feature(async_fn_in_trait)]
 
diff --git a/rtic/src/lib.rs b/rtic/src/lib.rs
index ac05d93d..53fe2875 100644
--- a/rtic/src/lib.rs
+++ b/rtic/src/lib.rs
@@ -30,7 +30,6 @@
     html_logo_url = "https://raw.githubusercontent.com/rtic-rs/rtic/master/book/en/src/RTIC.svg",
     html_favicon_url = "https://raw.githubusercontent.com/rtic-rs/rtic/master/book/en/src/RTIC.svg"
 )]
-//deny_warnings_placeholder_for_ci
 #![allow(clippy::inline_always)]
 
 pub use rtic_core::{prelude as mutex_prelude, Exclusive, Mutex};
diff --git a/xtask/src/argument_parsing.rs b/xtask/src/argument_parsing.rs
index 8c8f7d2a..d7c0262f 100644
--- a/xtask/src/argument_parsing.rs
+++ b/xtask/src/argument_parsing.rs
@@ -94,7 +94,11 @@ impl Package {
 pub struct TestMetadata {}
 
 impl TestMetadata {
-    pub fn match_package(package: Package, backend: Backends) -> CargoCommand<'static> {
+    pub fn match_package(
+        deny_warnings: bool,
+        package: Package,
+        backend: Backends,
+    ) -> CargoCommand<'static> {
         match package {
             Package::Rtic => {
                 let features = format!(
@@ -107,32 +111,38 @@ impl TestMetadata {
                     package: Some(package.name()),
                     features,
                     test: Some("ui".to_owned()),
+                    deny_warnings,
                 }
             }
             Package::RticMacros => CargoCommand::Test {
                 package: Some(package.name()),
                 features: Some(backend.to_rtic_macros_feature().to_owned()),
                 test: None,
+                deny_warnings,
             },
             Package::RticSync => CargoCommand::Test {
                 package: Some(package.name()),
                 features: Some("testing".to_owned()),
                 test: None,
+                deny_warnings,
             },
             Package::RticCommon => CargoCommand::Test {
                 package: Some(package.name()),
                 features: Some("testing".to_owned()),
                 test: None,
+                deny_warnings,
             },
             Package::RticMonotonics => CargoCommand::Test {
                 package: Some(package.name()),
                 features: None,
                 test: None,
+                deny_warnings,
             },
             Package::RticTime => CargoCommand::Test {
                 package: Some(package.name()),
                 features: Some("critical-section/std".into()),
                 test: None,
+                deny_warnings,
             },
         }
     }
@@ -192,6 +202,10 @@ pub enum BuildOrCheck {
 
 #[derive(Parser, Clone)]
 pub struct Globals {
+    /// Error out on warnings
+    #[arg(short = 'D', long)]
+    pub deny_warnings: bool,
+
     /// For which backend to build.
     #[arg(value_enum, short, default_value = "thumbv7", long, global = true)]
     pub backend: Option<Backends>,
diff --git a/xtask/src/cargo_command.rs b/xtask/src/cargo_command.rs
index 9cf4f650..09487cb6 100644
--- a/xtask/src/cargo_command.rs
+++ b/xtask/src/cargo_command.rs
@@ -28,6 +28,7 @@ pub enum CargoCommand<'a> {
         features: Option<String>,
         mode: BuildMode,
         dir: Option<PathBuf>,
+        deny_warnings: bool,
     },
     ExampleBuild {
         cargoarg: &'a Option<&'a str>,
@@ -36,6 +37,7 @@ pub enum CargoCommand<'a> {
         features: Option<String>,
         mode: BuildMode,
         dir: Option<PathBuf>,
+        deny_warnings: bool,
     },
     ExampleCheck {
         cargoarg: &'a Option<&'a str>,
@@ -43,6 +45,7 @@ pub enum CargoCommand<'a> {
         target: Option<Target<'a>>,
         features: Option<String>,
         mode: BuildMode,
+        deny_warnings: bool,
     },
     Build {
         cargoarg: &'a Option<&'a str>,
@@ -51,6 +54,7 @@ pub enum CargoCommand<'a> {
         features: Option<String>,
         mode: BuildMode,
         dir: Option<PathBuf>,
+        deny_warnings: bool,
     },
     Check {
         cargoarg: &'a Option<&'a str>,
@@ -59,6 +63,7 @@ pub enum CargoCommand<'a> {
         features: Option<String>,
         mode: BuildMode,
         dir: Option<PathBuf>,
+        deny_warnings: bool,
     },
     Clippy {
         cargoarg: &'a Option<&'a str>,
@@ -81,6 +86,7 @@ pub enum CargoCommand<'a> {
         package: Option<String>,
         features: Option<String>,
         test: Option<String>,
+        deny_warnings: bool,
     },
     Book {
         arguments: Option<ExtraArguments>,
@@ -123,6 +129,7 @@ impl core::fmt::Display for CargoCommand<'_> {
         }
 
         fn details(
+            deny_warnings: bool,
             target: &Option<Target>,
             mode: Option<&BuildMode>,
             features: &Option<String>,
@@ -150,14 +157,20 @@ impl core::fmt::Display for CargoCommand<'_> {
                 format!("debug")
             };
 
+            let deny_warnings = if deny_warnings {
+                format!("deny warnings, ")
+            } else {
+                format!("")
+            };
+
             if cargoarg.is_some() && path.is_some() {
-                format!("({target}, {mode}, {feat}, {carg}, {in_dir})")
+                format!("({deny_warnings}{target}, {mode}, {feat}, {carg}, {in_dir})")
             } else if cargoarg.is_some() {
-                format!("({target}, {mode}, {feat}, {carg})")
+                format!("({deny_warnings}{target}, {mode}, {feat}, {carg})")
             } else if path.is_some() {
-                format!("({target}, {mode}, {feat}, {in_dir})")
+                format!("({deny_warnings}{target}, {mode}, {feat}, {in_dir})")
             } else {
-                format!("({target}, {mode}, {feat})")
+                format!("({deny_warnings}{target}, {mode}, {feat})")
             }
         }
 
@@ -173,7 +186,7 @@ impl core::fmt::Display for CargoCommand<'_> {
                 write!(
                     f,
                     "Run example {example} {}",
-                    details(target, Some(mode), features, cargoarg, dir.as_ref())
+                    details(false, target, Some(mode), features, cargoarg, dir.as_ref())
                 )
             }
             CargoCommand::Qemu {
@@ -183,8 +196,10 @@ impl core::fmt::Display for CargoCommand<'_> {
                 features,
                 mode,
                 dir,
+                deny_warnings,
             } => {
-                let details = details(target, Some(mode), features, cargoarg, dir.as_ref());
+                let warns = *deny_warnings;
+                let details = details(warns, target, Some(mode), features, cargoarg, dir.as_ref());
                 write!(f, "Run example {example} in QEMU {details}",)
             }
             CargoCommand::ExampleBuild {
@@ -194,8 +209,10 @@ impl core::fmt::Display for CargoCommand<'_> {
                 features,
                 mode,
                 dir,
+                deny_warnings,
             } => {
-                let details = details(target, Some(mode), features, cargoarg, dir.as_ref());
+                let warns = *deny_warnings;
+                let details = details(warns, target, Some(mode), features, cargoarg, dir.as_ref());
                 write!(f, "Build example {example} {details}",)
             }
             CargoCommand::ExampleCheck {
@@ -204,10 +221,11 @@ impl core::fmt::Display for CargoCommand<'_> {
                 target,
                 features,
                 mode,
+                deny_warnings,
             } => write!(
                 f,
                 "Check example {example} {}",
-                details(target, Some(mode), features, cargoarg, None)
+                details(*deny_warnings, target, Some(mode), features, cargoarg, None)
             ),
             CargoCommand::Build {
                 cargoarg,
@@ -216,12 +234,14 @@ impl core::fmt::Display for CargoCommand<'_> {
                 features,
                 mode,
                 dir,
+                deny_warnings,
             } => {
                 let package = p(package);
+                let warns = *deny_warnings;
                 write!(
                     f,
                     "Build {package} {}",
-                    details(target, Some(mode), features, cargoarg, dir.as_ref())
+                    details(warns, target, Some(mode), features, cargoarg, dir.as_ref())
                 )
             }
 
@@ -232,12 +252,14 @@ impl core::fmt::Display for CargoCommand<'_> {
                 features,
                 mode,
                 dir,
+                deny_warnings,
             } => {
                 let package = p(package);
+                let warns = *deny_warnings;
                 write!(
                     f,
                     "Check {package} {}",
-                    details(target, Some(mode), features, cargoarg, dir.as_ref())
+                    details(warns, target, Some(mode), features, cargoarg, dir.as_ref())
                 )
             }
             CargoCommand::Clippy {
@@ -247,14 +269,9 @@ impl core::fmt::Display for CargoCommand<'_> {
                 features,
                 deny_warnings,
             } => {
-                let details = details(target, None, features, cargoarg, None);
+                let details = details(*deny_warnings, target, None, features, cargoarg, None);
                 let package = p(package);
-                let deny_warns = if *deny_warnings {
-                    format!(" (deny warnings)")
-                } else {
-                    format!("")
-                };
-                write!(f, "Clippy{deny_warns} {package} {details}")
+                write!(f, "Clippy {package} {details}")
             }
             CargoCommand::Format {
                 cargoarg,
@@ -297,14 +314,20 @@ impl core::fmt::Display for CargoCommand<'_> {
                 package,
                 features,
                 test,
+                deny_warnings,
             } => {
                 let p = p(package);
                 let test = test
                     .clone()
                     .map(|t| format!("test {t}"))
                     .unwrap_or("all tests".into());
+                let deny_warnings = if *deny_warnings {
+                    format!("deny warnings, ")
+                } else {
+                    format!("")
+                };
                 let feat = feat(features);
-                write!(f, "Run {test} in {p} (features: {feat})")
+                write!(f, "Run {test} in {p} ({deny_warnings}features: {feat})")
             }
             CargoCommand::Book { arguments: _ } => write!(f, "Build the book"),
             CargoCommand::ExampleSize {
@@ -316,7 +339,7 @@ impl core::fmt::Display for CargoCommand<'_> {
                 arguments: _,
                 dir,
             } => {
-                let details = details(target, Some(mode), features, cargoarg, dir.as_ref());
+                let details = details(false, target, Some(mode), features, cargoarg, dir.as_ref());
                 write!(f, "Compute size of example {example} {details}")
             }
         }
@@ -459,6 +482,8 @@ impl<'a> CargoCommand<'a> {
                 dir: _,
                 // Target is added by build_args
                 target: _,
+                // deny_warnings is exposed through `rustflags`
+                deny_warnings: _,
             } => self.build_args(
                 true,
                 cargoarg,
@@ -471,10 +496,12 @@ impl<'a> CargoCommand<'a> {
                 package,
                 features,
                 mode,
-                // Dir is exposed through `chdir`
-                dir: _,
                 // Target is added by build_args
                 target: _,
+                // Dir is exposed through `chdir`
+                dir: _,
+                // deny_warnings is exposed through `rustflags`
+                deny_warnings: _,
             } => self.build_args(true, cargoarg, features, Some(mode), p(package)),
             CargoCommand::Check {
                 cargoarg,
@@ -485,23 +512,25 @@ impl<'a> CargoCommand<'a> {
                 dir: _,
                 // Target is added by build_args
                 target: _,
+                // deny_warnings is exposed through `rustflags`
+                deny_warnings: _,
             } => self.build_args(true, cargoarg, features, Some(mode), p(package)),
             CargoCommand::Clippy {
                 cargoarg,
                 package,
                 features,
-                deny_warnings,
                 // Target is added by build_args
                 target: _,
+                deny_warnings,
             } => {
-                let package = p(package);
-                let extra = if *deny_warnings {
-                    vec!["--", "-D", "warnings"].into_iter()
+                let deny_warnings = if *deny_warnings {
+                    vec!["--", "-D", "warnings"]
                 } else {
-                    vec![].into_iter()
+                    vec![]
                 };
 
-                self.build_args(true, cargoarg, features, None, package.chain(extra))
+                let extra = p(package).chain(deny_warnings);
+                self.build_args(true, cargoarg, features, None, extra)
             }
             CargoCommand::Doc {
                 cargoarg,
@@ -515,6 +544,8 @@ impl<'a> CargoCommand<'a> {
                 package,
                 features,
                 test,
+                // deny_warnings is exposed through `rustflags`
+                deny_warnings: _,
             } => {
                 let extra = if let Some(test) = test {
                     vec!["--test", test]
@@ -564,6 +595,8 @@ impl<'a> CargoCommand<'a> {
                 dir: _,
                 // Target is added by build_args
                 target: _,
+                // deny_warnings is exposed through `rustflags`
+                deny_warnings: _,
             } => self.build_args(
                 true,
                 cargoarg,
@@ -578,6 +611,8 @@ impl<'a> CargoCommand<'a> {
                 mode,
                 // Target is added by build_args
                 target: _,
+                // deny_warnings is exposed through `rustflags`
+                deny_warnings: _,
             } => self.build_args(
                 true,
                 cargoarg,
@@ -632,6 +667,27 @@ impl<'a> CargoCommand<'a> {
         }
     }
 
+    pub fn rustflags(&self) -> Option<&str> {
+        match self {
+            // Clippy is a special case: it sets deny warnings
+            // through an argument to rustc.
+            CargoCommand::Clippy { .. } => None,
+            CargoCommand::Check { deny_warnings, .. }
+            | CargoCommand::ExampleCheck { deny_warnings, .. }
+            | CargoCommand::Build { deny_warnings, .. }
+            | CargoCommand::ExampleBuild { deny_warnings, .. }
+            | CargoCommand::Test { deny_warnings, .. }
+            | CargoCommand::Qemu { deny_warnings, .. } => {
+                if *deny_warnings {
+                    Some("-D warnings")
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        }
+    }
+
     pub fn print_stdout_intermediate(&self) -> bool {
         match self {
             Self::ExampleSize { .. } => true,
diff --git a/xtask/src/run/mod.rs b/xtask/src/run/mod.rs
index ac35f5ce..4032ea8f 100644
--- a/xtask/src/run/mod.rs
+++ b/xtask/src/run/mod.rs
@@ -172,6 +172,7 @@ pub fn cargo<'c>(
                     features,
                     mode: BuildMode::Release,
                     dir: None,
+                    deny_warnings: globals.deny_warnings,
                 },
                 BuildOrCheck::Build => CargoCommand::Build {
                     cargoarg,
@@ -180,6 +181,7 @@ pub fn cargo<'c>(
                     features,
                     mode: BuildMode::Release,
                     dir: None,
+                    deny_warnings: globals.deny_warnings,
                 },
             };
 
@@ -209,6 +211,7 @@ pub fn cargo_usage_example(
                     package: None,
                     target: None,
                     features: None,
+                    deny_warnings: globals.deny_warnings,
                 },
                 BuildOrCheck::Build => CargoCommand::Build {
                     cargoarg: &None,
@@ -217,6 +220,7 @@ pub fn cargo_usage_example(
                     features: None,
                     mode: BuildMode::Release,
                     dir: Some(path.into()),
+                    deny_warnings: globals.deny_warnings,
                 },
             };
             (globals, command, false)
@@ -244,6 +248,7 @@ pub fn cargo_example<'c>(
                 target: Some(backend.to_target()),
                 features,
                 mode: BuildMode::Release,
+                deny_warnings: globals.deny_warnings,
             },
             BuildOrCheck::Build => CargoCommand::ExampleBuild {
                 cargoarg,
@@ -252,6 +257,7 @@ pub fn cargo_example<'c>(
                 features,
                 mode: BuildMode::Release,
                 dir: Some(PathBuf::from("./rtic")),
+                deny_warnings: globals.deny_warnings,
             },
         };
         (globals, command, false)
@@ -337,7 +343,10 @@ pub fn cargo_test<'c>(
 ) -> Vec<FinalRunResult<'c>> {
     package
         .packages()
-        .map(|p| (globals, TestMetadata::match_package(p, backend), false))
+        .map(|p| {
+            let meta = TestMetadata::match_package(globals.deny_warnings, p, backend);
+            (globals, meta, false)
+        })
         .run_and_coalesce()
 }
 
@@ -378,6 +387,7 @@ pub fn qemu_run_examples<'c>(
                 features: features.clone(),
                 mode: BuildMode::Release,
                 dir: Some(PathBuf::from("./rtic")),
+                deny_warnings: globals.deny_warnings,
             };
 
             let cmd_qemu = CargoCommand::Qemu {
@@ -387,6 +397,7 @@ pub fn qemu_run_examples<'c>(
                 features: features.clone(),
                 mode: BuildMode::Release,
                 dir: Some(PathBuf::from("./rtic")),
+                deny_warnings: globals.deny_warnings,
             };
 
             into_iter([cmd_build, cmd_qemu])
@@ -417,7 +428,9 @@ pub fn build_and_check_size<'c>(
             features: features.clone(),
             mode: BuildMode::Release,
             dir: Some(PathBuf::from("./rtic")),
+            deny_warnings: globals.deny_warnings,
         };
+
         if let Err(err) = command_parser(globals, &cmd, false) {
             error!("{err}");
         }
@@ -455,6 +468,10 @@ fn run_command(
         process.current_dir(dir.canonicalize()?);
     }
 
+    if let Some(rustflags) = command.rustflags() {
+        process.env("RUSTFLAGS", rustflags);
+    }
+
     let result = process.output()?;
 
     let exit_status = result.status;