diff options
| -rw-r--r-- | src/bootstrap/builder.rs | 170 | ||||
| -rw-r--r-- | src/bootstrap/builder/tests.rs | 9 | ||||
| -rw-r--r-- | src/bootstrap/cache.rs | 4 | ||||
| -rw-r--r-- | src/bootstrap/lib.rs | 1 | ||||
| -rw-r--r-- | src/bootstrap/test.rs | 35 |
5 files changed, 153 insertions, 66 deletions
diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 38d4f15d3c8..c3ee30b65fe 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -91,7 +91,7 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash { pub struct RunConfig<'a> { pub builder: &'a Builder<'a>, pub target: TargetSelection, - pub path: PathBuf, + pub paths: Vec<PathSet>, } impl RunConfig<'_> { @@ -150,11 +150,16 @@ impl Debug for TaskPath { /// Collection of paths used to match a task rule. #[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)] pub enum PathSet { - /// A collection of individual paths. + /// A collection of individual paths or aliases. /// /// These are generally matched as a path suffix. For example, a - /// command-line value of `libstd` will match if `src/libstd` is in the + /// command-line value of `std` will match if `library/std` is in the /// set. + /// + /// NOTE: the paths within a set should always be aliases of one another. + /// For example, `src/librustdoc` and `src/tools/rustdoc` should be in the same set, + /// but `library/core` and `library/std` generally should not, unless there's no way (for that Step) + /// to build them separately. Set(BTreeSet<TaskPath>), /// A "suite" of paths. /// @@ -177,26 +182,65 @@ impl PathSet { } fn has(&self, needle: &Path, module: Option<Kind>) -> bool { - let check = |p: &TaskPath| { - if let (Some(p_kind), Some(kind)) = (&p.kind, module) { - p.path.ends_with(needle) && *p_kind == kind - } else { - p.path.ends_with(needle) + match self { + PathSet::Set(set) => set.iter().any(|p| Self::check(p, needle, module)), + PathSet::Suite(suite) => Self::check(suite, needle, module), + } + } + + // internal use only + fn check(p: &TaskPath, needle: &Path, module: Option<Kind>) -> bool { + if let (Some(p_kind), Some(kind)) = (&p.kind, module) { + p.path.ends_with(needle) && *p_kind == kind + } else { + p.path.ends_with(needle) + } + } + + /// Return all `TaskPath`s in `Self` that contain any of the `needles`, removing the + /// matched needles. + /// + /// This is used for `StepDescription::krate`, which passes all matching crates at once to + /// `Step::make_run`, rather than calling it many times with a single crate. + /// See `tests.rs` for examples. + fn intersection_removing_matches( + &self, + needles: &mut Vec<&Path>, + module: Option<Kind>, + ) -> PathSet { + let mut check = |p| { + for (i, n) in needles.iter().enumerate() { + let matched = Self::check(p, n, module); + if matched { + needles.remove(i); + return true; + } } + false }; - match self { - PathSet::Set(set) => set.iter().any(check), - PathSet::Suite(suite) => check(suite), + PathSet::Set(set) => PathSet::Set(set.iter().filter(|&p| check(p)).cloned().collect()), + PathSet::Suite(suite) => { + if check(suite) { + self.clone() + } else { + PathSet::empty() + } + } } } - fn path(&self, builder: &Builder<'_>) -> PathBuf { + /// 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::krate`], [`ShouldRun::path`], or [`ShouldRun::alias`]. + #[track_caller] + pub fn assert_single_path(&self) -> &TaskPath { match self { PathSet::Set(set) => { - set.iter().next().map(|p| &p.path).unwrap_or(&builder.build.src).clone() + assert_eq!(set.len(), 1, "called assert_single_path on multiple paths"); + set.iter().next().unwrap() } - PathSet::Suite(path) => path.path.clone(), + PathSet::Suite(_) => unreachable!("called assert_single_path on a Suite path"), } } } @@ -213,8 +257,8 @@ impl StepDescription { } } - fn maybe_run(&self, builder: &Builder<'_>, pathset: &PathSet) { - if self.is_excluded(builder, pathset) { + fn maybe_run(&self, builder: &Builder<'_>, pathsets: Vec<PathSet>) { + if pathsets.iter().any(|set| self.is_excluded(builder, set)) { return; } @@ -222,7 +266,7 @@ impl StepDescription { let targets = if self.only_hosts { &builder.hosts } else { &builder.targets }; for target in targets { - let run = RunConfig { builder, path: pathset.path(builder), target: *target }; + let run = RunConfig { builder, paths: pathsets.clone(), target: *target }; (self.make_run)(run); } } @@ -261,46 +305,51 @@ impl StepDescription { for (desc, should_run) in v.iter().zip(&should_runs) { if desc.default && should_run.is_really_default() { for pathset in &should_run.paths { - desc.maybe_run(builder, pathset); + desc.maybe_run(builder, vec![pathset.clone()]); } } } } - for path in paths { - // strip CurDir prefix if present - let path = match path.strip_prefix(".") { - Ok(p) => p, - Err(_) => path, - }; + // strip CurDir prefix if present + let mut paths: Vec<_> = + paths.into_iter().map(|p| p.strip_prefix(".").unwrap_or(p)).collect(); - let mut attempted_run = false; + // 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) { - attempted_run = true; - desc.maybe_run(builder, suite); - } else if let Some(pathset) = should_run.pathset_for_path(path, desc.kind) { - attempted_run = true; - desc.maybe_run(builder, pathset); + if let Some(suite) = should_run.is_suite_path(&path) { + desc.maybe_run(builder, vec![suite.clone()]); + return false; } } + true + }); - if !attempted_run { - eprintln!( - "error: no `{}` rules matched '{}'", - builder.kind.as_str(), - path.display() - ); - 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!`" - ); - std::process::exit(1); + if paths.is_empty() { + return; + } + + // Handle all PathSets. + for (desc, should_run) in v.iter().zip(&should_runs) { + let pathsets = should_run.pathset_for_paths_removing_matches(&mut paths, desc.kind); + if !pathsets.is_empty() { + desc.maybe_run(builder, pathsets); } } + + 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!`" + ); + std::process::exit(1); + } } } @@ -370,7 +419,7 @@ impl<'a> ShouldRun<'a> { /// Indicates it should run if the command-line selects the given crate or /// any of its (local) dependencies. /// - /// `make_run` will be called separately for each matching command-line path. + /// `make_run` will be called a single time with all matching command-line paths. pub fn krate(mut self, name: &str) -> Self { for krate in self.builder.in_tree_crates(name, None) { let path = krate.local_path(self.builder); @@ -417,9 +466,10 @@ impl<'a> ShouldRun<'a> { self } - pub fn is_suite_path(&self, path: &Path) -> Option<&PathSet> { + /// 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(p) => path.starts_with(&p.path), + PathSet::Suite(suite) => requested_path.starts_with(&suite.path), PathSet::Set(_) => false, }) } @@ -435,8 +485,28 @@ impl<'a> ShouldRun<'a> { self } - fn pathset_for_path(&self, path: &Path, kind: Kind) -> Option<&PathSet> { - self.paths.iter().find(|pathset| pathset.has(path, Some(kind))) + /// 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 pathset_for_paths_removing_matches( + &self, + paths: &mut Vec<&Path>, + kind: Kind, + ) -> Vec<PathSet> { + let mut sets = vec![]; + for pathset in &self.paths { + let subset = pathset.intersection_removing_matches(paths, Some(kind)); + if subset != PathSet::empty() { + sets.push(subset); + } + } + sets } } diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index 4ab502e9052..f030917d7a4 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -47,6 +47,13 @@ fn run_build(paths: &[PathBuf], config: Config) -> Cache { } #[test] +fn test_intersection() { + let set = PathSet::Set(["library/core", "library/alloc", "library/std"].into_iter().map(TaskPath::parse).collect()); + let subset = set.intersection(&[Path::new("library/core"), Path::new("library/alloc"), Path::new("library/stdarch")], None); + assert_eq!(subset, PathSet::Set(["library/core", "library/alloc"].into_iter().map(TaskPath::parse).collect())); +} + +#[test] fn test_exclude() { let mut config = configure("test", &["A"], &["A"]); config.exclude = vec![TaskPath::parse("src/tools/tidy")]; @@ -539,7 +546,7 @@ mod dist { target: host, mode: Mode::Std, test_kind: test::TestKind::Test, - krate: INTERNER.intern_str("std"), + crates: vec![INTERNER.intern_str("std")], },] ); } diff --git a/src/bootstrap/cache.rs b/src/bootstrap/cache.rs index fac5d8db511..97f0bfdc484 100644 --- a/src/bootstrap/cache.rs +++ b/src/bootstrap/cache.rs @@ -270,7 +270,7 @@ impl Cache { #[cfg(test)] impl Cache { - pub fn all<S: Ord + Copy + Step>(&mut self) -> Vec<(S, S::Output)> { + pub fn all<S: Ord + Clone + Step>(&mut self) -> Vec<(S, S::Output)> { let cache = self.0.get_mut(); let type_id = TypeId::of::<S>(); let mut v = cache @@ -278,7 +278,7 @@ impl Cache { .map(|b| b.downcast::<HashMap<S, S::Output>>().expect("correct type")) .map(|m| m.into_iter().collect::<Vec<_>>()) .unwrap_or_default(); - v.sort_by_key(|&(a, _)| a); + v.sort_by_key(|(s, _)| s.clone()); v } diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index b4333566f07..2ae49b59fd9 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -171,6 +171,7 @@ mod job { pub unsafe fn setup(_build: &mut crate::Build) {} } +pub use crate::builder::PathSet; use crate::cache::{Interned, INTERNER}; pub use crate::config::Config; pub use crate::flags::Subcommand; diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index fdce078bbed..9958306b576 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1856,12 +1856,12 @@ impl Step for RustcGuide { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct CrateLibrustc { compiler: Compiler, target: TargetSelection, test_kind: TestKind, - krate: Interned<String>, + crates: Vec<Interned<String>>, } impl Step for CrateLibrustc { @@ -1877,10 +1877,14 @@ impl Step for CrateLibrustc { let builder = run.builder; let host = run.build_triple(); let compiler = builder.compiler_for(builder.top_stage, host, host); - let krate = builder.crate_paths[&run.path]; + let crates = run + .paths + .iter() + .map(|p| builder.crate_paths[&p.assert_single_path().path].clone()) + .collect(); let test_kind = builder.kind.into(); - builder.ensure(CrateLibrustc { compiler, target: run.target, test_kind, krate }); + builder.ensure(CrateLibrustc { compiler, target: run.target, test_kind, crates }); } fn run(self, builder: &Builder<'_>) { @@ -1889,18 +1893,18 @@ impl Step for CrateLibrustc { target: self.target, mode: Mode::Rustc, test_kind: self.test_kind, - krate: self.krate, + crates: self.crates, }); } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Crate { pub compiler: Compiler, pub target: TargetSelection, pub mode: Mode, pub test_kind: TestKind, - pub krate: Interned<String>, + pub crates: Vec<Interned<String>>, } impl Step for Crate { @@ -1916,9 +1920,13 @@ impl Step for Crate { let host = run.build_triple(); let compiler = builder.compiler_for(builder.top_stage, host, host); let test_kind = builder.kind.into(); - let krate = builder.crate_paths[&run.path]; + let crates = run + .paths + .iter() + .map(|p| builder.crate_paths[&p.assert_single_path().path].clone()) + .collect(); - builder.ensure(Crate { compiler, target: run.target, mode: Mode::Std, test_kind, krate }); + builder.ensure(Crate { compiler, target: run.target, mode: Mode::Std, test_kind, crates }); } /// Runs all unit tests plus documentation tests for a given crate defined @@ -1934,7 +1942,6 @@ impl Step for Crate { let target = self.target; let mode = self.mode; let test_kind = self.test_kind; - let krate = self.krate; builder.ensure(compile::Std { compiler, target }); builder.ensure(RemoteCopyLibs { compiler, target }); @@ -1975,7 +1982,9 @@ impl Step for Crate { DocTests::Yes => {} } - cargo.arg("-p").arg(krate); + for krate in &self.crates { + cargo.arg("-p").arg(krate); + } // The tests are going to run with the *target* libraries, so we need to // ensure that those libraries show up in the LD_LIBRARY_PATH equivalent. @@ -2011,8 +2020,8 @@ impl Step for Crate { } builder.info(&format!( - "{} {} stage{} ({} -> {})", - test_kind, krate, compiler.stage, &compiler.host, target + "{} {:?} stage{} ({} -> {})", + test_kind, self.crates, compiler.stage, &compiler.host, target )); let _time = util::timeit(&builder); try_run(builder, &mut cargo.into()); |