Auto merge of #48105 - Mark-Simulacrum:exclude-paths, r=alexcrichton

Implement excluding a build-step via --exclude

First step to fixing #47911. This doesn't change any CI configuration, but implements what I believe necessary to make that feasible in rustbuild.

In theory this should be sufficient to allow someone to open a PR against .travis.yml and appveyor.yml which splits the Windows 32-bit tests and maybe the OS X tests into multiple builders (depending on what our cost-concerns are) to reduce runtimes.

r? @alexcrichton
cc @kennytm

Author: bors

src/bootstrap/builder.rs

@@ -95,7 +95,7 @@ pub struct RunConfig<'a> {
     pub builder: &'a Builder<'a>,
     pub host: Interned<String>,
     pub target: Interned<String>,
-    pub path: Option<&'a Path>,
+    pub path: PathBuf,
 }
 
 struct StepDescription {
@@ -105,6 +105,32 @@ struct StepDescription {
     only_build: bool,
     should_run: fn(ShouldRun) -> ShouldRun,
     make_run: fn(RunConfig),
+    name: &'static str,
+}
+
+#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)]
+struct PathSet {
+    set: BTreeSet<PathBuf>,
+}
+
+impl PathSet {
+    fn empty() -> PathSet {
+        PathSet { set: BTreeSet::new() }
+    }
+
+    fn one<P: Into<PathBuf>>(path: P) -> PathSet {
+        let mut set = BTreeSet::new();
+        set.insert(path.into());
+        PathSet { set }
+    }
+
+    fn has(&self, needle: &Path) -> bool {
+        self.set.iter().any(|p| p.ends_with(needle))
+    }
+
+    fn path(&self, builder: &Builder) -> PathBuf {
+        self.set.iter().next().unwrap_or(&builder.build.src).to_path_buf()
+    }
 }
 
 impl StepDescription {
@@ -116,10 +142,18 @@ impl StepDescription {
             only_build: S::ONLY_BUILD,
             should_run: S::should_run,
             make_run: S::make_run,
+            name: unsafe { ::std::intrinsics::type_name::<S>() },
         }
     }
 
-    fn maybe_run(&self, builder: &Builder, path: Option<&Path>) {
+    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);
+            return;
+        } else if !builder.config.exclude.is_empty() {
+            eprintln!("{:?} not skipped for {:?} -- not in {:?}", pathset,
+                self.name, builder.config.exclude);
+        }
         let build = builder.build;
         let hosts = if self.only_build_targets || self.only_build {
             build.build_triple()
@@ -144,7 +178,7 @@ impl StepDescription {
             for target in targets {
                 let run = RunConfig {
                     builder,
-                    path,
+                    path: pathset.path(builder),
                     host: *host,
                     target: *target,
                 };
@@ -157,19 +191,28 @@ impl StepDescription {
         let should_runs = v.iter().map(|desc| {
             (desc.should_run)(ShouldRun::new(builder))
         }).collect::<Vec<_>>();
+
+        // sanity checks on rules
+        for (desc, should_run) in v.iter().zip(&should_runs) {
+            assert!(!should_run.paths.is_empty(),
+                "{:?} should have at least one pathset", desc.name);
+        }
+
         if paths.is_empty() {
             for (desc, should_run) in v.iter().zip(should_runs) {
                 if desc.default && should_run.is_really_default {
-                    desc.maybe_run(builder, None);
+                    for pathset in &should_run.paths {
+                        desc.maybe_run(builder, pathset);
+                    }
                 }
             }
         } else {
             for path in paths {
                 let mut attempted_run = false;
                 for (desc, should_run) in v.iter().zip(&should_runs) {
-                    if should_run.run(path) {
+                    if let Some(pathset) = should_run.pathset_for_path(path) {
                         attempted_run = true;
-                        desc.maybe_run(builder, Some(path));
+                        desc.maybe_run(builder, pathset);
                     }
                 }
 
@@ -185,7 +228,7 @@ impl StepDescription {
 pub struct ShouldRun<'a> {
     pub builder: &'a Builder<'a>,
     // use a BTreeSet to maintain sort order
-    paths: BTreeSet<PathBuf>,
+    paths: BTreeSet<PathSet>,
 
     // If this is a default rule, this is an additional constraint placed on
     // it's run. Generally something like compiler docs being enabled.
@@ -206,25 +249,46 @@ impl<'a> ShouldRun<'a> {
         self
     }
 
+    // Unlike `krate` this will create just one pathset. As such, it probably shouldn't actually
+    // ever be used, but as we transition to having all rules properly handle passing krate(...) by
+    // actually doing something different for every crate passed.
+    pub fn all_krates(mut self, name: &str) -> Self {
+        let mut set = BTreeSet::new();
+        for krate in self.builder.in_tree_crates(name) {
+            set.insert(PathBuf::from(&krate.path));
+        }
+        self.paths.insert(PathSet { set });
+        self
+    }
+
     pub fn krate(mut self, name: &str) -> Self {
-        for (_, krate_path) in self.builder.crates(name) {
-            self.paths.insert(PathBuf::from(krate_path));
+        for krate in self.builder.in_tree_crates(name) {
+            self.paths.insert(PathSet::one(&krate.path));
         }
         self
     }
 
-    pub fn path(mut self, path: &str) -> Self {
-        self.paths.insert(PathBuf::from(path));
+    // single, non-aliased path
+    pub fn path(self, path: &str) -> Self {
+        self.paths(&[path])
+    }
+
+    // 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
     }
 
     // allows being more explicit about why should_run in Step returns the value passed to it
-    pub fn never(self) -> ShouldRun<'a> {
+    pub fn never(mut self) -> ShouldRun<'a> {
+        self.paths.insert(PathSet::empty());
         self
     }
 
-    fn run(&self, path: &Path) -> bool {
-        self.paths.iter().any(|p| path.ends_with(p))
+    fn pathset_for_path(&self, path: &Path) -> Option<&PathSet> {
+        self.paths.iter().find(|pathset| pathset.has(path))
     }
 }
 
@@ -254,19 +318,23 @@ impl<'a> Builder<'a> {
                 tool::RustInstaller, tool::Cargo, tool::Rls, tool::Rustdoc, tool::Clippy,
                 native::Llvm, tool::Rustfmt, tool::Miri),
             Kind::Check => describe!(check::Std, check::Test, check::Rustc),
-            Kind::Test => describe!(test::Tidy, test::Bootstrap, test::DefaultCompiletest,
-                test::HostCompiletest, test::Crate, test::CrateLibrustc, test::Rustdoc,
-                test::Linkcheck, test::Cargotest, test::Cargo, test::Rls, test::Docs,
-                test::ErrorIndex, test::Distcheck, test::Rustfmt, test::Miri, test::Clippy,
-                test::RustdocJS, test::RustdocTheme),
+            Kind::Test => describe!(test::Tidy, test::Bootstrap, test::Ui, test::RunPass,
+                test::CompileFail, test::ParseFail, test::RunFail, test::RunPassValgrind,
+                test::MirOpt, test::Codegen, test::CodegenUnits, test::Incremental, test::Debuginfo,
+                test::UiFullDeps, test::RunPassFullDeps, test::RunFailFullDeps,
+                test::CompileFailFullDeps, test::IncrementalFullDeps, test::Rustdoc, test::Pretty,
+                test::RunPassPretty, test::RunFailPretty, test::RunPassValgrindPretty,
+                test::RunPassFullDepsPretty, test::RunFailFullDepsPretty, test::RunMake,
+                test::Crate, test::CrateLibrustc, test::Rustdoc, test::Linkcheck, test::Cargotest,
+                test::Cargo, test::Rls, test::Docs, test::ErrorIndex, test::Distcheck,
+                test::Rustfmt, test::Miri, test::Clippy, test::RustdocJS, test::RustdocTheme),
             Kind::Bench => describe!(test::Crate, test::CrateLibrustc),
             Kind::Doc => describe!(doc::UnstableBook, doc::UnstableBookGen, doc::TheBook,
                 doc::Standalone, doc::Std, doc::Test, doc::Rustc, doc::ErrorIndex, doc::Nomicon,
                 doc::Reference, doc::Rustdoc, doc::RustByExample, doc::CargoBook),
             Kind::Dist => describe!(dist::Docs, dist::Mingw, dist::Rustc, dist::DebuggerScripts,
                 dist::Std, dist::Analysis, dist::Src, dist::PlainSourceTarball, dist::Cargo,
-                dist::Rls, dist::Rustfmt, dist::Extended, dist::HashSign,
-                dist::DontDistWithMiriEnabled),
+                dist::Rls, dist::Rustfmt, dist::Extended, dist::HashSign),
             Kind::Install => describe!(install::Docs, install::Std, install::Cargo, install::Rls,
                 install::Rustfmt, install::Analysis, install::Src, install::Rustc),
         }
@@ -297,8 +365,10 @@ impl<'a> Builder<'a> {
             should_run = (desc.should_run)(should_run);
         }
         let mut help = String::from("Available paths:\n");
-        for path in should_run.paths {
-            help.push_str(format!("    ./x.py {} {}\n", subcommand, path.display()).as_str());
+        for pathset in should_run.paths {
+            for path in pathset.set {
+                help.push_str(format!("    ./x.py {} {}\n", subcommand, path.display()).as_str());
+            }
         }
         Some(help)
     }
@@ -323,6 +393,12 @@ impl<'a> Builder<'a> {
             stack: RefCell::new(Vec::new()),
         };
 
+        if kind == Kind::Dist {
+            assert!(!build.config.test_miri, "Do not distribute with miri enabled.\n\
+                The distributed libraries would include all MIR (increasing binary size).
+                The distributed MIR would include validation statements.");
+        }
+
         StepDescription::run(&Builder::get_step_descriptions(builder.kind), &builder, paths);
     }
 

src/bootstrap/check.rs

@@ -26,7 +26,7 @@ impl Step for Std {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/libstd").krate("std")
+        run.all_krates("std")
     }
 
     fn make_run(run: RunConfig) {
@@ -67,7 +67,7 @@ impl Step for Rustc {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/librustc").krate("rustc-main")
+        run.all_krates("rustc-main")
     }
 
     fn make_run(run: RunConfig) {
@@ -114,7 +114,7 @@ impl Step for Test {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/libtest").krate("test")
+        run.all_krates("test")
     }
 
     fn make_run(run: RunConfig) {

src/bootstrap/compile.rs

@@ -48,7 +48,7 @@ impl Step for Std {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/libstd").krate("std")
+        run.all_krates("std")
     }
 
     fn make_run(run: RunConfig) {
@@ -320,7 +320,7 @@ impl Step for Test {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/libtest").krate("test")
+        run.all_krates("test")
     }
 
     fn make_run(run: RunConfig) {
@@ -436,7 +436,7 @@ impl Step for Rustc {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/librustc").krate("rustc-main")
+        run.all_krates("rustc-main")
     }
 
     fn make_run(run: RunConfig) {
@@ -593,7 +593,7 @@ impl Step for CodegenBackend {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/librustc_trans")
+        run.all_krates("rustc_trans")
     }
 
     fn make_run(run: RunConfig) {
@@ -828,7 +828,7 @@ impl Step for Assemble {
     type Output = Compiler;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/rustc")
+        run.all_krates("rustc-main")
     }
 
     /// Prepare a new compiler from the artifacts in `stage`

src/bootstrap/config.rs

@@ -56,6 +56,7 @@ pub struct Config {
     pub sanitizers: bool,
     pub profiler: bool,
     pub ignore_git: bool,
+    pub exclude: Vec<PathBuf>,
 
     pub run_host_only: bool,
 
@@ -311,6 +312,7 @@ impl Config {
         let flags = Flags::parse(&args);
         let file = flags.config.clone();
         let mut config = Config::default();
+        config.exclude = flags.exclude;
         config.llvm_enabled = true;
         config.llvm_optimize = true;
         config.llvm_version_check = true;

src/bootstrap/dist.rs

@@ -1233,31 +1233,6 @@ impl Step for Rustfmt {
     }
 }
 
-
-#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
-pub struct DontDistWithMiriEnabled;
-
-impl Step for DontDistWithMiriEnabled {
-    type Output = PathBuf;
-    const DEFAULT: bool = true;
-
-    fn should_run(run: ShouldRun) -> ShouldRun {
-        let build_miri = run.builder.build.config.test_miri;
-        run.default_condition(build_miri)
-    }
-
-    fn make_run(run: RunConfig) {
-        run.builder.ensure(DontDistWithMiriEnabled);
-    }
-
-    fn run(self, _: &Builder) -> PathBuf {
-        panic!("Do not distribute with miri enabled.\n\
-                The distributed libraries would include all MIR (increasing binary size).
-                The distributed MIR would include validation statements.");
-    }
-}
-
-
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
 pub struct Extended {
     stage: u32,

src/bootstrap/doc.rs

@@ -429,7 +429,7 @@ impl Step for Std {
 
     fn should_run(run: ShouldRun) -> ShouldRun {
         let builder = run.builder;
-        run.krate("std").default_condition(builder.build.config.docs)
+        run.all_krates("std").default_condition(builder.build.config.docs)
     }
 
     fn make_run(run: RunConfig) {