user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #91965 - ferrocene:pa-more-granular-exclude, r=Mark-Simulacrum

Browse source 
Add more granular `--exclude` in `x.py`

x.py has support for excluding some steps from the current invocation, but unfortunately that's not granular enough: some steps have the same name in different modules, and that prevents excluding only *some* of them.

As a practical example, let's say you need to run everything in `./x.py test` except for the standard library tests, as those tests require IPv6 and need to be executed on a separate machine. Before this commit, if you were to just run this:

    ./x.py test --exclude library/std

...the invocation would eventually fail, as that would not only exclude running the tests for the standard library (`library/std` in the `test` module), it would also exclude generating its documentation (`library/std` in the `doc` module), breaking linkchecker.

This commit adds support to the `--exclude` flag for prefixing paths with the name of the module their step is defined in, allowing the user to choose which module to exclude from:

    ./x.py test --exclude test::library/std

This maintains backward compatibility with existing invocations, while allowing more ganular exclusion. Examples of the behavior:

| `--exclude`         | Docs    | Tests   |
| ------------------- | ------- | ------- |
| `library/std`       | Skipped | Skipped |
| `doc::library/std`  | Skipped | Run     |
| `test::library/std` | Run     | Skipped |

Note that this PR only changes the `--exclude` flag, and not in other `x.py` arguments or flags yet.

In the implementation I tried to limit the impact this would have with rustbuild as a whole as much as possible. The module name is extracted from the step by parsing the result of `std::any::type_name()`: unfortunately that output can change at any point in time, but IMO it's better than having to annotate all the existing and future `Step` implementations with the module name. I added a test to ensure the parsing works as expected, so hopefully if anyone makes changes to the output of `std::any::type_name()` they'll also notice they have to update rustbuild.

r? `@Mark-Simulacrum`
This commit is contained in:
Matthias Krüger 2022-01-21 22:03:11 +01:00 • committed by GitHub
commit fc694064e8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 146 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

170
src/bootstrap/builder.rs
View file

@ -7,7 +7,7 @@ use std::fmt::Debug;
use std::fs;
use std::hash::Hash;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};
use std::process::Command;
use std::time::{Duration, Instant};
@ -105,6 +105,44 @@ struct StepDescription {
should_run: fn(ShouldRun<'_>) -> ShouldRun<'_>,
make_run: fn(RunConfig<'_>),
name: &'static str,
kind: Kind,
}
#[derive(Clone, PartialOrd, Ord, PartialEq, Eq)]
pub struct TaskPath {
pub path: PathBuf,
pub kind: Option<Kind>,
}
impl TaskPath {
pub fn parse(path: impl Into<PathBuf>) -> TaskPath {
let mut kind = None;
let mut path = path.into();
let mut components = path.components();
if let Some(Component::Normal(os_str)) = components.next() {
if let Some(str) = os_str.to_str() {
if let Some((found_kind, found_prefix)) = str.split_once("::") {
if found_kind.is_empty() {
panic!("empty kind in task path {}", path.display());
}
kind = Some(Kind::parse(found_kind));
path = Path::new(found_prefix).join(components.as_path());
}
}
}
TaskPath { path, kind }
}
}
impl Debug for TaskPath {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(kind) = &self.kind {
write!(f, "{}::", kind.as_str())?;
}
write!(f, "{}", self.path.display())
}
}
/// Collection of paths used to match a task rule.
@ -115,14 +153,14 @@ pub enum PathSet {
/// These are generally matched as a path suffix. For example, a
/// command-line value of `libstd` will match if `src/libstd` is in the
/// set.
Set(BTreeSet<PathBuf>),
Set(BTreeSet<TaskPath>),
/// A "suite" of paths.
///
/// These can match as a path suffix (like `Set`), or as a prefix. For
/// example, a command-line value of `src/test/ui/abi/variadic-ffi.rs`
/// will match `src/test/ui`. A command-line value of `ui` would also
/// match `src/test/ui`.
Suite(PathBuf),
Suite(TaskPath),
}
impl PathSet {
@ -130,35 +168,46 @@ impl PathSet {
PathSet::Set(BTreeSet::new())
}
fn one<P: Into<PathBuf>>(path: P) -> PathSet {
fn one<P: Into<PathBuf>>(path: P, kind: Kind) -> PathSet {
let mut set = BTreeSet::new();
set.insert(path.into());
set.insert(TaskPath { path: path.into(), kind: Some(kind.into()) });
PathSet::Set(set)
}
fn has(&self, needle: &Path) -> bool {
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| p.ends_with(needle)),
PathSet::Suite(suite) => suite.ends_with(needle),
PathSet::Set(set) => set.iter().any(check),
PathSet::Suite(suite) => check(suite),
}
}
fn path(&self, builder: &Builder<'_>) -> PathBuf {
match self {
PathSet::Set(set) => set.iter().next().unwrap_or(&builder.build.src).to_path_buf(),
PathSet::Suite(path) => PathBuf::from(path),
PathSet::Set(set) => {
set.iter().next().map(|p| &p.path).unwrap_or(&builder.build.src).clone()
}
PathSet::Suite(path) => path.path.clone(),
}
}
}
impl StepDescription {
fn from<S: Step>() -> StepDescription {
fn from<S: Step>(kind: Kind) -> StepDescription {
StepDescription {
default: S::DEFAULT,
only_hosts: S::ONLY_HOSTS,
should_run: S::should_run,
make_run: S::make_run,
name: std::any::type_name::<S>(),
kind,
}
}
@ -177,7 +226,7 @@ impl StepDescription {
}
fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet) -> bool {
if builder.config.exclude.iter().any(|e| pathset.has(e)) {
if builder.config.exclude.iter().any(|e| pathset.has(&e.path, e.kind)) {
eprintln!("Skipping {:?} because it is excluded", pathset);
return true;
}
@ -192,8 +241,10 @@ impl StepDescription {
}
fn run(v: &[StepDescription], builder: &Builder<'_>, paths: &[PathBuf]) {
let should_runs =
v.iter().map(|desc| (desc.should_run)(ShouldRun::new(builder))).collect::<Vec<_>>();
let should_runs = v
.iter()
.map(|desc| (desc.should_run)(ShouldRun::new(builder, desc.kind)))
.collect::<Vec<_>>();
// sanity checks on rules
for (desc, should_run) in v.iter().zip(&should_runs) {
@ -226,7 +277,7 @@ impl StepDescription {
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) {
} else if let Some(pathset) = should_run.pathset_for_path(path, desc.kind) {
attempted_run = true;
desc.maybe_run(builder, pathset);
}
@ -246,6 +297,8 @@ enum ReallyDefault<'a> {
pub struct ShouldRun<'a> {
pub builder: &'a Builder<'a>,
kind: Kind,
// use a BTreeSet to maintain sort order
paths: BTreeSet<PathSet>,
@ -255,9 +308,10 @@ pub struct ShouldRun<'a> {
}
impl<'a> ShouldRun<'a> {
fn new(builder: &'a Builder<'_>) -> ShouldRun<'a> {
fn new(builder: &'a Builder<'_>, kind: Kind) -> ShouldRun<'a> {
ShouldRun {
builder,
kind,
paths: BTreeSet::new(),
is_really_default: ReallyDefault::Bool(true), // by default no additional conditions
}
@ -293,7 +347,7 @@ impl<'a> ShouldRun<'a> {
let mut set = BTreeSet::new();
for krate in self.builder.in_tree_crates(name, None) {
let path = krate.local_path(self.builder);
set.insert(path);
set.insert(TaskPath { path, kind: Some(self.kind) });
}
self.paths.insert(PathSet::Set(set));
self
@ -306,7 +360,7 @@ impl<'a> ShouldRun<'a> {
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);
self.paths.insert(PathSet::one(path));
self.paths.insert(PathSet::one(path, self.kind));
}
self
}
@ -318,19 +372,25 @@ impl<'a> ShouldRun<'a> {
// multiple aliases for the same job
pub fn paths(mut self, paths: &[&str]) -> Self {
self.paths.insert(PathSet::Set(paths.iter().map(PathBuf::from).collect()));
self.paths.insert(PathSet::Set(
paths
.iter()
.map(|p| TaskPath { path: p.into(), kind: Some(self.kind.into()) })
.collect(),
));
self
}
pub fn is_suite_path(&self, path: &Path) -> Option<&PathSet> {
self.paths.iter().find(|pathset| match pathset {
PathSet::Suite(p) => path.starts_with(p),
PathSet::Suite(p) => path.starts_with(&p.path),
PathSet::Set(_) => false,
})
}
pub fn suite_path(mut self, suite: &str) -> Self {
self.paths.insert(PathSet::Suite(PathBuf::from(suite)));
self.paths
.insert(PathSet::Suite(TaskPath { path: suite.into(), kind: Some(self.kind.into()) }));
self
}
@ -340,12 +400,12 @@ impl<'a> ShouldRun<'a> {
self
}
fn pathset_for_path(&self, path: &Path) -> Option<&PathSet> {
self.paths.iter().find(|pathset| pathset.has(path))
fn pathset_for_path(&self, path: &Path, kind: Kind) -> Option<&PathSet> {
self.paths.iter().find(|pathset| pathset.has(path, Some(kind)))
}
}
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
pub enum Kind {
Build,
Check,
@ -359,11 +419,44 @@ pub enum Kind {
Run,
}
impl Kind {
fn parse(string: &str) -> Kind {
match string {
"build" => Kind::Build,
"check" => Kind::Check,
"clippy" => Kind::Clippy,
"fix" => Kind::Fix,
"test" => Kind::Test,
"bench" => Kind::Bench,
"dist" => Kind::Dist,
"doc" => Kind::Doc,
"install" => Kind::Install,
"run" => Kind::Run,
other => panic!("unknown kind: {}", other),
}
}
fn as_str(&self) -> &'static str {
match self {
Kind::Build => "build",
Kind::Check => "check",
Kind::Clippy => "clippy",
Kind::Fix => "fix",
Kind::Test => "test",
Kind::Bench => "bench",
Kind::Dist => "dist",
Kind::Doc => "doc",
Kind::Install => "install",
Kind::Run => "run",
}
}
}
impl<'a> Builder<'a> {
fn get_step_descriptions(kind: Kind) -> Vec<StepDescription> {
macro_rules! describe {
($($rule:ty),+ $(,)?) => {{
vec![$(StepDescription::from::<$rule>()),+]
vec![$(StepDescription::from::<$rule>(kind)),+]
}};
}
match kind {
@ -540,8 +633,11 @@ impl<'a> Builder<'a> {
let builder = Self::new_internal(build, kind, vec![]);
let builder = &builder;
let mut should_run = ShouldRun::new(builder);
// The "build" kind here is just a placeholder, it will be replaced with something else in
// the following statement.
let mut should_run = ShouldRun::new(builder, Kind::Build);
for desc in Builder::get_step_descriptions(builder.kind) {
should_run.kind = desc.kind;
should_run = (desc.should_run)(should_run);
}
let mut help = String::from("Available paths:\n");
@ -552,11 +648,11 @@ impl<'a> Builder<'a> {
match pathset {
PathSet::Set(set) => {
for path in set {
add_path(&path);
add_path(&path.path);
}
}
PathSet::Suite(path) => {
add_path(&path.join("..."));
add_path(&path.path.join("..."));
}
}
}
@ -1626,9 +1722,10 @@ impl<'a> Builder<'a> {
pub(crate) fn ensure_if_default<T, S: Step<Output = Option<T>>>(
&'a self,
step: S,
kind: Kind,
) -> S::Output {
let desc = StepDescription::from::<S>();
let should_run = (desc.should_run)(ShouldRun::new(self));
let desc = StepDescription::from::<S>(kind);
let should_run = (desc.should_run)(ShouldRun::new(self, desc.kind));
// Avoid running steps contained in --exclude
for pathset in &should_run.paths {
@ -1642,13 +1739,16 @@ impl<'a> Builder<'a> {
}
/// Checks if any of the "should_run" paths is in the `Builder` paths.
pub(crate) fn was_invoked_explicitly<S: Step>(&'a self) -> bool {
let desc = StepDescription::from::<S>();
let should_run = (desc.should_run)(ShouldRun::new(self));
pub(crate) fn was_invoked_explicitly<S: Step>(&'a self, kind: Kind) -> bool {
let desc = StepDescription::from::<S>(kind);
let should_run = (desc.should_run)(ShouldRun::new(self, desc.kind));
for path in &self.paths {
if should_run.paths.iter().any(|s| s.has(path))
&& !desc.is_excluded(self, &PathSet::Suite(path.clone()))
if should_run.paths.iter().any(|s| s.has(path, Some(desc.kind)))
&& !desc.is_excluded(
self,
&PathSet::Suite(TaskPath { path: path.clone(), kind: Some(desc.kind.into()) }),
)
{
return true;
}

4
src/bootstrap/builder/tests.rs
View file

@ -499,7 +499,7 @@ mod dist {
let host = TargetSelection::from_user("A");
builder.run_step_descriptions(
&[StepDescription::from::<test::Crate>()],
&[StepDescription::from::<test::Crate>(Kind::Test)],
&["library/std".into()],
);
@ -520,7 +520,7 @@ mod dist {
#[test]
fn test_exclude() {
let mut config = configure(&["A"], &["A"]);
config.exclude = vec!["src/tools/tidy".into()];
config.exclude = vec![TaskPath::parse("src/tools/tidy")];
config.cmd = Subcommand::Test {
paths: Vec::new(),
test_args: Vec::new(),

5
src/bootstrap/config.rs
View file

@ -12,6 +12,7 @@ use std::fs;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use crate::builder::TaskPath;
use crate::cache::{Interned, INTERNER};
use crate::channel::GitInfo;
pub use crate::flags::Subcommand;
@ -62,7 +63,7 @@ pub struct Config {
pub sanitizers: bool,
pub profiler: bool,
pub ignore_git: bool,
pub exclude: Vec<PathBuf>,
pub exclude: Vec<TaskPath>,
pub include_default_paths: bool,
pub rustc_error_format: Option<String>,
pub json_output: bool,
@ -635,7 +636,7 @@ impl Config {
let flags = Flags::parse(&args);
let mut config = Config::default_opts();
config.exclude = flags.exclude;
config.exclude = flags.exclude.into_iter().map(|path| TaskPath::parse(path)).collect();
config.include_default_paths = flags.include_default_paths;
config.rustc_error_format = flags.rustc_error_format;
config.json_output = flags.json_output;

4
src/bootstrap/dist.rs
View file

@ -16,7 +16,7 @@ use std::process::Command;
use build_helper::{output, t};
use crate::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
use crate::cache::{Interned, INTERNER};
use crate::compile;
use crate::config::TargetSelection;
@ -1368,7 +1368,7 @@ impl Step for Extended {
let mut built_tools = HashSet::new();
macro_rules! add_component {
($name:expr => $step:expr) => {
if let Some(tarball) = builder.ensure_if_default($step) {
if let Some(tarball) = builder.ensure_if_default($step, Kind::Dist) {
tarballs.push(tarball);
built_tools.insert($name);
}

8
src/bootstrap/doc.rs
View file

@ -15,7 +15,7 @@ use std::path::{Path, PathBuf};
use crate::Mode;
use build_helper::{t, up_to_date};
use crate::builder::{Builder, Compiler, RunConfig, ShouldRun, Step};
use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step};
use crate::cache::{Interned, INTERNER};
use crate::compile;
use crate::config::{Config, TargetSelection};
@ -240,7 +240,7 @@ impl Step for TheBook {
invoke_rustdoc(builder, compiler, target, path);
}
if builder.was_invoked_explicitly::<Self>() {
if builder.was_invoked_explicitly::<Self>(Kind::Doc) {
let out = builder.doc_out(target);
let index = out.join("book").join("index.html");
open(builder, &index);
@ -400,7 +400,7 @@ impl Step for Standalone {
// We open doc/index.html as the default if invoked as `x.py doc --open`
// with no particular explicit doc requested (e.g. library/core).
if builder.paths.is_empty() || builder.was_invoked_explicitly::<Self>() {
if builder.paths.is_empty() || builder.was_invoked_explicitly::<Self>(Kind::Doc) {
let index = out.join("index.html");
open(builder, &index);
}
@ -902,7 +902,7 @@ impl Step for RustcBook {
name: INTERNER.intern_str("rustc"),
src: INTERNER.intern_path(out_base),
});
if builder.was_invoked_explicitly::<Self>() {
if builder.was_invoked_explicitly::<Self>(Kind::Doc) {
let out = builder.doc_out(self.target);
let index = out.join("rustc").join("index.html");
open(builder, &index);