rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Remove current implementation of ./x suggest

Browse Source 
This is quite a bit of implementation complexity, yet it is quite
broken, and we don't have the maintenance bandwidth to address.

Remove the current implementation if only to reduce bootstrap's
implementation complexity; the `suggest` flow comes with its own set of
hacks.
This commit is contained in:
Jieyou Xu
2025-07-08 17:52:00 +08:00
parent 904273c58e
commit 91d064b442
20 changed files with 10 additions and 348 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
Cargo.lock
View File

@@ -5188,14 +5188,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"

1
Cargo.toml
View File

@@ -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",

1
src/bootstrap/src/core/build_steps/mod.rs
View File

@@ -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;

68
src/bootstrap/src/core/build_steps/suggest.rs
View File

@@ -1,68 +0,0 @@
//! Attempt to magically identify good tests to run
use std::path::PathBuf;
use std::str::FromStr;
use clap::Parser;
use crate::core::build_steps::tool::Tool;
use crate::core::builder::Builder;
/// Suggests a list of possible `x.py` commands to run based on modified files in branch.
pub fn suggest(builder: &Builder<'_>, run: bool) {
let git_config = builder.config.git_config();
let suggestions = builder
.tool_cmd(Tool::SuggestTests)
.env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch)
.env("SUGGEST_TESTS_MERGE_COMMIT_EMAIL", git_config.git_merge_commit_email)
.run_capture_stdout(builder)
.stdout();
let suggestions = suggestions
.lines()
.map(|line| {
let mut sections = line.split_ascii_whitespace();
// this code expects one suggestion per line in the following format:
// <x_subcommand> {some number of flags} [optional stage number]
let cmd = sections.next().unwrap();
let stage = sections.next_back().and_then(|s| str::parse(s).ok());
let paths: Vec<PathBuf> = sections.map(|p| PathBuf::from_str(p).unwrap()).collect();
(cmd, stage, paths)
})
.collect::<Vec<_>>();
if !suggestions.is_empty() {
println!("==== SUGGESTIONS ====");
for sug in &suggestions {
print!("x {} ", sug.0);
if let Some(stage) = sug.1 {
print!("--stage {stage} ");
}
for path in &sug.2 {
print!("{} ", path.display());
}
println!();
}
println!("=====================");
} else {
println!("No suggestions found!");
return;
}
if run {
for sug in suggestions {
let mut build: crate::Build = builder.build.clone();
build.config.paths = sug.2;
build.config.cmd = crate::core::config::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");
}
}

9
src/bootstrap/src/core/build_steps/test.rs
View File

@@ -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.

1
src/bootstrap/src/core/build_steps/tool.rs
View File

@@ -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;

2
src/bootstrap/src/core/builder/cargo.rs
View File

@@ -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);
}

6
src/bootstrap/src/core/builder/mod.rs
View File

@@ -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)),

2
src/bootstrap/src/core/config/config.rs
View File

@@ -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 { .. } => {}
}

8
src/bootstrap/src/core/config/flags.rs
View File

@@ -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,
}

1
src/bootstrap/src/core/sanity.rs
View File

@@ -216,7 +216,6 @@ than building it.
build.config.cmd,
Subcommand::Clean { .. }
| Subcommand::Check { .. }
| Subcommand::Suggest { .. }
| Subcommand::Format { .. }
| Subcommand::Setup { .. }
);

11
src/bootstrap/src/lib.rs
View File

@@ -651,11 +651,9 @@ impl Build {
// Handle hard-coded subcommands.
{
#[cfg(feature = "tracing")]
let _hardcoded_span = span!(
tracing::Level::DEBUG,
"handling hardcoded subcommands (Format, Suggest, Perf)"
)
.entered();
let _hardcoded_span =
span!(tracing::Level::DEBUG, "handling hardcoded subcommands (Format, Perf)")
.entered();
match &self.config.cmd {
Subcommand::Format { check, all } => {
@@ -666,9 +664,6 @@ impl Build {
&self.config.paths,
);
}
Subcommand::Suggest { run } => {
return core::build_steps::suggest::suggest(&builder::Builder::new(self), *run);
}
Subcommand::Perf(args) => {
return core::build_steps::perf::perf(&builder::Builder::new(self), args);
}

1
src/bootstrap/src/utils/cc_detect.rs
View File

@@ -67,7 +67,6 @@ pub fn fill_compilers(build: &mut Build) {
// We don't need to check cross targets for these commands.
crate::Subcommand::Clean { .. }
| crate::Subcommand::Check { .. }
| crate::Subcommand::Suggest { .. }
| crate::Subcommand::Format { .. }
| crate::Subcommand::Setup { .. } => {
build.hosts.iter().cloned().chain(iter::once(build.host_target)).collect()

2
src/bootstrap/src/utils/metrics.rs
View File

@@ -43,7 +43,7 @@ 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.
// NOTE: this isn't really cloning anything, but necessary for `Build: Clone`.
impl Clone for BuildMetrics {
fn clone(&self) -> Self {
Self::init()

8
src/tools/suggest-tests/Cargo.toml
View File

@@ -1,8 +0,0 @@
[package]
name = "suggest-tests"
version = "0.1.0"
edition = "2021"
[dependencies]
glob = "0.3.0"
build_helper = { version = "0.1.0", path = "../../build_helper" }

32
src/tools/suggest-tests/src/dynamic_suggestions.rs
View File

@@ -1,32 +0,0 @@
use std::path::Path;
use crate::Suggestion;
type DynamicSuggestion = fn(&Path) -> Vec<Suggestion>;
pub(crate) const DYNAMIC_SUGGESTIONS: &[DynamicSuggestion] = &[
|path: &Path| -> Vec<Suggestion> {
if path.starts_with("compiler/") || path.starts_with("library/") {
let path = path.components().take(2).collect::<Vec<_>>();
vec![Suggestion::with_single_path(
"test",
None,
&format!(
"{}/{}",
path[0].as_os_str().to_str().unwrap(),
path[1].as_os_str().to_str().unwrap()
),
)]
} else {
Vec::new()
}
},
|path: &Path| -> Vec<Suggestion> {
if path.starts_with("compiler/rustc_pattern_analysis") {
vec![Suggestion::new("test", None, &["tests/ui", "--test-args", "pattern"])]
} else {
Vec::new()
}
},
];

96
src/tools/suggest-tests/src/lib.rs
View File

@@ -1,96 +0,0 @@
use std::fmt::{self, Display};
use std::path::Path;
use dynamic_suggestions::DYNAMIC_SUGGESTIONS;
use glob::Pattern;
use static_suggestions::static_suggestions;
mod dynamic_suggestions;
mod static_suggestions;
#[cfg(test)]
mod tests;
macro_rules! sug {
($cmd:expr) => {
Suggestion::new($cmd, None, &[])
};
($cmd:expr, $paths:expr) => {
Suggestion::new($cmd, None, $paths.as_slice())
};
($cmd:expr, $stage:expr, $paths:expr) => {
Suggestion::new($cmd, Some($stage), $paths.as_slice())
};
}
pub(crate) use sug;
pub fn get_suggestions<T: AsRef<str>>(modified_files: &[T]) -> Vec<Suggestion> {
let mut suggestions = Vec::new();
// static suggestions
for (globs, sugs) in static_suggestions().iter() {
let globs = globs
.iter()
.map(|glob| Pattern::new(glob).expect("Found invalid glob pattern!"))
.collect::<Vec<_>>();
let matches_some_glob = |file: &str| globs.iter().any(|glob| glob.matches(file));
if modified_files.iter().map(AsRef::as_ref).any(matches_some_glob) {
suggestions.extend_from_slice(sugs);
}
}
// dynamic suggestions
for sug in DYNAMIC_SUGGESTIONS {
for file in modified_files {
let sugs = sug(Path::new(file.as_ref()));
suggestions.extend_from_slice(&sugs);
}
}
suggestions.sort();
suggestions.dedup();
suggestions
}
#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Debug)]
pub struct Suggestion {
pub cmd: String,
pub stage: Option<u32>,
pub paths: Vec<String>,
}
impl Suggestion {
pub fn new(cmd: &str, stage: Option<u32>, paths: &[&str]) -> Self {
Self { cmd: cmd.to_owned(), stage, paths: paths.iter().map(|p| p.to_string()).collect() }
}
pub fn with_single_path(cmd: &str, stage: Option<u32>, path: &str) -> Self {
Self::new(cmd, stage, &[path])
}
}
impl Display for Suggestion {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
write!(f, "{} ", self.cmd)?;
for path in &self.paths {
write!(f, "{} ", path)?;
}
if let Some(stage) = self.stage {
write!(f, "{}", stage)?;
} else {
// write a sentinel value here (in place of a stage) to be consumed
// by the shim in bootstrap, it will be read and ignored.
write!(f, "N/A")?;
}
Ok(())
}
}

40
src/tools/suggest-tests/src/main.rs
View File

@@ -1,40 +0,0 @@
use std::process::ExitCode;
use build_helper::git::{GitConfig, get_git_modified_files};
use suggest_tests::get_suggestions;
fn main() -> ExitCode {
let modified_files = get_git_modified_files(
&GitConfig {
nightly_branch: &env("SUGGEST_TESTS_NIGHTLY_BRANCH"),
git_merge_commit_email: &env("SUGGEST_TESTS_MERGE_COMMIT_EMAIL"),
},
None,
&Vec::new(),
);
let modified_files = match modified_files {
Ok(files) => files,
Err(err) => {
eprintln!("Could not get modified files from git: \"{err}\"");
return ExitCode::FAILURE;
}
};
let suggestions = get_suggestions(&modified_files);
for sug in &suggestions {
println!("{sug}");
}
ExitCode::SUCCESS
}
fn env(key: &str) -> String {
match std::env::var(key) {
Ok(var) => var,
Err(err) => {
eprintln!("suggest-tests: failed to read environment variable {key}: {err}");
std::process::exit(1);
}
}
}

40
src/tools/suggest-tests/src/static_suggestions.rs
View File

@@ -1,40 +0,0 @@
use std::sync::OnceLock;
use crate::{Suggestion, sug};
// FIXME: perhaps this could use `std::lazy` when it is stabilized
macro_rules! static_suggestions {
($( [ $( $glob:expr ),* $(,)? ] => [ $( $suggestion:expr ),* $(,)? ] ),* $(,)? ) => {
pub(crate) fn static_suggestions() -> &'static [(Vec<&'static str>, Vec<Suggestion>)]
{
static S: OnceLock<Vec<(Vec<&'static str>, Vec<Suggestion>)>> = OnceLock::new();
S.get_or_init(|| vec![ $( (vec![ $($glob),* ], vec![ $($suggestion),* ]) ),*])
}
}
}
static_suggestions! {
["*.md"] => [
sug!("test", 0, ["linkchecker"]),
],
["compiler/*"] => [
sug!("check"),
sug!("test", 1, ["tests/ui", "tests/run-make"]),
],
["compiler/rustc_mir_transform/*"] => [
sug!("test", 1, ["mir-opt"]),
],
[
"compiler/rustc_mir_transform/src/coverage/*",
"compiler/rustc_codegen_llvm/src/coverageinfo/*",
] => [
sug!("test", 1, ["coverage"]),
],
["src/librustdoc/*"] => [
sug!("test", 1, ["rustdoc"]),
],
}

21
src/tools/suggest-tests/src/tests.rs
View File

@@ -1,21 +0,0 @@
macro_rules! sugg_test {
( $( $name:ident: $paths:expr => $suggestions:expr ),* ) => {
$(
#[test]
fn $name() {
let suggestions = crate::get_suggestions(&$paths).into_iter().map(|s| s.to_string()).collect::<Vec<_>>();
assert_eq!(suggestions, $suggestions);
}
)*
};
}
sugg_test! {
test_error_code_docs: ["compiler/rustc_error_codes/src/error_codes/E0000.md"] =>
["check N/A", "test compiler/rustc_error_codes N/A", "test linkchecker 0", "test tests/ui tests/run-make 1"],
test_rustdoc: ["src/librustdoc/src/lib.rs"] => ["test rustdoc 1"],
test_rustdoc_and_libstd: ["src/librustdoc/src/lib.rs", "library/std/src/lib.rs"] =>
["test library/std N/A", "test rustdoc 1"]
}