Rollup merge of #143630 - jieyouxu:drop-suggest, r=Mark-Simulacrum

Drop `./x suggest`

This PR removes the current `./x suggest` implementation (#109933, #106249) and associated docs for several reasons:

1. Primarily, `./x suggest` is another "flow" in bootstrap that incurs extra complexity and more invariants that bootstrap has to maintain. This causes more friction when trying to investigate and fix staging problems. As far as I know, this flow has not been actively maintained in quite a while, and I'm not aware of interest in maintaining it. Bootstrap really could use less implementation complexity with a very limited maintenance bandwidth.
2. The current `./x suggest` implementation "bypasses" the usual stage defaults for the various check/build/test/etc. flows, and it's not really possible to have a stage default because `./x suggest --run` produces a *sequence* of suggestions like [`./x check`, `./x test library/std`, ..] and then tries to run all of them in sequence, based on which files are modified.
3. We've not seen a lot of interest both in using it or extending static/dynamic test suggestions. Last extensions were #117961 and #120763. I'm not convinced the extra implementation complexity is worth it. This was discussed in:
    - [#t-infra/bootstrap > Dropping the current `./x suggest` flow implementation](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Dropping.20the.20current.20.60.2E.2Fx.20suggest.60.20flow.20implementation/with/527456699)
    - [#t-compiler > Dropping current `./x suggest` implementation](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Dropping.20current.20.60.2E.2Fx.20suggest.60.20implementation/with/527528696)

Closes #109933 (the current implementation is being removed).
Closes #143569 (by removing `./x suggest` altogether).

Author: samueltardieu

Cargo.lock

@@ -5069,14 +5069,6 @@ dependencies = [
  "syn 1.0.109",
 ]
 
-[[package]]
-name = "suggest-tests"
-version = "0.1.0"
-dependencies = [
- "build_helper",
- "glob",
-]
-
 [[package]]
 name = "syn"
 version = "1.0.109"

Cargo.toml

@@ -39,7 +39,6 @@ members = [
   "src/tools/rustdoc-gui-test",
   "src/tools/rustdoc-themes",
   "src/tools/rustfmt",
-  "src/tools/suggest-tests",
   "src/tools/test-float-parse",
   "src/tools/tidy",
   "src/tools/tier-check",

src/bootstrap/src/core/build_steps/mod.rs

@@ -11,7 +11,6 @@ pub(crate) mod llvm;
 pub(crate) mod perf;
 pub(crate) mod run;
 pub(crate) mod setup;
-pub(crate) mod suggest;
 pub(crate) mod synthetic_targets;
 pub(crate) mod test;
 pub(crate) mod tool;

src/bootstrap/src/core/build_steps/suggest.rs

@@ -47,12 +47,11 @@ impl Step for CrateBootstrap {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        // This step is responsible for several different tool paths. By default
-        // it will test all of them, but requesting specific tools on the
-        // command-line (e.g. `./x test suggest-tests`) will test only the
-        // specified tools.
+        // This step is responsible for several different tool paths.
+        //
+        // By default, it will test all of them, but requesting specific tools on the command-line
+        // (e.g. `./x test src/tools/coverage-dump`) will test only the specified tools.
         run.path("src/tools/jsondoclint")
-            .path("src/tools/suggest-tests")
             .path("src/tools/replace-version-placeholder")
             .path("src/tools/coverage-dump")
             // We want `./x test tidy` to _run_ the tidy tool, not its tests.

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

@@ -517,7 +517,6 @@ bootstrap_tool!(
     ReplaceVersionPlaceholder, "src/tools/replace-version-placeholder", "replace-version-placeholder";
     CollectLicenseMetadata, "src/tools/collect-license-metadata", "collect-license-metadata";
     GenerateCopyright, "src/tools/generate-copyright", "generate-copyright";
-    SuggestTests, "src/tools/suggest-tests", "suggest-tests";
     GenerateWindowsSys, "src/tools/generate-windows-sys", "generate-windows-sys";
     // rustdoc-gui-test has a crate dependency on compiletest, so it needs the same unstable features.
     RustdocGUITest, "src/tools/rustdoc-gui-test", "rustdoc-gui-test", is_unstable_tool = true, allow_features = COMPILETEST_ALLOW_FEATURES;

src/bootstrap/src/core/build_steps/tool.rs

@@ -113,7 +113,7 @@ impl Cargo {
 
         match cmd_kind {
             // No need to configure the target linker for these command types.
-            Kind::Clean | Kind::Check | Kind::Suggest | Kind::Format | Kind::Setup => {}
+            Kind::Clean | Kind::Check | Kind::Format | Kind::Setup => {}
             _ => {
                 cargo.configure_linker(builder, mode);
             }

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

@@ -845,7 +845,6 @@ pub enum Kind {
     #[value(alias = "r")]
     Run,
     Setup,
-    Suggest,
     Vendor,
     Perf,
 }
@@ -869,7 +868,6 @@ impl Kind {
             Kind::Install => "install",
             Kind::Run => "run",
             Kind::Setup => "setup",
-            Kind::Suggest => "suggest",
             Kind::Vendor => "vendor",
             Kind::Perf => "perf",
         }
@@ -881,7 +879,6 @@ impl Kind {
             Kind::Bench => "Benchmarking",
             Kind::Doc => "Documenting",
             Kind::Run => "Running",
-            Kind::Suggest => "Suggesting",
             Kind::Clippy => "Linting",
             Kind::Perf => "Profiling & benchmarking",
             _ => {
@@ -1201,7 +1198,7 @@ impl<'a> Builder<'a> {
             Kind::Clean => describe!(clean::CleanAll, clean::Rustc, clean::Std),
             Kind::Vendor => describe!(vendor::Vendor),
             // special-cased in Build::build()
-            Kind::Format | Kind::Suggest | Kind::Perf => vec![],
+            Kind::Format | Kind::Perf => vec![],
             Kind::MiriTest | Kind::MiriSetup => unreachable!(),
         }
     }
@@ -1269,7 +1266,6 @@ impl<'a> Builder<'a> {
             Subcommand::Run { .. } => (Kind::Run, &paths[..]),
             Subcommand::Clean { .. } => (Kind::Clean, &paths[..]),
             Subcommand::Format { .. } => (Kind::Format, &[][..]),
-            Subcommand::Suggest { .. } => (Kind::Suggest, &[][..]),
             Subcommand::Setup { profile: ref path } => (
                 Kind::Setup,
                 path.as_ref().map_or([].as_slice(), |path| std::slice::from_ref(path)),

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

@@ -1050,7 +1050,6 @@ impl Config {
             | Subcommand::Run { .. }
             | Subcommand::Setup { .. }
             | Subcommand::Format { .. }
-            | Subcommand::Suggest { .. }
             | Subcommand::Vendor { .. } => flags_stage.unwrap_or(0),
         };
 
@@ -1098,7 +1097,6 @@ impl Config {
                 | Subcommand::Run { .. }
                 | Subcommand::Setup { .. }
                 | Subcommand::Format { .. }
-                | Subcommand::Suggest { .. }
                 | Subcommand::Vendor { .. }
                 | Subcommand::Perf { .. } => {}
             }

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

@@ -481,13 +481,6 @@ Arguments:
         #[arg(value_name = "<PROFILE>|hook|editor|link")]
         profile: Option<PathBuf>,
     },
-    /// Suggest a subset of tests to run, based on modified files
-    #[command(long_about = "\n")]
-    Suggest {
-        /// run suggested tests
-        #[arg(long)]
-        run: bool,
-    },
     /// Vendor dependencies
     Vendor {
         /// Additional `Cargo.toml` to sync and vendor
@@ -518,7 +511,6 @@ impl Subcommand {
             Subcommand::Install => Kind::Install,
             Subcommand::Run { .. } => Kind::Run,
             Subcommand::Setup { .. } => Kind::Setup,
-            Subcommand::Suggest { .. } => Kind::Suggest,
             Subcommand::Vendor { .. } => Kind::Vendor,
             Subcommand::Perf { .. } => Kind::Perf,
         }