Skip to content

Commit

Permalink
Use rustup run <toolchain> instead of cargo +<toolchain>
Browse files Browse the repository at this point in the history
  • Loading branch information
@taiki-e
taiki-e committed Sep 7, 2023
1 parent a70540a commit b0e4b4b
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 46 deletions.
6 changes: 4 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,12 +316,14 @@ 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 `rustup run 1.46 cargo check` on cargo-hack (1/2)
...
info: running `cargo +1.47 check` on cargo-hack (2/2)
info: running `rustup run 1.47 cargo check` on cargo-hack (2/2)
...
```

(We use `rustup run <toolchain> cargo` instead of `cargo +<toolchain>` to work around a [rustup bug](https://github.com/rust-lang/rustup/issues/3036).)

This might be useful for catching issues like [termcolor#35], [regex#685],
[rust-clippy#6324].

Expand Down
6 changes: 5 additions & 1 deletion src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ fn try_main() -> Result<()> {
progress.total += total * cx.target.len();
}
}
let line = cmd!("cargo");
// Do not use `cargo +<toolchain>` due to a rustup bug: https://github.com/rust-lang/rustup/issues/3036
let mut line = cmd!("rustup");
line.leading_arg("run");

// First, generate the lockfile using the oldest cargo specified.
// https://github.com/taiki-e/cargo-hack/issues/105
Expand All @@ -84,6 +86,7 @@ fn try_main() -> Result<()> {
if generate_lockfile || regenerate_lockfile_on_51_or_up && *cargo_version >= 51 {
let mut line = line.clone();
line.leading_arg(toolchain);
line.leading_arg("cargo");
line.arg("generate-lockfile");
if let Some(pid) = cx.current_package() {
let package = cx.packages(pid);
Expand Down Expand Up @@ -111,6 +114,7 @@ fn try_main() -> Result<()> {

let mut line = line.clone();
line.leading_arg(toolchain);
line.leading_arg("cargo");
line.apply_context(cx);
exec_on_packages(
cx,
Expand Down
4 changes: 2 additions & 2 deletions src/metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl Metadata {
restore: &restore::Manager,
) -> Result<Self> {
let stable_cargo_version =
cargo::version(cmd!("cargo", "+stable")).map(|v| v.minor).unwrap_or(0);
cargo::version(cmd!("rustup", "run", "stable", "cargo")).map(|v| v.minor).unwrap_or(0);

let mut cmd;
let json = if stable_cargo_version > cargo_version {
Expand All @@ -79,7 +79,7 @@ impl Metadata {
// Try with stable cargo because if workspace member has
// a dependency that requires newer cargo features, `cargo metadata`
// with older cargo may fail.
cmd = cmd!("cargo", "+stable", "metadata", "--format-version=1");
cmd = cmd!("rustup", "run", "stable", "cargo", "metadata", "--format-version=1");
if let Some(manifest_path) = &args.manifest_path {
cmd.arg("--manifest-path");
cmd.arg(manifest_path);
Expand Down
6 changes: 3 additions & 3 deletions src/rustup.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub(crate) fn version_range(range: VersionRange, cx: &Context) -> Result<Vec<(u3
Ok(stable_version)
} else {
install_toolchain("stable", &[], false)?;
let version = cargo::version(cmd!("cargo", "+stable"))?;
let version = cargo::version(cmd!("rustup", "run", "stable", "cargo"))?;
stable_version = Some(version);
Ok(version)
}
Expand Down Expand Up @@ -100,7 +100,7 @@ pub(crate) fn version_range(range: VersionRange, cx: &Context) -> Result<Vec<(u3

let versions: Vec<_> = (start_inclusive.minor..=end_inclusive.minor)
.step_by(cx.version_step as _)
.map(|minor| (minor, format!("+1.{minor}")))
.map(|minor| (minor, format!("1.{minor}")))
.collect();
if versions.is_empty() {
bail!("specified version range `{range}` is empty");
Expand All @@ -116,7 +116,7 @@ pub(crate) fn install_toolchain(
toolchain = toolchain.strip_prefix('+').unwrap_or(toolchain);

if target.is_empty()
&& cmd!("cargo", format!("+{toolchain}"), "--version").run_with_output().is_ok()
&& cmd!("rustup", "run", toolchain, "cargo", "--version").run_with_output().is_ok()
{
// Do not run `rustup toolchain add` if the toolchain already has installed.
return Ok(());
Expand Down
16 changes: 5 additions & 11 deletions tests/auxiliary/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
env::{self, consts::EXE_SUFFIX},
env,
ffi::OsStr,
path::{Path, PathBuf},
process::{Command, ExitStatus},
Expand All @@ -26,14 +26,6 @@ pub fn cargo_bin_exe() -> Command {
cmd
}

fn test_toolchain() -> String {
if let Some(toolchain) = test_version() {
format!("+1.{toolchain} ")
} else {
String::new()
}
}

fn test_version() -> Option<u32> {
static TEST_VERSION: OnceLock<Option<u32>> = OnceLock::new();
*TEST_VERSION.get_or_init(|| {
Expand Down Expand Up @@ -145,10 +137,12 @@ struct AssertOutputInner {
}

fn replace_command(lines: &str) -> String {
if lines.contains("cargo +") || lines.contains(&format!("cargo{EXE_SUFFIX} +")) {
if lines.contains("rustup run") {
lines.to_owned()
} else if let Some(minor) = test_version() {
lines.replace("cargo ", &format!("rustup run 1.{minor} cargo "))
} else {
lines.replace("cargo ", &format!("cargo {}", test_toolchain()))
lines.to_owned()
}
}
fn line_separated(lines: &str) -> impl Iterator<Item = &'_ str> {
Expand Down
44 changes: 17 additions & 27 deletions tests/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1315,13 +1315,12 @@ fn default_feature_behavior() {
.stdout_not_contains("has default feature!");
}

#[cfg_attr(windows, ignore)] // rustup bug: https://github.com/rust-lang/rustup/issues/3036
#[test]
fn version_range() {
cargo_hack(["check", "--version-range", "1.63..=1.64"]).assert_success("real").stderr_contains(
"
running `cargo +1.63 check` on real (1/2)
running `cargo +1.64 check` on real (2/2)
running `rustup run 1.63 cargo check` on real (1/2)
running `rustup run 1.64 cargo check` on real (2/2)
",
);

Expand All @@ -1330,49 +1329,41 @@ fn version_range() {
.stderr_contains(
"
warning: using `..` for inclusive range is deprecated; consider using `1.63..=1.64`
running `cargo +1.63 check` on real (1/2)
running `cargo +1.64 check` on real (2/2)
running `rustup run 1.63 cargo check` on real (1/2)
running `rustup run 1.64 cargo check` on real (2/2)
",
);

cargo_hack(["check", "--version-range", "1.63..=1.64", "--target", TARGET])
.assert_success("real")
.stderr_contains(format!(
"
running `cargo +1.63 check --target {TARGET}` on real (1/2)
running `cargo +1.64 check --target {TARGET}` on real (2/2)
running `rustup run 1.63 cargo check --target {TARGET}` on real (1/2)
running `rustup run 1.64 cargo check --target {TARGET}` on real (2/2)
",
));

cargo_hack(["check", "--version-range", "..=1.64"])
.assert_success("rust-version")
.stderr_contains(
"
running `cargo +1.64 check` on real (1/1)
running `rustup run 1.64 cargo check` on real (1/1)
",
);
}

#[cfg_attr(windows, ignore)] // rustup bug: https://github.com/rust-lang/rustup/issues/3036
#[test]
fn rust_version() {
cargo_hack(["check", "--rust-version"]).assert_success("rust-version").stderr_contains(
"
running `cargo +1.64 check` on real (1/1)
",
running `rustup run 1.64 cargo check` on real (1/1)
",
);
}

#[cfg_attr(windows, ignore)] // rustup bug: https://github.com/rust-lang/rustup/issues/3036
#[test]
fn multi_target() {
let target_suffix = if cfg!(target_os = "linux") && cfg!(target_env = "gnu") {
"-unknown-linux-gnu"
} else if cfg!(target_os = "macos") {
"-apple-darwin"
} else {
unimplemented!()
};
let target_suffix = String::from("-") + TARGET.split_once('-').unwrap().1;

cargo_hack([
"check",
Expand All @@ -1384,8 +1375,8 @@ fn multi_target() {
.assert_success("real")
.stderr_contains(format!(
"
running `cargo +1.63 check --target aarch64{target_suffix}` on real (1/2)
running `cargo +1.64 check --target aarch64{target_suffix}` on real (2/2)
running `rustup run 1.63 cargo check --target aarch64{target_suffix}` on real (1/2)
running `rustup run 1.64 cargo check --target aarch64{target_suffix}` on real (2/2)
"
));

Expand All @@ -1401,9 +1392,9 @@ fn multi_target() {
.assert_success("real")
.stderr_contains(format!(
"
running `cargo +1.63 check --target aarch64{target_suffix}` on real (1/3)
running `cargo +1.63 check --target x86_64{target_suffix}` on real (2/3)
running `cargo +1.64 check --target aarch64{target_suffix} --target x86_64{target_suffix}` on real (3/3)
running `rustup run 1.63 cargo check --target aarch64{target_suffix}` on real (1/3)
running `rustup run 1.63 cargo check --target x86_64{target_suffix}` on real (2/3)
running `rustup run 1.64 cargo check --target aarch64{target_suffix} --target x86_64{target_suffix}` on real (3/3)
",
));

Expand All @@ -1419,13 +1410,12 @@ fn multi_target() {
.assert_success("real")
.stderr_contains(format!(
"
running `cargo +1.63 check --target x86_64{target_suffix}` on real (1/2)
running `cargo +1.64 check --target x86_64{target_suffix}` on real (2/2)
running `rustup run 1.63 cargo check --target x86_64{target_suffix}` on real (1/2)
running `rustup run 1.64 cargo check --target x86_64{target_suffix}` on real (2/2)
",
));
}

#[cfg_attr(windows, ignore)] // rustup bug: https://github.com/rust-lang/rustup/issues/3036
#[test]
fn version_range_failure() {
// zero step
Expand Down

0 comments on commit b0e4b4b

Please sign in to comment.