Skip to content

Commit

Permalink
Merge pull request #825 from foresterre/split-install-target
Browse files Browse the repository at this point in the history
  • Loading branch information
foresterre authored Nov 12, 2023
2 parents abab837 + d2e571f commit 968e5e3
Show file tree
Hide file tree
Showing 14 changed files with 233 additions and 151 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
233 changes: 132 additions & 101 deletions src/check/rustup_toolchain_check.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<Outcome> {
let settings = &self.settings;

Expand All @@ -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 {
Expand All @@ -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<Outcome> {
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<Item = &'arg str>,
) -> TResult<Outcome> {
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
Expand All @@ -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
}
Expand All @@ -202,3 +208,28 @@ impl<'env, 'cc> Settings<'env, 'cc> {
self.environment.lock()
}
}

pub struct RunCommand {
command: Vec<String>,
}

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<String>) -> Self {
Self { command }
}

pub fn components(&self) -> &[String] {
self.command.as_ref()
}
}
4 changes: 2 additions & 2 deletions src/cli/custom_check_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
#[arg(last = true)]
pub custom_check_command: Option<Vec<String>>,
}
4 changes: 4 additions & 0 deletions src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ impl RustupCommand {
self.execute(OsStr::new("show"))
}

pub fn target(self) -> TResult<RustupOutput> {
self.execute(OsStr::new("target"))
}

/// Execute a given `rustup` command.
///
/// See also:
Expand Down
20 changes: 5 additions & 15 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -193,24 +193,14 @@ impl TryFrom<ToolchainOpts> for ToolchainContext {
#[derive(Debug)]
pub struct CheckCmdContext {
/// The custom `Rustup` command to invoke for a toolchain.
pub rustup_command: Vec<String>,
pub rustup_command: Option<Vec<String>>,
}

impl From<CustomCheckOpts> 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,
}
}
}

Expand Down
Loading

0 comments on commit 968e5e3

Please sign in to comment.