Skip to content

feat(cli)!: set log level to INFO/DEBUG on --quiet/--verbose if RUSTUP_LOG is unset #3987

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 5 commits into from
Sep 4, 2024
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
4 changes: 2 additions & 2 deletions rustup-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ Usage: rustup-init[EXE] [OPTIONS]

Options:
-v, --verbose
Enable verbose output
Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
-q, --quiet
Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
-y
Disable confirmation prompt
--default-host <DEFAULT_HOST>
Expand Down
2 changes: 1 addition & 1 deletion src/bin/rustup-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ async fn run_rustup_inner(
utils::current_exe()?;

match process.name().as_deref() {
Some("rustup") => rustup_mode::main(current_dir, process).await,
Some("rustup") => rustup_mode::main(current_dir, process, console_filter).await,
Some(n) if n.starts_with("rustup-setup") || n.starts_with("rustup-init") => {
// NB: The above check is only for the prefix of the file
// name. Browsers rename duplicates to
Expand Down
46 changes: 33 additions & 13 deletions src/cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use std::{cmp, env};
use anyhow::{anyhow, Context, Result};
use git_testament::{git_testament, render_testament};
use tracing::{debug, error, info, trace, warn};
use tracing_subscriber::{reload::Handle, EnvFilter, Registry};

use super::self_update;
use crate::{
Expand Down Expand Up @@ -127,15 +128,13 @@ pub(crate) fn read_line(process: &Process) -> Result<String> {
pub(super) struct Notifier {
tracker: Mutex<DownloadTracker>,
ram_notice_shown: RefCell<bool>,
verbose: bool,
}

impl Notifier {
pub(super) fn new(verbose: bool, quiet: bool, process: &Process) -> Self {
pub(super) fn new(quiet: bool, process: &Process) -> Self {
Self {
tracker: Mutex::new(DownloadTracker::new_with_display_progress(!quiet, process)),
ram_notice_shown: RefCell::new(false),
verbose,
}
}

Expand All @@ -158,9 +157,7 @@ impl Notifier {
for n in format!("{n}").lines() {
match level {
NotificationLevel::Debug => {
if self.verbose {
debug!("{}", n);
}
debug!("{}", n);
}
NotificationLevel::Info => {
info!("{}", n);
Expand All @@ -180,13 +177,8 @@ impl Notifier {
}

#[tracing::instrument(level = "trace")]
pub(crate) fn set_globals(
current_dir: PathBuf,
verbose: bool,
quiet: bool,
process: &Process,
) -> Result<Cfg<'_>> {
let notifier = Notifier::new(verbose, quiet, process);
pub(crate) fn set_globals(current_dir: PathBuf, quiet: bool, process: &Process) -> Result<Cfg<'_>> {
let notifier = Notifier::new(quiet, process);
Cfg::from_env(current_dir, Arc::new(move |n| notifier.handle(n)), process)
}

Expand Down Expand Up @@ -661,3 +653,31 @@ pub(crate) fn warn_if_host_is_emulated(process: &Process) {
warn!("For best compatibility and performance you should reinstall rustup for your native CPU.");
}
}

/// Updates the console logger level according to whether `quiet` or `verbose` is set to `true`.
///
/// Does nothing if at least one of the following conditions is met:
/// - The `RUSTUP_LOG` environment variable is set.
/// - Both `quiet` and `verbose` are set to `true`.
pub(super) fn update_console_filter(
process: &Process,
filter: &Handle<EnvFilter, Registry>,
quiet: bool,
verbose: bool,
) {
if process.var("RUSTUP_LOG").is_ok() {
return;
}

let maybe_directives = match (quiet, verbose) {
(true, _) => Some("rustup=WARN"),
(_, true) => Some("rustup=DEBUG"),
(_, _) => None,
};

if let Some(directives) = maybe_directives {
filter
.modify(|it| *it = EnvFilter::new(directives))
.expect("error reloading `EnvFilter` for console_logger");
}
}
2 changes: 1 addition & 1 deletion src/cli/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ where
(logger.compact().with_filter(env_filter).boxed(), handle)
} else {
// Receive log lines from Rustup only.
let (env_filter, handle) = reload::Layer::new(EnvFilter::new("rustup=DEBUG"));
let (env_filter, handle) = reload::Layer::new(EnvFilter::new("rustup=INFO"));
(
logger
.event_format(EventFormatter)
Expand Down
2 changes: 1 addition & 1 deletion src/cli/proxy_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub async fn main(arg0: &str, current_dir: PathBuf, process: &Process) -> Result
.skip(1 + toolchain.is_some() as usize)
.collect();

let cfg = set_globals(current_dir, false, true, process)?;
let cfg = set_globals(current_dir, true, process)?;
let cmd = cfg.resolve_local_toolchain(toolchain)?.command(arg0)?;
run_command_for_dir(cmd, arg0, &cmd_args)
}
21 changes: 14 additions & 7 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ use clap::{builder::PossibleValue, Args, CommandFactory, Parser, Subcommand, Val
use clap_complete::Shell;
use itertools::Itertools;
use tracing::{info, trace, warn};
use tracing_subscriber::{reload::Handle, EnvFilter, Registry};

use crate::{
cli::{
common::{self, PackageUpdate},
common::{self, update_console_filter, PackageUpdate},
errors::CLIError,
help::*,
self_update::{self, check_rustup_update, SelfUpdateMode},
Expand Down Expand Up @@ -68,11 +69,11 @@ fn handle_epipe(res: Result<utils::ExitCode>) -> Result<utils::ExitCode> {
after_help = RUSTUP_HELP,
)]
struct Rustup {
/// Enable verbose output
#[arg(short, long)]
/// Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
#[arg(short, long, conflicts_with = "quiet")]
verbose: bool,

/// Disable progress output
/// Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
#[arg(short, long, conflicts_with = "verbose")]
quiet: bool,

Expand Down Expand Up @@ -532,7 +533,11 @@ enum SetSubcmd {
}

#[tracing::instrument(level = "trace", fields(args = format!("{:?}", process.args_os().collect::<Vec<_>>())))]
pub async fn main(current_dir: PathBuf, process: &Process) -> Result<utils::ExitCode> {
pub async fn main(
current_dir: PathBuf,
process: &Process,
console_filter: Handle<EnvFilter, Registry>,
) -> Result<utils::ExitCode> {
self_update::cleanup_self_updater(process)?;

use clap::error::ErrorKind::*;
Expand All @@ -545,7 +550,7 @@ pub async fn main(current_dir: PathBuf, process: &Process) -> Result<utils::Exit
Err(err) if err.kind() == DisplayVersion => {
write!(process.stdout().lock(), "{err}")?;
info!("This is the version for the rustup toolchain manager, not the rustc compiler.");
let mut cfg = common::set_globals(current_dir, false, true, process)?;
let mut cfg = common::set_globals(current_dir, true, process)?;
match cfg.active_rustc_version() {
Ok(Some(version)) => info!("The currently active `rustc` version is `{version}`"),
Ok(None) => info!("No `rustc` is currently active"),
Expand All @@ -570,7 +575,9 @@ pub async fn main(current_dir: PathBuf, process: &Process) -> Result<utils::Exit
}
};

let cfg = &mut common::set_globals(current_dir, matches.verbose, matches.quiet, process)?;
update_console_filter(process, &console_filter, matches.quiet, matches.verbose);

let cfg = &mut common::set_globals(current_dir, matches.quiet, process)?;

if let Some(t) = &matches.plus_toolchain {
cfg.set_toolchain_override(t);
Expand Down
9 changes: 3 additions & 6 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,6 @@ fn canonical_cargo_home(process: &Process) -> Result<Cow<'static, str>> {
pub(crate) async fn install(
current_dir: PathBuf,
no_prompt: bool,
verbose: bool,
quiet: bool,
mut opts: InstallOpts<'_>,
process: &Process,
Expand Down Expand Up @@ -549,7 +548,7 @@ pub(crate) async fn install(
}

let no_modify_path = opts.no_modify_path;
if let Err(e) = maybe_install_rust(current_dir, verbose, quiet, opts, process).await {
if let Err(e) = maybe_install_rust(current_dir, quiet, opts, process).await {
report_error(&e, process);

// On windows, where installation happens in a console
Expand Down Expand Up @@ -804,7 +803,6 @@ pub(crate) fn install_proxies(process: &Process) -> Result<()> {

async fn maybe_install_rust(
current_dir: PathBuf,
verbose: bool,
quiet: bool,
opts: InstallOpts<'_>,
process: &Process,
Expand All @@ -828,7 +826,7 @@ async fn maybe_install_rust(
fs::create_dir_all(home).context("unable to create ~/.rustup")?;
}

let mut cfg = common::set_globals(current_dir, verbose, quiet, process)?;
let mut cfg = common::set_globals(current_dir, quiet, process)?;

let (components, targets) = (opts.components, opts.targets);
let toolchain = opts.install(&mut cfg)?;
Expand Down Expand Up @@ -1230,8 +1228,7 @@ mod tests {
home.apply(&mut vars);
let tp = TestProcess::with_vars(vars);
let mut cfg =
common::set_globals(tp.process.current_dir().unwrap(), false, false, &tp.process)
.unwrap();
common::set_globals(tp.process.current_dir().unwrap(), false, &tp.process).unwrap();

let opts = InstallOpts {
default_host_triple: None,
Expand Down
18 changes: 7 additions & 11 deletions src/cli/setup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use tracing_subscriber::{reload::Handle, EnvFilter, Registry};

use crate::{
cli::{
common,
common::{self, update_console_filter},
self_update::{self, InstallOpts},
},
dist::Profile,
Expand All @@ -25,12 +25,12 @@ use crate::{
before_help = format!("rustup-init {}", common::version()),
)]
struct RustupInit {
/// Enable verbose output
#[arg(short, long)]
/// Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
#[arg(short, long, conflicts_with = "quiet")]
verbose: bool,

/// Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
#[arg(short, long)]
/// Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
#[arg(short, long, conflicts_with = "verbose")]
quiet: bool,

/// Disable confirmation prompt
Expand Down Expand Up @@ -115,11 +115,7 @@ pub async fn main(
warn!("{}", common::WARN_COMPLETE_PROFILE);
}

if quiet && process.var("RUSTUP_LOG").is_err() {
console_filter
.modify(|it| *it = EnvFilter::new("rustup=WARN"))
.expect("error reloading `EnvFilter` for console_logger");
}
update_console_filter(process, &console_filter, quiet, verbose);

let opts = InstallOpts {
default_host_triple: default_host,
Expand All @@ -131,5 +127,5 @@ pub async fn main(
targets: &target.iter().map(|s| &**s).collect::<Vec<_>>(),
};

self_update::install(current_dir, no_prompt, verbose, quiet, opts, process).await
self_update::install(current_dir, no_prompt, quiet, opts, process).await
}
8 changes: 6 additions & 2 deletions src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use anyhow::{Context, Result};
use tracing::subscriber::DefaultGuard;
#[cfg(feature = "test")]
use tracing_subscriber::util::SubscriberInitExt;
#[cfg(feature = "test")]
use tracing_subscriber::{reload::Handle, EnvFilter, Registry};

pub mod filesource;
pub mod terminalsource;
Expand Down Expand Up @@ -177,6 +179,7 @@ impl Default for OsProcess {
#[cfg(feature = "test")]
pub struct TestProcess {
pub process: Process,
pub console_filter: Handle<EnvFilter, Registry>,
_guard: DefaultGuard, // guard is dropped at the end of the test
}

Expand Down Expand Up @@ -230,10 +233,11 @@ impl TestProcess {
impl From<TestContext> for TestProcess {
fn from(inner: TestContext) -> Self {
let inner = Process::TestProcess(inner);
let guard = crate::cli::log::tracing_subscriber(&inner).0.set_default();
let (tracing_subscriber, console_filter) = crate::cli::log::tracing_subscriber(&inner);
Self {
process: inner,
_guard: guard,
console_filter,
_guard: tracing_subscriber.set_default(),
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/test/mock/clitools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,12 @@ impl Config {
}

let tp = process::TestProcess::new(&*self.workdir.borrow(), &arg_strings, vars, "");
let process_res = rustup_mode::main(tp.process.current_dir().unwrap(), &tp.process).await;
let process_res = rustup_mode::main(
tp.process.current_dir().unwrap(),
&tp.process,
tp.console_filter.clone(),
)
.await;
// convert Err's into an ec
let ec = match process_res {
Ok(process_res) => process_res,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ Usage: rustup-init[EXE] [OPTIONS]

Options:
-v, --verbose
Enable verbose output
Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
-q, --quiet
Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
-y
Disable confirmation prompt
--default-host <DEFAULT_HOST>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ Usage: rustup-init[EXE] [OPTIONS]

Options:
-v, --verbose
Enable verbose output
Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
-q, --quiet
Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
-y
Disable confirmation prompt
--default-host <DEFAULT_HOST>
Expand Down
4 changes: 2 additions & 2 deletions tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ Arguments:
[+toolchain] Release channel (e.g. +stable) or custom toolchain to set override

Options:
-v, --verbose Enable verbose output
-q, --quiet Disable progress output
-v, --verbose Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
-q, --quiet Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
-h, --help Print help
-V, --version Print version

Expand Down
4 changes: 2 additions & 2 deletions tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ Arguments:
[+toolchain] Release channel (e.g. +stable) or custom toolchain to set override

Options:
-v, --verbose Enable verbose output
-q, --quiet Disable progress output
-v, --verbose Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset
-q, --quiet Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset
-h, --help Print help
-V, --version Print version

Expand Down
4 changes: 2 additions & 2 deletions tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ Arguments:

Options:
-v, --verbose
Enable verbose output
Set log level to 'DEBUG' if 'RUSTUP_LOG' is unset

-q, --quiet
Disable progress output
Disable progress output, set log level to 'WARN' if 'RUSTUP_LOG' is unset

-h, --help
Print help
Expand Down
Loading
Loading