From 065802115605b1ec15cba2276b1c6b0a858aaec1 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sat, 5 Dec 2020 22:17:08 +0900 Subject: [PATCH] Add --version-range option --- .github/workflows/ci.yml | 16 ++++++ README.md | 27 ++++++++++ src/cargo.rs | 8 +-- src/cli.rs | 30 ++++++++++- src/context.rs | 7 +-- src/main.rs | 35 ++++++++++--- src/process.rs | 52 +++++++++++++------- src/rustup.rs | 104 +++++++++++++++++++++++++++++++++++++++ tests/long-help.txt | 10 ++++ tests/short-help.txt | 2 + tests/test.rs | 32 ++++++++++++ 11 files changed, 289 insertions(+), 34 deletions(-) create mode 100644 src/rustup.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19aadc03..76438959 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,6 +54,22 @@ jobs: - if: startsWith(matrix.rust, 'nightly') && matrix.target == '' run: bash scripts/check-minimal-versions.sh + build: + strategy: + matrix: + range: + # This is the minimum supported Rust version of this crate. + # When updating this, the reminder to update the minimum supported Rust version in README.md. + - 1.36..1.40 + - 1.41..1.45 + - 1.46.. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + # TODO: replace this with `cargo install cargo-hack` when cargo-hack 0.5.0 released + - run: cargo +stable install --path . + - run: cargo hack build --all --ignore-private --no-dev-deps --version-range ${{ matrix.range }} + run: strategy: matrix: diff --git a/README.md b/README.md index 3ca42f8d..4ef9a5b7 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,32 @@ cargo-hack is usually runnable with Cargo versions older than the Rust version r Remove artifacts for that package before running the command. + This also works as a workaround for [rust-lang/rust-clippy#4612]. + +* **`--version-range`** + + Perform commands on a specified (inclusive) range of Rust versions. + + ```console + $ cargo hack check --version-range 1.46..1.47 + info: running `cargo +1.46 check` on cargo-hack (1/2) + ... + info: running `cargo +1.47 check` on cargo-hack (2/2) + ... + ``` + + If the given range is unclosed, the latest stable compiler is treated as the upper bound. + + This might be useful for catching issues like [BurntSushi/termcolor#35], [rust-lang/regex#685], [rust-lang/rust-clippy#6324]. + + [BurntSushi/termcolor#35]: https://github.com/BurntSushi/termcolor/issues/35 + [rust-lang/regex#685]: https://github.com/rust-lang/regex/issues/685 + [rust-lang/rust-clippy#6324]: https://github.com/rust-lang/rust-clippy/issues/6324. + +* **`--version-step`** + + Specify the version interval of `--version-range`. + The following flags can be used with `--each-feature` and `--feature-powerset`. * **`--optional-deps`** @@ -141,6 +167,7 @@ The following flags can be used with `--each-feature` and `--feature-powerset`. [rust-lang/cargo#5015]: https://github.com/rust-lang/cargo/issues/4463 [rust-lang/cargo#5364]: https://github.com/rust-lang/cargo/issues/5364 [rust-lang/cargo#6195]: https://github.com/rust-lang/cargo/issues/6195 +[rust-lang/rust-clippy#4612]: https://github.com/rust-lang/cargo/issues/4612 [cargo-metadata]: https://doc.rust-lang.org/cargo/commands/cargo-metadata.html ## License diff --git a/src/cargo.rs b/src/cargo.rs index 056c4845..c8cad2f1 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -14,7 +14,7 @@ impl Cargo { // If failed to determine cargo version, assign 0 to skip all version-dependent decisions. let version = minor_version(&mut ProcessBuilder::new(&path)) - .map_err(|e| warn!("unable to determine cargo version: {}", e)) + .map_err(|e| warn!("unable to determine cargo version: {:#}", e)) .unwrap_or(0); Self { path, version } @@ -38,7 +38,9 @@ pub(crate) fn minor_version(cmd: &mut ProcessBuilder<'_>) -> Result { .lines() .find(|line| line.starts_with("release: ")) .map(|line| &line["release: ".len()..]) - .ok_or_else(|| format_err!("could not find rustc release from output of {}", cmd))?; + .ok_or_else(|| { + format_err!("could not find rustc release from output of {}: {}", cmd, output) + })?; // Split the version and channel info. let mut version_channel = release.split('-'); @@ -47,7 +49,7 @@ pub(crate) fn minor_version(cmd: &mut ProcessBuilder<'_>) -> Result { let version = parse_version(version)?; if version.major != 1 || version.patch.is_none() { - bail!("unexpected output from {}", cmd); + bail!("unexpected output from {}: {}", cmd, output); } Ok(version.minor) diff --git a/src/cli.rs b/src/cli.rs index 4239de35..ea85fb46 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,7 +1,7 @@ use anyhow::{bail, format_err, Error}; use std::{env, fmt, mem}; -use crate::{term, Cargo, Feature, Result}; +use crate::{rustup, term, Cargo, Feature, Result, Rustup}; fn print_version() { println!("{0} {1}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")) @@ -81,6 +81,16 @@ const HELP: &[(&str, &str, &str, &[&str])] = &[ "If used this flag with --workspace, --each-feature, or --feature-powerset, artifacts will be removed before each run.", "Note that dependencies artifacts will be preserved.", ]), + ( + "", + "--version-range ..[END]", + "Perform commands on a specified (inclusive) range of Rust versions", + &[ + "If the given range is unclosed, the latest stable compiler is treated as the upper bound.", + "Note that ranges are always inclusive ranges.", + ], + ), + ("", "--version-step ", "Specify the version interval of --version-range", &[]), ("-v", "--verbose", "Use verbose output", &[]), ("", "--color ", "Coloring: auto, always, never", &[ "This flag will be propagated to cargo.", @@ -208,6 +218,8 @@ pub(crate) struct Args<'a> { pub(crate) ignore_unknown_features: bool, /// --clean-per-run pub(crate) clean_per_run: bool, + /// --version-range and --version-step + pub(crate) version_range: Option>, // options for --each-feature and --feature-powerset /// --optional-deps [DEPS]... @@ -250,7 +262,7 @@ pub(crate) fn raw() -> RawArgs { pub(crate) struct RawArgs(Vec); -pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result> { +pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo, rustup: &Rustup) -> Result> { let mut iter = raw.0.iter(); let mut args = iter.by_ref().map(String::as_str).peekable(); match args.next() { @@ -282,6 +294,8 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result let mut ignore_private = false; let mut ignore_unknown_features = false; let mut clean_per_run = false; + let mut version_range = None; + let mut version_step = None; let mut optional_deps = None; let mut include_features = Vec::new(); @@ -397,6 +411,8 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result parse_opt!(manifest_path, false, "--manifest-path", "--manifest-path "); parse_opt!(depth, false, "--depth", "--depth "); parse_opt!(color, true, "--color", "--color "); + parse_opt!(version_range, false, "--version-range", "--version-range ..[END]"); + parse_opt!(version_step, false, "--version-step", "--version-step "); parse_multi_opt!(package, false, true, "--package", "--package ..."); parse_multi_opt!(package, false, true, "-p", "--package ..."); @@ -544,6 +560,10 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result bail!("--group-features can only be used together with --feature-powerset"); } } + if version_range.is_none() && version_step.is_some() { + bail!("--version-step can only be used together with --version-range"); + } + let depth = depth.map(str::parse::).transpose()?; let group_features = group_features.iter().try_fold(Vec::with_capacity(group_features.len()), |mut v, g| { @@ -560,6 +580,8 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result v.push(Feature::group(g)); Ok(v) })?; + let version_range = + version_range.map(|range| rustup::version_range(range, version_step)).transpose()?; if let Some(subcommand) = subcommand { if subcommand == "test" || subcommand == "bench" { @@ -629,6 +651,9 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result if cargo.version < 41 && include_deps_features { bail!("--include-deps-features requires Cargo 1.41 or leter"); } + if rustup.version < 23 && version_range.is_some() { + bail!("--version-range requires rustup 1.23 or leter"); + } if subcommand.is_none() { if leading.contains(&"-h") { @@ -682,6 +707,7 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &Cargo) -> Result clean_per_run, include_features: include_features.into_iter().map(Into::into).collect(), include_deps_features, + version_range, depth, group_features, diff --git a/src/context.rs b/src/context.rs index 85f90375..b2a11274 100644 --- a/src/context.rs +++ b/src/context.rs @@ -11,7 +11,7 @@ use crate::{ features::Features, manifest::Manifest, metadata::{Metadata, Package, PackageId}, - Cargo, ProcessBuilder, Result, + Cargo, ProcessBuilder, Result, Rustup, }; pub(crate) struct Context<'a> { @@ -26,9 +26,10 @@ pub(crate) struct Context<'a> { impl<'a> Context<'a> { pub(crate) fn new(args: &'a RawArgs) -> Result { let cargo = Cargo::new(); + let rustup = Rustup::new(); let current_dir = env::current_dir()?; - let args = cli::parse_args(args, &cargo)?; + let args = cli::parse_args(args, &cargo, &rustup)?; assert!( args.subcommand.is_some() || args.remove_dev_deps, "no subcommand or valid flag specified" @@ -112,7 +113,7 @@ impl<'a> Context<'a> { (self.cargo.version < 39 && self.ignore_private) || self.no_dev_deps || self.remove_dev_deps } - pub(crate) fn process(&self) -> ProcessBuilder<'_> { + pub(crate) fn cargo(&self) -> ProcessBuilder<'_> { let mut cmd = self.cargo.process(); if self.verbose { cmd.display_manifest_path(); diff --git a/src/main.rs b/src/main.rs index 88003132..8d25faac 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,6 +23,7 @@ mod metadata; mod process; mod remove_dev_deps; mod restore; +mod rustup; mod version; use anyhow::{bail, Context as _}; @@ -30,7 +31,7 @@ use std::{fmt::Write, fs}; use crate::{ cargo::Cargo, context::Context, features::Feature, metadata::PackageId, - process::ProcessBuilder, restore::Restore, + process::ProcessBuilder, restore::Restore, rustup::Rustup, }; type Result = std::result::Result; @@ -58,13 +59,31 @@ fn exec_on_workspace(cx: &Context<'_>) -> Result<()> { // ) // } - let line = cx.process().with_args(cx); - - let restore = Restore::new(cx); let mut progress = Progress::default(); - determine_package_list(cx, &mut progress)? - .iter() - .try_for_each(|(id, kind)| exec_on_package(cx, id, kind, &line, &restore, &mut progress)) + let packages = determine_package_list(cx, &mut progress)?; + let restore = Restore::new(cx); + if let Some(range) = &cx.version_range { + progress.total *= range.len(); + let mut line = ProcessBuilder::new("cargo".as_ref()); + if cx.verbose { + line.display_manifest_path(); + } + range.iter().try_for_each(|toolchain| { + rustup::install_toolchain(&toolchain[1..])?; + let mut line = line.clone(); + line.leading_arg(toolchain); + line.with_args(cx); + packages.iter().try_for_each(|(id, kind)| { + exec_on_package(cx, id, kind, &line, &restore, &mut progress) + }) + }) + } else { + let mut line = cx.cargo(); + line.with_args(cx); + packages.iter().try_for_each(|(id, kind)| { + exec_on_package(cx, id, kind, &line, &restore, &mut progress) + }) + } } #[derive(Default)] @@ -344,7 +363,7 @@ fn exec_cargo( } fn cargo_clean(cx: &Context<'_>, id: &PackageId) -> Result<()> { - let mut line = cx.process(); + let mut line = cx.cargo(); line.arg("clean"); line.arg("--package"); line.arg(&cx.packages(id).name); diff --git a/src/process.rs b/src/process.rs index e19ad702..d5727d25 100644 --- a/src/process.rs +++ b/src/process.rs @@ -14,15 +14,19 @@ use crate::{Context, PackageId, Result}; /// A builder object for an external process, similar to `std::process::Command`. #[derive(Clone)] +#[must_use] pub(crate) struct ProcessBuilder<'a> { + // $program $leading_args $propagated_leading_args $args $propagated_trailing_args /// The program to execute. program: &'a OsStr, /// A list of arguments to pass to the program (until '--'). - leading_args: &'a [&'a str], + propagated_leading_args: &'a [&'a str], /// A list of arguments to pass to the program (after '--'). trailing_args: &'a [String], - /// A list of arguments to pass to the program (between `leading_args` and '--'). + /// A list of arguments to pass to the program (between `program` and 'propagated_leading_args'). + leading_args: Vec, + /// A list of arguments to pass to the program (between `propagated_leading_args` and '--'). args: Vec, /// A comma-separated list of features. /// This list always has a trailing comma if it is not empty. @@ -40,8 +44,9 @@ impl<'a> ProcessBuilder<'a> { pub(crate) fn new(program: &'a OsStr) -> Self { Self { program, - leading_args: &[], + propagated_leading_args: &[], trailing_args: &[], + leading_args: Vec::new(), args: Vec::new(), features: String::new(), display_program_path: false, @@ -49,8 +54,8 @@ impl<'a> ProcessBuilder<'a> { } } - pub(crate) fn with_args(mut self, cx: &'a Context<'_>) -> Self { - self.leading_args = &cx.leading_args; + pub(crate) fn with_args(&mut self, cx: &'a Context<'_>) -> &mut Self { + self.propagated_leading_args = &cx.leading_args; self.trailing_args = cx.trailing_args; self.display_manifest_path = cx.verbose; self @@ -79,6 +84,12 @@ impl<'a> ProcessBuilder<'a> { } } + /// (chainable) Adds `arg` to the leading args list. + pub(crate) fn leading_arg(&mut self, arg: impl Into) -> &mut Self { + self.leading_args.push(arg.into()); + self + } + /// (chainable) Adds `arg` to the args list. pub(crate) fn arg(&mut self, arg: impl AsRef) -> &mut Self { self.args.push(arg.as_ref().to_os_string()); @@ -126,9 +137,9 @@ impl<'a> ProcessBuilder<'a> { /// Executes the process, waiting for completion, and mapping non-success exit codes to an error. pub(crate) fn exec(&mut self) -> Result<()> { - let mut command = self.build_command(); + let mut cmd = self.build_command(); - let exit = command.status().with_context(|| { + let exit = cmd.status().with_context(|| { self.display_all(); ProcessError::new(&format!("could not execute process {}", self), None, None) })?; @@ -148,9 +159,9 @@ impl<'a> ProcessBuilder<'a> { /// Executes the process, returning the stdio output, or an error if non-zero exit status. pub(crate) fn exec_with_output(&mut self) -> Result { - let mut command = self.build_command(); + let mut cmd = self.build_command(); - let output = command.output().with_context(|| { + let output = cmd.output().with_context(|| { self.display_all(); ProcessError::new(&format!("could not execute process {}", self), None, None) })?; @@ -171,20 +182,21 @@ impl<'a> ProcessBuilder<'a> { /// Converts `ProcessBuilder` into a `std::process::Command`, and handles the jobserver, if /// present. fn build_command(&self) -> Command { - let mut command = Command::new(&*self.program); + let mut cmd = Command::new(&*self.program); - command.args(&*self.leading_args); - command.args(&self.args); + cmd.args(&*self.leading_args); + cmd.args(&*self.propagated_leading_args); + cmd.args(&self.args); if !self.features.is_empty() { - command.arg("--features"); - command.arg(self.get_features()); + cmd.arg("--features"); + cmd.arg(self.get_features()); } if !self.trailing_args.is_empty() { - command.arg("--"); - command.args(&*self.trailing_args); + cmd.arg("--"); + cmd.args(&*self.trailing_args); } - command + cmd } } @@ -198,7 +210,11 @@ impl fmt::Display for ProcessBuilder<'_> { write!(f, "{}", Path::new(&*self.program).file_stem().unwrap().to_string_lossy())?; } - for arg in self.leading_args { + for arg in &self.leading_args { + write!(f, " {}", arg)?; + } + + for arg in self.propagated_leading_args { write!(f, " {}", arg)?; } diff --git a/src/rustup.rs b/src/rustup.rs new file mode 100644 index 00000000..093daea4 --- /dev/null +++ b/src/rustup.rs @@ -0,0 +1,104 @@ +use anyhow::{bail, format_err, Context as _}; +use std::str; + +use crate::{ + cargo, + version::{parse_version, Version}, + ProcessBuilder, Result, +}; + +pub(crate) struct Rustup { + pub(crate) version: u32, +} + +impl Rustup { + pub(crate) fn new() -> Self { + // If failed to determine rustup version, assign 0 to skip all version-dependent decisions. + let version = minor_version() + .map_err(|e| warn!("unable to determine rustup version: {:#}", e)) + .unwrap_or(0); + + Self { version } + } +} + +pub(crate) fn version_range(range: &str, step: Option<&str>) -> Result> { + let check = |version: &Version| { + if version.major != 1 { + bail!("major version must be 1"); + } + if let Some(patch) = version.patch { + warn!( + "--version-range always selects the latest patch release per minor release, \ + not the specified patch release `{}`", + patch + ) + } + Ok(()) + }; + + let mut split = range.splitn(2, ".."); + let start = split.next().map(parse_version).unwrap()?; + check(&start)?; + + let end = match split.next() { + Some("") | None => { + install_toolchain("stable")?; + cargo::minor_version(ProcessBuilder::new("cargo".as_ref()).args(&["+stable"]))? + } + Some(end) => { + let end = parse_version(end)?; + check(&end)?; + end.minor + } + }; + + let step = step.map(str::parse::).transpose()?.unwrap_or(1); + if step == 0 { + bail!("--version-step cannot be zero"); + } + + let versions: Vec<_> = + (start.minor..=end).step_by(step as _).map(|minor| format!("+1.{}", minor)).collect(); + if versions.is_empty() { + bail!("specified version range `{}` is empty", range); + } + Ok(versions) +} + +pub(crate) fn install_toolchain(toolchain: &str) -> Result<()> { + // In Github Actions and Azure Pipelines, --no-self-update is necessary + // because the windows environment cannot self-update rustup.exe. + rustup() + .args(&["toolchain", "install", toolchain, "--no-self-update"]) + .exec_with_output() + .map(drop) +} + +fn rustup<'a>() -> ProcessBuilder<'a> { + ProcessBuilder::new("rustup".as_ref()) +} + +fn minor_version() -> Result { + let mut cmd = rustup(); + cmd.args(&["--version"]); + let output = cmd.exec_with_output()?; + + let output = str::from_utf8(&output.stdout) + .with_context(|| format!("failed to parse output of {}", cmd))?; + + let version = (|| { + let mut output = output.split(' '); + if output.next()? != "rustup" { + return None; + } + output.next() + })() + .ok_or_else(|| format_err!("unexpected output from {}: {}", cmd, output))?; + let version = parse_version(version)?; + if version.major != 1 || version.patch.is_none() { + bail!("unexpected output from {}: {}", cmd, output); + } + + Ok(version.minor) +} diff --git a/tests/long-help.txt b/tests/long-help.txt index 7d11d11e..4fb77203 100644 --- a/tests/long-help.txt +++ b/tests/long-help.txt @@ -110,6 +110,16 @@ OPTIONS: Note that dependencies artifacts will be preserved. + --version-range ..[END] + Perform commands on a specified (inclusive) range of Rust versions. + + If the given range is unclosed, the latest stable compiler is treated as the upper bound. + + Note that ranges are always inclusive ranges. + + --version-step + Specify the version interval of --version-range. + -v, --verbose Use verbose output. diff --git a/tests/short-help.txt b/tests/short-help.txt index 5bcc653a..9e454625 100644 --- a/tests/short-help.txt +++ b/tests/short-help.txt @@ -28,6 +28,8 @@ OPTIONS: --ignore-private Skip to perform on `publish = false` packages --ignore-unknown-features Skip passing --features flag to `cargo` if that feature does not exist in the package --clean-per-run Remove artifacts for that package before running the command + --version-range ..[END] Perform commands on a specified (inclusive) range of Rust versions + --version-step Specify the version interval of --version-range -v, --verbose Use verbose output --color Coloring: auto, always, never -h, --help Prints help information diff --git a/tests/test.rs b/tests/test.rs index 4fae706d..8d8536ab 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1422,3 +1422,35 @@ fn default_feature_behavior() { .stdout_contains("no default feature!") .stdout_not_contains("has default feature!"); } + +// It seems rustup is not installed in the docker image provided by cross. +#[cfg_attr(any(not(target_arch = "x86_64"), target_env = "musl"), ignore)] +#[test] +fn version_range_failure() { + // zero step + cargo_hack(["check", "--version-range", "1.45..", "--version-step", "0"]) + .test_dir("tests/fixtures/real") + .assert_failure() + .stderr_contains("--version-step cannot be zero"); + + // empty + cargo_hack(["check", "--version-range", "1.45..1.44"]) + .test_dir("tests/fixtures/real") + .assert_failure() + .stderr_contains("specified version range `1.45..1.44` is empty"); + + // v0 + cargo_hack(["check", "--version-range", "0.45.."]) + .test_dir("tests/fixtures/real") + .assert_failure() + .stderr_contains("major version must be 1"); + + // patch version + cargo_hack(["check", "--version-range", "1.45.2.."]) + .test_dir("tests/fixtures/real") + .assert_success() // warn + .stderr_contains( + "--version-range always selects the latest patch release per minor release, \ + not the specified patch release `2`", + ); +}