Skip to content
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

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

Merged
merged 1 commit into from
Feb 5, 2025
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
12 changes: 5 additions & 7 deletions src/bootstrap/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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);

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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")]
Expand Down
87 changes: 43 additions & 44 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
};
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/bootstrap/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
49 changes: 49 additions & 0 deletions src/bootstrap/src/utils/tracing.rs
Original file line number Diff line number Diff line change
@@ -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)*)
}
}
Loading