Skip to content

Commit 58d5ce4

Browse files
committed
Auto merge of rust-lang#76197 - Mark-Simulacrum:no-llvm-no-ninja, r=pietroalbini
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. r? @pietroalbini -- this should fix nightlies
2 parents 397db05 + d77c351 commit 58d5ce4

File tree

5 files changed

+51
-45
lines changed

5 files changed

+51
-45
lines changed

Diff for: src/bootstrap/builder/tests.rs

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

Diff for: src/bootstrap/config.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ use serde::Deserialize;
3232
#[derive(Default)]
3333
pub struct Config {
3434
pub ccache: Option<String>,
35-
pub ninja: bool,
35+
/// Call Build::ninja() instead of this.
36+
pub ninja_in_file: bool,
3637
pub verbose: usize,
3738
pub submodules: bool,
3839
pub fast_submodules: bool,
@@ -450,7 +451,7 @@ impl Config {
450451
pub fn default_opts() -> Config {
451452
let mut config = Config::default();
452453
config.llvm_optimize = true;
453-
config.ninja = true;
454+
config.ninja_in_file = true;
454455
config.llvm_version_check = true;
455456
config.backtrace = true;
456457
config.rust_optimize = true;
@@ -606,7 +607,7 @@ impl Config {
606607
}
607608
Some(StringOrBool::Bool(false)) | None => {}
608609
}
609-
set(&mut config.ninja, llvm.ninja);
610+
set(&mut config.ninja_in_file, llvm.ninja);
610611
llvm_assertions = llvm.assertions;
611612
llvm_skip_rebuild = llvm_skip_rebuild.or(llvm.skip_rebuild);
612613
set(&mut config.llvm_optimize, llvm.optimize);

Diff for: src/bootstrap/lib.rs

+38-1
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ impl Build {
650650
}
651651
} else {
652652
let base = self.llvm_out(self.config.build).join("build");
653-
let base = if !self.config.ninja && self.config.build.contains("msvc") {
653+
let base = if !self.ninja() && self.config.build.contains("msvc") {
654654
if self.config.llvm_optimize {
655655
if self.config.llvm_release_debuginfo {
656656
base.join("RelWithDebInfo")
@@ -1328,6 +1328,43 @@ impl Build {
13281328
}
13291329
fs::remove_file(f).unwrap_or_else(|_| panic!("failed to remove {:?}", f));
13301330
}
1331+
1332+
/// Returns if config.ninja is enabled, and checks for ninja existence,
1333+
/// exiting with a nicer error message if not.
1334+
fn ninja(&self) -> bool {
1335+
let mut cmd_finder = crate::sanity::Finder::new();
1336+
1337+
if self.config.ninja_in_file {
1338+
// Some Linux distros rename `ninja` to `ninja-build`.
1339+
// CMake can work with either binary name.
1340+
if cmd_finder.maybe_have("ninja-build").is_none()
1341+
&& cmd_finder.maybe_have("ninja").is_none()
1342+
{
1343+
eprintln!(
1344+
"
1345+
Couldn't find required command: ninja
1346+
You should install ninja, or set ninja=false in config.toml
1347+
"
1348+
);
1349+
std::process::exit(1);
1350+
}
1351+
}
1352+
1353+
// If ninja isn't enabled but we're building for MSVC then we try
1354+
// doubly hard to enable it. It was realized in #43767 that the msbuild
1355+
// CMake generator for MSVC doesn't respect configuration options like
1356+
// disabling LLVM assertions, which can often be quite important!
1357+
//
1358+
// In these cases we automatically enable Ninja if we find it in the
1359+
// environment.
1360+
if !self.config.ninja_in_file && self.config.build.contains("msvc") {
1361+
if cmd_finder.maybe_have("ninja").is_some() {
1362+
return true;
1363+
}
1364+
}
1365+
1366+
self.config.ninja_in_file
1367+
}
13311368
}
13321369

13331370
#[cfg(unix)]

Diff for: src/bootstrap/native.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub fn prebuilt_llvm_config(
5656
let out_dir = builder.llvm_out(target);
5757

5858
let mut llvm_config_ret_dir = builder.llvm_out(builder.config.build);
59-
if !builder.config.build.contains("msvc") || builder.config.ninja {
59+
if !builder.config.build.contains("msvc") || builder.ninja() {
6060
llvm_config_ret_dir.push("build");
6161
}
6262
llvm_config_ret_dir.push("bin");
@@ -363,7 +363,7 @@ fn configure_cmake(
363363
// own build directories.
364364
cfg.env("DESTDIR", "");
365365

366-
if builder.config.ninja {
366+
if builder.ninja() {
367367
cfg.generator("Ninja");
368368
}
369369
cfg.target(&target.triple).host(&builder.config.build.triple);
@@ -395,7 +395,7 @@ fn configure_cmake(
395395
// MSVC with CMake uses msbuild by default which doesn't respect these
396396
// vars that we'd otherwise configure. In that case we just skip this
397397
// entirely.
398-
if target.contains("msvc") && !builder.config.ninja {
398+
if target.contains("msvc") && !builder.ninja() {
399399
return;
400400
}
401401

@@ -405,7 +405,7 @@ fn configure_cmake(
405405
};
406406

407407
// Handle msvc + ninja + ccache specially (this is what the bots use)
408-
if target.contains("msvc") && builder.config.ninja && builder.config.ccache.is_some() {
408+
if target.contains("msvc") && builder.ninja() && builder.config.ccache.is_some() {
409409
let mut wrap_cc = env::current_exe().expect("failed to get cwd");
410410
wrap_cc.set_file_name("sccache-plus-cl.exe");
411411

Diff for: src/bootstrap/sanity.rs

+4-36
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,17 @@ use build_helper::{output, t};
2020
use crate::config::Target;
2121
use crate::Build;
2222

23-
struct Finder {
23+
pub struct Finder {
2424
cache: HashMap<OsString, Option<PathBuf>>,
2525
path: OsString,
2626
}
2727

2828
impl Finder {
29-
fn new() -> Self {
29+
pub fn new() -> Self {
3030
Self { cache: HashMap::new(), path: env::var_os("PATH").unwrap_or_default() }
3131
}
3232

33-
fn maybe_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> Option<PathBuf> {
33+
pub fn maybe_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> Option<PathBuf> {
3434
let cmd: OsString = cmd.as_ref().into();
3535
let path = &self.path;
3636
self.cache
@@ -54,7 +54,7 @@ impl Finder {
5454
.clone()
5555
}
5656

57-
fn must_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> PathBuf {
57+
pub fn must_have<S: AsRef<OsStr>>(&mut self, cmd: S) -> PathBuf {
5858
self.maybe_have(&cmd).unwrap_or_else(|| {
5959
panic!("\n\ncouldn't find required command: {:?}\n\n", cmd.as_ref());
6060
})
@@ -95,38 +95,6 @@ pub fn check(build: &mut Build) {
9595
cmd_finder.must_have("cmake");
9696
}
9797

98-
// Ninja is currently only used for LLVM itself.
99-
if building_llvm {
100-
if build.config.ninja {
101-
// Some Linux distros rename `ninja` to `ninja-build`.
102-
// CMake can work with either binary name.
103-
if cmd_finder.maybe_have("ninja-build").is_none()
104-
&& cmd_finder.maybe_have("ninja").is_none()
105-
{
106-
eprintln!(
107-
"
108-
Couldn't find required command: ninja
109-
You should install ninja, or set ninja=false in config.toml
110-
"
111-
);
112-
std::process::exit(1);
113-
}
114-
}
115-
116-
// If ninja isn't enabled but we're building for MSVC then we try
117-
// doubly hard to enable it. It was realized in #43767 that the msbuild
118-
// CMake generator for MSVC doesn't respect configuration options like
119-
// disabling LLVM assertions, which can often be quite important!
120-
//
121-
// In these cases we automatically enable Ninja if we find it in the
122-
// environment.
123-
if !build.config.ninja && build.config.build.contains("msvc") {
124-
if cmd_finder.maybe_have("ninja").is_some() {
125-
build.config.ninja = true;
126-
}
127-
}
128-
}
129-
13098
build.config.python = build
13199
.config
132100
.python

0 commit comments

Comments
 (0)