Rollup merge of #147038 - Kobzol:bootstrap-verbose-fn, r=jieyouxu

Rename verbosity functions in bootstrap

Just a small cleanup, these function names have been bothering me for a while. I realized that we can delete some of them outright, rather than just renaming them.

r? ``@jieyouxu``

Author: matthiaskrgr

src/bootstrap/src/core/build_steps/compile.rs

@@ -1832,8 +1832,9 @@ impl Step for Sysroot {
         let sysroot = sysroot_dir(compiler.stage);
         trace!(stage = ?compiler.stage, ?sysroot);
 
-        builder
-            .verbose(|| println!("Removing sysroot {} to avoid caching bugs", sysroot.display()));
+        builder.do_if_verbose(|| {
+            println!("Removing sysroot {} to avoid caching bugs", sysroot.display())
+        });
         let _ = fs::remove_dir_all(&sysroot);
         t!(fs::create_dir_all(&sysroot));
 
@@ -1902,12 +1903,7 @@ impl Step for Sysroot {
                 if !path.parent().is_none_or(|p| p.ends_with(&suffix)) {
                     return true;
                 }
-                if !filtered_files.iter().all(|f| f != path.file_name().unwrap()) {
-                    builder.verbose_than(1, || println!("ignoring {}", path.display()));
-                    false
-                } else {
-                    true
-                }
+                filtered_files.iter().all(|f| f != path.file_name().unwrap())
             });
         }
 
@@ -2596,7 +2592,7 @@ pub fn stream_cargo(
         cmd.arg(arg);
     }
 
-    builder.verbose(|| println!("running: {cmd:?}"));
+    builder.do_if_verbose(|| println!("running: {cmd:?}"));
 
     let streaming_command = cmd.stream_capture_stdout(&builder.config.exec_ctx);
 

src/bootstrap/src/core/build_steps/dist.rs

@@ -2304,7 +2304,7 @@ fn maybe_install_llvm(
         let mut cmd = command(host_llvm_config);
         cmd.cached();
         cmd.arg("--libfiles");
-        builder.verbose(|| println!("running {cmd:?}"));
+        builder.do_if_verbose(|| println!("running {cmd:?}"));
         let files = cmd.run_capture_stdout(builder).stdout();
         let build_llvm_out = &builder.llvm_out(builder.config.host_target);
         let target_llvm_out = &builder.llvm_out(target);

src/bootstrap/src/core/build_steps/gcc.rs

@@ -128,7 +128,7 @@ fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<Pa
         &builder.config,
         builder.config.rust_info.is_managed_git_subrepository(),
     );
-    builder.verbose(|| {
+    builder.do_if_verbose(|| {
         eprintln!("GCC freshness: {source:?}");
     });
     match source {

src/bootstrap/src/core/build_steps/test.rs

@@ -582,11 +582,11 @@ impl Miri {
         // We re-use the `cargo` from above.
         cargo.arg("--print-sysroot");
 
-        builder.verbose(|| println!("running: {cargo:?}"));
+        builder.do_if_verbose(|| println!("running: {cargo:?}"));
         let stdout = cargo.run_capture_stdout(builder).stdout();
         // Output is "<sysroot>\n".
         let sysroot = stdout.trim_end();
-        builder.verbose(|| println!("`cargo miri setup --print-sysroot` said: {sysroot:?}"));
+        builder.do_if_verbose(|| println!("`cargo miri setup --print-sysroot` said: {sysroot:?}"));
         PathBuf::from(sysroot)
     }
 }
@@ -2675,7 +2675,7 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
         return true;
     }
 
-    builder.verbose(|| println!("doc tests for: {}", markdown.display()));
+    builder.do_if_verbose(|| println!("doc tests for: {}", markdown.display()));
     let mut cmd = builder.rustdoc_cmd(compiler);
     builder.add_rust_test_threads(&mut cmd);
     // allow for unstable options such as new editions

src/bootstrap/src/core/builder/cargo.rs

@@ -1139,7 +1139,7 @@ impl Builder<'_> {
             cargo.env("RUSTC_BACKTRACE_ON_ICE", "1");
         }
 
-        if self.is_verbose_than(1) {
+        if self.verbosity >= 2 {
             // This provides very useful logs especially when debugging build cache-related stuff.
             cargo.env("CARGO_LOG", "cargo::core::compiler::fingerprint=info");
         }

src/bootstrap/src/core/builder/mod.rs

@@ -545,7 +545,7 @@ impl StepDescription {
         if !builder.config.skip.is_empty()
             && !matches!(builder.config.get_dry_run(), DryRun::SelfCheck)
         {
-            builder.verbose(|| {
+            builder.do_if_verbose(|| {
                 println!(
                     "{:?} not skipped for {:?} -- not in {:?}",
                     pathset, self.name, builder.config.skip
@@ -947,7 +947,7 @@ impl Step for Libdir {
             // Sysroot`).
             if !builder.download_rustc() {
                 let sysroot_target_libdir = sysroot.join(self.target).join("lib");
-                builder.verbose(|| {
+                builder.do_if_verbose(|| {
                     eprintln!(
                         "Removing sysroot {} to avoid caching bugs",
                         sysroot_target_libdir.display()

src/bootstrap/src/core/config/config.rs

@@ -1571,8 +1571,8 @@ impl Config {
     }
 
     /// Runs a function if verbosity is greater than 0
-    pub fn verbose(&self, f: impl Fn()) {
-        self.exec_ctx.verbose(f);
+    pub fn do_if_verbose(&self, f: impl Fn()) {
+        self.exec_ctx.do_if_verbose(f);
     }
 
     pub fn any_sanitizers_to_build(&self) -> bool {
@@ -2061,7 +2061,7 @@ pub fn download_ci_rustc_commit<'a>(
         // Look for a version to compare to based on the current commit.
         // Only commits merged by bors will have CI artifacts.
         let freshness = check_path_modifications_(dwn_ctx, RUSTC_IF_UNCHANGED_ALLOWED_PATHS);
-        dwn_ctx.exec_ctx.verbose(|| {
+        dwn_ctx.exec_ctx.do_if_verbose(|| {
             eprintln!("rustc freshness: {freshness:?}");
         });
         match freshness {

src/bootstrap/src/core/download.rs

@@ -106,7 +106,7 @@ enum DownloadSource {
 /// Functions that are only ever called once, but named for clarity and to avoid thousand-line functions.
 impl Config {
     pub(crate) fn download_clippy(&self) -> PathBuf {
-        self.verbose(|| println!("downloading stage0 clippy artifacts"));
+        self.do_if_verbose(|| println!("downloading stage0 clippy artifacts"));
 
         let date = &self.stage0_metadata.compiler.date;
         let version = &self.stage0_metadata.compiler.version;
@@ -151,7 +151,9 @@ impl Config {
     }
 
     pub(crate) fn download_ci_rustc(&self, commit: &str) {
-        self.verbose(|| println!("using downloaded stage2 artifacts from CI (commit {commit})"));
+        self.do_if_verbose(|| {
+            println!("using downloaded stage2 artifacts from CI (commit {commit})")
+        });
 
         let version = self.artifact_version_part(commit);
         // download-rustc doesn't need its own cargo, it can just use beta's. But it does need the
@@ -258,7 +260,7 @@ impl Config {
         let llvm_root = self.ci_llvm_root();
         let llvm_freshness =
             detect_llvm_freshness(self, self.rust_info.is_managed_git_subrepository());
-        self.verbose(|| {
+        self.do_if_verbose(|| {
             eprintln!("LLVM freshness: {llvm_freshness:?}");
         });
         let llvm_sha = match llvm_freshness {
@@ -557,7 +559,7 @@ pub(crate) fn download_beta_toolchain<'a>(dwn_ctx: impl AsRef<DownloadContext<'a
 #[cfg(not(test))]
 pub(crate) fn download_beta_toolchain<'a>(dwn_ctx: impl AsRef<DownloadContext<'a>>, out: &Path) {
     let dwn_ctx = dwn_ctx.as_ref();
-    dwn_ctx.exec_ctx.verbose(|| {
+    dwn_ctx.exec_ctx.do_if_verbose(|| {
         println!("downloading stage0 beta artifacts");
     });
 
@@ -812,7 +814,7 @@ fn download_component<'a>(
                 unpack(dwn_ctx.exec_ctx, &tarball, &bin_root, prefix);
                 return;
             } else {
-                dwn_ctx.exec_ctx.verbose(|| {
+                dwn_ctx.exec_ctx.do_if_verbose(|| {
                     println!(
                         "ignoring cached file {} due to failed verification",
                         tarball.display()
@@ -853,7 +855,7 @@ download-rustc = false
 pub(crate) fn verify(exec_ctx: &ExecutionContext, path: &Path, expected: &str) -> bool {
     use sha2::Digest;
 
-    exec_ctx.verbose(|| {
+    exec_ctx.do_if_verbose(|| {
         println!("verifying {}", path.display());
     });
 
@@ -934,7 +936,7 @@ fn unpack(exec_ctx: &ExecutionContext, tarball: &Path, dst: &Path, pattern: &str
         short_path = short_path.strip_prefix(pattern).unwrap_or(short_path);
         let dst_path = dst.join(short_path);
 
-        exec_ctx.verbose(|| {
+        exec_ctx.do_if_verbose(|| {
             println!("extracting {} to {}", original_path.display(), dst.display());
         });
 
@@ -965,7 +967,7 @@ fn download_file<'a>(
 ) {
     let dwn_ctx = dwn_ctx.as_ref();
 
-    dwn_ctx.exec_ctx.verbose(|| {
+    dwn_ctx.exec_ctx.do_if_verbose(|| {
         println!("download {url}");
     });
     // Use a temporary file in case we crash while downloading, to avoid a corrupt download in cache/.

src/bootstrap/src/lib.rs

@@ -415,7 +415,7 @@ macro_rules! forward {
 }
 
 forward! {
-    verbose(f: impl Fn()),
+    do_if_verbose(f: impl Fn()),
     is_verbose() -> bool,
     create(path: &Path, s: &str),
     remove(f: &Path),
@@ -601,19 +601,19 @@ impl Build {
             .unwrap()
             .trim();
         if local_release.split('.').take(2).eq(version.split('.').take(2)) {
-            build.verbose(|| println!("auto-detected local-rebuild {local_release}"));
+            build.do_if_verbose(|| println!("auto-detected local-rebuild {local_release}"));
             build.local_rebuild = true;
         }
 
-        build.verbose(|| println!("finding compilers"));
+        build.do_if_verbose(|| println!("finding compilers"));
         utils::cc_detect::fill_compilers(&mut build);
         // When running `setup`, the profile is about to change, so any requirements we have now may
         // be different on the next invocation. Don't check for them until the next time x.py is
         // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
         //
         // Similarly, for `setup` we don't actually need submodules or cargo metadata.
         if !matches!(build.config.cmd, Subcommand::Setup { .. }) {
-            build.verbose(|| println!("running sanity check"));
+            build.do_if_verbose(|| println!("running sanity check"));
             crate::core::sanity::check(&mut build);
 
             // Make sure we update these before gathering metadata so we don't get an error about missing
@@ -631,7 +631,7 @@ impl Build {
             // Now, update all existing submodules.
             build.update_existing_submodules();
 
-            build.verbose(|| println!("learning about cargo"));
+            build.do_if_verbose(|| println!("learning about cargo"));
             crate::core::metadata::build(&mut build);
         }
 
@@ -1087,18 +1087,6 @@ impl Build {
         })
     }
 
-    /// Check if verbosity is greater than the `level`
-    pub fn is_verbose_than(&self, level: usize) -> bool {
-        self.verbosity > level
-    }
-
-    /// Runs a function if verbosity is greater than `level`.
-    fn verbose_than(&self, level: usize, f: impl Fn()) {
-        if self.is_verbose_than(level) {
-            f()
-        }
-    }
-
     fn info(&self, msg: &str) {
         match self.config.get_dry_run() {
             DryRun::SelfCheck => (),
@@ -1816,7 +1804,6 @@ impl Build {
         if self.config.dry_run() {
             return;
         }
-        self.verbose_than(1, || println!("Copy/Link {src:?} to {dst:?}"));
         if src == dst {
             return;
         }
@@ -1933,7 +1920,10 @@ impl Build {
             return;
         }
         let dst = dstdir.join(src.file_name().unwrap());
-        self.verbose_than(1, || println!("Install {src:?} to {dst:?}"));
+
+        #[cfg(feature = "tracing")]
+        let _span = trace_io!("install", ?src, ?dst);
+
         t!(fs::create_dir_all(dstdir));
         if !src.exists() {
             panic!("ERROR: File \"{}\" not found!", src.display());

src/bootstrap/src/utils/build_stamp.rs

@@ -112,7 +112,7 @@ pub fn clear_if_dirty(builder: &Builder<'_>, dir: &Path, input: &Path) -> bool {
     let stamp = BuildStamp::new(dir);
     let mut cleared = false;
     if mtime(stamp.path()) < mtime(input) {
-        builder.verbose(|| println!("Dirty - {}", dir.display()));
+        builder.do_if_verbose(|| println!("Dirty - {}", dir.display()));
         let _ = fs::remove_dir_all(dir);
         cleared = true;
     } else if stamp.path().exists() {