From a588ceeb4940b9eb4584965e34c8627d63c949b1 Mon Sep 17 00:00:00 2001 From: Zachary Harrold Date: Wed, 16 Oct 2024 10:45:23 +1100 Subject: [PATCH] Improve and Debug CI `compile-check-no-std` Command (#15935) # Objective - Fix bug where `cargo run -p ci` fails due to differing implementations for default values between `Default` trait and `argh` - Automatically install target via `rustup` to make first-run simpler. ## Solution The command will now attempt to install the target via `rustup` by default, which will provide a cleaner error message if a malformed target is passed. It will also avoid confusion when people attempt to use this tool for the first time without the target already installed. I've added a flag, --skip-install, to disable the attempted installation just-in-case. (e.g., maybe rustup isn't in their path but cargo is?). Also fixed a bug where the default value for `target` was different between the `Default` trait and `argh`, causing `cargo run -p ci` to fail. ## Testing - CI - Subcommand ran directly ## Notes This issue was originally discovered by @targrub (on Discord): > Unfortunately, running `cargo run -p ci` still gives that same error as I initially reported (though `cargo run -p ci -- compile-check-no-std` succeeds). This is after having run `rustup target add x86_64-unknown-none`. --- tools/ci/src/commands/compile_check_no_std.rs | 44 +++++++++++++++---- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/tools/ci/src/commands/compile_check_no_std.rs b/tools/ci/src/commands/compile_check_no_std.rs index 86109c6279..ddb7c5b4e8 100644 --- a/tools/ci/src/commands/compile_check_no_std.rs +++ b/tools/ci/src/commands/compile_check_no_std.rs @@ -3,39 +3,65 @@ use argh::FromArgs; use xshell::cmd; /// Checks that the project compiles for a `no_std` target. -#[derive(FromArgs, Default)] +/// Note that this tool will attempt to install the target via rustup. +/// This can be skipped by passing the "--skip-install" flag. +#[derive(FromArgs)] #[argh(subcommand, name = "compile-check-no-std")] pub struct CompileCheckNoStdCommand { /// the target to check against. /// Defaults to "x86_64-unknown-none" - #[argh(option, default = "String::from(\"x86_64-unknown-none\")")] + #[argh(option, default = "Self::default().target")] target: String, + /// skip attempting the installation of the target. + #[argh(switch)] + skip_install: bool, +} + +impl Default for CompileCheckNoStdCommand { + fn default() -> Self { + Self { + target: String::from("x86_64-unknown-none"), + skip_install: false, + } + } } impl Prepare for CompileCheckNoStdCommand { fn prepare<'a>(&self, sh: &'a xshell::Shell, _flags: Flag) -> Vec> { let target = self.target.as_str(); - vec![PreparedCommand::new::( + let mut commands = Vec::new(); + + if !self.skip_install { + commands.push(PreparedCommand::new::( + cmd!(sh, "rustup target add {target}"), + "Unable to add the required target via rustup, is it spelled correctly?", + )); + } + + commands.push(PreparedCommand::new::( cmd!( sh, "cargo check -p bevy_ptr --no-default-features --target {target}" ), "Please fix compiler errors in output above for bevy_ptr no_std compatibility.", - ), - PreparedCommand::new::( + )); + + commands.push(PreparedCommand::new::( cmd!( sh, "cargo check -p bevy_utils --no-default-features --target {target}" ), "Please fix compiler errors in output above for bevy_utils no_std compatibility.", - ), - PreparedCommand::new::( + )); + + commands.push(PreparedCommand::new::( cmd!( sh, "cargo check -p bevy_mikktspace --no-default-features --features libm --target {target}" ), "Please fix compiler errors in output above for bevy_mikktspace no_std compatibility.", - ) - ] + )); + + commands } }