Skip to content

Commit

Permalink
Merge pull request #976 from foresterre/rename-add-component
Browse files Browse the repository at this point in the history
Rename --add-component to --component
  • Loading branch information
foresterre authored Aug 10, 2024
2 parents a825a5d + 4bb554c commit 18f47cb
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 40 deletions.
18 changes: 10 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ the [discussions section](https://github.com/foresterre/cargo-msrv/discussions).
* Added a 'minimal' output option intended for machine-readable use when full json output is undesirable.
* Added `--features` option, `--all-features` flag and `--no-default-features` flag, which are forwarded to the default
compatibility check command
* Added `--add-component` option, which can be used to add a Rust component to a toolchain.
* Added `--component` option, which can be used to add one or more Rust components to a toolchain.
* `cargo msrv verify` now supports
Cargo [workspace inheritance](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-package-table), and will
now correctly inherit the MSRV (i.e. `package.rust-version`) defined by a workspace
Expand Down Expand Up @@ -55,9 +55,10 @@ the [discussions section](https://github.com/foresterre/cargo-msrv/discussions).
* Use compilation target instead of build machine target for MSRV checks.
* Fix issue where `--manifest-path Cargo.toml` would yield an empty manifest path.
* Supply provided components to `verify` subcommand.
* The CLI arguments `--target` and `--add-component` were previously inadvertently ignored when provided
* The CLI arguments `--target` and `--component` were previously inadvertently ignored when provided
to `cargo msrv verify`.
* Fixed issue where some errors were not being reported (e.g. `cargo msrv verify` did not print an error if it wasn't possible to resolve the MSRV to check against).
* Fixed issue where some errors were not being reported (e.g. `cargo msrv verify` did not print an error if it wasn't
possible to resolve the MSRV to check against).

### Removed

Expand All @@ -71,13 +72,14 @@ the [discussions section](https://github.com/foresterre/cargo-msrv/discussions).
arguments `--features`, `--all-features`, `--no-default-features`, `--min`, `--max`, `--include-all-patch-releases`
and `--release-source` are ignored when provided to the `verify` subcommand. Workaround: supply these arguments
directly to the top-level command, e.g. `cargo msrv --all-features verify`.
* The CLI arguments `--target` and `--add-component` can be provided to both the top-level `cargo msrv` command, and
* The CLI arguments `--target` and `--component` can be provided to both the top-level `cargo msrv` command, and
the `cargo msrv verify` subcommand, however if they're provided to both, then only the arguments of the subcommand are
considered.
Example: `cargo msrv --target x --add-component a --add-component b verify --target y --add-component c --add-component d`
does not reject or collect the `--target x --add-component a --add-component b` portion; the program will only be
aware of the `--target y --add-component c --add-component d` arguments.
* The CLI arguments `--target` and `--add-component` are shown to be available globally (i.e. both at the top
Example:
`cargo msrv --target x --component a --component b verify --target y --component c --component d`
does not reject or collect the `--target x --component a --component b` portion; the program will only be
aware of the `--target y --component c --component d` arguments.
* The CLI arguments `--target` and `--component` are shown to be available globally (i.e. both at the top
level `cargo msrv` command and at the subcommand level, e.g. `cargo msrv verify`), even for subcommands which do not
consume these arguments like `cargo msrv list`.

Expand Down
4 changes: 2 additions & 2 deletions src/check/rustup_toolchain_check.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::check::Check;
use crate::context::EnvironmentContext;
use crate::download::{DownloadToolchain, ToolchainDownloader};
use crate::error::{IoError, IoErrorSource};
use crate::external_command::cargo_command::CargoCommand;
use crate::external_command::rustup_command::RustupCommand;
use crate::lockfile::LockfileHandler;
use crate::reporter::event::{CheckMethod, CheckResult, CheckToolchain, Method};
use crate::setup_toolchain::{SetupRustupToolchain, SetupToolchain};
use crate::toolchain::ToolchainSpec;
use crate::{lockfile, CargoMSRVError, Outcome, Reporter, TResult};
use camino::{Utf8Path, Utf8PathBuf};
Expand Down Expand Up @@ -87,7 +87,7 @@ impl<R: Reporter> fmt::Debug for RustupToolchainCheck<'_, '_, R> {
}

fn setup_toolchain(reporter: &impl Reporter, toolchain: &ToolchainSpec) -> TResult<()> {
let downloader = ToolchainDownloader::new(reporter);
let downloader = SetupRustupToolchain::new(reporter);
downloader.download(toolchain)?;

Ok(())
Expand Down
4 changes: 3 additions & 1 deletion src/cli/toolchain_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub struct ToolchainOpts {
/// Components be added to the toolchain
///
/// Can be supplied multiple times to add multiple components.
///
/// For example: --component rustc --component cargo
#[arg(long, value_name = "COMPONENT", global = true)]
pub add_component: Vec<String>,
pub component: Vec<String>,
}
2 changes: 1 addition & 1 deletion src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl TryFrom<ToolchainOpts> for ToolchainContext {
let target: &'static str = String::leak(target);

let components: &'static [&'static str] = Vec::leak(
opts.add_component
opts.component
.into_iter()
.map(|s| {
let s: &'static str = String::leak(s);
Expand Down
47 changes: 36 additions & 11 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub enum CargoMSRVError {
RustReleasesEmptyReleaseSet,

#[error(transparent)]
RustupInstallFailed(#[from] RustupInstallFailed),
RustupError(#[from] RustupError),

#[error("Check toolchain (with `rustup run <toolchain> <command>`) failed.")]
RustupRunWithCommandFailed,
Expand Down Expand Up @@ -250,24 +250,49 @@ impl<T> From<storyteller::EventReporterError<T>> for CargoMSRVError {
}
}

#[derive(Debug, thiserror::Error)]
#[error(transparent)]
pub enum RustupError {
Install(#[from] RustupInstallError),
AddComponent(#[from] RustupAddComponentError),
AddTarget(#[from] RustupAddTargetError),
}

#[derive(Debug, thiserror::Error)]
#[error(
"Unable to install toolchain '{}', rustup reported:\n {}",
toolchain_spec,
stderr.trim_end().lines().collect::<Vec<_>>().join("\n ").dimmed()
)]
pub struct RustupInstallFailed {
pub(crate) toolchain_spec: String,
pub(crate) stderr: String,
pub struct RustupInstallError {
pub toolchain_spec: String,
pub stderr: String,
}

impl RustupInstallFailed {
pub fn new(toolchain_spec: impl Into<String>, stderr: impl Into<String>) -> Self {
Self {
toolchain_spec: toolchain_spec.into(),
stderr: stderr.into(),
}
}
#[derive(Debug, thiserror::Error)]
#[error(
"Unable to add components '{}' to toolchain '{}', rustup reported:\n {}",
components,
toolchain_spec,
stderr.trim_end().lines().collect::<Vec<_>>().join("\n ").dimmed()
)]
pub struct RustupAddComponentError {
pub components: String,
pub toolchain_spec: String,
pub stderr: String,
}

#[derive(Debug, thiserror::Error)]
#[error(
"Unable to add target '{}' to toolchain '{}', rustup reported:\n {}",
targets,
toolchain_spec,
stderr.trim_end().lines().collect::<Vec<_>>().join("\n ").dimmed()
)]
pub struct RustupAddTargetError {
pub targets: String,
pub toolchain_spec: String,
pub stderr: String,
}

#[derive(Debug, thiserror::Error)]
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ pub mod toolchain;
mod context;
pub(crate) mod default_target;
pub(crate) mod dependency_graph;
pub(crate) mod download;
mod external_command;
pub(crate) mod formatting;
pub(crate) mod lockfile;
Expand All @@ -55,6 +54,7 @@ mod release_index;
pub(crate) mod releases_filter;
mod rust_release;
pub(crate) mod search_method;
pub(crate) mod setup_toolchain;
pub(crate) mod sub_command;
pub(crate) mod typed_bool;
pub(crate) mod writer;
Expand Down
45 changes: 29 additions & 16 deletions src/download.rs → src/setup_toolchain.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,31 @@
use crate::error::RustupInstallFailed;
use crate::error::{
RustupAddComponentError, RustupAddTargetError, RustupError, RustupInstallError,
};
use crate::external_command::rustup_command::RustupCommand;
use crate::reporter::event::SetupToolchain;
use crate::reporter::event::SetupToolchain as SetupToolchainEvent;
use crate::toolchain::ToolchainSpec;
use crate::{CargoMSRVError, Reporter, TResult};

pub trait DownloadToolchain {
pub trait SetupToolchain {
fn download(&self, toolchain: &ToolchainSpec) -> TResult<()>;
}

#[derive(Debug)]
pub struct ToolchainDownloader<'reporter, R: Reporter> {
pub struct SetupRustupToolchain<'reporter, R: Reporter> {
reporter: &'reporter R,
}

impl<'reporter, R: Reporter> ToolchainDownloader<'reporter, R> {
impl<'reporter, R: Reporter> SetupRustupToolchain<'reporter, R> {
pub fn new(reporter: &'reporter R) -> Self {
Self { reporter }
}
}

impl<'reporter, R: Reporter> DownloadToolchain for ToolchainDownloader<'reporter, R> {
impl<'reporter, R: Reporter> SetupToolchain for SetupRustupToolchain<'reporter, R> {
#[instrument(skip(self, toolchain))]
fn download(&self, toolchain: &ToolchainSpec) -> TResult<()> {
self.reporter
.run_scoped_event(SetupToolchain::new(toolchain.to_owned()), || {
.run_scoped_event(SetupToolchainEvent::new(toolchain.to_owned()), || {
install_toolchain(toolchain)
.and_then(|_| add_target(toolchain))
.and_then(|_| {
Expand Down Expand Up @@ -57,9 +59,12 @@ fn install_toolchain(toolchain: &ToolchainSpec) -> TResult<()> {
"rustup failed to install toolchain"
);

return Err(CargoMSRVError::RustupInstallFailed(
RustupInstallFailed::new(toolchain.spec(), rustup.stderr()),
));
return Err(CargoMSRVError::RustupError(RustupError::Install(
RustupInstallError {
toolchain_spec: toolchain.spec().to_string(),
stderr: rustup.stderr().to_string(),
},
)));
}

Ok(())
Expand Down Expand Up @@ -94,9 +99,13 @@ fn add_target(toolchain: &ToolchainSpec) -> TResult<()> {
"rustup failed to add target to toolchain"
);

return Err(CargoMSRVError::RustupInstallFailed(
RustupInstallFailed::new(toolchain.spec(), rustup.stderr()),
));
return Err(CargoMSRVError::RustupError(RustupError::AddTarget(
RustupAddTargetError {
targets: toolchain.target().to_string(),
toolchain_spec: toolchain.spec().to_string(),
stderr: rustup.stderr().to_string(),
},
)));
}

Ok(())
Expand Down Expand Up @@ -135,9 +144,13 @@ fn add_components(toolchain: &ToolchainSpec) -> TResult<()> {
"rustup failed to add component(s) to toolchain"
);

return Err(CargoMSRVError::RustupInstallFailed(
RustupInstallFailed::new(toolchain.spec(), rustup.stderr()),
));
return Err(CargoMSRVError::RustupError(RustupError::AddComponent(
RustupAddComponentError {
components: toolchain.components().join(", "),
toolchain_spec: toolchain.spec().to_string(),
stderr: rustup.stderr().to_string(),
},
)));
}

Ok(())
Expand Down

0 comments on commit 18f47cb

Please sign in to comment.