Skip to content

Commit 98079d9

Browse files
borsbors
committed
Auto merge of #13479 - epage:cfg, r=weihanglo
fix(cli): Respect CARGO_TERM_COLOR in '--list' and '-Zhelp' ### What does this PR try to resolve? Similar to #9012, we aren't respecting `CARGO_TERM_COLOR` for `-Zhelp` and other places. This corrects that. ### How should we test and review this PR? #9012 was about initialization order to get the value. Here, the problem is that we don't update `Shell` with `CARGO_TERM_COLOR`. In doing this, I was concerned about keeping track of where it is safe to call `config_configure` without running it twice. To this end, I refactored `main` to make it clear that each call to `config_configure` is in a mutually exclusive path that exists immediately. ### Additional information Found this with the test for `-Zhelp` in #13461.
2 parents b425030 + 2ec0461 commit 98079d9

File tree

2 files changed

+166
-150
lines changed

2 files changed

+166
-150
lines changed

‎src/bin/cargo/cli.rs

Lines changed: 133 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -48,141 +48,155 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {
4848

4949
let (expanded_args, global_args) = expand_aliases(gctx, args, vec![])?;
5050

51+
let is_verbose = expanded_args.verbose() > 0;
52+
5153
if expanded_args
5254
.get_one::<String>("unstable-features")
5355
.map(String::as_str)
5456
== Some("help")
5557
{
56-
let header = style::HEADER;
57-
let literal = style::LITERAL;
58-
let placeholder = style::PLACEHOLDER;
59-
60-
let options = CliUnstable::help();
61-
let max_length = options
62-
.iter()
63-
.filter(|(_, help)| help.is_some())
64-
.map(|(option_name, _)| option_name.len())
65-
.max()
66-
.unwrap_or(0);
67-
let z_flags = options
68-
.iter()
69-
.filter(|(_, help)| help.is_some())
70-
.map(|(opt, help)| {
71-
let opt = opt.replace("_", "-");
72-
let help = help.unwrap();
73-
format!(" {literal}-Z {opt:<max_length$}{literal:#} {help}")
74-
})
75-
.join("\n");
76-
drop_println!(
58+
// Don't let config errors get in the way of parsing arguments
59+
let _ = config_configure(gctx, &expanded_args, None, global_args, None);
60+
print_zhelp(gctx);
61+
} else if expanded_args.flag("version") {
62+
// Don't let config errors get in the way of parsing arguments
63+
let _ = config_configure(gctx, &expanded_args, None, global_args, None);
64+
let version = get_version_string(is_verbose);
65+
drop_print!(gctx, "{}", version);
66+
} else if let Some(code) = expanded_args.get_one::<String>("explain") {
67+
// Don't let config errors get in the way of parsing arguments
68+
let _ = config_configure(gctx, &expanded_args, None, global_args, None);
69+
let mut procss = gctx.load_global_rustc(None)?.process();
70+
procss.arg("--explain").arg(code).exec()?;
71+
} else if expanded_args.flag("list") {
72+
// Don't let config errors get in the way of parsing arguments
73+
let _ = config_configure(gctx, &expanded_args, None, global_args, None);
74+
print_list(gctx, is_verbose);
75+
} else {
76+
let (cmd, subcommand_args) = match expanded_args.subcommand() {
77+
Some((cmd, args)) => (cmd, args),
78+
_ => {
79+
// No subcommand provided.
80+
cli(gctx).print_help()?;
81+
return Ok(());
82+
}
83+
};
84+
let exec = Exec::infer(cmd)?;
85+
config_configure(
7786
gctx,
78-
"\
87+
&expanded_args,
88+
Some(subcommand_args),
89+
global_args,
90+
Some(&exec),
91+
)?;
92+
super::init_git(gctx);
93+
94+
exec.exec(gctx, subcommand_args)?;
95+
}
96+
Ok(())
97+
}
98+
99+
fn print_zhelp(gctx: &GlobalContext) {
100+
let header = style::HEADER;
101+
let literal = style::LITERAL;
102+
let placeholder = style::PLACEHOLDER;
103+
104+
let options = CliUnstable::help();
105+
let max_length = options
106+
.iter()
107+
.filter(|(_, help)| help.is_some())
108+
.map(|(option_name, _)| option_name.len())
109+
.max()
110+
.unwrap_or(0);
111+
let z_flags = options
112+
.iter()
113+
.filter(|(_, help)| help.is_some())
114+
.map(|(opt, help)| {
115+
let opt = opt.replace("_", "-");
116+
let help = help.unwrap();
117+
format!(" {literal}-Z {opt:<max_length$}{literal:#} {help}")
118+
})
119+
.join("\n");
120+
drop_println!(
121+
gctx,
122+
"\
79123
{header}Available unstable (nightly-only) flags:{header:#}
80124
81125
{z_flags}
82126
83127
Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder:#}`",
84-
);
85-
if !gctx.nightly_features_allowed {
86-
drop_println!(
87-
gctx,
88-
"\nUnstable flags are only available on the nightly channel \
89-
of Cargo, but this is the `{}` channel.\n\
90-
{}",
91-
features::channel(),
92-
features::SEE_CHANNELS
93-
);
94-
}
128+
);
129+
if !gctx.nightly_features_allowed {
95130
drop_println!(
96131
gctx,
97-
"\nSee https://doc.rust-lang.org/nightly/cargo/reference/unstable.html \
98-
for more information about these flags."
132+
"\nUnstable flags are only available on the nightly channel \
133+
of Cargo, but this is the `{}` channel.\n\
134+
{}",
135+
features::channel(),
136+
features::SEE_CHANNELS
99137
);
100-
return Ok(());
101-
}
102-
103-
let is_verbose = expanded_args.verbose() > 0;
104-
if expanded_args.flag("version") {
105-
let version = get_version_string(is_verbose);
106-
drop_print!(gctx, "{}", version);
107-
return Ok(());
108-
}
109-
110-
if let Some(code) = expanded_args.get_one::<String>("explain") {
111-
let mut procss = gctx.load_global_rustc(None)?.process();
112-
procss.arg("--explain").arg(code).exec()?;
113-
return Ok(());
114138
}
139+
drop_println!(
140+
gctx,
141+
"\nSee https://doc.rust-lang.org/nightly/cargo/reference/unstable.html \
142+
for more information about these flags."
143+
);
144+
}
115145

116-
if expanded_args.flag("list") {
117-
// Maps from commonly known external commands (not builtin to cargo)
118-
// to their description, for the help page. Reserved for external
119-
// subcommands that are core within the rust ecosystem (esp ones that
120-
// might become internal in the future).
121-
let known_external_command_descriptions = HashMap::from([
122-
(
123-
"clippy",
124-
"Checks a package to catch common mistakes and improve your Rust code.",
125-
),
126-
(
127-
"fmt",
128-
"Formats all bin and lib files of the current crate using rustfmt.",
129-
),
130-
]);
131-
drop_println!(
132-
gctx,
133-
color_print::cstr!("<green,bold>Installed Commands:</>")
134-
);
135-
for (name, command) in list_commands(gctx) {
136-
let known_external_desc = known_external_command_descriptions.get(name.as_str());
137-
let literal = style::LITERAL;
138-
match command {
139-
CommandInfo::BuiltIn { about } => {
140-
assert!(
141-
known_external_desc.is_none(),
142-
"known_external_commands shouldn't contain builtin `{name}`",
143-
);
144-
let summary = about.unwrap_or_default();
145-
let summary = summary.lines().next().unwrap_or(&summary); // display only the first line
146-
drop_println!(gctx, " {literal}{name:<20}{literal:#} {summary}");
147-
}
148-
CommandInfo::External { path } => {
149-
if let Some(desc) = known_external_desc {
150-
drop_println!(gctx, " {literal}{name:<20}{literal:#} {desc}");
151-
} else if is_verbose {
152-
drop_println!(
153-
gctx,
154-
" {literal}{name:<20}{literal:#} {}",
155-
path.display()
156-
);
157-
} else {
158-
drop_println!(gctx, " {literal}{name}{literal:#}");
159-
}
160-
}
161-
CommandInfo::Alias { target } => {
146+
fn print_list(gctx: &GlobalContext, is_verbose: bool) {
147+
// Maps from commonly known external commands (not builtin to cargo)
148+
// to their description, for the help page. Reserved for external
149+
// subcommands that are core within the rust ecosystem (esp ones that
150+
// might become internal in the future).
151+
let known_external_command_descriptions = HashMap::from([
152+
(
153+
"clippy",
154+
"Checks a package to catch common mistakes and improve your Rust code.",
155+
),
156+
(
157+
"fmt",
158+
"Formats all bin and lib files of the current crate using rustfmt.",
159+
),
160+
]);
161+
drop_println!(
162+
gctx,
163+
color_print::cstr!("<green,bold>Installed Commands:</>")
164+
);
165+
for (name, command) in list_commands(gctx) {
166+
let known_external_desc = known_external_command_descriptions.get(name.as_str());
167+
let literal = style::LITERAL;
168+
match command {
169+
CommandInfo::BuiltIn { about } => {
170+
assert!(
171+
known_external_desc.is_none(),
172+
"known_external_commands shouldn't contain builtin `{name}`",
173+
);
174+
let summary = about.unwrap_or_default();
175+
let summary = summary.lines().next().unwrap_or(&summary); // display only the first line
176+
drop_println!(gctx, " {literal}{name:<20}{literal:#} {summary}");
177+
}
178+
CommandInfo::External { path } => {
179+
if let Some(desc) = known_external_desc {
180+
drop_println!(gctx, " {literal}{name:<20}{literal:#} {desc}");
181+
} else if is_verbose {
162182
drop_println!(
163183
gctx,
164-
" {literal}{name:<20}{literal:#} alias: {}",
165-
target.iter().join(" ")
184+
" {literal}{name:<20}{literal:#} {}",
185+
path.display()
166186
);
187+
} else {
188+
drop_println!(gctx, " {literal}{name}{literal:#}");
167189
}
168190
}
191+
CommandInfo::Alias { target } => {
192+
drop_println!(
193+
gctx,
194+
" {literal}{name:<20}{literal:#} alias: {}",
195+
target.iter().join(" ")
196+
);
197+
}
169198
}
170-
return Ok(());
171199
}
172-
173-
let (cmd, subcommand_args) = match expanded_args.subcommand() {
174-
Some((cmd, args)) => (cmd, args),
175-
_ => {
176-
// No subcommand provided.
177-
cli(gctx).print_help()?;
178-
return Ok(());
179-
}
180-
};
181-
let exec = Exec::infer(cmd)?;
182-
config_configure(gctx, &expanded_args, subcommand_args, global_args, &exec)?;
183-
super::init_git(gctx);
184-
185-
exec.exec(gctx, subcommand_args)
186200
}
187201

188202
pub fn get_version_string(is_verbose: bool) -> String {
@@ -365,16 +379,18 @@ For more information, see issue #12207 <https://github.com/rust-lang/cargo/issue
365379
fn config_configure(
366380
gctx: &mut GlobalContext,
367381
args: &ArgMatches,
368-
subcommand_args: &ArgMatches,
382+
subcommand_args: Option<&ArgMatches>,
369383
global_args: GlobalArgs,
370-
exec: &Exec,
384+
exec: Option<&Exec>,
371385
) -> CliResult {
372-
let arg_target_dir = &subcommand_args.value_of_path("target-dir", gctx);
386+
let arg_target_dir = &subcommand_args.and_then(|a| a.value_of_path("target-dir", gctx));
373387
let mut verbose = global_args.verbose + args.verbose();
374388
// quiet is unusual because it is redefined in some subcommands in order
375389
// to provide custom help text.
376-
let mut quiet = args.flag("quiet") || subcommand_args.flag("quiet") || global_args.quiet;
377-
if matches!(exec, Exec::Manifest(_)) && !quiet {
390+
let mut quiet = args.flag("quiet")
391+
|| subcommand_args.map(|a| a.flag("quiet")).unwrap_or_default()
392+
|| global_args.quiet;
393+
if matches!(exec, Some(Exec::Manifest(_))) && !quiet {
378394
// Verbosity is shifted quieter for `Exec::Manifest` as it is can be used as if you ran
379395
// `cargo install` and we especially shouldn't pollute programmatic output.
380396
//

‎src/cargo/util/config/mod.rs

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,41 +1018,14 @@ impl GlobalContext {
10181018
unstable_flags: &[String],
10191019
cli_config: &[String],
10201020
) -> CargoResult<()> {
1021-
for warning in self
1022-
.unstable_flags
1023-
.parse(unstable_flags, self.nightly_features_allowed)?
1024-
{
1025-
self.shell().warn(warning)?;
1026-
}
1027-
if !unstable_flags.is_empty() {
1028-
// store a copy of the cli flags separately for `load_unstable_flags_from_config`
1029-
// (we might also need it again for `reload_rooted_at`)
1030-
self.unstable_flags_cli = Some(unstable_flags.to_vec());
1031-
}
1032-
if !cli_config.is_empty() {
1033-
self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect());
1034-
self.merge_cli_args()?;
1035-
}
1036-
if self.unstable_flags.config_include {
1037-
// If the config was already loaded (like when fetching the
1038-
// `[alias]` table), it was loaded with includes disabled because
1039-
// the `unstable_flags` hadn't been set up, yet. Any values
1040-
// fetched before this step will not process includes, but that
1041-
// should be fine (`[alias]` is one of the only things loaded
1042-
// before configure). This can be removed when stabilized.
1043-
self.reload_rooted_at(self.cwd.clone())?;
1044-
}
1045-
let extra_verbose = verbose >= 2;
1046-
let verbose = verbose != 0;
1047-
10481021
// Ignore errors in the configuration files. We don't want basic
10491022
// commands like `cargo version` to error out due to config file
10501023
// problems.
10511024
let term = self.get::<TermConfig>("term").unwrap_or_default();
10521025

1053-
let color = color.or_else(|| term.color.as_deref());
1054-
10551026
// The command line takes precedence over configuration.
1027+
let extra_verbose = verbose >= 2;
1028+
let verbose = verbose != 0;
10561029
let verbosity = match (verbose, quiet) {
10571030
(true, true) => bail!("cannot set both --verbose and --quiet"),
10581031
(true, false) => Verbosity::Verbose,
@@ -1066,16 +1039,17 @@ impl GlobalContext {
10661039
_ => Verbosity::Normal,
10671040
},
10681041
};
1069-
1070-
let cli_target_dir = target_dir.as_ref().map(|dir| Filesystem::new(dir.clone()));
1071-
10721042
self.shell().set_verbosity(verbosity);
1043+
self.extra_verbose = extra_verbose;
1044+
1045+
let color = color.or_else(|| term.color.as_deref());
10731046
self.shell().set_color_choice(color)?;
10741047
if let Some(hyperlinks) = term.hyperlinks {
10751048
self.shell().set_hyperlinks(hyperlinks)?;
10761049
}
1050+
10771051
self.progress_config = term.progress.unwrap_or_default();
1078-
self.extra_verbose = extra_verbose;
1052+
10791053
self.frozen = frozen;
10801054
self.locked = locked;
10811055
self.offline = offline
@@ -1084,8 +1058,34 @@ impl GlobalContext {
10841058
.ok()
10851059
.and_then(|n| n.offline)
10861060
.unwrap_or(false);
1061+
let cli_target_dir = target_dir.as_ref().map(|dir| Filesystem::new(dir.clone()));
10871062
self.target_dir = cli_target_dir;
10881063

1064+
for warning in self
1065+
.unstable_flags
1066+
.parse(unstable_flags, self.nightly_features_allowed)?
1067+
{
1068+
self.shell().warn(warning)?;
1069+
}
1070+
if !unstable_flags.is_empty() {
1071+
// store a copy of the cli flags separately for `load_unstable_flags_from_config`
1072+
// (we might also need it again for `reload_rooted_at`)
1073+
self.unstable_flags_cli = Some(unstable_flags.to_vec());
1074+
}
1075+
if !cli_config.is_empty() {
1076+
self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect());
1077+
self.merge_cli_args()?;
1078+
}
1079+
if self.unstable_flags.config_include {
1080+
// If the config was already loaded (like when fetching the
1081+
// `[alias]` table), it was loaded with includes disabled because
1082+
// the `unstable_flags` hadn't been set up, yet. Any values
1083+
// fetched before this step will not process includes, but that
1084+
// should be fine (`[alias]` is one of the only things loaded
1085+
// before configure). This can be removed when stabilized.
1086+
self.reload_rooted_at(self.cwd.clone())?;
1087+
}
1088+
10891089
self.load_unstable_flags_from_config()?;
10901090

10911091
Ok(())

0 commit comments

Comments
 (0)