allow excluding paths only from a single module

x.py has support for excluding some steps from the 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 execution would fail, as that would not only exclude running the
tests for the standard library, it would also exclude generating its
documentation (breaking linkchecker).

This commit adds support for an optional module annotation in --exclude
paths, allowing the user to choose which module to exclude from:

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

This maintains backward compatibility, but also allows for more ganular
exclusion. More examples on how this works:

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

Note that the new behavior only works in the `--exclude` flag, and not
in other x.py arguments or flags yet.

Author: pietroalbini

src/bootstrap/builder.rs

@@ -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,17 +105,43 @@ struct StepDescription {
     should_run: fn(ShouldRun<'_>) -> ShouldRun<'_>,
     make_run: fn(RunConfig<'_>),
     name: &'static str,
+    kind: Kind,
 }
 
-#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)]
+#[derive(Clone, PartialOrd, Ord, PartialEq, Eq)]
 pub struct TaskPath {
     pub path: PathBuf,
-    pub module: Option<String>,
+    pub kind: Option<Kind>,
 }
 
 impl TaskPath {
     pub fn parse(path: impl Into<PathBuf>) -> TaskPath {
-        TaskPath { path: path.into(), module: None }
+        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())
     }
 }
 
@@ -142,16 +168,24 @@ 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(TaskPath::parse(path));
+        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.path.ends_with(needle)),
-            PathSet::Suite(suite) => suite.path.ends_with(needle),
+            PathSet::Set(set) => set.iter().any(check),
+            PathSet::Suite(suite) => check(suite),
         }
     }
 
@@ -166,13 +200,14 @@ impl PathSet {
 }
 
 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,
         }
     }
 
@@ -191,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;
         }
@@ -206,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) {
@@ -240,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);
                 }
@@ -260,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>,
 
@@ -269,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
         }
@@ -307,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(TaskPath::parse(path));
+            set.insert(TaskPath { path, kind: Some(self.kind) });
         }
         self.paths.insert(PathSet::Set(set));
         self
@@ -320,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
     }
@@ -332,7 +372,12 @@ 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(|p| TaskPath::parse(p)).collect()));
+        self.paths.insert(PathSet::Set(
+            paths
+                .iter()
+                .map(|p| TaskPath { path: p.into(), kind: Some(self.kind.into()) })
+                .collect(),
+        ));
         self
     }
 
@@ -344,7 +389,8 @@ impl<'a> ShouldRun<'a> {
     }
 
     pub fn suite_path(mut self, suite: &str) -> Self {
-        self.paths.insert(PathSet::Suite(TaskPath::parse(suite)));
+        self.paths
+            .insert(PathSet::Suite(TaskPath { path: suite.into(), kind: Some(self.kind.into()) }));
         self
     }
 
@@ -354,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,
@@ -373,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 {
@@ -554,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");
@@ -1640,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 {
@@ -1656,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(TaskPath::parse(path)))
+            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;
             }

src/bootstrap/builder/tests.rs

@@ -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(),

src/bootstrap/config.rs

@@ -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;

src/bootstrap/dist.rs

@@ -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);
                 }