From 0db70ca26313b847738fd9405c67e9c43e9ee21b Mon Sep 17 00:00:00 2001
From: Joshua Nelson <jnelson@cloudflare.com>
Date: Sun, 10 Apr 2022 17:09:56 -0500
Subject: [PATCH] Require all paths passed to `ShouldRun::paths` to exist on
 disk

This has two benefits:
1. There is a clearer mental model of how bootstrap works. Steps correspond to paths on disk unless it's strictly impossible for them to do so (e.g. dist components).
2. Bootstrap has better checks for internal consistency. This caught several issues:
  - `src/sanitizers` doesn't exist; I changed it to just be a `sanitizers` alias.
  - `src/tools/lld` doesn't exist; I removed it, since `lld` alone already works.
  - `src/llvm` doesn't exist; removed it since `llvm` and `src/llvm-project` both work.
  - `src/lldb_batchmode.py` doesn't exist, it was moved to `src/etc`.
  - `install` was still using `src/librustc` instead of `compiler/rustc`.
  - None of the tools in `dist` / `install` allowed using `src/tools/X` to build them. This might be intentional - I can change them to aliases if you like.
---
 src/bootstrap/builder.rs   | 25 ++++++++++++++++++++-
 src/bootstrap/dist.rs      | 45 +++++++++++++++++++-------------------
 src/bootstrap/install.rs   | 28 ++++++++++++------------
 src/bootstrap/native.rs    |  6 ++---
 src/bootstrap/test.rs      |  2 +-
 src/bootstrap/toolstate.rs |  2 +-
 6 files changed, 66 insertions(+), 42 deletions(-)

diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
index 8c06a007013..cfcc807d293 100644
--- a/src/bootstrap/builder.rs
+++ b/src/bootstrap/builder.rs
@@ -364,6 +364,19 @@ impl<'a> ShouldRun<'a> {
         self
     }
 
+    // single alias, which does not correspond to any on-disk path
+    pub fn alias(mut self, alias: &str) -> Self {
+        assert!(
+            !self.builder.src.join(alias).exists(),
+            "use `builder.path()` for real paths: {}",
+            alias
+        );
+        self.paths.insert(PathSet::Set(
+            std::iter::once(TaskPath { path: alias.into(), kind: Some(self.kind) }).collect(),
+        ));
+        self
+    }
+
     // single, non-aliased path
     pub fn path(self, path: &str) -> Self {
         self.paths(&[path])
@@ -372,7 +385,17 @@ 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 { path: p.into(), kind: Some(self.kind) }).collect(),
+            paths
+                .iter()
+                .map(|p| {
+                    assert!(
+                        self.builder.src.join(p).exists(),
+                        "`should_run.paths` should correspond to real on-disk paths - use `alias` if there is no relevant path: {}",
+                        p
+                    );
+                    TaskPath { path: p.into(), kind: Some(self.kind) }
+                })
+                .collect(),
         ));
         self
     }
diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs
index fdd1581d9cd..e3287e35227 100644
--- a/src/bootstrap/dist.rs
+++ b/src/bootstrap/dist.rs
@@ -61,7 +61,7 @@ impl Step for Docs {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = run.builder.config.docs;
-        run.path("rust-docs").default_condition(default)
+        run.alias("rust-docs").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -94,7 +94,7 @@ impl Step for RustcDocs {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let builder = run.builder;
-        run.path("rustc-docs").default_condition(builder.config.compiler_docs)
+        run.alias("rustc-docs").default_condition(builder.config.compiler_docs)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -272,7 +272,7 @@ impl Step for Mingw {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("rust-mingw")
+        run.alias("rust-mingw")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -313,7 +313,7 @@ impl Step for Rustc {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("rustc")
+        run.alias("rustc")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -456,7 +456,7 @@ impl Step for DebuggerScripts {
     type Output = ();
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("src/lldb_batchmode.py")
+        run.path("src/etc/lldb_batchmode.py")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -547,7 +547,7 @@ impl Step for Std {
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("rust-std")
+        run.alias("rust-std")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -594,7 +594,7 @@ impl Step for RustcDev {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("rustc-dev")
+        run.alias("rustc-dev")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -653,7 +653,7 @@ impl Step for Analysis {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "analysis");
-        run.path("rust-analysis").default_condition(default)
+        run.alias("rust-analysis").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -790,7 +790,7 @@ impl Step for Src {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("rust-src")
+        run.alias("rust-src")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -848,7 +848,7 @@ impl Step for PlainSourceTarball {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let builder = run.builder;
-        run.path("rustc-src").default_condition(builder.config.rust_dist_src)
+        run.alias("rustc-src").default_condition(builder.config.rust_dist_src)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -942,7 +942,7 @@ impl Step for Cargo {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "cargo");
-        run.path("cargo").default_condition(default)
+        run.alias("cargo").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -998,7 +998,7 @@ impl Step for Rls {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "rls");
-        run.path("rls").default_condition(default)
+        run.alias("rls").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1045,7 +1045,7 @@ impl Step for RustAnalyzer {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "rust-analyzer");
-        run.path("rust-analyzer").default_condition(default)
+        run.alias("rust-analyzer").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1101,7 +1101,7 @@ impl Step for Clippy {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "clippy");
-        run.path("clippy").default_condition(default)
+        run.alias("clippy").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1152,7 +1152,7 @@ impl Step for Miri {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "miri");
-        run.path("miri").default_condition(default)
+        run.alias("miri").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1212,7 +1212,7 @@ impl Step for Rustfmt {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "rustfmt");
-        run.path("rustfmt").default_condition(default)
+        run.alias("rustfmt").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1271,7 +1271,7 @@ impl Step for RustDemangler {
         // we run the step by default when only `extended = true`, and decide whether to actually
         // run it or not later.
         let default = run.builder.config.extended;
-        run.path("rust-demangler").default_condition(default)
+        run.alias("rust-demangler").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1324,7 +1324,7 @@ impl Step for Extended {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let builder = run.builder;
-        run.path("extended").default_condition(builder.config.extended)
+        run.alias("extended").default_condition(builder.config.extended)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1968,7 +1968,8 @@ impl Step for LlvmTools {
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         let default = should_build_extended_tool(&run.builder, "llvm-tools");
-        run.path("llvm-tools").default_condition(default)
+        // FIXME: allow using the names of the tools themselves?
+        run.alias("llvm-tools").default_condition(default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -2022,7 +2023,7 @@ impl Step for RustDev {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("rust-dev")
+        run.alias("rust-dev")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -2098,7 +2099,7 @@ impl Step for BuildManifest {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("build-manifest")
+        run.alias("build-manifest")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -2130,7 +2131,7 @@ impl Step for ReproducibleArtifacts {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("reproducible-artifacts")
+        run.alias("reproducible-artifacts")
     }
 
     fn make_run(run: RunConfig<'_>) {
diff --git a/src/bootstrap/install.rs b/src/bootstrap/install.rs
index 27b9196d986..08e37a16279 100644
--- a/src/bootstrap/install.rs
+++ b/src/bootstrap/install.rs
@@ -93,7 +93,7 @@ fn prepare_dir(mut path: PathBuf) -> String {
 macro_rules! install {
     (($sel:ident, $builder:ident, $_config:ident),
        $($name:ident,
-       $path:expr,
+       $condition_name: ident = $path_or_alias: literal,
        $default_cond:expr,
        only_hosts: $only_hosts:expr,
        $run_item:block $(, $c:ident)*;)+) => {
@@ -108,7 +108,7 @@ macro_rules! install {
             #[allow(dead_code)]
             fn should_build(config: &Config) -> bool {
                 config.extended && config.tools.as_ref()
-                    .map_or(true, |t| t.contains($path))
+                    .map_or(true, |t| t.contains($path_or_alias))
             }
         }
 
@@ -120,7 +120,7 @@ macro_rules! install {
 
             fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
                 let $_config = &run.builder.config;
-                run.path($path).default_condition($default_cond)
+                run.$condition_name($path_or_alias).default_condition($default_cond)
             }
 
             fn make_run(run: RunConfig<'_>) {
@@ -138,11 +138,11 @@ macro_rules! install {
 }
 
 install!((self, builder, _config),
-    Docs, "src/doc", _config.docs, only_hosts: false, {
+    Docs, path = "src/doc", _config.docs, only_hosts: false, {
         let tarball = builder.ensure(dist::Docs { host: self.target }).expect("missing docs");
         install_sh(builder, "docs", self.compiler.stage, Some(self.target), &tarball);
     };
-    Std, "library/std", true, only_hosts: false, {
+    Std, path = "library/std", true, only_hosts: false, {
         for target in &builder.targets {
             // `expect` should be safe, only None when host != build, but this
             // only runs when host == build
@@ -153,13 +153,13 @@ install!((self, builder, _config),
             install_sh(builder, "std", self.compiler.stage, Some(*target), &tarball);
         }
     };
-    Cargo, "cargo", Self::should_build(_config), only_hosts: true, {
+    Cargo, alias = "cargo", Self::should_build(_config), only_hosts: true, {
         let tarball = builder
             .ensure(dist::Cargo { compiler: self.compiler, target: self.target })
             .expect("missing cargo");
         install_sh(builder, "cargo", self.compiler.stage, Some(self.target), &tarball);
     };
-    Rls, "rls", Self::should_build(_config), only_hosts: true, {
+    Rls, alias = "rls", Self::should_build(_config), only_hosts: true, {
         if let Some(tarball) = builder.ensure(dist::Rls { compiler: self.compiler, target: self.target }) {
             install_sh(builder, "rls", self.compiler.stage, Some(self.target), &tarball);
         } else {
@@ -168,7 +168,7 @@ install!((self, builder, _config),
             );
         }
     };
-    RustAnalyzer, "rust-analyzer", Self::should_build(_config), only_hosts: true, {
+    RustAnalyzer, alias = "rust-analyzer", Self::should_build(_config), only_hosts: true, {
         if let Some(tarball) =
             builder.ensure(dist::RustAnalyzer { compiler: self.compiler, target: self.target })
         {
@@ -179,13 +179,13 @@ install!((self, builder, _config),
             );
         }
     };
-    Clippy, "clippy", Self::should_build(_config), only_hosts: true, {
+    Clippy, alias = "clippy", Self::should_build(_config), only_hosts: true, {
         let tarball = builder
             .ensure(dist::Clippy { compiler: self.compiler, target: self.target })
             .expect("missing clippy");
         install_sh(builder, "clippy", self.compiler.stage, Some(self.target), &tarball);
     };
-    Miri, "miri", Self::should_build(_config), only_hosts: true, {
+    Miri, alias = "miri", Self::should_build(_config), only_hosts: true, {
         if let Some(tarball) = builder.ensure(dist::Miri { compiler: self.compiler, target: self.target }) {
             install_sh(builder, "miri", self.compiler.stage, Some(self.target), &tarball);
         } else {
@@ -194,7 +194,7 @@ install!((self, builder, _config),
             );
         }
     };
-    Rustfmt, "rustfmt", Self::should_build(_config), only_hosts: true, {
+    Rustfmt, alias = "rustfmt", Self::should_build(_config), only_hosts: true, {
         if let Some(tarball) = builder.ensure(dist::Rustfmt {
             compiler: self.compiler,
             target: self.target
@@ -206,7 +206,7 @@ install!((self, builder, _config),
             );
         }
     };
-    RustDemangler, "rust-demangler", Self::should_build(_config), only_hosts: true, {
+    RustDemangler, alias = "rust-demangler", Self::should_build(_config), only_hosts: true, {
         // Note: Even though `should_build` may return true for `extended` default tools,
         // dist::RustDemangler may still return None, unless the target-dependent `profiler` config
         // is also true, or the `tools` array explicitly includes "rust-demangler".
@@ -222,7 +222,7 @@ install!((self, builder, _config),
             );
         }
     };
-    Analysis, "analysis", Self::should_build(_config), only_hosts: false, {
+    Analysis, alias = "analysis", Self::should_build(_config), only_hosts: false, {
         // `expect` should be safe, only None with host != build, but this
         // only uses the `build` compiler
         let tarball = builder.ensure(dist::Analysis {
@@ -234,7 +234,7 @@ install!((self, builder, _config),
         }).expect("missing analysis");
         install_sh(builder, "analysis", self.compiler.stage, Some(self.target), &tarball);
     };
-    Rustc, "src/librustc", true, only_hosts: true, {
+    Rustc, path = "compiler/rustc", true, only_hosts: true, {
         let tarball = builder.ensure(dist::Rustc {
             compiler: builder.compiler(builder.top_stage, self.target),
         });
diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs
index 6d7ca9a94cf..73fb2dad1e3 100644
--- a/src/bootstrap/native.rs
+++ b/src/bootstrap/native.rs
@@ -122,7 +122,7 @@ impl Step for Llvm {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("src/llvm-project").path("src/llvm-project/llvm").path("src/llvm")
+        run.path("src/llvm-project").path("src/llvm-project/llvm")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -605,7 +605,7 @@ impl Step for Lld {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("src/llvm-project/lld").path("src/tools/lld")
+        run.path("src/llvm-project/lld")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -771,7 +771,7 @@ impl Step for Sanitizers {
     type Output = Vec<SanitizerRuntime>;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("src/llvm-project/compiler-rt").path("src/sanitizers")
+        run.alias("sanitizers")
     }
 
     fn make_run(run: RunConfig<'_>) {
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index b88684791bc..703b876f301 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -2295,7 +2295,7 @@ impl Step for Distcheck {
     type Output = ();
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("distcheck")
+        run.alias("distcheck")
     }
 
     fn make_run(run: RunConfig<'_>) {
diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs
index c7ea254c5b1..3ee6a42d987 100644
--- a/src/bootstrap/toolstate.rs
+++ b/src/bootstrap/toolstate.rs
@@ -234,7 +234,7 @@ impl Step for ToolStateCheck {
     }
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("check-tools")
+        run.alias("check-tools")
     }
 
     fn make_run(run: RunConfig<'_>) {