Merge #168

168: Support multitarget r=taiki-e a=taiki-e

This supports multiple `--target` options. This uses cargo 1.64's [multi-target builds](https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html#cargo-improvements-workspace-inheritance-and-multi-target-builds) on cargo 1.64+, otherwise fallback to perform command per targets.

```console
$ cargo hack build --target x86_64-apple-darwin --target aarch64-apple-darwin --version-range 1.63..1.64
info: running `cargo +1.63 build --target x86_64-apple-darwin` on cargo-hack (1/3)
...
info: running `cargo +1.63 build --target aarch64-apple-darwin` on cargo-hack (2/3)
...
info: running `cargo +1.64 build --target x86_64-apple-darwin --target aarch64-apple-darwin` on cargo-hack (3/3)
...
```

Closes #167

Co-authored-by: Taiki Endo <te316e89@gmail.com>

CHANGELOG.md

@@ -10,6 +10,8 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Support multiple `--target` options. This uses cargo 1.64's [multi-target builds](https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html#cargo-improvements-workspace-inheritance-and-multi-target-builds) on cargo 1.64+, otherwise fallback to perform command per targets. ([#168](https://github.com/taiki-e/cargo-hack/pull/168))
+
 ## [0.5.19] - 2022-09-22
 
 - Fix "failed to parse `rust-version` field from manifest" error when workspace inheritance is used. ([#165](https://github.com/taiki-e/cargo-hack/pull/165))

src/cli.rs

@@ -75,14 +75,12 @@ pub(crate) struct Args {
     // options that will be propagated to cargo
     /// --features <FEATURES>...
     pub(crate) features: Vec<String>,
+    /// --target <TRIPLE>...
+    pub(crate) target: Vec<String>,
 
     // propagated to cargo (as a part of leading_args)
     /// --no-default-features
     pub(crate) no_default_features: bool,
-    // Note: specifying multiple `--target` flags requires unstable `-Z multitarget`,
-    // so cargo-hack currently only supports a single `--target`.
-    /// --target <TRIPLE>...
-    pub(crate) target: Option<String>,
 }
 
 impl Args {
@@ -155,7 +153,7 @@ impl Args {
         let mut verbose = 0;
         let mut no_default_features = false;
         let mut all_features = false;
-        let mut target = None;
+        let mut target = vec![];
 
         let mut parser = lexopt::Parser::from_args(args);
         let mut next_flag: Option<OwnedFlag> = None;
@@ -209,7 +207,7 @@ impl Args {
 
             match arg {
                 Long("color") => parse_opt!(color, true),
-                Long("target") => parse_opt!(target, true),
+                Long("target") => target.push(parser.value()?.parse()?),
 
                 Long("manifest-path") => parse_opt!(manifest_path, false),
                 Long("depth") => parse_opt!(depth, false),

src/context.rs

@@ -10,6 +10,7 @@ use std::{
 use anyhow::{bail, Result};
 
 use crate::{
+    cargo,
     cli::Args,
     features::Features,
     manifest::Manifest,
@@ -23,9 +24,10 @@ pub(crate) struct Context {
     manifests: HashMap<PackageId, Manifest>,
     pkg_features: HashMap<PackageId, Features>,
     cargo: PathBuf,
+    pub(crate) cargo_version: u32,
     pub(crate) restore: restore::Manager,
     pub(crate) current_dir: PathBuf,
-    pub(crate) version_range: Option<Vec<String>>,
+    pub(crate) version_range: Option<Vec<(u32, String)>>,
 }
 
 impl Context {
@@ -38,8 +40,13 @@ impl Context {
             "no subcommand or valid flag specified"
         );
 
+        // If failed to determine cargo version, assign 0 to skip all version-dependent decisions.
+        let cargo_version = cargo::minor_version(cmd!(&cargo))
+            .map_err(|e| warn!("unable to determine cargo version: {:#}", e))
+            .unwrap_or(0);
+
         let mut restore = restore::Manager::new(true);
-        let metadata = Metadata::new(&args, &cargo, &restore)?;
+        let metadata = Metadata::new(&args, &cargo, cargo_version, &restore)?;
         // if `--remove-dev-deps` flag is off, restore manifest file.
         restore.needs_restore = args.no_dev_deps && !args.remove_dev_deps;
         if metadata.cargo_version < 41 && args.include_deps_features {
@@ -63,6 +70,7 @@ impl Context {
             manifests,
             pkg_features,
             cargo: cargo.into(),
+            cargo_version,
             restore,
             current_dir: env::current_dir()?,
             version_range: None,

src/main.rs

@@ -63,13 +63,21 @@ fn exec_on_workspace(cx: &Context) -> Result<()> {
     let packages = determine_package_list(cx, &mut progress)?;
     let mut keep_going = KeepGoing::default();
     if let Some(range) = &cx.version_range {
-        progress.total *= range.len();
+        let total = progress.total;
+        progress.total = 0;
+        for (cargo_version, _) in range {
+            if cx.target.is_empty() || *cargo_version >= 64 {
+                progress.total += total;
+            } else {
+                progress.total += total * cx.target.len();
+            }
+        }
         let line = cmd!("cargo");
         {
             // First, generate the lockfile using the oldest cargo specified.
             // https://github.com/taiki-e/cargo-hack/issues/105
-            let toolchain = &range[0];
-            rustup::install_toolchain(toolchain, cx.target.as_deref(), true)?;
+            let toolchain = &range[0].1;
+            rustup::install_toolchain(toolchain, &cx.target, true)?;
             let mut line = line.clone();
             line.leading_arg(toolchain);
             line.arg("generate-lockfile");
@@ -86,9 +94,9 @@ fn exec_on_workspace(cx: &Context) -> Result<()> {
             line.run_with_output()?;
         }
 
-        range.iter().enumerate().try_for_each(|(i, toolchain)| {
+        range.iter().enumerate().try_for_each(|(i, (cargo_version, toolchain))| {
             if i != 0 {
-                rustup::install_toolchain(toolchain, cx.target.as_deref(), true)?;
+                rustup::install_toolchain(toolchain, &cx.target, true)?;
             }
 
             if cx.clean_per_version {
@@ -98,16 +106,12 @@ fn exec_on_workspace(cx: &Context) -> Result<()> {
             let mut line = line.clone();
             line.leading_arg(toolchain);
             line.apply_context(cx);
-            packages.iter().try_for_each(|(id, kind)| {
-                exec_on_package(cx, id, kind, &line, &mut progress, &mut keep_going)
-            })
+            exec_on_packages(cx, &packages, line, &mut progress, &mut keep_going, *cargo_version)
         })?;
     } else {
         let mut line = cx.cargo();
         line.apply_context(cx);
-        packages.iter().try_for_each(|(id, kind)| {
-            exec_on_package(cx, id, kind, &line, &mut progress, &mut keep_going)
-        })?;
+        exec_on_packages(cx, &packages, line, &mut progress, &mut keep_going, cx.cargo_version)?;
     }
     if keep_going.count > 0 {
         eprintln!();
@@ -287,6 +291,35 @@ fn determine_package_list<'a>(
     })
 }
 
+fn exec_on_packages(
+    cx: &Context,
+    packages: &[(&PackageId, Kind<'_>)],
+    mut line: ProcessBuilder<'_>,
+    progress: &mut Progress,
+    keep_going: &mut KeepGoing,
+    cargo_version: u32,
+) -> Result<()> {
+    if cx.target.is_empty() || cargo_version >= 64 {
+        // TODO: Test that cargo multitarget does not break the resolver behavior required for a correct check.
+        for target in &cx.target {
+            line.arg("--target");
+            line.arg(target);
+        }
+        packages
+            .iter()
+            .try_for_each(|(id, kind)| exec_on_package(cx, id, kind, &line, progress, keep_going))
+    } else {
+        cx.target.iter().try_for_each(|target| {
+            let mut line = line.clone();
+            line.arg("--target");
+            line.arg(target);
+            packages.iter().try_for_each(|(id, kind)| {
+                exec_on_package(cx, id, kind, &line, progress, keep_going)
+            })
+        })
+    }
+}
+
 fn exec_on_package(
     cx: &Context,
     id: &PackageId,

src/metadata.rs

@@ -46,11 +46,12 @@ pub(crate) struct Metadata {
 }
 
 impl Metadata {
-    pub(crate) fn new(args: &Args, cargo: &OsStr, restore: &restore::Manager) -> Result<Self> {
-        // If failed to determine cargo version, assign 0 to skip all version-dependent decisions.
-        let mut cargo_version = cargo::minor_version(cmd!(cargo))
-            .map_err(|e| warn!("unable to determine cargo version: {:#}", e))
-            .unwrap_or(0);
+    pub(crate) fn new(
+        args: &Args,
+        cargo: &OsStr,
+        mut cargo_version: u32,
+        restore: &restore::Manager,
+    ) -> Result<Self> {
         let stable_cargo_version = cargo::minor_version(cmd!("cargo", "+stable")).unwrap_or(0);
 
         let mut cmd;

src/rustup.rs

@@ -19,7 +19,11 @@ impl Rustup {
     }
 }
 
-pub(crate) fn version_range(range: &str, step: Option<&str>, cx: &Context) -> Result<Vec<String>> {
+pub(crate) fn version_range(
+    range: &str,
+    step: Option<&str>,
+    cx: &Context,
+) -> Result<Vec<(u32, String)>> {
     let check = |version: &Version| {
         if version.major != 1 {
             bail!("major version must be 1");
@@ -59,7 +63,7 @@ pub(crate) fn version_range(range: &str, step: Option<&str>, cx: &Context) -> Re
 
     let end = match split.next() {
         Some("") | None => {
-            install_toolchain("stable", None, false)?;
+            install_toolchain("stable", &[], false)?;
             cargo::minor_version(cmd!("cargo", "+stable"))?
         }
         Some(end) => {
@@ -74,8 +78,10 @@ pub(crate) fn version_range(range: &str, step: Option<&str>, cx: &Context) -> Re
         bail!("--version-step cannot be zero");
     }
 
-    let versions: Vec<_> =
-        (start.minor..=end).step_by(step as _).map(|minor| format!("+1.{}", minor)).collect();
+    let versions: Vec<_> = (start.minor..=end)
+        .step_by(step as _)
+        .map(|minor| (minor, format!("+1.{}", minor)))
+        .collect();
     if versions.is_empty() {
         bail!("specified version range `{}` is empty", range);
     }
@@ -84,12 +90,12 @@ pub(crate) fn version_range(range: &str, step: Option<&str>, cx: &Context) -> Re
 
 pub(crate) fn install_toolchain(
     mut toolchain: &str,
-    target: Option<&str>,
+    target: &[String],
     print_output: bool,
 ) -> Result<()> {
     toolchain = toolchain.strip_prefix('+').unwrap_or(toolchain);
 
-    if target.is_none()
+    if target.is_empty()
         && cmd!("cargo", format!("+{}", toolchain), "--version").run_with_output().is_ok()
     {
         // Do not run `rustup toolchain install` if the toolchain already has installed.
@@ -99,8 +105,8 @@ pub(crate) fn install_toolchain(
     // In Github Actions and Azure Pipelines, --no-self-update is necessary
     // because the windows environment cannot self-update rustup.exe.
     let mut cmd = cmd!("rustup", "toolchain", "add", toolchain, "--no-self-update");
-    if let Some(target) = target {
-        cmd.args(["--target", target]);
+    if !target.is_empty() {
+        cmd.args(["--target", &target.join(",")]);
     }
 
     if print_output {

tests/test.rs

@@ -1340,6 +1340,24 @@ fn version_range() {
             running `cargo +1.59 check --target x86_64-unknown-linux-musl` on real (2/2)
             ",
         );
+
+        cargo_hack([
+            "check",
+            "--version-range",
+            "1.63..1.64",
+            "--target",
+            "x86_64-unknown-linux-gnu",
+            "--target",
+            "x86_64-unknown-linux-musl",
+        ])
+        .assert_success("real")
+        .stderr_contains(
+            "
+            running `cargo +1.63 check --target x86_64-unknown-linux-gnu` on real (1/3)
+            running `cargo +1.63 check --target x86_64-unknown-linux-musl` on real (2/3)
+            running `cargo +1.64 check --target x86_64-unknown-linux-gnu --target x86_64-unknown-linux-musl` on real (3/3)
+            ",
+        );
     }
 }