Auto merge of rust-lang#117814 - RalfJung:rustc-logger-without-set-var, r=TaKO8Ki

rustc_log: provide a way to init logging based on the values, not names, of the env vars

Miri wants to affect how rustc does logging. So far this required setting environment variables before calling `rustc_driver::init_rustc_env_logger`. However, `set_var` is a function one should really [avoid calling](rust-lang#90308), so this adds the necessary APIs to rustc such that Miri can just pass it the *values* of all the log-relevant environment variables, rather than having to change the global environment.

Author: bors

compiler/rustc_driver_impl/src/lib.rs

@@ -1520,14 +1520,14 @@ fn report_ice(
 /// This allows tools to enable rust logging without having to magically match rustc's
 /// tracing crate version.
 pub fn init_rustc_env_logger(handler: &EarlyErrorHandler) {
-    init_env_logger(handler, "RUSTC_LOG");
+    init_logger(handler, rustc_log::LoggerConfig::from_env("RUSTC_LOG"));
 }
 
 /// This allows tools to enable rust logging without having to magically match rustc's
-/// tracing crate version. In contrast to `init_rustc_env_logger` it allows you to choose an env var
-/// other than `RUSTC_LOG`.
-pub fn init_env_logger(handler: &EarlyErrorHandler, env: &str) {
-    if let Err(error) = rustc_log::init_env_logger(env) {
+/// tracing crate version. In contrast to `init_rustc_env_logger` it allows you to choose
+/// the values directly rather than having to set an environment variable.
+pub fn init_logger(handler: &EarlyErrorHandler, cfg: rustc_log::LoggerConfig) {
+    if let Err(error) = rustc_log::init_logger(cfg) {
         handler.early_error(error.to_string());
     }
 }

compiler/rustc_log/src/lib.rs

@@ -14,7 +14,7 @@
 //!
 //! ```
 //! fn main() {
-//!     rustc_log::init_env_logger("LOG").unwrap();
+//!     rustc_log::init_logger(rustc_log::LoggerConfig::from_env("LOG")).unwrap();
 //!
 //!     let edition = rustc_span::edition::Edition::Edition2021;
 //!     rustc_span::create_session_globals_then(edition, || {
@@ -52,13 +52,36 @@ use tracing_subscriber::fmt::{
 };
 use tracing_subscriber::layer::SubscriberExt;
 
-pub fn init_env_logger(env: &str) -> Result<(), Error> {
-    let filter = match env::var(env) {
+/// The values of all the environment variables that matter for configuring a logger.
+/// Errors are explicitly preserved so that we can share error handling.
+pub struct LoggerConfig {
+    pub filter: Result<String, VarError>,
+    pub color_logs: Result<String, VarError>,
+    pub verbose_entry_exit: Result<String, VarError>,
+    pub verbose_thread_ids: Result<String, VarError>,
+    pub backtrace: Result<String, VarError>,
+}
+
+impl LoggerConfig {
+    pub fn from_env(env: &str) -> Self {
+        LoggerConfig {
+            filter: env::var(env),
+            color_logs: env::var(format!("{env}_COLOR")),
+            verbose_entry_exit: env::var(format!("{env}_ENTRY_EXIT")),
+            verbose_thread_ids: env::var(format!("{env}_THREAD_IDS")),
+            backtrace: env::var(format!("{env}_BACKTRACE")),
+        }
+    }
+}
+
+/// Initialize the logger with the given values for the filter, coloring, and other options env variables.
+pub fn init_logger(cfg: LoggerConfig) -> Result<(), Error> {
+    let filter = match cfg.filter {
         Ok(env) => EnvFilter::new(env),
         _ => EnvFilter::default().add_directive(Directive::from(LevelFilter::WARN)),
     };
 
-    let color_logs = match env::var(String::from(env) + "_COLOR") {
+    let color_logs = match cfg.color_logs {
         Ok(value) => match value.as_ref() {
             "always" => true,
             "never" => false,
@@ -69,14 +92,14 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> {
         Err(VarError::NotUnicode(_value)) => return Err(Error::NonUnicodeColorValue),
     };
 
-    let verbose_entry_exit = match env::var_os(String::from(env) + "_ENTRY_EXIT") {
-        None => false,
-        Some(v) => &v != "0",
+    let verbose_entry_exit = match cfg.verbose_entry_exit {
+        Ok(v) => &v != "0",
+        Err(_) => false,
     };
 
-    let verbose_thread_ids = match env::var_os(String::from(env) + "_THREAD_IDS") {
-        None => false,
-        Some(v) => &v == "1",
+    let verbose_thread_ids = match cfg.verbose_thread_ids {
+        Ok(v) => &v == "1",
+        Err(_) => false,
     };
 
     let layer = tracing_tree::HierarchicalLayer::default()
@@ -91,7 +114,7 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> {
         .with_thread_names(verbose_thread_ids);
 
     let subscriber = tracing_subscriber::Registry::default().with(filter).with(layer);
-    match env::var(format!("{env}_BACKTRACE")) {
+    match cfg.backtrace {
         Ok(str) => {
             let fmt_layer = tracing_subscriber::fmt::layer()
                 .with_writer(io::stderr)

src/librustdoc/lib.rs

@@ -53,6 +53,7 @@ extern crate rustc_interface;
 extern crate rustc_lexer;
 extern crate rustc_lint;
 extern crate rustc_lint_defs;
+extern crate rustc_log;
 extern crate rustc_macros;
 extern crate rustc_metadata;
 extern crate rustc_middle;
@@ -175,7 +176,7 @@ pub fn main() {
     // in the sysroot), and all of rustdoc's logging goes to its version (the one in Cargo.toml).
 
     init_logging(&handler);
-    rustc_driver::init_env_logger(&handler, "RUSTDOC_LOG");
+    rustc_driver::init_logger(&handler, rustc_log::LoggerConfig::from_env("RUSTDOC_LOG"));
 
     let exit_code = rustc_driver::catch_with_exit_code(|| match get_args(&handler) {
         Some(args) => main_args(&mut handler, &args, using_internal_features),

src/tools/error_index_generator/main.rs

@@ -1,6 +1,7 @@
 #![feature(rustc_private)]
 
 extern crate rustc_driver;
+extern crate rustc_log;
 extern crate rustc_session;
 
 use std::env;
@@ -173,7 +174,7 @@ fn parse_args() -> (OutputFormat, PathBuf) {
 fn main() {
     let handler =
         rustc_session::EarlyErrorHandler::new(rustc_session::config::ErrorOutputType::default());
-    rustc_driver::init_env_logger(&handler, "RUST_LOG");
+    rustc_driver::init_logger(&handler, rustc_log::LoggerConfig::from_env("RUST_LOG"));
     let (format, dst) = parse_args();
     let result = main_with_result(format, &dst);
     if let Err(e) = result {

src/tools/miri/src/bin/miri.rs

@@ -9,11 +9,12 @@ extern crate rustc_data_structures;
 extern crate rustc_driver;
 extern crate rustc_hir;
 extern crate rustc_interface;
+extern crate rustc_log;
 extern crate rustc_metadata;
 extern crate rustc_middle;
 extern crate rustc_session;
 
-use std::env;
+use std::env::{self, VarError};
 use std::num::NonZeroU64;
 use std::path::PathBuf;
 use std::str::FromStr;
@@ -183,45 +184,54 @@ macro_rules! show_error {
     ($($tt:tt)*) => { show_error(&format_args!($($tt)*)) };
 }
 
-fn init_early_loggers(handler: &EarlyErrorHandler) {
-    // Note that our `extern crate log` is *not* the same as rustc's; as a result, we have to
-    // initialize them both, and we always initialize `miri`'s first.
-    let env = env_logger::Env::new().filter("MIRI_LOG").write_style("MIRI_LOG_STYLE");
-    env_logger::init_from_env(env);
-    // Enable verbose entry/exit logging by default if MIRI_LOG is set.
-    if env::var_os("MIRI_LOG").is_some() && env::var_os("RUSTC_LOG_ENTRY_EXIT").is_none() {
-        env::set_var("RUSTC_LOG_ENTRY_EXIT", "1");
-    }
-    // We only initialize `rustc` if the env var is set (so the user asked for it).
-    // If it is not set, we avoid initializing now so that we can initialize
-    // later with our custom settings, and *not* log anything for what happens before
-    // `miri` gets started.
-    if env::var_os("RUSTC_LOG").is_some() {
-        rustc_driver::init_rustc_env_logger(handler);
-    }
-}
+fn rustc_logger_config() -> rustc_log::LoggerConfig {
+    // Start with the usual env vars.
+    let mut cfg = rustc_log::LoggerConfig::from_env("RUSTC_LOG");
 
-fn init_late_loggers(handler: &EarlyErrorHandler, tcx: TyCtxt<'_>) {
-    // We initialize loggers right before we start evaluation. We overwrite the `RUSTC_LOG`
-    // env var if it is not set, control it based on `MIRI_LOG`.
-    // (FIXME: use `var_os`, but then we need to manually concatenate instead of `format!`.)
+    // Overwrite if MIRI_LOG is set.
     if let Ok(var) = env::var("MIRI_LOG") {
-        if env::var_os("RUSTC_LOG").is_none() {
+        // MIRI_LOG serves as default for RUSTC_LOG, if that is not set.
+        if matches!(cfg.filter, Err(VarError::NotPresent)) {
             // We try to be a bit clever here: if `MIRI_LOG` is just a single level
             // used for everything, we only apply it to the parts of rustc that are
             // CTFE-related. Otherwise, we use it verbatim for `RUSTC_LOG`.
             // This way, if you set `MIRI_LOG=trace`, you get only the right parts of
             // rustc traced, but you can also do `MIRI_LOG=miri=trace,rustc_const_eval::interpret=debug`.
             if log::Level::from_str(&var).is_ok() {
-                env::set_var(
-                    "RUSTC_LOG",
-                    format!("rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var}"),
-                );
+                cfg.filter = Ok(format!(
+                    "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var}"
+                ));
             } else {
-                env::set_var("RUSTC_LOG", &var);
+                cfg.filter = Ok(var);
             }
-            rustc_driver::init_rustc_env_logger(handler);
         }
+        // Enable verbose entry/exit logging by default if MIRI_LOG is set.
+        if matches!(cfg.verbose_entry_exit, Err(VarError::NotPresent)) {
+            cfg.verbose_entry_exit = Ok(format!("1"));
+        }
+    }
+
+    cfg
+}
+
+fn init_early_loggers(handler: &EarlyErrorHandler) {
+    // Note that our `extern crate log` is *not* the same as rustc's; as a result, we have to
+    // initialize them both, and we always initialize `miri`'s first.
+    let env = env_logger::Env::new().filter("MIRI_LOG").write_style("MIRI_LOG_STYLE");
+    env_logger::init_from_env(env);
+    // Now for rustc. We only initialize `rustc` if the env var is set (so the user asked for it).
+    // If it is not set, we avoid initializing now so that we can initialize later with our custom
+    // settings, and *not* log anything for what happens before `miri` gets started.
+    if env::var_os("RUSTC_LOG").is_some() {
+        rustc_driver::init_logger(handler, rustc_logger_config());
+    }
+}
+
+fn init_late_loggers(handler: &EarlyErrorHandler, tcx: TyCtxt<'_>) {
+    // If `RUSTC_LOG` is not set, then `init_early_loggers` did not call
+    // `rustc_driver::init_logger`, so we have to do this now.
+    if env::var_os("RUSTC_LOG").is_none() {
+        rustc_driver::init_logger(handler, rustc_logger_config());
     }
 
     // If `MIRI_BACKTRACE` is set and `RUSTC_CTFE_BACKTRACE` is not, set `RUSTC_CTFE_BACKTRACE`.