Skip to content

Commit 45009fb

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 45009fb

File tree

2 files changed

+44
-38
lines changed

2 files changed

+44
-38
lines changed

src/bootstrap/src/core/builder.rs

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ pub fn crate_description(crates: &[impl AsRef<str>]) -> String {
191191
descr
192192
}
193193

194+
#[derive(PartialEq)]
194195
struct StepDescription {
195196
default: bool,
196197
only_hosts: bool,
@@ -265,23 +266,9 @@ impl PathSet {
265266
}
266267
}
267268

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-
};
269+
/// Return all `TaskPath`s in `Self` that contain the `path`.
270+
fn intersection_pathset(&self, path: &Path, module: Kind) -> PathSet {
271+
let check = |p| Self::check(p, path, module);
285272
match self {
286273
PathSet::Set(set) => PathSet::Set(set.iter().filter(|&p| check(p)).cloned().collect()),
287274
PathSet::Suite(suite) => {
@@ -473,12 +460,35 @@ impl StepDescription {
473460
return;
474461
}
475462

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);
463+
// Special type that helps bootstrap to:
464+
// 1. keep the given CLI order on step execution
465+
// 2. keep all matching crates so we can pass them to `Step::make_run` at once,
466+
// rather than calling it many times with a single crate.
467+
let mut steps_to_run: Vec<(&StepDescription, Vec<PathSet>)> = vec![];
468+
469+
// Start given steps and remove them from `paths`.
470+
paths.retain(|path| {
471+
for (desc, should_run) in v.iter().zip(&should_runs) {
472+
let pathsets = should_run.resolve_pathsets(path, desc.kind);
473+
if !pathsets.is_empty() {
474+
if let Some((_, paths)) =
475+
steps_to_run.iter_mut().find(|(step_desc, _)| *step_desc == desc)
476+
{
477+
paths.extend(pathsets);
478+
} else {
479+
steps_to_run.push((desc, pathsets));
480+
}
481+
482+
return false;
483+
}
481484
}
485+
486+
true
487+
});
488+
489+
// Execute steps
490+
for (desc, pathsets) in steps_to_run {
491+
desc.maybe_run(builder, pathsets);
482492
}
483493

484494
if !paths.is_empty() {
@@ -632,23 +642,18 @@ impl<'a> ShouldRun<'a> {
632642
self
633643
}
634644

635-
/// Given a set of requested paths, return the subset which match the Step for this `ShouldRun`,
636-
/// removing the matches from `paths`.
645+
/// Return the subset from given `path` and `kind` which match the `Step` for this `ShouldRun`.
637646
///
638647
/// NOTE: this returns multiple PathSets to allow for the possibility of multiple units of work
639648
/// within the same step. For example, `test::Crate` allows testing multiple crates in the same
640649
/// cargo invocation, which are put into separate sets because they aren't aliases.
641650
///
642651
/// The reason we return PathSet instead of PathBuf is to allow for aliases that mean the same thing
643652
/// (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> {
653+
fn resolve_pathsets(&self, path: &Path, kind: Kind) -> Vec<PathSet> {
649654
let mut sets = vec![];
650655
for pathset in &self.paths {
651-
let subset = pathset.intersection_removing_matches(paths, kind);
656+
let subset = pathset.intersection_pathset(path, kind);
652657
if subset != PathSet::empty() {
653658
sets.push(subset);
654659
}

src/bootstrap/src/core/builder/tests.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,15 @@ fn test_intersection() {
122122
PathSet::Set(paths.into_iter().map(|p| TaskPath { path: p.into(), kind: None }).collect())
123123
};
124124
let library_set = set(&["library/core", "library/alloc", "library/std"]);
125-
let mut command_paths = vec![
126-
PathBuf::from("library/core"),
127-
PathBuf::from("library/alloc"),
128-
PathBuf::from("library/stdarch"),
129-
];
130-
let subset = library_set.intersection_removing_matches(&mut command_paths, Kind::Build);
131-
assert_eq!(subset, set(&["library/core", "library/alloc"]),);
132-
assert_eq!(command_paths, vec![PathBuf::from("library/stdarch")]);
125+
126+
let subset = library_set.intersection_pathset(&PathBuf::from("alloc"), Kind::Build);
127+
assert_eq!(subset, set(&["library/alloc"]));
128+
129+
let subset = library_set.intersection_pathset(&PathBuf::from("library/std"), Kind::Build);
130+
assert_eq!(subset, set(&["library/std"]));
131+
132+
let subset = library_set.intersection_pathset(&PathBuf::from("invalid"), Kind::Build);
133+
assert_eq!(subset, set(&[]));
133134
}
134135

135136
#[test]

0 commit comments

Comments
 (0)