diff --git a/src/bootstrap/src/core/builder/cli_paths.rs b/src/bootstrap/src/core/builder/cli_paths.rs index 51304bcd140b6..94e111966d322 100644 --- a/src/bootstrap/src/core/builder/cli_paths.rs +++ b/src/bootstrap/src/core/builder/cli_paths.rs @@ -2,7 +2,8 @@ //! command-line, extracted from `core/builder/mod.rs` because that file is //! large and hard to navigate. -use std::fmt::{self, Debug}; +use std::collections::{HashMap, HashSet}; +use std::hash::Hash; use std::path::PathBuf; use crate::core::builder::{Builder, CommandLineStepDescription, Kind, PathSet, ShouldRun}; @@ -10,36 +11,12 @@ use crate::core::builder::{Builder, CommandLineStepDescription, Kind, PathSet, S #[cfg(test)] mod tests; -#[derive(Clone, PartialEq)] -pub(crate) struct CLIStepPath { - pub(crate) path: PathBuf, - pub(crate) will_be_executed: bool, -} - -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 } - } -} - /// Combines a [`CommandLineStepDescription`] with its corresponding [`ShouldRun`]. struct StepExtra<'a> { desc: &'a CommandLineStepDescription, should_run: ShouldRun<'a>, } -struct StepToRun<'a> { - sort_index: usize, - desc: &'a CommandLineStepDescription, - pathsets: Vec, -} - pub(crate) fn match_paths_to_steps_and_run( builder: &Builder<'_>, step_descs: &[CommandLineStepDescription], @@ -67,6 +44,7 @@ pub(crate) fn match_paths_to_steps_and_run( assert!(!should_run.paths.is_empty(), "{:?} should have at least one pathset", desc.name); } + // Run default steps if appropriate. if paths.is_empty() || builder.config.include_default_paths { for StepExtra { desc, should_run } in &steps { if (desc.is_default_step_fn)(builder) { @@ -86,7 +64,7 @@ pub(crate) fn match_paths_to_steps_and_run( // // It is also possible that someone passed a relative path starting with . or .. // In that case, we have to remove that path prefix. - let mut paths = paths + let paths = paths .iter() .map(|path| { // Here we "launder" the path through builder.src, to normalize relative path prefixes @@ -111,7 +89,6 @@ pub(crate) fn match_paths_to_steps_and_run( path } }) - .map(|p| p.to_owned()) .collect::>(); // If any absolute paths couldn't be made relative, stop now and report them. @@ -123,66 +100,48 @@ pub(crate) fn match_paths_to_steps_and_run( crate::exit!(1); } - // 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 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; - } - } - 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(); - - // 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 { - let pathsets = should_run.pathsets_for_paths_flagging_matches(&mut paths); - - // 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; + // When matching selectors to steps, we want to balance two conflicting goals: + // - Ideally, steps should run in the order specified by command-line arguments. + // - A selected step should be invoked only once, not multiple times. + // + // We therefore build up: + // - An ordered list of steps to run, each represented by its index in `steps`. + // - For each step (by index), the list of its anchors that were matched. + let mut step_queue = Vec::::with_capacity(paths.len()); + let mut step_anchors = HashMap::>::with_capacity(steps.len()); + let mut unmatched_paths = vec![]; + + // For each command-line selector, enqueue the steps that it matches. + for path in &paths { + let mut path_matched = false; + + for (step_ix, step) in steps.iter().enumerate() { + let matched_anchors = step + .should_run + .paths + .iter() + .filter(|anchor| { + // The extra `starts_with` here allows an argument like + // `tests/ui/asm/cfg.rs` to select the suite anchor `tests/ui`. + anchor.has(path) + || matches!(anchor, PathSet::Suite(suite) if path.starts_with(&suite.path)) + }) + .collect::>(); + + if !matched_anchors.is_empty() { + step_queue.push(step_ix); + step_anchors.entry(step_ix).or_default().extend(matched_anchors); + path_matched = true; } } - 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(|step| step.sort_index); - - // Handle all PathSets. - for StepToRun { sort_index: _, desc, pathsets } in steps_to_run { - if !pathsets.is_empty() { - desc.maybe_run(builder, pathsets); + if !path_matched { + unmatched_paths.push(path); } } - paths.retain(|p| !p.will_be_executed); - - if !paths.is_empty() { - eprintln!("ERROR: no `{}` rules matched {:?}", builder.kind.as_str(), paths); + if !unmatched_paths.is_empty() { + eprintln!("ERROR: no `{}` rules matched {unmatched_paths:?}", builder.kind.as_str()); eprintln!( "HELP: run `x.py {} --help --verbose` to show a list of available paths", builder.kind.as_str() @@ -192,4 +151,23 @@ pub(crate) fn match_paths_to_steps_and_run( ); crate::exit!(1); } + + fn dedup_vec(vec: &mut Vec) { + let mut seen = HashSet::::with_capacity(vec.len()); + vec.retain(|&x| seen.insert(x)); + } + + // Deduplicate the queue of steps to run, and the list of anchors to run for each step. + dedup_vec(&mut step_queue); + for anchors in step_anchors.values_mut() { + dedup_vec(anchors); + } + + // Run the steps that were selected, in (roughly) command-line order. + // For each step, pass all of its matched anchors, regardless of position. + for &step_ix in &step_queue { + let step = &steps[step_ix]; + let anchors = step_anchors[&step_ix].iter().map(|p| PathSet::clone(p)).collect::>(); + step.desc.maybe_run(builder, anchors); + } } diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_library_core_and_alloc_and_stdarch.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_library_core_and_alloc_and_stdarch.snap index 2664ab7a404cc..ad349efdde6e7 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_library_core_and_alloc_and_stdarch.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_library_core_and_alloc_and_stdarch.snap @@ -4,8 +4,8 @@ expression: test library/core library/alloc library/stdarch --- [Test] test::Crate targets: [aarch64-unknown-linux-gnu] - - Set({library/alloc}) - Set({library/core}) + - Set({library/alloc}) [Test] test::StdarchVerify targets: [x86_64-unknown-linux-gnu] - Set({library/stdarch/crates/stdarch-verify}) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_coverage_trivial_rs.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_coverage_trivial_rs.snap new file mode 100644 index 0000000000000..8b78c288a550a --- /dev/null +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_coverage_trivial_rs.snap @@ -0,0 +1,7 @@ +--- +source: src/bootstrap/src/core/builder/cli_paths/tests.rs +expression: test tests/coverage/trivial.rs +--- +[Test] test::Coverage + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/coverage) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_coverage_trivial_rs_and_attr_impl_rs.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_coverage_trivial_rs_and_attr_impl_rs.snap new file mode 100644 index 0000000000000..4e11821b7cbcb --- /dev/null +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_coverage_trivial_rs_and_attr_impl_rs.snap @@ -0,0 +1,7 @@ +--- +source: src/bootstrap/src/core/builder/cli_paths/tests.rs +expression: test tests/coverage/trivial.rs tests/coverage/attr/impl.rs +--- +[Test] test::Coverage + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/coverage) diff --git a/src/bootstrap/src/core/builder/cli_paths/tests.rs b/src/bootstrap/src/core/builder/cli_paths/tests.rs index b4dad0013b2de..e18a274c75f49 100644 --- a/src/bootstrap/src/core/builder/cli_paths/tests.rs +++ b/src/bootstrap/src/core/builder/cli_paths/tests.rs @@ -197,6 +197,11 @@ declare_tests!( (x_test_src_tools_miri, "test src/tools/miri"), (x_test_src_tools_miri_and_cargo_miri, "test src/tools/miri src/tools/miri/cargo-miri"), (x_test_tests, "test tests"), + (x_test_tests_coverage_trivial_rs, "test tests/coverage/trivial.rs"), + ( + x_test_tests_coverage_trivial_rs_and_attr_impl_rs, + "test tests/coverage/trivial.rs tests/coverage/attr/impl.rs" + ), (x_test_tests_skip_coverage, "test tests --skip=coverage"), (x_test_tests_ui, "test tests/ui"), (x_test_tests_ui_dot_prefix, "test ./tests/ui"), diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index 8d0461f0787ed..c21322740fa61 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -20,7 +20,6 @@ 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::builder::step_stack::StepRecord; pub use crate::core::builder::step_stack::StepStack; use crate::core::config::flags::Subcommand; @@ -361,7 +360,7 @@ struct CommandLineStepDescription { kind: Kind, } -#[derive(Clone, PartialOrd, Ord, PartialEq, Eq)] +#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Hash)] pub struct TaskPath { pub path: PathBuf, } @@ -373,7 +372,7 @@ impl Debug for TaskPath { } /// Collection of paths used to match a task rule. -#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)] +#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)] pub enum PathSet { /// A collection of individual paths or aliases. /// @@ -415,34 +414,6 @@ impl PathSet { p.path.ends_with(needle) || p.path.starts_with(needle) } - /// Returns true if self is matched by any of the command-line selectors, - /// and mutates those selectors to flag them as will-be-executed. - fn match_and_flag_selectors(&self, selectors: &mut [CLIStepPath]) -> bool { - let mut check_and_flag = |p| { - let mut result = false; - for selector in selectors.iter_mut() { - let matched = Self::check(p, &selector.path); - if matched { - selector.will_be_executed = true; - result = true; - } - } - result - }; - - match self { - PathSet::Set(set) => { - // Flag all matching selectors, not just the first match. - let mut matched = false; - for p in set { - matched |= check_and_flag(p); - } - matched - } - PathSet::Suite(suite) => check_and_flag(suite), - } - } - /// A convenience wrapper for Steps which know they have no aliases and all their sets contain only a single path. /// /// This can be used with [`ShouldRun::crate_or_deps`], [`ShouldRun::path`], or [`ShouldRun::alias`]. @@ -633,38 +604,11 @@ impl<'a> ShouldRun<'a> { self } - /// Handles individual files (not directories) within a test suite. - fn is_suite_path(&self, requested_path: &Path) -> Option<&PathSet> { - self.paths.iter().find(|pathset| match pathset { - PathSet::Suite(suite) => requested_path.starts_with(&suite.path), - PathSet::Set(_) => false, - }) - } - pub fn suite_path(mut self, suite: &str) -> Self { self.paths.insert(PathSet::Suite(TaskPath { path: suite.into() })); self } - /// Given a set of requested paths, return the subset which match the Step for this `ShouldRun`, - /// removing the matches from `paths`. - /// - /// NOTE: this returns multiple PathSets to allow for the possibility of multiple units of work - /// within the same step. For example, `test::Crate` allows testing multiple crates in the same - /// cargo invocation, which are put into separate sets because they aren't aliases. - /// - /// The reason we return PathSet instead of PathBuf is to allow for aliases that mean the same thing - /// (for now, just `all_krates` and `paths`, but we may want to add an `aliases` function in the future?) - fn pathsets_for_paths_flagging_matches(&self, paths: &mut [CLIStepPath]) -> Vec { - let mut sets = vec![]; - for pathset in &self.paths { - if pathset.match_and_flag_selectors(paths) { - sets.push(pathset.clone()); - } - } - sets - } - /// When the corresponding step is run "by default" (without explicit command-line paths), /// act as though the user had explicitly specified these paths. fn default_pathsets(&self) -> Vec { diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 4ae1ee53537f2..499900226d026 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -2119,11 +2119,11 @@ mod snapshot { [test] compiletest-run-make 2 [build] rustc 1 -> rustc 2 [build] rustdoc 1 + [build] rustc 2 -> std 2 + [build] rustdoc 2 [build] rustc 0 -> RustdocGUITest 1 [test] rustdoc-gui 2 [test] compiletest-incremental 2 - [build] rustc 2 -> std 2 - [build] rustdoc 2 "); } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index c084c12ae4ab8..e4adb264ff3e2 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -15,7 +15,11 @@ //! //! More documentation can be found in each respective module below, and you can //! also check out the `src/bootstrap/README.md` file for more information. + +// tidy-alphabetical-start #![allow(clippy::assertions_on_constants, reason = "false positive for `assert!(cfg!(..))`")] +#![allow(clippy::map_clone, reason = "false positive for `|x: &&Foo| Foo::clone(x)`")] +// tidy-alphabetical-end use std::cell::Cell; use std::collections::{BTreeSet, HashMap, HashSet};