Skip to content

Commit

Permalink
Rollup merge of #87282 - pietroalbini:refactor-extended, r=Mark-Simul…
Browse files Browse the repository at this point in the history
…acrum

Ensure `./x.py dist` adheres to `build.tools`

According to `config.toml.example`, the way to produce dist artifacts for both the compiler and a *subset* of tools would be to enable the extended build and manually specify the list of tools to build:

```toml
[build]
extended = true
tools = ["cargo", "rustfmt"]
```

This works as expected for `./x.py build` and `./x.py install`, but *not* for `./x.py dist`. Before this PR `./x.py dist` simply ignored the contents of `build.tools`, building just rustc/rustdoc if `build.extended = false` and all of the tools otherwise. This PR does two things:

* Changes `./x.py dist extended` to only build the tools defined in `build.tools`, if `build.tools` is not empty. The rest of the extended step was refactored to simplify the code.
* Changes how dist jobs for tools are gated: instead of `assert!(builder.config.extended)` to prevent tools from being built with `build.extended = false`, tools are simply built by default depending on `build.extended` and `build.tools`. This also enables to **explicitly** dist tools even with `build.extended = false`.

This PR is best reviewed commit-by-commit.

Fixes #86436
  • Loading branch information
JohnTitor authored Aug 2, 2021
2 parents f386ae3 + 69f712c commit 46f01ca
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 135 deletions.
44 changes: 37 additions & 7 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,8 @@ impl StepDescription {
}

fn maybe_run(&self, builder: &Builder<'_>, pathset: &PathSet) {
if builder.config.exclude.iter().any(|e| pathset.has(e)) {
eprintln!("Skipping {:?} because it is excluded", pathset);
if self.is_excluded(builder, pathset) {
return;
} else if !builder.config.exclude.is_empty() {
eprintln!(
"{:?} not skipped for {:?} -- not in {:?}",
pathset, self.name, builder.config.exclude
);
}

// Determine the targets participating in this rule.
Expand All @@ -182,6 +176,21 @@ impl StepDescription {
}
}

fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet) -> bool {
if builder.config.exclude.iter().any(|e| pathset.has(e)) {
eprintln!("Skipping {:?} because it is excluded", pathset);
return true;
}

if !builder.config.exclude.is_empty() {
eprintln!(
"{:?} not skipped for {:?} -- not in {:?}",
pathset, self.name, builder.config.exclude
);
}
false
}

fn run(v: &[StepDescription], builder: &Builder<'_>, paths: &[PathBuf]) {
let should_runs =
v.iter().map(|desc| (desc.should_run)(ShouldRun::new(builder))).collect::<Vec<_>>();
Expand Down Expand Up @@ -1579,6 +1588,27 @@ impl<'a> Builder<'a> {
self.cache.put(step, out.clone());
out
}

/// Ensure that a given step is built *only if it's supposed to be built by default*, returning
/// its output. This will cache the step, so it's safe (and good!) to call this as often as
/// needed to ensure that all dependencies are build.
pub(crate) fn ensure_if_default<T, S: Step<Output = Option<T>>>(
&'a self,
step: S,
) -> S::Output {
let desc = StepDescription::from::<S>();
let should_run = (desc.should_run)(ShouldRun::new(self));

// Avoid running steps contained in --exclude
for pathset in &should_run.paths {
if desc.is_excluded(self, pathset) {
return None;
}
}

// Only execute if it's supposed to run as default
if desc.default && should_run.is_really_default() { self.ensure(step) } else { None }
}
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 46f01ca

Please sign in to comment.