Skip to content

Commit 4abd67f

Browse files
committed
maintain the given order on step execution
Previously step execution disregarded the CLI order and this change executes the given steps in the order specified on CLI. For example, running `x $kind a b c` will execute `$kind` step for `a`, then `b`, then `c` crates in the specified order. Signed-off-by: onur-ozkan <work@onurozkan.dev>
1 parent e1f45a1 commit 4abd67f

File tree

1 file changed

+37
-31
lines changed

1 file changed

+37
-31
lines changed

src/bootstrap/src/core/builder.rs

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::any::{type_name, Any};
22
use std::cell::{Cell, RefCell};
3-
use std::collections::BTreeSet;
3+
use std::collections::{BTreeSet, HashMap};
44
use std::env;
55
use std::ffi::{OsStr, OsString};
66
use std::fmt::{Debug, Write};
@@ -265,23 +265,9 @@ impl PathSet {
265265
}
266266
}
267267

268-
/// Return all `TaskPath`s in `Self` that contain any of the `needles`, removing the
269-
/// matched needles.
270-
///
271-
/// This is used for `StepDescription::krate`, which passes all matching crates at once to
272-
/// `Step::make_run`, rather than calling it many times with a single crate.
273-
/// See `tests.rs` for examples.
274-
fn intersection_removing_matches(&self, needles: &mut Vec<PathBuf>, module: Kind) -> PathSet {
275-
let mut check = |p| {
276-
for (i, n) in needles.iter().enumerate() {
277-
let matched = Self::check(p, n, module);
278-
if matched {
279-
needles.remove(i);
280-
return true;
281-
}
282-
}
283-
false
284-
};
268+
/// Return all `TaskPath`s in `Self` that contain the `path`.
269+
fn intersection_pathset(&self, path: &Path, module: Kind) -> PathSet {
270+
let check = |p| Self::check(p, path, module);
285271
match self {
286272
PathSet::Set(set) => PathSet::Set(set.iter().filter(|&p| check(p)).cloned().collect()),
287273
PathSet::Suite(suite) => {
@@ -473,14 +459,39 @@ impl StepDescription {
473459
return;
474460
}
475461

476-
// Handle all PathSets.
477-
for (desc, should_run) in v.iter().zip(&should_runs) {
478-
let pathsets = should_run.pathset_for_paths_removing_matches(&mut paths, desc.kind);
479-
if !pathsets.is_empty() {
480-
desc.maybe_run(builder, pathsets);
462+
// Special type that helps bootstrap to:
463+
// 1. keep the given CLI order on step execution
464+
// 2. keep all matching crates so we can pass them to `Step::make_run` at once,
465+
// rather than calling it many times with a single crate.
466+
let mut steps_to_run: HashMap<PathBuf, (&StepDescription, Vec<PathSet>)> = HashMap::new();
467+
468+
// Handle all PathSets
469+
for path in &paths {
470+
for (desc, should_run) in v.iter().zip(&should_runs) {
471+
let pathsets = should_run.resolve_pathsets(path, desc.kind);
472+
if !pathsets.is_empty() {
473+
steps_to_run
474+
.entry(path.clone())
475+
.or_insert_with(|| (desc, Vec::new()))
476+
.1
477+
.extend(pathsets);
478+
}
481479
}
482480
}
483481

482+
// Start given steps and remove them from `paths`.
483+
paths.retain(|path| {
484+
if let Some((desc, pathsets)) = steps_to_run.remove(path) {
485+
desc.maybe_run(builder, pathsets);
486+
487+
false
488+
} else {
489+
true
490+
}
491+
});
492+
493+
assert!(steps_to_run.is_empty(), "Some steps did not executed.");
494+
484495
if !paths.is_empty() {
485496
eprintln!("ERROR: no `{}` rules matched {:?}", builder.kind.as_str(), paths,);
486497
eprintln!(
@@ -632,23 +643,18 @@ impl<'a> ShouldRun<'a> {
632643
self
633644
}
634645

635-
/// Given a set of requested paths, return the subset which match the Step for this `ShouldRun`,
636-
/// removing the matches from `paths`.
646+
/// Return the subset from given `path` and `kind` which match the `Step` for this `ShouldRun`.
637647
///
638648
/// NOTE: this returns multiple PathSets to allow for the possibility of multiple units of work
639649
/// within the same step. For example, `test::Crate` allows testing multiple crates in the same
640650
/// cargo invocation, which are put into separate sets because they aren't aliases.
641651
///
642652
/// The reason we return PathSet instead of PathBuf is to allow for aliases that mean the same thing
643653
/// (for now, just `all_krates` and `paths`, but we may want to add an `aliases` function in the future?)
644-
fn pathset_for_paths_removing_matches(
645-
&self,
646-
paths: &mut Vec<PathBuf>,
647-
kind: Kind,
648-
) -> Vec<PathSet> {
654+
fn resolve_pathsets(&self, path: &Path, kind: Kind) -> Vec<PathSet> {
649655
let mut sets = vec![];
650656
for pathset in &self.paths {
651-
let subset = pathset.intersection_removing_matches(paths, kind);
657+
let subset = pathset.intersection_pathset(path, kind);
652658
if subset != PathSet::empty() {
653659
sets.push(subset);
654660
}

0 commit comments

Comments
 (0)