Rollup merge of #113644 - jyn514:bootstrap-cleanups, r=albertlarsan68

misc bootstrap cleanups

- rename `detail_exit_macro` to `exit`
- remove unnecessary `Builder::new_standalone` function
- support `x suggest` with build-metrics

src/bootstrap/builder.rs

@@ -395,7 +395,7 @@ impl StepDescription {
             eprintln!(
                 "note: if you are adding a new Step to bootstrap itself, make sure you register it with `describe!`"
             );
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
     }
 }
@@ -939,21 +939,6 @@ impl<'a> Builder<'a> {
         Self::new_internal(build, kind, paths.to_owned())
     }
 
-    /// Creates a new standalone builder for use outside of the normal process
-    pub fn new_standalone(
-        build: &mut Build,
-        kind: Kind,
-        paths: Vec<PathBuf>,
-        stage: Option<u32>,
-    ) -> Builder<'_> {
-        // FIXME: don't mutate `build`
-        if let Some(stage) = stage {
-            build.config.stage = stage;
-        }
-
-        Self::new_internal(build, kind, paths.to_owned())
-    }
-
     pub fn execute_cli(&self) {
         self.run_step_descriptions(&Builder::get_step_descriptions(self.kind), &self.paths);
     }
@@ -1375,7 +1360,7 @@ impl<'a> Builder<'a> {
                         "error: `x.py clippy` requires a host `rustc` toolchain with the `clippy` component"
                     );
                     eprintln!("help: try `rustup component add clippy`");
-                    crate::detail_exit_macro!(1);
+                    crate::exit!(1);
                 });
                 if !t!(std::str::from_utf8(&output.stdout)).contains("nightly") {
                     rustflags.arg("--cfg=bootstrap");

src/bootstrap/compile.rs

@@ -1820,7 +1820,7 @@ pub fn run_cargo(
     });
 
     if !ok {
-        crate::detail_exit_macro!(1);
+        crate::exit!(1);
     }
 
     // Ok now we need to actually find all the files listed in `toplevel`. We've

src/bootstrap/config.rs

@@ -23,7 +23,7 @@ use crate::channel::{self, GitInfo};
 pub use crate::flags::Subcommand;
 use crate::flags::{Color, Flags, Warnings};
 use crate::util::{exe, output, t};
-use build_helper::detail_exit_macro;
+use build_helper::exit;
 use once_cell::sync::OnceCell;
 use semver::Version;
 use serde::{Deserialize, Deserializer};
@@ -646,7 +646,7 @@ macro_rules! define_config {
                                         panic!("overriding existing option")
                                     } else {
                                         eprintln!("overriding existing option: `{}`", stringify!($field));
-                                        detail_exit_macro!(2);
+                                        exit!(2);
                                     }
                                 } else {
                                     self.$field = other.$field;
@@ -745,7 +745,7 @@ impl<T> Merge for Option<T> {
                             panic!("overriding existing option")
                         } else {
                             eprintln!("overriding existing option");
-                            detail_exit_macro!(2);
+                            exit!(2);
                         }
                     } else {
                         *self = other;
@@ -1101,7 +1101,7 @@ impl Config {
                 .and_then(|table: toml::Value| TomlConfig::deserialize(table))
                 .unwrap_or_else(|err| {
                     eprintln!("failed to parse TOML configuration '{}': {err}", file.display());
-                    detail_exit_macro!(2);
+                    exit!(2);
                 })
         }
         Self::parse_inner(args, get_toml)
@@ -1135,7 +1135,7 @@ impl Config {
             eprintln!(
                 "Cannot use both `llvm_bolt_profile_generate` and `llvm_bolt_profile_use` at the same time"
             );
-            detail_exit_macro!(1);
+            exit!(1);
         }
 
         // Infer the rest of the configuration.
@@ -1259,7 +1259,7 @@ impl Config {
                 }
             }
             eprintln!("failed to parse override `{option}`: `{err}");
-            detail_exit_macro!(2)
+            exit!(2)
         }
         toml.merge(override_toml, ReplaceOpt::Override);
 
@@ -2007,7 +2007,7 @@ impl Config {
                 "Unexpected rustc version: {}, we should use {}/{} to build source with {}",
                 rustc_version, prev_version, source_version, source_version
             );
-            detail_exit_macro!(1);
+            exit!(1);
         }
     }
 
@@ -2043,7 +2043,7 @@ impl Config {
             println!("help: maybe your repository history is too shallow?");
             println!("help: consider disabling `download-rustc`");
             println!("help: or fetch enough history to include one upstream commit");
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
 
         // Warn if there were changes to the compiler or standard library since the ancestor commit.

src/bootstrap/download.rs

@@ -253,7 +253,7 @@ impl Config {
             if !help_on_error.is_empty() {
                 eprintln!("{}", help_on_error);
             }
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
     }
 

src/bootstrap/flags.rs

@@ -193,7 +193,7 @@ impl Flags {
             } else {
                 panic!("No paths available for subcommand `{}`", subcommand.as_str());
             }
-            crate::detail_exit_macro!(0);
+            crate::exit!(0);
         }
 
         Flags::parse_from(it)
@@ -538,7 +538,7 @@ pub fn get_completion<G: clap_complete::Generator>(shell: G, path: &Path) -> Opt
     } else {
         std::fs::read_to_string(path).unwrap_or_else(|_| {
             eprintln!("couldn't read {}", path.display());
-            crate::detail_exit_macro!(1)
+            crate::exit!(1)
         })
     };
     let mut buf = Vec::new();

src/bootstrap/format.rs

@@ -40,7 +40,7 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F
                         code, run `./x.py fmt` instead.",
                 cmd_debug,
             );
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
         true
     }
@@ -200,7 +200,7 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
 
     let rustfmt_path = build.initial_rustfmt().unwrap_or_else(|| {
         eprintln!("./x.py fmt is not supported on this channel");
-        crate::detail_exit_macro!(1);
+        crate::exit!(1);
     });
     assert!(rustfmt_path.exists(), "{}", rustfmt_path.display());
     let src = build.src.clone();

src/bootstrap/lib.rs

@@ -27,7 +27,7 @@ use std::process::{Command, Stdio};
 use std::str;
 
 use build_helper::ci::{gha, CiEnv};
-use build_helper::detail_exit_macro;
+use build_helper::exit;
 use channel::GitInfo;
 use config::{DryRun, Target};
 use filetime::FileTime;
@@ -191,7 +191,7 @@ pub enum GitRepo {
 /// although most functions are implemented as free functions rather than
 /// methods specifically on this structure itself (to make it easier to
 /// organize).
-#[cfg_attr(not(feature = "build-metrics"), derive(Clone))]
+#[derive(Clone)]
 pub struct Build {
     /// User-specified configuration from `config.toml`.
     config: Config,
@@ -711,7 +711,7 @@ impl Build {
             for failure in failures.iter() {
                 eprintln!("  - {}\n", failure);
             }
-            detail_exit_macro!(1);
+            exit!(1);
         }
 
         #[cfg(feature = "build-metrics")]
@@ -1529,7 +1529,7 @@ impl Build {
                 "Error: Unable to find the stamp file {}, did you try to keep a nonexistent build stage?",
                 stamp.display()
             );
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
 
         let mut paths = Vec::new();
@@ -1721,7 +1721,7 @@ Alternatively, set `download-ci-llvm = true` in that `[llvm]` section
 to download LLVM rather than building it.
 "
                 );
-                detail_exit_macro!(1);
+                exit!(1);
             }
         }
 

src/bootstrap/metrics.rs

@@ -40,6 +40,13 @@ pub(crate) struct BuildMetrics {
     state: RefCell<MetricsState>,
 }
 
+/// NOTE: this isn't really cloning anything, but `x suggest` doesn't need metrics so this is probably ok.
+impl Clone for BuildMetrics {
+    fn clone(&self) -> Self {
+        Self::init()
+    }
+}
+
 impl BuildMetrics {
     pub(crate) fn init() -> Self {
         let state = RefCell::new(MetricsState {

src/bootstrap/render_tests.rs

@@ -30,7 +30,7 @@ pub(crate) fn try_run_tests(builder: &Builder<'_>, cmd: &mut Command, stream: bo
 
     if !run_tests(builder, cmd, stream) {
         if builder.fail_fast {
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         } else {
             let mut failures = builder.delayed_failures.borrow_mut();
             failures.push(format!("{cmd:?}"));

src/bootstrap/sanity.rs

@@ -104,7 +104,7 @@ You should install cmake, or set `download-ci-llvm = true` in the
 than building it.
 "
             );
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
     }
 

src/bootstrap/setup.rs

@@ -203,7 +203,7 @@ fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) {
             "note: this will use the configuration in {}",
             profile.include_path(&config.src).display()
         );
-        crate::detail_exit_macro!(1);
+        crate::exit!(1);
     }
 
     let settings = format!(
@@ -389,7 +389,7 @@ pub fn interactive_path() -> io::Result<Profile> {
         io::stdin().read_line(&mut input)?;
         if input.is_empty() {
             eprintln!("EOF on stdin, when expecting answer to question.  Giving up.");
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
         break match parse_with_abbrev(&input) {
             Ok(profile) => profile,

src/bootstrap/suggest.rs

@@ -4,18 +4,11 @@ use std::str::FromStr;
 
 use std::path::PathBuf;
 
-use crate::{
-    builder::{Builder, Kind},
-    tool::Tool,
-};
+use clap::Parser;
 
-#[cfg(feature = "build-metrics")]
-pub fn suggest(builder: &Builder<'_>, run: bool) {
-    panic!("`x suggest` is not supported with `build-metrics`")
-}
+use crate::{builder::Builder, tool::Tool};
 
 /// Suggests a list of possible `x.py` commands to run based on modified files in branch.
-#[cfg(not(feature = "build-metrics"))]
 pub fn suggest(builder: &Builder<'_>, run: bool) {
     let suggestions =
         builder.tool_cmd(Tool::SuggestTests).output().expect("failed to run `suggest-tests` tool");
@@ -67,12 +60,13 @@ pub fn suggest(builder: &Builder<'_>, run: bool) {
 
     if run {
         for sug in suggestions {
-            let mut build = builder.build.clone();
-
-            let builder =
-                Builder::new_standalone(&mut build, Kind::parse(&sug.0).unwrap(), sug.2, sug.1);
-
-            builder.execute_cli()
+            let mut build: crate::Build = builder.build.clone();
+            build.config.paths = sug.2;
+            build.config.cmd = crate::flags::Flags::parse_from(["x.py", sug.0]).cmd;
+            if let Some(stage) = sug.1 {
+                build.config.stage = stage;
+            }
+            build.build();
         }
     } else {
         println!("help: consider using the `--run` flag to automatically run suggested tests");

src/bootstrap/test.rs

@@ -833,7 +833,7 @@ impl Step for Clippy {
         }
 
         if !builder.config.cmd.bless() {
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
     }
 }
@@ -1141,7 +1141,7 @@ help: to skip test's attempt to check tidiness, pass `--exclude src/tools/tidy`
                     PATH = inferred_rustfmt_dir.display(),
                     CHAN = builder.config.channel,
                 );
-                crate::detail_exit_macro!(1);
+                crate::exit!(1);
             }
             crate::format::format(&builder, !builder.config.cmd.bless(), &[]);
         }
@@ -1164,7 +1164,7 @@ help: to skip test's attempt to check tidiness, pass `--exclude src/tools/tidy`
                 eprintln!(
                     "x.py completions were changed; run `x.py run generate-completions` to update them"
                 );
-                crate::detail_exit_macro!(1);
+                crate::exit!(1);
             }
         }
     }
@@ -1475,7 +1475,7 @@ help: to test the compiler, use `--stage 1` instead
 help: to test the standard library, use `--stage 0 library/std` instead
 note: if you're sure you want to do this, please open an issue as to why. In the meantime, you can override this with `COMPILETEST_FORCE_STAGE0=1`."
             );
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
 
         let mut compiler = self.compiler;

src/bootstrap/tool.rs

@@ -117,7 +117,7 @@ impl Step for ToolBuild {
 
         if !is_expected {
             if !is_optional_tool {
-                crate::detail_exit_macro!(1);
+                crate::exit!(1);
             } else {
                 None
             }

src/bootstrap/toolstate.rs

@@ -91,7 +91,7 @@ fn print_error(tool: &str, submodule: &str) {
     eprintln!("If you do NOT intend to update '{}', please ensure you did not accidentally", tool);
     eprintln!("change the submodule at '{}'. You may ask your reviewer for the", submodule);
     eprintln!("proper steps.");
-    crate::detail_exit_macro!(3);
+    crate::exit!(3);
 }
 
 fn check_changed_files(toolstates: &HashMap<Box<str>, ToolState>) {
@@ -106,7 +106,7 @@ fn check_changed_files(toolstates: &HashMap<Box<str>, ToolState>) {
         Ok(o) => o,
         Err(e) => {
             eprintln!("Failed to get changed files: {:?}", e);
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
     };
 
@@ -177,7 +177,7 @@ impl Step for ToolStateCheck {
         }
 
         if did_error {
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
 
         check_changed_files(&toolstates);
@@ -223,7 +223,7 @@ impl Step for ToolStateCheck {
         }
 
         if did_error {
-            crate::detail_exit_macro!(1);
+            crate::exit!(1);
         }
 
         if builder.config.channel == "nightly" && env::var_os("TOOLSTATE_PUBLISH").is_some() {

src/bootstrap/util.rs

@@ -229,7 +229,7 @@ pub fn is_valid_test_suite_arg<'a, P: AsRef<Path>>(
 
 pub fn run(cmd: &mut Command, print_cmd_on_fail: bool) {
     if try_run(cmd, print_cmd_on_fail).is_err() {
-        crate::detail_exit_macro!(1);
+        crate::exit!(1);
     }
 }
 
@@ -253,7 +253,7 @@ pub fn check_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool {
 
 pub fn run_suppressed(cmd: &mut Command) {
     if !try_run_suppressed(cmd) {
-        crate::detail_exit_macro!(1);
+        crate::exit!(1);
     }
 }
 

src/tools/build_helper/src/util.rs

@@ -1,10 +1,12 @@
 use std::process::Command;
 
 /// Invokes `build_helper::util::detail_exit` with `cfg!(test)`
+///
+/// This is a macro instead of a function so that it uses `cfg(test)` in the *calling* crate, not in build helper.
 #[macro_export]
-macro_rules! detail_exit_macro {
+macro_rules! exit {
     ($code:expr) => {
-        build_helper::util::detail_exit($code, cfg!(test));
+        $crate::util::detail_exit($code, cfg!(test));
     };
 }