Skip to content

fix(script): Be quiet on programmatic output #12305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 72 additions & 46 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use anyhow::{anyhow, Context as _};
use cargo::core::shell::Shell;
use cargo::core::{features, CliUnstable};
use cargo::{self, drop_print, drop_println, CliResult, Config};
use cargo::{self, drop_print, drop_println, CargoResult, CliResult, Config};
use clap::{Arg, ArgMatches};
use itertools::Itertools;
use std::collections::HashMap;
Expand Down Expand Up @@ -175,10 +175,11 @@ Run with 'cargo -Z [FLAG] [COMMAND]'",
return Ok(());
}
};
config_configure(config, &expanded_args, subcommand_args, global_args)?;
let exec = Exec::infer(cmd)?;
config_configure(config, &expanded_args, subcommand_args, global_args, &exec)?;
super::init_git(config);

execute_subcommand(config, cmd, subcommand_args)
exec.exec(config, subcommand_args)
}

pub fn get_version_string(is_verbose: bool) -> String {
Expand Down Expand Up @@ -363,12 +364,26 @@ fn config_configure(
args: &ArgMatches,
subcommand_args: &ArgMatches,
global_args: GlobalArgs,
exec: &Exec,
) -> CliResult {
let arg_target_dir = &subcommand_args.value_of_path("target-dir", config);
let verbose = global_args.verbose + args.verbose();
let mut verbose = global_args.verbose + args.verbose();
// quiet is unusual because it is redefined in some subcommands in order
// to provide custom help text.
let quiet = args.flag("quiet") || subcommand_args.flag("quiet") || global_args.quiet;
let mut quiet = args.flag("quiet") || subcommand_args.flag("quiet") || global_args.quiet;
if matches!(exec, Exec::Manifest(_)) && !quiet {
// Verbosity is shifted quieter for `Exec::Manifest` as it is can be used as if you ran
// `cargo install` and we especially shouldn't pollute programmatic output.
//
// For now, interactive output has the same default output as `cargo run` but that is
// subject to change.
if let Some(lower) = verbose.checked_sub(1) {
verbose = lower;
} else if !config.shell().is_err_tty() {
// Don't pollute potentially-scripted output
quiet = true;
}
}
let global_color = global_args.color; // Extract so it can take reference.
let color = args
.get_one::<String>("color")
Expand Down Expand Up @@ -399,53 +414,64 @@ fn config_configure(
Ok(())
}

/// Precedence isn't the most obvious from this function because
/// - Some is determined by `expand_aliases`
/// - Some is enforced by `avoid_ambiguity_between_builtins_and_manifest_commands`
///
/// In actuality, it is:
/// 1. built-ins xor manifest-command
/// 2. aliases
/// 3. external subcommands
fn execute_subcommand(config: &mut Config, cmd: &str, subcommand_args: &ArgMatches) -> CliResult {
if let Some(exec) = commands::builtin_exec(cmd) {
return exec(config, subcommand_args);
enum Exec {
Builtin(commands::Exec),
Manifest(String),
External(String),
}

impl Exec {
/// Precedence isn't the most obvious from this function because
/// - Some is determined by `expand_aliases`
/// - Some is enforced by `avoid_ambiguity_between_builtins_and_manifest_commands`
///
/// In actuality, it is:
/// 1. built-ins xor manifest-command
/// 2. aliases
/// 3. external subcommands
fn infer(cmd: &str) -> CargoResult<Self> {
if let Some(exec) = commands::builtin_exec(cmd) {
Ok(Self::Builtin(exec))
} else if commands::run::is_manifest_command(cmd) {
Ok(Self::Manifest(cmd.to_owned()))
} else {
Ok(Self::External(cmd.to_owned()))
}
}

if commands::run::is_manifest_command(cmd) {
let ext_path = super::find_external_subcommand(config, cmd);
if !config.cli_unstable().script && ext_path.is_some() {
config.shell().warn(format_args!(
"\
fn exec(self, config: &mut Config, subcommand_args: &ArgMatches) -> CliResult {
match self {
Self::Builtin(exec) => exec(config, subcommand_args),
Self::Manifest(cmd) => {
let ext_path = super::find_external_subcommand(config, &cmd);
if !config.cli_unstable().script && ext_path.is_some() {
config.shell().warn(format_args!(
"\
external subcommand `{cmd}` has the appearance of a manfiest-command
This was previously accepted but will be phased out when `-Zscript` is stabilized.
For more information, see issue #12207 <https://github.com/rust-lang/cargo/issues/12207>.",
))?;
let mut ext_args = vec![OsStr::new(cmd)];
ext_args.extend(
subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, cmd, &ext_args)
} else {
let ext_args: Vec<OsString> = subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.cloned()
.collect();
commands::run::exec_manifest_command(config, cmd, &ext_args)
))?;
Self::External(cmd).exec(config, subcommand_args)
} else {
let ext_args: Vec<OsString> = subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.cloned()
.collect();
commands::run::exec_manifest_command(config, &cmd, &ext_args)
}
}
Self::External(cmd) => {
let mut ext_args = vec![OsStr::new(&cmd)];
ext_args.extend(
subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, &cmd, &ext_args)
}
}
} else {
let mut ext_args = vec![OsStr::new(cmd)];
ext_args.extend(
subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, cmd, &ext_args)
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/bin/cargo/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ pub fn builtin() -> Vec<Command> {
]
}

pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResult> {
pub type Exec = fn(&mut Config, &ArgMatches) -> CliResult;

pub fn builtin_exec(cmd: &str) -> Option<Exec> {
let f = match cmd {
"add" => add::exec,
"bench" => bench::exec,
Expand Down
1 change: 1 addition & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,7 @@ A parameter is identified as a manifest-command if it has one of:

Differences between `cargo run --manifest-path <path>` and `cargo <path>`
- `cargo <path>` runs with the config for `<path>` and not the current dir, more like `cargo install --path <path>`
- `cargo <path>` is at a verbosity level below the normal default. Pass `-v` to get normal output.

### `[lints]`

Expand Down
Loading