Move ninja requirements to a dynamic check, when actually building

It isn't practical to determine whether we'll build LLVM very early in the
pipeline, so move the ninja checking to a dynamic check.

Author: Mark-Simulacrum

src/bootstrap/builder/tests.rs

@@ -8,7 +8,7 @@ fn configure(host: &[&str], target: &[&str]) -> Config {
     config.save_toolstates = None;
     config.skip_only_host_steps = false;
     config.dry_run = true;
-    config.ninja = false;
+    config.ninja_in_file = false;
     // try to avoid spurious failures in dist where we create/delete each others file
     let dir = config
         .out

src/bootstrap/config.rs

@@ -32,7 +32,8 @@ use serde::Deserialize;
 #[derive(Default)]
 pub struct Config {
     pub ccache: Option<String>,
-    pub ninja: bool,
+    /// Call Build::ninja() instead of this.
+    pub ninja_in_file: bool,
     pub verbose: usize,
     pub submodules: bool,
     pub fast_submodules: bool,
@@ -450,7 +451,7 @@ impl Config {
     pub fn default_opts() -> Config {
         let mut config = Config::default();
         config.llvm_optimize = true;
-        config.ninja = true;
+        config.ninja_in_file = true;
         config.llvm_version_check = true;
         config.backtrace = true;
         config.rust_optimize = true;
@@ -606,7 +607,7 @@ impl Config {
                 }
                 Some(StringOrBool::Bool(false)) | None => {}
             }
-            set(&mut config.ninja, llvm.ninja);
+            set(&mut config.ninja_in_file, llvm.ninja);
             llvm_assertions = llvm.assertions;
             llvm_skip_rebuild = llvm_skip_rebuild.or(llvm.skip_rebuild);
             set(&mut config.llvm_optimize, llvm.optimize);

src/bootstrap/lib.rs

@@ -650,7 +650,7 @@ impl Build {
             }
         } else {
             let base = self.llvm_out(self.config.build).join("build");
-            let base = if !self.config.ninja && self.config.build.contains("msvc") {
+            let base = if !self.ninja() && self.config.build.contains("msvc") {
                 if self.config.llvm_optimize {
                     if self.config.llvm_release_debuginfo {
                         base.join("RelWithDebInfo")
@@ -1328,6 +1328,43 @@ impl Build {
         }
         fs::remove_file(f).unwrap_or_else(|_| panic!("failed to remove {:?}", f));
     }
+
+    /// Returns if config.ninja is enabled, and checks for ninja existence,
+    /// exiting with a nicer error message if not.
+    fn ninja(&self) -> bool {
+        let mut cmd_finder = crate::sanity::Finder::new();
+
+        if self.config.ninja_in_file {
+            // Some Linux distros rename `ninja` to `ninja-build`.
+            // CMake can work with either binary name.
+            if cmd_finder.maybe_have("ninja-build").is_none()
+                && cmd_finder.maybe_have("ninja").is_none()
+            {
+                eprintln!(
+                    "
+Couldn't find required command: ninja
+You should install ninja, or set ninja=false in config.toml
+"
+                );
+                std::process::exit(1);
+            }
+        }
+
+        // If ninja isn't enabled but we're building for MSVC then we try
+        // doubly hard to enable it. It was realized in #43767 that the msbuild
+        // CMake generator for MSVC doesn't respect configuration options like
+        // disabling LLVM assertions, which can often be quite important!
+        //
+        // In these cases we automatically enable Ninja if we find it in the
+        // environment.
+        if !self.config.ninja_in_file && self.config.build.contains("msvc") {
+            if cmd_finder.maybe_have("ninja").is_some() {
+                return true;
+            }
+        }
+
+        self.config.ninja_in_file
+    }
 }
 
 #[cfg(unix)]

src/bootstrap/native.rs

@@ -56,7 +56,7 @@ pub fn prebuilt_llvm_config(
     let out_dir = builder.llvm_out(target);
 
     let mut llvm_config_ret_dir = builder.llvm_out(builder.config.build);
-    if !builder.config.build.contains("msvc") || builder.config.ninja {
+    if !builder.config.build.contains("msvc") || builder.ninja() {
         llvm_config_ret_dir.push("build");
     }
     llvm_config_ret_dir.push("bin");
@@ -363,7 +363,7 @@ fn configure_cmake(
     // own build directories.
     cfg.env("DESTDIR", "");
 
-    if builder.config.ninja {
+    if builder.ninja() {
         cfg.generator("Ninja");
     }
     cfg.target(&target.triple).host(&builder.config.build.triple);
@@ -395,7 +395,7 @@ fn configure_cmake(
     // MSVC with CMake uses msbuild by default which doesn't respect these
     // vars that we'd otherwise configure. In that case we just skip this
     // entirely.
-    if target.contains("msvc") && !builder.config.ninja {
+    if target.contains("msvc") && !builder.ninja() {
         return;
     }
 
@@ -405,7 +405,7 @@ fn configure_cmake(
     };
 
     // Handle msvc + ninja + ccache specially (this is what the bots use)
-    if target.contains("msvc") && builder.config.ninja && builder.config.ccache.is_some() {
+    if target.contains("msvc") && builder.ninja() && builder.config.ccache.is_some() {
         let mut wrap_cc = env::current_exe().expect("failed to get cwd");
         wrap_cc.set_file_name("sccache-plus-cl.exe");
 

src/bootstrap/sanity.rs

@@ -20,17 +20,17 @@ use build_helper::{output, t};
 use crate::config::Target;
 use crate::Build;
 
-struct Finder {
+pub struct Finder {
     cache: HashMap<OsString, Option<PathBuf>>,
     path: OsString,
 }
 
 impl Finder {
-    fn new() -> Self {
+    pub fn new() -> Self {
         Self { cache: HashMap::new(), path: env::var_os("PATH").unwrap_or_default() }
     }
 
-    fn maybe_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> Option<PathBuf> {
+    pub fn maybe_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> Option<PathBuf> {
         let cmd: OsString = cmd.as_ref().into();
         let path = &self.path;
         self.cache
@@ -54,7 +54,7 @@ impl Finder {
             .clone()
     }
 
-    fn must_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> PathBuf {
+    pub fn must_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> PathBuf {
         self.maybe_have(&cmd).unwrap_or_else(|| {
             panic!("\n\ncouldn't find required command: {:?}\n\n", cmd.as_ref());
         })
@@ -95,38 +95,6 @@ pub fn check(build: &mut Build) {
         cmd_finder.must_have("cmake");
     }
 
-    // Ninja is currently only used for LLVM itself.
-    if building_llvm {
-        if build.config.ninja {
-            // Some Linux distros rename `ninja` to `ninja-build`.
-            // CMake can work with either binary name.
-            if cmd_finder.maybe_have("ninja-build").is_none()
-                && cmd_finder.maybe_have("ninja").is_none()
-            {
-                eprintln!(
-                    "
-Couldn't find required command: ninja
-You should install ninja, or set ninja=false in config.toml
-"
-                );
-                std::process::exit(1);
-            }
-        }
-
-        // If ninja isn't enabled but we're building for MSVC then we try
-        // doubly hard to enable it. It was realized in #43767 that the msbuild
-        // CMake generator for MSVC doesn't respect configuration options like
-        // disabling LLVM assertions, which can often be quite important!
-        //
-        // In these cases we automatically enable Ninja if we find it in the
-        // environment.
-        if !build.config.ninja && build.config.build.contains("msvc") {
-            if cmd_finder.maybe_have("ninja").is_some() {
-                build.config.ninja = true;
-            }
-        }
-    }
-
     build.config.python = build
         .config
         .python