From 2db26c1015f0a32fe43e71d60f5fd75dbb25f40c Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sun, 16 Apr 2023 12:51:11 +0200 Subject: [PATCH] Deny on warnings in xtasks --- .github/workflows/build.yml | 24 ++----- rtic-common/src/lib.rs | 1 - rtic-macros/src/lib.rs | 1 - rtic-monotonics/src/lib.rs | 1 - rtic-sync/src/lib.rs | 1 - rtic-time/src/lib.rs | 1 - rtic/src/lib.rs | 1 - xtask/src/argument_parsing.rs | 16 ++++- xtask/src/cargo_command.rs | 116 +++++++++++++++++++++++++--------- xtask/src/run/mod.rs | 19 +++++- 10 files changed, 123 insertions(+), 58 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4fa2b8079d..ad0f5eda40 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 03d0306373..e8f5af7e71 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 cd2a9245db..e0043d916e 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 04ce4e2451..714200a693 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 ac062205a8..fd8b6c3ad0 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 1ed1e9d6ce..7c0c7645a2 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 ac05d93d6c..53fe287575 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 8c8f7d2a2d..d7c0262fb7 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, diff --git a/xtask/src/cargo_command.rs b/xtask/src/cargo_command.rs index 9cf4f6505b..09487cb61b 100644 --- a/xtask/src/cargo_command.rs +++ b/xtask/src/cargo_command.rs @@ -28,6 +28,7 @@ pub enum CargoCommand<'a> { features: Option, mode: BuildMode, dir: Option, + deny_warnings: bool, }, ExampleBuild { cargoarg: &'a Option<&'a str>, @@ -36,6 +37,7 @@ pub enum CargoCommand<'a> { features: Option, mode: BuildMode, dir: Option, + deny_warnings: bool, }, ExampleCheck { cargoarg: &'a Option<&'a str>, @@ -43,6 +45,7 @@ pub enum CargoCommand<'a> { target: Option>, features: Option, mode: BuildMode, + deny_warnings: bool, }, Build { cargoarg: &'a Option<&'a str>, @@ -51,6 +54,7 @@ pub enum CargoCommand<'a> { features: Option, mode: BuildMode, dir: Option, + deny_warnings: bool, }, Check { cargoarg: &'a Option<&'a str>, @@ -59,6 +63,7 @@ pub enum CargoCommand<'a> { features: Option, mode: BuildMode, dir: Option, + deny_warnings: bool, }, Clippy { cargoarg: &'a Option<&'a str>, @@ -81,6 +86,7 @@ pub enum CargoCommand<'a> { package: Option, features: Option, test: Option, + deny_warnings: bool, }, Book { arguments: Option, @@ -123,6 +129,7 @@ impl core::fmt::Display for CargoCommand<'_> { } fn details( + deny_warnings: bool, target: &Option, mode: Option<&BuildMode>, features: &Option, @@ -150,14 +157,20 @@ impl core::fmt::Display for CargoCommand<'_> { format!("debug") }; - if cargoarg.is_some() && path.is_some() { - format!("({target}, {mode}, {feat}, {carg}, {in_dir})") - } else if cargoarg.is_some() { - format!("({target}, {mode}, {feat}, {carg})") - } else if path.is_some() { - format!("({target}, {mode}, {feat}, {in_dir})") + let deny_warnings = if deny_warnings { + format!("deny warnings, ") } else { - format!("({target}, {mode}, {feat})") + format!("") + }; + + if cargoarg.is_some() && path.is_some() { + format!("({deny_warnings}{target}, {mode}, {feat}, {carg}, {in_dir})") + } else if cargoarg.is_some() { + format!("({deny_warnings}{target}, {mode}, {feat}, {carg})") + } else if path.is_some() { + format!("({deny_warnings}{target}, {mode}, {feat}, {in_dir})") + } else { + 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 ac35f5ce81..4032ea8fed 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> { 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;