Auto merge of #12305 - epage:quiet, r=weihanglo

fix(script): Be quiet on programmatic output

### What does this PR try to resolve?

This is a part of #12207

The goal is that we shouldn't interfere with end-user output when
"cargo script"s are used programmatically.  The only way to detect this
is when piping.  CI will also look like this.

My thought is that if someone does want to do `#!/usr/bin/env -S cargo -v`, it
should have a consistent meaning between local development
(`cargo run --manifest-path`) and "script mode" (`cargo`), so I
effectively added a new verbosity level in these cases.  To get normal
output in all cases, add a `-v` like the tests do.  Do `-vv` if you want
the normal `-v` mode.  If you want it always quiet, do `--quiet`.

I want to see the default verbosity for interactive "script mode" a bit
quieter to the point that all normal output cargo makes is cleared before
running the built binary.  I am holding off on that now as that could
tie into bigger conversations / refactors
(see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output).

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

Initial writing of tests and refactors are split split out.  The tests in the final commit will let you see what impact this had on behavior.
This commit is contained in:
bors 2023-06-23 23:27:46 +00:00
commit 03bc66b55c
4 changed files with 144 additions and 73 deletions

View File

@ -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;
@ -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 {
@ -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")
@ -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)
}
}

View File

@ -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,

View File

@ -1484,6 +1484,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]`

View File

@ -26,7 +26,7 @@ fn basic_rs() {
.file("echo.rs", ECHO_SCRIPT)
.build();
p.cargo("-Zscript echo.rs")
p.cargo("-Zscript -v echo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/echo[EXE]
@ -50,7 +50,7 @@ fn basic_path() {
.file("echo", ECHO_SCRIPT)
.build();
p.cargo("-Zscript ./echo")
p.cargo("-Zscript -v ./echo")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/echo[EXE]
@ -74,7 +74,7 @@ fn basic_cargo_toml() {
.file("src/main.rs", ECHO_SCRIPT)
.build();
p.cargo("-Zscript Cargo.toml")
p.cargo("-Zscript -v Cargo.toml")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: target/debug/foo[EXE]
@ -97,7 +97,7 @@ fn path_required() {
.file("echo", ECHO_SCRIPT)
.build();
p.cargo("-Zscript echo")
p.cargo("-Zscript -v echo")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
@ -126,7 +126,7 @@ fn manifest_precedence_over_plugins() {
path.push(p.root().join("path-test"));
let path = std::env::join_paths(path.iter()).unwrap();
p.cargo("-Zscript echo.rs")
p.cargo("-Zscript -v echo.rs")
.env("PATH", &path)
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
@ -157,7 +157,7 @@ fn warn_when_plugin_masks_manifest_on_stable() {
path.push(p.root().join("path-test"));
let path = std::env::join_paths(path.iter()).unwrap();
p.cargo("echo.rs")
p.cargo("-v echo.rs")
.env("PATH", &path)
.with_stdout("")
.with_stderr(
@ -176,7 +176,7 @@ fn requires_nightly() {
.file("echo.rs", ECHO_SCRIPT)
.build();
p.cargo("echo.rs")
p.cargo("-v echo.rs")
.with_status(101)
.with_stdout("")
.with_stderr(
@ -193,7 +193,7 @@ fn requires_z_flag() {
.file("echo.rs", ECHO_SCRIPT)
.build();
p.cargo("echo.rs")
p.cargo("-v echo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
@ -221,7 +221,7 @@ fn main() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"Hello world!
@ -252,7 +252,7 @@ fn main() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"Hello world!
@ -281,7 +281,7 @@ fn main() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"msg = undefined
@ -298,7 +298,7 @@ fn main() {
.run();
// Verify we don't rebuild
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"msg = undefined
@ -314,7 +314,7 @@ fn main() {
.run();
// Verify we do rebuild
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.env("_MESSAGE", "hello")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
@ -373,6 +373,48 @@ args: ["-NotAnArg"]
.run();
}
#[cargo_test]
fn default_programmatic_verbosity() {
let script = ECHO_SCRIPT;
let p = cargo_test_support::project()
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs -NotAnArg")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/script[EXE]
args: ["-NotAnArg"]
"#,
)
.with_stderr(
"\
",
)
.run();
}
#[cargo_test]
fn quiet() {
let script = ECHO_SCRIPT;
let p = cargo_test_support::project()
.file("script.rs", script)
.build();
p.cargo("-Zscript -q script.rs -NotAnArg")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/script[EXE]
args: ["-NotAnArg"]
"#,
)
.with_stderr(
"\
",
)
.run();
}
#[cargo_test]
fn test_line_numbering_preserved() {
let script = r#"#!/usr/bin/env cargo
@ -385,7 +427,7 @@ fn main() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"line: 4
@ -409,7 +451,7 @@ fn test_escaped_hyphen_arg() {
.file("script.rs", script)
.build();
p.cargo("-Zscript -- script.rs -NotAnArg")
p.cargo("-Zscript -v -- script.rs -NotAnArg")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/script[EXE]
@ -434,7 +476,7 @@ fn test_unescaped_hyphen_arg() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs -NotAnArg")
p.cargo("-Zscript -v script.rs -NotAnArg")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/script[EXE]
@ -459,7 +501,7 @@ fn test_same_flags() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs --help")
p.cargo("-Zscript -v script.rs --help")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/script[EXE]
@ -484,7 +526,7 @@ fn test_name_has_weird_chars() {
.file("s-h.w§c!.rs", script)
.build();
p.cargo("-Zscript s-h.w§c!.rs")
p.cargo("-Zscript -v s-h.w§c!.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [..]/debug/s-h-w-c-[EXE]
@ -507,7 +549,7 @@ fn script_like_dir() {
.file("script.rs/foo", "something")
.build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stderr(
@ -522,7 +564,7 @@ error: manifest path `script.rs` is a directory but expected a file
fn missing_script_rs() {
let p = cargo_test_support::project().build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stderr(
@ -550,7 +592,7 @@ fn main() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs --help")
p.cargo("-Zscript -v script.rs --help")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"Hello world!
@ -590,7 +632,7 @@ fn main() {
.file("bar/src/lib.rs", "pub fn bar() {}")
.build();
p.cargo("-Zscript script.rs --help")
p.cargo("-Zscript -v script.rs --help")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"Hello world!
@ -620,7 +662,7 @@ fn main() {
.file("build.rs", "broken")
.build();
p.cargo("-Zscript script.rs --help")
p.cargo("-Zscript -v script.rs --help")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"Hello world!
@ -649,7 +691,7 @@ fn main() {
.file("src/bin/not-script/main.rs", "fn main() {}")
.build();
p.cargo("-Zscript script.rs --help")
p.cargo("-Zscript -v script.rs --help")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"Hello world!
@ -673,7 +715,7 @@ fn implicit_target_dir() {
.file("script.rs", script)
.build();
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [ROOT]/home/.cargo/target/[..]/debug/script[EXE]
@ -701,7 +743,7 @@ fn no_local_lockfile() {
assert!(!local_lockfile_path.exists());
p.cargo("-Zscript script.rs")
p.cargo("-Zscript -v script.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_stdout(
r#"bin: [ROOT]/home/.cargo/target/[..]/debug/script[EXE]