Rollup merge of rust-lang#136392 - jieyouxu:wrap-tracing, r=onur-ozkan

bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros

Follow-up to rust-lang#136091 (comment).

- Add wrapper macros for `error!`, `warn!`, `info!`, `debug!` and `trace!`, which `cfg(feature = "tracing")`-gates the underlying `tracing` macros. They expand to nothing if `"tracing"` feature is not enabled.
- This is not done for `span!` or `event!` because they can return span guards, and you can't really wrap that.
- This is also not possible for `tracing::instrument` attribute proc-macro unless you use another attribute proc-macro to wrap that.

It's not *great*, because `tracing::instrument` and `tracing::{span,event}` can't be wrapped this way.

Can test locally with:

```bash
$ BOOTSTRAP_TRACING=bootstrap=TRACE ./x check src/bootstrap/
```

r? `@onur-ozkan` (or reroll)

Author: jhpratt

src/bootstrap/src/bin/main.rs

@@ -11,12 +11,12 @@ use std::str::FromStr;
 use std::{env, process};
 
 use bootstrap::{
-    Build, CONFIG_CHANGE_HISTORY, Config, Flags, Subcommand, find_recent_config_change_ids,
+    Build, CONFIG_CHANGE_HISTORY, Config, Flags, Subcommand, debug, find_recent_config_change_ids,
     human_readable_changes, t,
 };
 use build_helper::ci::CiEnv;
 #[cfg(feature = "tracing")]
-use tracing::{debug, instrument};
+use tracing::instrument;
 
 #[cfg_attr(feature = "tracing", instrument(level = "trace", name = "main"))]
 fn main() {
@@ -29,10 +29,8 @@ fn main() {
         return;
     }
 
-    #[cfg(feature = "tracing")]
     debug!("parsing flags");
     let flags = Flags::parse(&args);
-    #[cfg(feature = "tracing")]
     debug!("parsing config based on flags");
     let config = Config::parse(flags);
 
@@ -95,7 +93,6 @@ fn main() {
     let dump_bootstrap_shims = config.dump_bootstrap_shims;
     let out_dir = config.out.clone();
 
-    #[cfg(feature = "tracing")]
     debug!("creating new build based on config");
     Build::new(config).build();
 
@@ -207,8 +204,9 @@ fn check_version(config: &Config) -> Option<String> {
 // Due to the conditional compilation via the `tracing` cargo feature, this means that `tracing`
 // usages in bootstrap need to be also gated behind the `tracing` feature:
 //
-// - `tracing` macros (like `trace!`) and anything from `tracing`, `tracing_subscriber` and
-//   `tracing-tree` will need to be gated by `#[cfg(feature = "tracing")]`.
+// - `tracing` macros with log levels (`trace!`, `debug!`, `warn!`, `info`, `error`) should not be
+//   used *directly*. You should use the wrapped `tracing` macros which gate the actual invocations
+//   behind `feature = "tracing"`.
 // - `tracing`'s `#[instrument(..)]` macro will need to be gated like `#![cfg_attr(feature =
 //   "tracing", instrument(..))]`.
 #[cfg(feature = "tracing")]

src/bootstrap/src/lib.rs

@@ -28,8 +28,6 @@ use std::{env, fs, io, str};
 use build_helper::ci::gha;
 use build_helper::exit;
 use termcolor::{ColorChoice, StandardStream, WriteColor};
-#[cfg(feature = "tracing")]
-use tracing::{debug, instrument, span, trace};
 use utils::build_stamp::BuildStamp;
 use utils::channel::GitInfo;
 
@@ -46,6 +44,8 @@ pub use core::builder::PathSet;
 pub use core::config::Config;
 pub use core::config::flags::{Flags, Subcommand};
 
+#[cfg(feature = "tracing")]
+use tracing::{instrument, span};
 pub use utils::change_tracker::{
     CONFIG_CHANGE_HISTORY, find_recent_config_change_ids, human_readable_changes,
 };
@@ -541,72 +541,71 @@ impl Build {
     /// Executes the entire build, as configured by the flags and configuration.
     #[cfg_attr(feature = "tracing", instrument(level = "debug", name = "Build::build", skip_all))]
     pub fn build(&mut self) {
-        #[cfg(feature = "tracing")]
         trace!("setting up job management");
         unsafe {
             crate::utils::job::setup(self);
         }
 
-        #[cfg(feature = "tracing")]
-        trace!("downloading rustfmt early");
-
         // Download rustfmt early so that it can be used in rust-analyzer configs.
+        trace!("downloading rustfmt early");
         let _ = &builder::Builder::new(self).initial_rustfmt();
 
-        #[cfg(feature = "tracing")]
-        let hardcoded_span =
-            span!(tracing::Level::DEBUG, "handling hardcoded subcommands (Format, Suggest, Perf)")
-                .entered();
-
-        // hardcoded subcommands
-        match &self.config.cmd {
-            Subcommand::Format { check, all } => {
-                return core::build_steps::format::format(
-                    &builder::Builder::new(self),
-                    *check,
-                    *all,
-                    &self.config.paths,
-                );
-            }
-            Subcommand::Suggest { run } => {
-                return core::build_steps::suggest::suggest(&builder::Builder::new(self), *run);
-            }
-            Subcommand::Perf { .. } => {
-                return core::build_steps::perf::perf(&builder::Builder::new(self));
-            }
-            _cmd => {
-                #[cfg(feature = "tracing")]
-                debug!(cmd = ?_cmd, "not a hardcoded subcommand; returning to normal handling");
+        // Handle hard-coded subcommands.
+        {
+            #[cfg(feature = "tracing")]
+            let _hardcoded_span = span!(
+                tracing::Level::DEBUG,
+                "handling hardcoded subcommands (Format, Suggest, Perf)"
+            )
+            .entered();
+
+            match &self.config.cmd {
+                Subcommand::Format { check, all } => {
+                    return core::build_steps::format::format(
+                        &builder::Builder::new(self),
+                        *check,
+                        *all,
+                        &self.config.paths,
+                    );
+                }
+                Subcommand::Suggest { run } => {
+                    return core::build_steps::suggest::suggest(&builder::Builder::new(self), *run);
+                }
+                Subcommand::Perf { .. } => {
+                    return core::build_steps::perf::perf(&builder::Builder::new(self));
+                }
+                _cmd => {
+                    debug!(cmd = ?_cmd, "not a hardcoded subcommand; returning to normal handling");
+                }
             }
-        }
 
-        #[cfg(feature = "tracing")]
-        drop(hardcoded_span);
-        #[cfg(feature = "tracing")]
-        debug!("handling subcommand normally");
+            debug!("handling subcommand normally");
+        }
 
         if !self.config.dry_run() {
             #[cfg(feature = "tracing")]
             let _real_run_span = span!(tracing::Level::DEBUG, "executing real run").entered();
 
+            // We first do a dry-run. This is a sanity-check to ensure that
+            // steps don't do anything expensive in the dry-run.
             {
                 #[cfg(feature = "tracing")]
                 let _sanity_check_span =
                     span!(tracing::Level::DEBUG, "(1) executing dry-run sanity-check").entered();
-
-                // We first do a dry-run. This is a sanity-check to ensure that
-                // steps don't do anything expensive in the dry-run.
                 self.config.dry_run = DryRun::SelfCheck;
                 let builder = builder::Builder::new(self);
                 builder.execute_cli();
             }
 
-            #[cfg(feature = "tracing")]
-            let _actual_run_span =
-                span!(tracing::Level::DEBUG, "(2) executing actual run").entered();
-            self.config.dry_run = DryRun::Disabled;
-            let builder = builder::Builder::new(self);
-            builder.execute_cli();
+            // Actual run.
+            {
+                #[cfg(feature = "tracing")]
+                let _actual_run_span =
+                    span!(tracing::Level::DEBUG, "(2) executing actual run").entered();
+                self.config.dry_run = DryRun::Disabled;
+                let builder = builder::Builder::new(self);
+                builder.execute_cli();
+            }
         } else {
             #[cfg(feature = "tracing")]
             let _dry_run_span = span!(tracing::Level::DEBUG, "executing dry run").entered();

src/bootstrap/src/utils/mod.rs

@@ -14,6 +14,8 @@ pub(crate) mod render_tests;
 pub(crate) mod shared_helpers;
 pub(crate) mod tarball;
 
+pub(crate) mod tracing;
+
 #[cfg(feature = "build-metrics")]
 pub(crate) mod metrics;
 

src/bootstrap/src/utils/tracing.rs

@@ -0,0 +1,49 @@
+//! Wrapper macros for `tracing` macros to avoid having to write `cfg(feature = "tracing")`-gated
+//! `debug!`/`trace!` everytime, e.g.
+//!
+//! ```rust,ignore (example)
+//! #[cfg(feature = "tracing")]
+//! trace!("...");
+//! ```
+//!
+//! When `feature = "tracing"` is inactive, these macros expand to nothing.
+
+#[macro_export]
+macro_rules! trace {
+    ($($tokens:tt)*) => {
+        #[cfg(feature = "tracing")]
+        ::tracing::trace!($($tokens)*)
+    }
+}
+
+#[macro_export]
+macro_rules! debug {
+    ($($tokens:tt)*) => {
+        #[cfg(feature = "tracing")]
+        ::tracing::debug!($($tokens)*)
+    }
+}
+
+#[macro_export]
+macro_rules! warn {
+    ($($tokens:tt)*) => {
+        #[cfg(feature = "tracing")]
+        ::tracing::warn!($($tokens)*)
+    }
+}
+
+#[macro_export]
+macro_rules! info {
+    ($($tokens:tt)*) => {
+        #[cfg(feature = "tracing")]
+        ::tracing::info!($($tokens)*)
+    }
+}
+
+#[macro_export]
+macro_rules! error {
+    ($($tokens:tt)*) => {
+        #[cfg(feature = "tracing")]
+        ::tracing::error!($($tokens)*)
+    }
+}