diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d5d1ad5..bba9ed57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,6 @@ the [issue tracker](https://github.com/foresterre/cargo-msrv/issues), or open a * `cargo-msrv` now requires paths to be UTF-8. * `--write-msrv` now writes two, instead of three component version numbers . - #### Infra * Changed release artifact name of `cargo-msrv` packages on Github, such that they can be installed with `cargo-binstall` out of the box. @@ -41,6 +40,7 @@ the [issue tracker](https://github.com/foresterre/cargo-msrv/issues), or open a * The program will no longer return an unformatted message when a command failed and the output format was set to json. * Fix issue where reading the fallback MSRV from a TOML inline table was not possible. * Fix an index out-of-bounds panic which occurred if the filtered Rust releases search space was empty +* Use compilation target instead of build machine target for MSRV checks ### Removed @@ -70,7 +70,7 @@ This release does not contain user-facing changes, hence the lack of changelog e ### Changed * ⚠️ Breaking change: Changed default cargo-msrv (find) check command from `cargo check --all` to `cargo check`. - * Revert to the old behaviour by running cargo-msrv with a custom check command: `cargo msrv -- cargo check --all`. + * To revert to the old behaviour, run cargo-msrv with the following custom check command: `cargo msrv -- cargo check --all`. ### Removed diff --git a/src/check.rs b/src/check.rs index 0b376421..68d1d2b4 100644 --- a/src/check.rs +++ b/src/check.rs @@ -5,7 +5,8 @@ mod rustup_toolchain_check; mod testing; use crate::{Outcome, TResult}; -pub use rustup_toolchain_check::RustupToolchainCheck; +pub use rustup_toolchain_check::{RunCommand, RustupToolchainCheck}; + #[cfg(test)] pub use testing::TestRunner; diff --git a/src/check/rustup_toolchain_check.rs b/src/check/rustup_toolchain_check.rs index 410ef44c..e28a0b97 100644 --- a/src/check/rustup_toolchain_check.rs +++ b/src/check/rustup_toolchain_check.rs @@ -1,6 +1,6 @@ use crate::check::Check; use crate::command::RustupCommand; -use crate::context::{CheckCmdContext, EnvironmentContext}; +use crate::context::EnvironmentContext; use crate::download::{DownloadToolchain, ToolchainDownloader}; use crate::error::{IoError, IoErrorSource}; use crate::lockfile::LockfileHandler; @@ -9,12 +9,32 @@ use crate::toolchain::ToolchainSpec; use crate::{lockfile, CargoMSRVError, Outcome, Reporter, TResult}; use camino::{Utf8Path, Utf8PathBuf}; -pub struct RustupToolchainCheck<'reporter, 'env, 'cc, R: Reporter> { +pub struct RustupToolchainCheck<'reporter, 'env, R: Reporter> { reporter: &'reporter R, - settings: Settings<'env, 'cc>, + settings: Settings<'env>, } -impl<'reporter, 'env, 'cc, R: Reporter> Check for RustupToolchainCheck<'reporter, 'env, 'cc, R> { +impl<'reporter, 'env, R: Reporter> RustupToolchainCheck<'reporter, 'env, R> { + pub fn new( + reporter: &'reporter R, + ignore_lockfile: bool, + no_check_feedback: bool, + environment: &'env EnvironmentContext, + run_command: RunCommand, + ) -> Self { + Self { + reporter, + settings: Settings { + ignore_lockfile, + no_check_feedback, + environment, + check_cmd: run_command, + }, + } + } +} + +impl<'reporter, 'env, R: Reporter> Check for RustupToolchainCheck<'reporter, 'env, R> { fn check(&self, toolchain: &ToolchainSpec) -> TResult { let settings = &self.settings; @@ -28,22 +48,24 @@ impl<'reporter, 'env, 'cc, R: Reporter> Check for RustupToolchainCheck<'reporter .map(|handle| handle.move_lockfile()) .transpose()?; - self.setup_toolchain(toolchain)?; + setup_toolchain(self.reporter, toolchain)?; if handle_wrap.is_some() { remove_lockfile(&settings.lockfile_path())?; } let crate_root = settings.crate_root_path(); + let cmd = &self.settings.check_cmd; - let outcome = self.run_check_command_via_rustup( + let outcome = run_check_command_via_rustup( + self.reporter, toolchain, crate_root, - settings.check_cmd.rustup_command.iter().map(|s| s.as_str()), + cmd.components(), )?; // report outcome to UI - self.report_outcome(&outcome, settings.no_check_feedback())?; + report_outcome(self.reporter, &outcome, settings.no_check_feedback())?; // move the lockfile back if let Some(handle) = handle_wrap { @@ -55,103 +77,87 @@ impl<'reporter, 'env, 'cc, R: Reporter> Check for RustupToolchainCheck<'reporter } } -impl<'reporter, 'env, 'cc, R: Reporter> RustupToolchainCheck<'reporter, 'env, 'cc, R> { - pub fn new( - reporter: &'reporter R, - ignore_lockfile: bool, - no_check_feedback: bool, - environment: &'env EnvironmentContext, - check_cmd: &'cc CheckCmdContext, - ) -> Self { - Self { - reporter, - settings: Settings { - ignore_lockfile, - no_check_feedback, - environment, - check_cmd, - }, - } - } +fn setup_toolchain(reporter: &impl Reporter, toolchain: &ToolchainSpec) -> TResult<()> { + let downloader = ToolchainDownloader::new(reporter); + downloader.download(toolchain)?; - fn setup_toolchain(&self, toolchain: &ToolchainSpec) -> TResult<()> { - let downloader = ToolchainDownloader::new(self.reporter); - downloader.download(toolchain)?; + Ok(()) +} - Ok(()) - } +fn run_check_command_via_rustup( + reporter: &impl Reporter, + toolchain: &ToolchainSpec, + dir: &Utf8Path, + check: &[String], +) -> TResult { + let version = format!("{}", toolchain.version()); + let mut cmd = vec![version.as_str()]; + cmd.extend(check.iter().map(|s| s.as_str())); + + reporter.report_event(CheckMethod::new( + toolchain.to_owned(), + Method::rustup_run(&cmd, dir), + ))?; + + let rustup_output = RustupCommand::new() + .with_args(cmd.iter()) + .with_dir(dir) + .with_stderr() + .run() + .map_err(|_| CargoMSRVError::UnableToRunCheck { + command: cmd[1..].join(" "), + cwd: dir.to_path_buf(), + })?; + + let status = rustup_output.exit_status(); - fn run_check_command_via_rustup<'arg>( - &self, - toolchain: &'arg ToolchainSpec, - dir: &Utf8Path, - check: impl Iterator, - ) -> TResult { - let mut cmd: Vec<&str> = vec![toolchain.spec()]; - cmd.extend(check); + if status.success() { + Ok(Outcome::new_success(toolchain.to_owned())) + } else { + let stderr = rustup_output.stderr(); + let command = cmd.join(" "); - self.reporter.report_event(CheckMethod::new( + info!( + ?toolchain, + stderr, + cmd = command.as_str(), + "try_building run failed" + ); + + Ok(Outcome::new_failure( toolchain.to_owned(), - Method::rustup_run(&cmd, dir), - ))?; - - let rustup_output = RustupCommand::new() - .with_args(cmd.iter()) - .with_dir(dir) - .with_stderr() - .run() - .map_err(|_| CargoMSRVError::UnableToRunCheck { - command: cmd[1..].join(" "), - cwd: dir.to_path_buf(), - })?; - - let status = rustup_output.exit_status(); - - if status.success() { - Ok(Outcome::new_success(toolchain.to_owned())) - } else { - let stderr = rustup_output.stderr(); - let command = cmd.join(" "); - - info!( - ?toolchain, - stderr, - cmd = command.as_str(), - "try_building run failed" - ); - - Ok(Outcome::new_failure( - toolchain.to_owned(), - stderr.to_string(), - )) - } + stderr.to_string(), + )) } +} - fn report_outcome(&self, outcome: &Outcome, no_error_report: bool) -> TResult<()> { - match outcome { - Outcome::Success(outcome) => { - // report compatibility with this toolchain - self.reporter - .report_event(CheckResult::compatible(outcome.toolchain_spec.to_owned()))? - } - Outcome::Failure(outcome) if no_error_report => { - // report incompatibility with this toolchain - self.reporter.report_event(CheckResult::incompatible( - outcome.toolchain_spec.to_owned(), - None, - ))? - } - Outcome::Failure(outcome) => { - // report incompatibility with this toolchain - self.reporter.report_event(CheckResult::incompatible( - outcome.toolchain_spec.to_owned(), - Some(outcome.error_message.clone()), - ))? - } - }; - - Ok(()) - } +fn report_outcome( + reporter: &impl Reporter, + outcome: &Outcome, + no_error_report: bool, +) -> TResult<()> { + match outcome { + Outcome::Success(outcome) => { + // report compatibility with this toolchain + reporter.report_event(CheckResult::compatible(outcome.toolchain_spec.to_owned()))? + } + Outcome::Failure(outcome) if no_error_report => { + // report incompatibility with this toolchain + reporter.report_event(CheckResult::incompatible( + outcome.toolchain_spec.to_owned(), + None, + ))? + } + Outcome::Failure(outcome) => { + // report incompatibility with this toolchain + reporter.report_event(CheckResult::incompatible( + outcome.toolchain_spec.to_owned(), + Some(outcome.error_message.clone()), + ))? + } + }; + + Ok(()) } /// Creates a lockfile handle, iff the lockfile exists and the user opted @@ -177,15 +183,15 @@ fn remove_lockfile(lock_file: &Utf8Path) -> TResult<()> { Ok(()) } -struct Settings<'env, 'cc> { +struct Settings<'env> { ignore_lockfile: bool, no_check_feedback: bool, environment: &'env EnvironmentContext, - check_cmd: &'cc CheckCmdContext, + check_cmd: RunCommand, } -impl<'env, 'cc> Settings<'env, 'cc> { +impl<'env> Settings<'env> { pub fn ignore_lockfile(&self) -> bool { self.ignore_lockfile } @@ -202,3 +208,28 @@ impl<'env, 'cc> Settings<'env, 'cc> { self.environment.lock() } } + +pub struct RunCommand { + command: Vec, +} + +impl RunCommand { + pub fn default(target: impl ToString) -> Self { + let command = vec![ + "cargo".to_string(), + "check".to_string(), + "--target".to_string(), + target.to_string(), + ]; + + Self { command } + } + + pub fn custom(command: Vec) -> Self { + Self { command } + } + + pub fn components(&self) -> &[String] { + self.command.as_ref() + } +} diff --git a/src/cli/custom_check_opts.rs b/src/cli/custom_check_opts.rs index 84aec19b..1f8026b5 100644 --- a/src/cli/custom_check_opts.rs +++ b/src/cli/custom_check_opts.rs @@ -4,6 +4,6 @@ use clap::Args; #[command(next_help_heading = "Custom check options")] pub struct CustomCheckOpts { /// Supply a custom `check` command to be used by cargo msrv - #[arg(last = true, required = false)] - pub custom_check_command: Vec, + #[arg(last = true)] + pub custom_check_command: Option>, } diff --git a/src/command.rs b/src/command.rs index 710208a3..4fb6e5ad 100644 --- a/src/command.rs +++ b/src/command.rs @@ -56,6 +56,10 @@ impl RustupCommand { self.execute(OsStr::new("show")) } + pub fn target(self) -> TResult { + self.execute(OsStr::new("target")) + } + /// Execute a given `rustup` command. /// /// See also: diff --git a/src/context.rs b/src/context.rs index a2c9e270..09ca8b21 100644 --- a/src/context.rs +++ b/src/context.rs @@ -6,7 +6,6 @@ //! //! Unlike the opts, the context is top down, not bottom up. -use crate::cli::custom_check_opts::CustomCheckOpts; use crate::cli::rust_releases_opts::RustReleasesOpts; use crate::cli::shared_opts::{SharedOpts, UserOutputOpts}; use crate::cli::toolchain_opts::ToolchainOpts; @@ -26,6 +25,7 @@ pub mod set; pub mod show; pub mod verify; +use crate::cli::custom_check_opts::CustomCheckOpts; use crate::cli::rust_releases_opts::Edition; use crate::cli::{CargoMsrvOpts, SubCommand}; use crate::default_target::default_target; @@ -193,24 +193,14 @@ impl TryFrom for ToolchainContext { #[derive(Debug)] pub struct CheckCmdContext { /// The custom `Rustup` command to invoke for a toolchain. - pub rustup_command: Vec, + pub rustup_command: Option>, } impl From for CheckCmdContext { fn from(opts: CustomCheckOpts) -> Self { - let rustup_command = if opts.custom_check_command.is_empty() { - vec!["cargo".to_string(), "check".to_string()] - } else { - opts.custom_check_command - }; - - Self { rustup_command } - } -} - -impl CheckCmdContext { - pub fn custom_rustup_command(&self) -> &[String] { - &self.rustup_command + Self { + rustup_command: opts.custom_check_command, + } } } diff --git a/src/context/find.rs b/src/context/find.rs index c0e7da7b..99968744 100644 --- a/src/context/find.rs +++ b/src/context/find.rs @@ -1,3 +1,4 @@ +use crate::check::RunCommand; use crate::cli::CargoMsrvOpts; use crate::context::{ CheckCmdContext, EnvironmentContext, RustReleasesContext, SearchMethod, ToolchainContext, @@ -70,3 +71,13 @@ impl TryFrom for FindContext { }) } } + +impl FindContext { + pub fn run_command(&self) -> RunCommand { + if let Some(custom) = &self.check_cmd.rustup_command { + RunCommand::custom(custom.clone()) + } else { + RunCommand::default(self.toolchain.target.clone()) + } + } +} diff --git a/src/context/verify.rs b/src/context/verify.rs index ca52f3dd..ac4a3013 100644 --- a/src/context/verify.rs +++ b/src/context/verify.rs @@ -3,6 +3,7 @@ use crate::context::{ CheckCmdContext, EnvironmentContext, RustReleasesContext, ToolchainContext, UserOutputContext, }; +use crate::check::RunCommand; use crate::error::CargoMSRVError; use crate::sub_command::verify::RustVersion; use std::convert::{TryFrom, TryInto}; @@ -69,3 +70,13 @@ impl TryFrom for VerifyContext { }) } } + +impl VerifyContext { + pub fn run_command(&self) -> RunCommand { + if let Some(custom) = &self.check_cmd.rustup_command { + RunCommand::custom(custom.clone()) + } else { + RunCommand::default(self.toolchain.target.clone()) + } + } +} diff --git a/src/download.rs b/src/download.rs index 63429a21..96c26a9a 100644 --- a/src/download.rs +++ b/src/download.rs @@ -26,28 +26,62 @@ impl<'reporter, R: Reporter> DownloadToolchain for ToolchainDownloader<'reporter self.reporter .run_scoped_event(SetupToolchain::new(toolchain.to_owned()), || { - let rustup = RustupCommand::new() - .with_stdout() - .with_stderr() - .with_args(["--profile", "minimal", toolchain.spec()]) - .install()?; - - let status = rustup.exit_status(); - - if !status.success() { - error!( - toolchain = toolchain.spec(), - stdout = rustup.stdout(), - stderr = rustup.stderr(), - "rustup failed to install toolchain" - ); - - return Err(CargoMSRVError::RustupInstallFailed( - RustupInstallFailed::new(toolchain.spec(), rustup.stderr()), - )); - } - - Ok(()) + install_toolchain(toolchain).and_then(|_| add_target(toolchain)) }) } } + +fn install_toolchain(toolchain: &ToolchainSpec) -> TResult<()> { + let rustup = RustupCommand::new() + .with_stdout() + .with_stderr() + .with_args(["--profile", "minimal", &format!("{}", toolchain.version())]) + .install()?; + + let status = rustup.exit_status(); + + if !status.success() { + error!( + toolchain = toolchain.spec(), + stdout = rustup.stdout(), + stderr = rustup.stderr(), + "rustup failed to install toolchain" + ); + + return Err(CargoMSRVError::RustupInstallFailed( + RustupInstallFailed::new(toolchain.spec(), rustup.stderr()), + )); + } + + Ok(()) +} + +fn add_target(toolchain: &ToolchainSpec) -> TResult<()> { + let rustup = RustupCommand::new() + .with_stdout() + .with_stderr() + .with_args([ + "add", + "--toolchain", + &format!("{}", toolchain.version()), + toolchain.target(), + ]) + .target()?; + + let status = rustup.exit_status(); + + if !status.success() { + error!( + toolchain = toolchain.spec(), + stdout = rustup.stdout(), + stderr = rustup.stderr(), + "rustup failed to add target to toolchain" + ); + + return Err(CargoMSRVError::RustupInstallFailed( + RustupInstallFailed::new(toolchain.spec(), rustup.stderr()), + )); + } + + Ok(()) +} diff --git a/src/lib.rs b/src/lib.rs index 419a9a4b..591eac54 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -65,12 +65,13 @@ pub fn run_app(ctx: &Context, reporter: &impl Reporter) -> TResult<()> { match ctx { Context::Find(ctx) => { let index = release_index::fetch_index(reporter, ctx.rust_releases.release_source)?; + let runner = RustupToolchainCheck::new( reporter, ctx.ignore_lockfile, ctx.no_check_feedback, &ctx.environment, - &ctx.check_cmd, + ctx.run_command(), ); Find::new(&index, runner).run(ctx, reporter)?; } @@ -86,12 +87,13 @@ pub fn run_app(ctx: &Context, reporter: &impl Reporter) -> TResult<()> { } Context::Verify(ctx) => { let index = release_index::fetch_index(reporter, ctx.rust_releases.release_source)?; + let runner = RustupToolchainCheck::new( reporter, ctx.ignore_lockfile, ctx.no_check_feedback, &ctx.environment, - &ctx.check_cmd, + ctx.run_command(), ); Verify::new(&index, runner).run(ctx, reporter)?; diff --git a/src/sub_command/find.rs b/src/sub_command/find.rs index 60ece6f9..504dbd47 100644 --- a/src/sub_command/find.rs +++ b/src/sub_command/find.rs @@ -50,7 +50,7 @@ fn find_msrv( info!("no minimal-compatible toolchain found"); Err(CargoMSRVError::UnableToFindAnyGoodVersion { - command: ctx.check_cmd.custom_rustup_command().join(" "), + command: ctx.run_command().components().join(" "), }) } MinimumSupportedRustVersion::Toolchain { toolchain } => { diff --git a/src/sub_command/find/tests.rs b/src/sub_command/find/tests.rs index d95276f3..519ebadf 100644 --- a/src/sub_command/find/tests.rs +++ b/src/sub_command/find/tests.rs @@ -248,7 +248,7 @@ fn create_test_context() -> FindContext { target: "x".to_string(), }, check_cmd: CheckCmdContext { - rustup_command: vec![], + rustup_command: None, }, environment: EnvironmentContext { crate_path: Utf8PathBuf::new(), diff --git a/tests/common/sub_cmd_find.rs b/tests/common/sub_cmd_find.rs index a4c3ee87..1dd71baf 100644 --- a/tests/common/sub_cmd_find.rs +++ b/tests/common/sub_cmd_find.rs @@ -69,14 +69,13 @@ pub fn find_msrv_with_releases< let ignore_toolchain = find_ctx.ignore_lockfile; let no_check_feedback = find_ctx.no_check_feedback; let env = &find_ctx.environment; - let check_cmd = &find_ctx.check_cmd; let runner = RustupToolchainCheck::new( device.reporter(), ignore_toolchain, no_check_feedback, env, - check_cmd, + find_ctx.run_command(), ); // Determine the MSRV from the index of available releases. diff --git a/tests/common/sub_cmd_verify.rs b/tests/common/sub_cmd_verify.rs index ededa480..cdac0d11 100644 --- a/tests/common/sub_cmd_verify.rs +++ b/tests/common/sub_cmd_verify.rs @@ -28,14 +28,13 @@ where let ignore_toolchain = verify_ctx.ignore_lockfile; let no_check_feedback = verify_ctx.no_check_feedback; let env = &verify_ctx.environment; - let check_cmd = &verify_ctx.check_cmd; let runner = RustupToolchainCheck::new( device.reporter(), ignore_toolchain, no_check_feedback, env, - check_cmd, + verify_ctx.run_command(), ); // Determine the MSRV from the index of available releases.