From 49205a1dc09c729ddcb887fa631dd0d0c63b70db Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 31 Oct 2025 13:36:14 +1100 Subject: [PATCH 1/3] Extract parts of `bootstrap::core::builder` into a `cli_paths` module --- src/bootstrap/src/core/builder/cli_paths.rs | 229 ++++++++++++++++++++ src/bootstrap/src/core/builder/mod.rs | 227 +------------------ src/bootstrap/src/core/builder/tests.rs | 1 + 3 files changed, 234 insertions(+), 223 deletions(-) create mode 100644 src/bootstrap/src/core/builder/cli_paths.rs diff --git a/src/bootstrap/src/core/builder/cli_paths.rs b/src/bootstrap/src/core/builder/cli_paths.rs new file mode 100644 index 0000000000000..de50ecee48ad1 --- /dev/null +++ b/src/bootstrap/src/core/builder/cli_paths.rs @@ -0,0 +1,229 @@ +//! Various pieces of code for dealing with "paths" passed to bootstrap on the +//! command-line, extracted from `core/builder/mod.rs` because that file is +//! large and hard to navigate. + +use std::fmt::{self, Debug}; +use std::path::PathBuf; + +use crate::core::builder::{Builder, Kind, ShouldRun, StepDescription}; + +pub(crate) const PATH_REMAP: &[(&str, &[&str])] = &[ + // bootstrap.toml uses `rust-analyzer-proc-macro-srv`, but the + // actual path is `proc-macro-srv-cli` + ("rust-analyzer-proc-macro-srv", &["src/tools/rust-analyzer/crates/proc-macro-srv-cli"]), + // Make `x test tests` function the same as `x t tests/*` + ( + "tests", + &[ + // tidy-alphabetical-start + "tests/assembly-llvm", + "tests/codegen-llvm", + "tests/codegen-units", + "tests/coverage", + "tests/coverage-run-rustdoc", + "tests/crashes", + "tests/debuginfo", + "tests/incremental", + "tests/mir-opt", + "tests/pretty", + "tests/run-make", + "tests/run-make-cargo", + "tests/rustdoc", + "tests/rustdoc-gui", + "tests/rustdoc-js", + "tests/rustdoc-js-std", + "tests/rustdoc-json", + "tests/rustdoc-ui", + "tests/ui", + "tests/ui-fulldeps", + // tidy-alphabetical-end + ], + ), +]; + +pub(crate) fn remap_paths(paths: &mut Vec) { + let mut remove = vec![]; + let mut add = vec![]; + for (i, path) in paths.iter().enumerate().filter_map(|(i, path)| path.to_str().map(|s| (i, s))) + { + for &(search, replace) in PATH_REMAP { + // Remove leading and trailing slashes so `tests/` and `tests` are equivalent + if path.trim_matches(std::path::is_separator) == search { + remove.push(i); + add.extend(replace.iter().map(PathBuf::from)); + break; + } + } + } + remove.sort(); + remove.dedup(); + for idx in remove.into_iter().rev() { + paths.remove(idx); + } + paths.append(&mut add); +} + +#[derive(Clone, PartialEq)] +pub(crate) struct CLIStepPath { + pub(crate) path: PathBuf, + pub(crate) will_be_executed: bool, +} + +#[cfg(test)] +impl CLIStepPath { + pub(crate) fn will_be_executed(mut self, will_be_executed: bool) -> Self { + self.will_be_executed = will_be_executed; + self + } +} + +impl Debug for CLIStepPath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.path.display()) + } +} + +impl From for CLIStepPath { + fn from(path: PathBuf) -> Self { + Self { path, will_be_executed: false } + } +} + +pub(crate) fn match_paths_to_steps_and_run( + builder: &Builder<'_>, + v: &[StepDescription], + paths: &[PathBuf], +) { + let should_runs = v + .iter() + .map(|desc| (desc.should_run)(ShouldRun::new(builder, desc.kind))) + .collect::>(); + + // FIXME(Zalathar): This particular check isn't related to path-to-step + // matching, and should probably be hoisted to somewhere much earlier. + if builder.download_rustc() && (builder.kind == Kind::Dist || builder.kind == Kind::Install) { + eprintln!( + "ERROR: '{}' subcommand is incompatible with `rust.download-rustc`.", + builder.kind.as_str() + ); + crate::exit!(1); + } + + // sanity checks on rules + for (desc, should_run) in v.iter().zip(&should_runs) { + assert!(!should_run.paths.is_empty(), "{:?} should have at least one pathset", desc.name); + } + + if paths.is_empty() || builder.config.include_default_paths { + for (desc, should_run) in v.iter().zip(&should_runs) { + if desc.default && should_run.is_really_default() { + desc.maybe_run(builder, should_run.paths.iter().cloned().collect()); + } + } + } + + // Attempt to resolve paths to be relative to the builder source directory. + let mut paths: Vec = paths + .iter() + .map(|original_path| { + let mut path = original_path.clone(); + + // Someone could run `x ` from a different repository than the source + // directory. + // In that case, we should not try to resolve the paths relative to the working + // directory, but rather relative to the source directory. + // So we forcefully "relocate" the path to the source directory here. + if !path.is_absolute() { + path = builder.src.join(path); + } + + // If the path does not exist, it may represent the name of a Step, such as `tidy` in `x test tidy` + if !path.exists() { + // Use the original path here + return original_path.clone(); + } + + // Make the path absolute, strip the prefix, and convert to a PathBuf. + match std::path::absolute(&path) { + Ok(p) => p.strip_prefix(&builder.src).unwrap_or(&p).to_path_buf(), + Err(e) => { + eprintln!("ERROR: {e:?}"); + panic!("Due to the above error, failed to resolve path: {path:?}"); + } + } + }) + .collect(); + + remap_paths(&mut paths); + + // Handle all test suite paths. + // (This is separate from the loop below to avoid having to handle multiple paths in `is_suite_path` somehow.) + paths.retain(|path| { + for (desc, should_run) in v.iter().zip(&should_runs) { + if let Some(suite) = should_run.is_suite_path(path) { + desc.maybe_run(builder, vec![suite.clone()]); + return false; + } + } + true + }); + + if paths.is_empty() { + return; + } + + let mut paths: Vec = paths.into_iter().map(|p| p.into()).collect(); + let mut path_lookup: Vec<(CLIStepPath, bool)> = + paths.clone().into_iter().map(|p| (p, false)).collect(); + + // List of `(usize, &StepDescription, Vec)` where `usize` is the closest index of a path + // compared to the given CLI paths. So we can respect to the CLI order by using this value to sort + // the steps. + let mut steps_to_run = vec![]; + + for (desc, should_run) in v.iter().zip(&should_runs) { + let pathsets = should_run.pathset_for_paths_removing_matches(&mut paths, desc.kind); + + // This value is used for sorting the step execution order. + // By default, `usize::MAX` is used as the index for steps to assign them the lowest priority. + // + // If we resolve the step's path from the given CLI input, this value will be updated with + // the step's actual index. + let mut closest_index = usize::MAX; + + // Find the closest index from the original list of paths given by the CLI input. + for (index, (path, is_used)) in path_lookup.iter_mut().enumerate() { + if !*is_used && !paths.contains(path) { + closest_index = index; + *is_used = true; + break; + } + } + + steps_to_run.push((closest_index, desc, pathsets)); + } + + // Sort the steps before running them to respect the CLI order. + steps_to_run.sort_by_key(|(index, _, _)| *index); + + // Handle all PathSets. + for (_index, desc, pathsets) in steps_to_run { + if !pathsets.is_empty() { + desc.maybe_run(builder, pathsets); + } + } + + paths.retain(|p| !p.will_be_executed); + + if !paths.is_empty() { + eprintln!("ERROR: no `{}` rules matched {:?}", builder.kind.as_str(), paths); + eprintln!( + "HELP: run `x.py {} --help --verbose` to show a list of available paths", + builder.kind.as_str() + ); + eprintln!( + "NOTE: if you are adding a new Step to bootstrap itself, make sure you register it with `describe!`" + ); + crate::exit!(1); + } +} diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index b5d0f11236143..c7490c7072bda 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -1,7 +1,7 @@ use std::any::{Any, type_name}; use std::cell::{Cell, RefCell}; use std::collections::BTreeSet; -use std::fmt::{self, Debug, Write}; +use std::fmt::{Debug, Write}; use std::hash::Hash; use std::ops::Deref; use std::path::{Path, PathBuf}; @@ -20,6 +20,7 @@ use crate::core::build_steps::tool::RustcPrivateCompilers; use crate::core::build_steps::{ check, clean, clippy, compile, dist, doc, gcc, install, llvm, run, setup, test, tool, vendor, }; +use crate::core::builder::cli_paths::CLIStepPath; use crate::core::config::flags::Subcommand; use crate::core::config::{DryRun, TargetSelection}; use crate::utils::build_stamp::BuildStamp; @@ -29,7 +30,7 @@ use crate::utils::helpers::{self, LldThreads, add_dylib_path, exe, libdir, linke use crate::{Build, Crate, trace}; mod cargo; - +mod cli_paths; #[cfg(test)] mod tests; @@ -424,88 +425,6 @@ impl PathSet { } } -const PATH_REMAP: &[(&str, &[&str])] = &[ - // bootstrap.toml uses `rust-analyzer-proc-macro-srv`, but the - // actual path is `proc-macro-srv-cli` - ("rust-analyzer-proc-macro-srv", &["src/tools/rust-analyzer/crates/proc-macro-srv-cli"]), - // Make `x test tests` function the same as `x t tests/*` - ( - "tests", - &[ - // tidy-alphabetical-start - "tests/assembly-llvm", - "tests/codegen-llvm", - "tests/codegen-units", - "tests/coverage", - "tests/coverage-run-rustdoc", - "tests/crashes", - "tests/debuginfo", - "tests/incremental", - "tests/mir-opt", - "tests/pretty", - "tests/run-make", - "tests/run-make-cargo", - "tests/rustdoc", - "tests/rustdoc-gui", - "tests/rustdoc-js", - "tests/rustdoc-js-std", - "tests/rustdoc-json", - "tests/rustdoc-ui", - "tests/ui", - "tests/ui-fulldeps", - // tidy-alphabetical-end - ], - ), -]; - -fn remap_paths(paths: &mut Vec) { - let mut remove = vec![]; - let mut add = vec![]; - for (i, path) in paths.iter().enumerate().filter_map(|(i, path)| path.to_str().map(|s| (i, s))) - { - for &(search, replace) in PATH_REMAP { - // Remove leading and trailing slashes so `tests/` and `tests` are equivalent - if path.trim_matches(std::path::is_separator) == search { - remove.push(i); - add.extend(replace.iter().map(PathBuf::from)); - break; - } - } - } - remove.sort(); - remove.dedup(); - for idx in remove.into_iter().rev() { - paths.remove(idx); - } - paths.append(&mut add); -} - -#[derive(Clone, PartialEq)] -struct CLIStepPath { - path: PathBuf, - will_be_executed: bool, -} - -#[cfg(test)] -impl CLIStepPath { - fn will_be_executed(mut self, will_be_executed: bool) -> Self { - self.will_be_executed = will_be_executed; - self - } -} - -impl Debug for CLIStepPath { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.path.display()) - } -} - -impl From for CLIStepPath { - fn from(path: PathBuf) -> Self { - Self { path, will_be_executed: false } - } -} - impl StepDescription { fn from(kind: Kind) -> StepDescription { StepDescription { @@ -554,144 +473,6 @@ impl StepDescription { } false } - - fn run(v: &[StepDescription], builder: &Builder<'_>, paths: &[PathBuf]) { - let should_runs = v - .iter() - .map(|desc| (desc.should_run)(ShouldRun::new(builder, desc.kind))) - .collect::>(); - - if builder.download_rustc() && (builder.kind == Kind::Dist || builder.kind == Kind::Install) - { - eprintln!( - "ERROR: '{}' subcommand is incompatible with `rust.download-rustc`.", - builder.kind.as_str() - ); - crate::exit!(1); - } - - // sanity checks on rules - for (desc, should_run) in v.iter().zip(&should_runs) { - assert!( - !should_run.paths.is_empty(), - "{:?} should have at least one pathset", - desc.name - ); - } - - if paths.is_empty() || builder.config.include_default_paths { - for (desc, should_run) in v.iter().zip(&should_runs) { - if desc.default && should_run.is_really_default() { - desc.maybe_run(builder, should_run.paths.iter().cloned().collect()); - } - } - } - - // Attempt to resolve paths to be relative to the builder source directory. - let mut paths: Vec = paths - .iter() - .map(|original_path| { - let mut path = original_path.clone(); - - // Someone could run `x ` from a different repository than the source - // directory. - // In that case, we should not try to resolve the paths relative to the working - // directory, but rather relative to the source directory. - // So we forcefully "relocate" the path to the source directory here. - if !path.is_absolute() { - path = builder.src.join(path); - } - - // If the path does not exist, it may represent the name of a Step, such as `tidy` in `x test tidy` - if !path.exists() { - // Use the original path here - return original_path.clone(); - } - - // Make the path absolute, strip the prefix, and convert to a PathBuf. - match std::path::absolute(&path) { - Ok(p) => p.strip_prefix(&builder.src).unwrap_or(&p).to_path_buf(), - Err(e) => { - eprintln!("ERROR: {e:?}"); - panic!("Due to the above error, failed to resolve path: {path:?}"); - } - } - }) - .collect(); - - remap_paths(&mut paths); - - // Handle all test suite paths. - // (This is separate from the loop below to avoid having to handle multiple paths in `is_suite_path` somehow.) - paths.retain(|path| { - for (desc, should_run) in v.iter().zip(&should_runs) { - if let Some(suite) = should_run.is_suite_path(path) { - desc.maybe_run(builder, vec![suite.clone()]); - return false; - } - } - true - }); - - if paths.is_empty() { - return; - } - - let mut paths: Vec = paths.into_iter().map(|p| p.into()).collect(); - let mut path_lookup: Vec<(CLIStepPath, bool)> = - paths.clone().into_iter().map(|p| (p, false)).collect(); - - // List of `(usize, &StepDescription, Vec)` where `usize` is the closest index of a path - // compared to the given CLI paths. So we can respect to the CLI order by using this value to sort - // the steps. - let mut steps_to_run = vec![]; - - for (desc, should_run) in v.iter().zip(&should_runs) { - let pathsets = should_run.pathset_for_paths_removing_matches(&mut paths, desc.kind); - - // This value is used for sorting the step execution order. - // By default, `usize::MAX` is used as the index for steps to assign them the lowest priority. - // - // If we resolve the step's path from the given CLI input, this value will be updated with - // the step's actual index. - let mut closest_index = usize::MAX; - - // Find the closest index from the original list of paths given by the CLI input. - for (index, (path, is_used)) in path_lookup.iter_mut().enumerate() { - if !*is_used && !paths.contains(path) { - closest_index = index; - *is_used = true; - break; - } - } - - steps_to_run.push((closest_index, desc, pathsets)); - } - - // Sort the steps before running them to respect the CLI order. - steps_to_run.sort_by_key(|(index, _, _)| *index); - - // Handle all PathSets. - for (_index, desc, pathsets) in steps_to_run { - if !pathsets.is_empty() { - desc.maybe_run(builder, pathsets); - } - } - - paths.retain(|p| !p.will_be_executed); - - if !paths.is_empty() { - eprintln!("ERROR: no `{}` rules matched {:?}", builder.kind.as_str(), paths); - eprintln!( - "HELP: run `x.py {} --help --verbose` to show a list of available paths", - builder.kind.as_str() - ); - eprintln!( - "NOTE: if you are adding a new Step to bootstrap itself, make sure you register it with `describe!`" - ); - crate::exit!(1); - } - } } enum ReallyDefault<'a> { @@ -1349,7 +1130,7 @@ impl<'a> Builder<'a> { } fn run_step_descriptions(&self, v: &[StepDescription], paths: &[PathBuf]) { - StepDescription::run(v, self, paths); + cli_paths::match_paths_to_steps_and_run(self, v, paths); } /// Returns if `std` should be statically linked into `rustc_driver`. diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index e0eb38d04aad0..b8ba1b4c2c340 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -7,6 +7,7 @@ use llvm::prebuilt_llvm_config; use super::*; use crate::Flags; use crate::core::build_steps::doc::DocumentationFormat; +use crate::core::builder::cli_paths::PATH_REMAP; use crate::core::config::Config; use crate::utils::cache::ExecutedStep; use crate::utils::helpers::get_host_target; From 70a9883135d8d8799561f4a6b4fce1ba324e22a5 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 31 Oct 2025 14:03:08 +1100 Subject: [PATCH 2/3] Replace repeated zips with a dedicated `StepExtra` struct --- src/bootstrap/src/core/builder/cli_paths.rs | 25 +++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/src/core/builder/cli_paths.rs b/src/bootstrap/src/core/builder/cli_paths.rs index de50ecee48ad1..294318f1d494a 100644 --- a/src/bootstrap/src/core/builder/cli_paths.rs +++ b/src/bootstrap/src/core/builder/cli_paths.rs @@ -89,14 +89,25 @@ impl From for CLIStepPath { } } +/// Combines a `StepDescription` with its corresponding `ShouldRun`. +struct StepExtra<'a> { + desc: &'a StepDescription, + should_run: ShouldRun<'a>, +} + pub(crate) fn match_paths_to_steps_and_run( builder: &Builder<'_>, - v: &[StepDescription], + step_descs: &[StepDescription], paths: &[PathBuf], ) { - let should_runs = v + // Obtain `ShouldRun` information for each step, so that we know which + // paths to match it against. + let steps = step_descs .iter() - .map(|desc| (desc.should_run)(ShouldRun::new(builder, desc.kind))) + .map(|desc| StepExtra { + desc, + should_run: (desc.should_run)(ShouldRun::new(builder, desc.kind)), + }) .collect::>(); // FIXME(Zalathar): This particular check isn't related to path-to-step @@ -110,12 +121,12 @@ pub(crate) fn match_paths_to_steps_and_run( } // sanity checks on rules - for (desc, should_run) in v.iter().zip(&should_runs) { + for StepExtra { desc, should_run } in &steps { assert!(!should_run.paths.is_empty(), "{:?} should have at least one pathset", desc.name); } if paths.is_empty() || builder.config.include_default_paths { - for (desc, should_run) in v.iter().zip(&should_runs) { + for StepExtra { desc, should_run } in &steps { if desc.default && should_run.is_really_default() { desc.maybe_run(builder, should_run.paths.iter().cloned().collect()); } @@ -159,7 +170,7 @@ pub(crate) fn match_paths_to_steps_and_run( // Handle all test suite paths. // (This is separate from the loop below to avoid having to handle multiple paths in `is_suite_path` somehow.) paths.retain(|path| { - for (desc, should_run) in v.iter().zip(&should_runs) { + for StepExtra { desc, should_run } in &steps { if let Some(suite) = should_run.is_suite_path(path) { desc.maybe_run(builder, vec![suite.clone()]); return false; @@ -181,7 +192,7 @@ pub(crate) fn match_paths_to_steps_and_run( // the steps. let mut steps_to_run = vec![]; - for (desc, should_run) in v.iter().zip(&should_runs) { + for StepExtra { desc, should_run } in &steps { let pathsets = should_run.pathset_for_paths_removing_matches(&mut paths, desc.kind); // This value is used for sorting the step execution order. From 250ee47708e3612d89389861103d4dbb2644cb88 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 31 Oct 2025 14:26:31 +1100 Subject: [PATCH 3/3] Replace bare tuple with a `StepToRun` struct --- src/bootstrap/src/core/builder/cli_paths.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/src/core/builder/cli_paths.rs b/src/bootstrap/src/core/builder/cli_paths.rs index 294318f1d494a..aa81c4684eab3 100644 --- a/src/bootstrap/src/core/builder/cli_paths.rs +++ b/src/bootstrap/src/core/builder/cli_paths.rs @@ -5,7 +5,7 @@ use std::fmt::{self, Debug}; use std::path::PathBuf; -use crate::core::builder::{Builder, Kind, ShouldRun, StepDescription}; +use crate::core::builder::{Builder, Kind, PathSet, ShouldRun, StepDescription}; pub(crate) const PATH_REMAP: &[(&str, &[&str])] = &[ // bootstrap.toml uses `rust-analyzer-proc-macro-srv`, but the @@ -95,6 +95,12 @@ struct StepExtra<'a> { should_run: ShouldRun<'a>, } +struct StepToRun<'a> { + sort_index: usize, + desc: &'a StepDescription, + pathsets: Vec, +} + pub(crate) fn match_paths_to_steps_and_run( builder: &Builder<'_>, step_descs: &[StepDescription], @@ -187,9 +193,8 @@ pub(crate) fn match_paths_to_steps_and_run( let mut path_lookup: Vec<(CLIStepPath, bool)> = paths.clone().into_iter().map(|p| (p, false)).collect(); - // List of `(usize, &StepDescription, Vec)` where `usize` is the closest index of a path - // compared to the given CLI paths. So we can respect to the CLI order by using this value to sort - // the steps. + // Before actually running (non-suite) steps, collect them into a list of structs + // so that we can then sort the list to preserve CLI order as much as possible. let mut steps_to_run = vec![]; for StepExtra { desc, should_run } in &steps { @@ -211,14 +216,14 @@ pub(crate) fn match_paths_to_steps_and_run( } } - steps_to_run.push((closest_index, desc, pathsets)); + steps_to_run.push(StepToRun { sort_index: closest_index, desc, pathsets }); } // Sort the steps before running them to respect the CLI order. - steps_to_run.sort_by_key(|(index, _, _)| *index); + steps_to_run.sort_by_key(|step| step.sort_index); // Handle all PathSets. - for (_index, desc, pathsets) in steps_to_run { + for StepToRun { sort_index: _, desc, pathsets } in steps_to_run { if !pathsets.is_empty() { desc.maybe_run(builder, pathsets); }