Skip to content

Commit

Permalink
Auto merge of #12607 - epage:target, r=weihanglo
Browse files Browse the repository at this point in the history
fix(cli): Help users know possible `--target` values

### What does this PR try to resolve?

I was needing to do some more cross-compilation and forgot what the target triple was that I needed to run.  I realized i had to re-remember the command yet again.   Especially with #12585 in memory, I realized that `--target` isn't working like `--package` and other arguments that can report supported values.

In working on it, I realized I probably didn't want to report supported values yet out of concern for how big the list is (see also #12585), so I decided to just list the relevant commands for now.  We *might* be able to parse the rustup output to report those targets but I didn't receive a glowing endorsement from the rustup team about parsing the list (more of "yes, hyrums law and at least its interactive rather than CI").

Before:
```
error: a value is required for '--target <TRIPLE>' but none was supplied

For more information, try '--help'.
```

After:
```
error: "--target" takes a target architecture as an argument.

Run `rustup target list` to see possible targets.
```
(quotes were used because the other "list available" use them, we should probably work to be uniform in how we quote)

### How should we test and review this PR?

First PR adds a test showing the existing output and then through the rest you can see how the output changed
  • Loading branch information
bors committed Aug 31, 2023
2 parents 11966a4 + 802cb38 commit 77a9b2d
Show file tree
Hide file tree
Showing 24 changed files with 77 additions and 42 deletions.
7 changes: 2 additions & 5 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::fmt::Write;
use super::commands;
use super::list_commands;
use crate::command_prelude::*;
use crate::util::is_rustup;
use cargo::core::features::HIDDEN;

pub fn main(config: &mut LazyConfig) -> CliResult {
Expand Down Expand Up @@ -511,11 +512,7 @@ impl GlobalArgs {
}

pub fn cli() -> Command {
// ALLOWED: `RUSTUP_HOME` should only be read from process env, otherwise
// other tools may point to executables from incompatible distributions.
#[allow(clippy::disallowed_methods)]
let is_rustup = std::env::var_os("RUSTUP_HOME").is_some();
let usage = if is_rustup {
let usage = if is_rustup() {
"cargo [+toolchain] [OPTIONS] [COMMAND]\n cargo [+toolchain] [OPTIONS] -Zscript <MANIFEST_RS> [ARGS]..."
} else {
"cargo [OPTIONS] [COMMAND]\n cargo [OPTIONS] -Zscript <MANIFEST_RS> [ARGS]..."
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let opts = CleanOptions {
config,
spec: values(args, "package"),
targets: args.targets(),
targets: args.targets()?,
requested_profile: args.get_profile_name(config, "dev", ProfileChecking::Custom)?,
profile_specified: args.contains_id("profile") || args.flag("release"),
doc: args.flag("doc"),
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {

let opts = FetchOptions {
config,
targets: args.targets(),
targets: args.targets()?,
};
let _ = ops::fetch(&ws, &opts)?;
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
check_metadata: !args.flag("no-metadata"),
allow_dirty: args.flag("allow-dirty"),
to_package: specs,
targets: args.targets(),
targets: args.targets()?,
jobs: args.jobs()?,
keep_going: args.keep_going(),
cli_features: args.cli_features()?,
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
verify: !args.flag("no-verify"),
allow_dirty: args.flag("allow-dirty"),
to_publish: args.packages_from_flags()?,
targets: args.targets(),
targets: args.targets()?,
jobs: args.jobs()?,
keep_going: args.keep_going(),
dry_run: args.dry_run(),
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
.warn("the --all-targets flag has been changed to --target=all")?;
vec!["all".to_string()]
} else {
args._values_of("target")
args.targets()?
};
let target = tree::Target::from_cli(targets);

Expand Down
24 changes: 20 additions & 4 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::core::{Edition, Workspace};
use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl};
use crate::util::important_paths::find_root_manifest_for_wd;
use crate::util::interning::InternedString;
use crate::util::is_rustup;
use crate::util::restricted_names::is_glob_pattern;
use crate::util::toml::{StringOrVec, TomlProfile};
use crate::util::validate_package_name;
Expand Down Expand Up @@ -218,7 +219,10 @@ pub trait CommandExt: Sized {
}

fn arg_target_triple(self, target: &'static str) -> Self {
self._arg(multi_opt("target", "TRIPLE", target).help_heading(heading::COMPILATION_OPTIONS))
self._arg(
optional_multi_opt("target", "TRIPLE", target)
.help_heading(heading::COMPILATION_OPTIONS),
)
}

fn arg_target_dir(self) -> Self {
Expand Down Expand Up @@ -440,8 +444,20 @@ pub trait ArgMatchesExt {
self.maybe_flag("keep-going")
}

fn targets(&self) -> Vec<String> {
self._values_of("target")
fn targets(&self) -> CargoResult<Vec<String>> {
if self.is_present_with_zero_values("target") {
let cmd = if is_rustup() {
"rustup target list"
} else {
"rustc --print target-list"
};
bail!(
"\"--target\" takes a target architecture as an argument.
Run `{cmd}` to see possible targets."
);
}
Ok(self._values_of("target"))
}

fn get_profile_name(
Expand Down Expand Up @@ -590,7 +606,7 @@ pub trait ArgMatchesExt {
config,
self.jobs()?,
self.keep_going(),
&self.targets(),
&self.targets()?,
mode,
)?;
build_config.message_format = message_format.unwrap_or(MessageFormat::Human);
Expand Down
7 changes: 7 additions & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ pub mod toml_mut;
mod vcs;
mod workspace;

pub fn is_rustup() -> bool {
// ALLOWED: `RUSTUP_HOME` should only be read from process env, otherwise
// other tools may point to executables from incompatible distributions.
#[allow(clippy::disallowed_methods)]
std::env::var_os("RUSTUP_HOME").is_some()
}

pub fn elapsed(duration: Duration) -> String {
let secs = duration.as_secs();

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_bench/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Feature Selection:
Compilation Options:
-j, --jobs <N> Number of parallel jobs, defaults to # of CPUs.
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_build/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Compilation Options:
--profile <PROFILE-NAME> Build artifacts with the specified profile
-j, --jobs <N> Number of parallel jobs, defaults to # of CPUs.
--keep-going Do not abort the build as soon as there is an error
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--out-dir <PATH> Copy final artifacts to this directory (unstable)
--build-plan Output the build plan in JSON (unstable)
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_check/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Compilation Options:
--keep-going Do not abort the build as soon as there is an error
-r, --release Check artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Check artifacts with the specified profile
--target <TRIPLE> Check for the target triple
--target [<TRIPLE>] Check for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_clean/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Package Selection:
Compilation Options:
-r, --release Whether or not to clean release artifacts
--profile <PROFILE-NAME> Clean artifacts of the specified profile
--target <TRIPLE> Target triple to clean output for
--target [<TRIPLE>] Target triple to clean output for
--target-dir <DIRECTORY> Directory for all generated artifacts

Manifest Options:
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_doc/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Compilation Options:
--keep-going Do not abort the build as soon as there is an error
-r, --release Build artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_fetch/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Options:
-h, --help Print help

Compilation Options:
--target <TRIPLE> Fetch dependencies for the target triple
--target [<TRIPLE>] Fetch dependencies for the target triple

Manifest Options:
--manifest-path <PATH> Path to Cargo.toml
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_fix/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Compilation Options:
--keep-going Do not abort the build as soon as there is an error
-r, --release Fix artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Fix for the target triple
--target [<TRIPLE>] Fix for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_install/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Compilation Options:
-j, --jobs <N> Number of parallel jobs, defaults to # of CPUs.
--keep-going Do not abort the build as soon as there is an error
--profile <PROFILE-NAME> Install artifacts with the specified profile
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_package/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Feature Selection:
--no-default-features Do not activate the `default` feature

Compilation Options:
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
-j, --jobs <N> Number of parallel jobs, defaults to # of CPUs.
--keep-going Do not abort the build as soon as there is an error
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_publish/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Feature Selection:
Compilation Options:
-j, --jobs <N> Number of parallel jobs, defaults to # of CPUs.
--keep-going Do not abort the build as soon as there is an error
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts

Manifest Options:
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_run/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Compilation Options:
--keep-going Do not abort the build as soon as there is an error
-r, --release Build artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_rustc/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Compilation Options:
--keep-going Do not abort the build as soon as there is an error
-r, --release Build artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Target triple which compiles will be for
--target [<TRIPLE>] Target triple which compiles will be for
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_rustdoc/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Compilation Options:
--keep-going Do not abort the build as soon as there is an error
-r, --release Build artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_test/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Compilation Options:
-j, --jobs <N> Number of parallel jobs, defaults to # of CPUs.
-r, --release Build artifacts in release mode, with optimizations
--profile <PROFILE-NAME> Build artifacts with the specified profile
--target <TRIPLE> Build for the target triple
--target [<TRIPLE>] Build for the target triple
--target-dir <DIRECTORY> Directory for all generated artifacts
--unit-graph Output build graph in JSON (unstable)
--timings[=<FMTS>] Timing output formats (unstable) (comma separated): html, json
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/cargo_tree/help/stdout.log
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ Feature Selection:
--no-default-features Do not activate the `default` feature

Compilation Options:
--target <TRIPLE> Filter dependencies matching the given target-triple (default host
platform). Pass `all` to include all targets.
--target [<TRIPLE>] Filter dependencies matching the given target-triple (default host
platform). Pass `all` to include all targets.

Manifest Options:
--manifest-path <PATH> Path to Cargo.toml
Expand Down
39 changes: 27 additions & 12 deletions tests/testsuite/list_availables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const BIN: u8 = 1 << 1;
const TEST: u8 = 1 << 2;
const BENCH: u8 = 1 << 3;
const PACKAGE: u8 = 1 << 4;
const TARGET: u8 = 1 << 5;

fn list_availables_test(command: &str, targets: u8) {
let full_project = project()
Expand Down Expand Up @@ -154,6 +155,20 @@ No benches available.
error: \"--test\" takes one argument.
No tests available.
",
)
.with_status(101)
.run();
}

if targets & TARGET != 0 {
empty_project
.cargo(&format!("{} --target", command))
.with_stderr(
"\
error: \"--target\" takes a target architecture as an argument.
Run `[..]` to see possible targets.
",
)
.with_status(101)
Expand All @@ -163,52 +178,52 @@ No tests available.

#[cargo_test]
fn build_list_availables() {
list_availables_test("build", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("build", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
fn check_list_availables() {
list_availables_test("check", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("check", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
fn doc_list_availables() {
list_availables_test("doc", BIN | PACKAGE);
list_availables_test("doc", BIN | PACKAGE | TARGET);
}

#[cargo_test]
fn fix_list_availables() {
list_availables_test("fix", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("fix", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
fn run_list_availables() {
list_availables_test("run", EXAMPLE | BIN | PACKAGE);
list_availables_test("run", EXAMPLE | BIN | PACKAGE | TARGET);
}

#[cargo_test]
fn test_list_availables() {
list_availables_test("test", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("test", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
fn bench_list_availables() {
list_availables_test("bench", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("bench", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
fn install_list_availables() {
list_availables_test("install", EXAMPLE | BIN);
list_availables_test("install", EXAMPLE | BIN | TARGET);
}

#[cargo_test]
fn rustdoc_list_availables() {
list_availables_test("rustdoc", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("rustdoc", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
fn rustc_list_availables() {
list_availables_test("rustc", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
list_availables_test("rustc", EXAMPLE | BIN | TEST | BENCH | PACKAGE | TARGET);
}

#[cargo_test]
Expand All @@ -218,12 +233,12 @@ fn pkgid_list_availables() {

#[cargo_test]
fn tree_list_availables() {
list_availables_test("tree", PACKAGE);
list_availables_test("tree", PACKAGE | TARGET);
}

#[cargo_test]
fn clean_list_availables() {
list_availables_test("clean", PACKAGE);
list_availables_test("clean", PACKAGE | TARGET);
}

#[cargo_test]
Expand Down

0 comments on commit 77a9b2d

Please sign in to comment.