Skip to content

Commit 4f82fb8

Browse files
authored
Rollup merge of #114613 - ferrocene:pa-fix-rebuild, r=lqd
Prevent constant rebuilds of `rustc-main` (and thus everything else) PR #114305 changed bootstrap to run `strip -g` on `librustc_driver.so` and `libllvm.so` on Linux when no debuginfo was requested. Unfortunately, that PR resulted in bootstrap always rebuilding everything starting from stage 1 `rustc-main` (including stage 1 libraries and tests) when invoking bootstrap multiple times. We noticed this because Ferrocene's CI times increased to between 2x and 3x total execution time, but the regression can also be reproduced locally by running `./x build library/sysroot --stage 1` twice. The explanation of the problem is in the code comments. r? ```@lqd``` cc ```@ozkanonur```
2 parents 54a9c2c + da00356 commit 4f82fb8

File tree

2 files changed

+36
-21
lines changed

2 files changed

+36
-21
lines changed

src/bootstrap/compile.rs

+29-8
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use crate::util::get_clang_cl_resource_dir;
3131
use crate::util::{exe, is_debug_info, is_dylib, output, symlink_dir, t, up_to_date};
3232
use crate::LLVM_TOOLS;
3333
use crate::{CLang, Compiler, DependencyType, GitRepo, Mode};
34+
use filetime::FileTime;
3435

3536
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
3637
pub struct Std {
@@ -904,19 +905,12 @@ impl Step for Rustc {
904905
// our LLVM wrapper. Unless we're explicitly requesting `librustc_driver` to be built with
905906
// debuginfo (via the debuginfo level of the executables using it): strip this debuginfo
906907
// away after the fact.
907-
// FIXME: to make things simpler for now, limit this to the host and target where we know
908-
// `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not
909-
// cross-compiling. Expand this to other appropriate targets in the future.
910908
if builder.config.rust_debuginfo_level_rustc == DebuginfoLevel::None
911909
&& builder.config.rust_debuginfo_level_tools == DebuginfoLevel::None
912-
&& target == "x86_64-unknown-linux-gnu"
913-
&& target == builder.config.build
914910
{
915911
let target_root_dir = stamp.parent().unwrap();
916912
let rustc_driver = target_root_dir.join("librustc_driver.so");
917-
if rustc_driver.exists() {
918-
output(Command::new("strip").arg("--strip-debug").arg(rustc_driver));
919-
}
913+
strip_debug(builder, target, &rustc_driver);
920914
}
921915

922916
builder.ensure(RustcLink::from_rustc(
@@ -1974,3 +1968,30 @@ pub enum CargoMessage<'a> {
19741968
success: bool,
19751969
},
19761970
}
1971+
1972+
pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) {
1973+
// FIXME: to make things simpler for now, limit this to the host and target where we know
1974+
// `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not
1975+
// cross-compiling. Expand this to other appropriate targets in the future.
1976+
if target != "x86_64-unknown-linux-gnu" || target != builder.config.build || !path.exists() {
1977+
return;
1978+
}
1979+
1980+
let previous_mtime = FileTime::from_last_modification_time(&path.metadata().unwrap());
1981+
// Note: `output` will propagate any errors here.
1982+
output(Command::new("strip").arg("--strip-debug").arg(path));
1983+
1984+
// After running `strip`, we have to set the file modification time to what it was before,
1985+
// otherwise we risk Cargo invalidating its fingerprint and rebuilding the world next time
1986+
// bootstrap is invoked.
1987+
//
1988+
// An example of this is if we run this on librustc_driver.so. In the first invocation:
1989+
// - Cargo will build librustc_driver.so (mtime of 1)
1990+
// - Cargo will build rustc-main (mtime of 2)
1991+
// - Bootstrap will strip librustc_driver.so (changing the mtime to 3).
1992+
//
1993+
// In the second invocation of bootstrap, Cargo will see that the mtime of librustc_driver.so
1994+
// is greater than the mtime of rustc-main, and will rebuild rustc-main. That will then cause
1995+
// everything else (standard library, future stages...) to be rebuilt.
1996+
t!(filetime::set_file_mtime(path, previous_mtime));
1997+
}

src/bootstrap/llvm.rs

+7-13
Original file line numberDiff line numberDiff line change
@@ -512,27 +512,21 @@ impl Step for Llvm {
512512
// When building LLVM as a shared library on linux, it can contain unexpected debuginfo:
513513
// some can come from the C++ standard library. Unless we're explicitly requesting LLVM to
514514
// be built with debuginfo, strip it away after the fact, to make dist artifacts smaller.
515-
// FIXME: to make things simpler for now, limit this to the host and target where we know
516-
// `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not
517-
// cross-compiling. Expand this to other appropriate targets in the future.
518515
if builder.llvm_link_shared()
519516
&& builder.config.llvm_optimize
520517
&& !builder.config.llvm_release_debuginfo
521-
&& target == "x86_64-unknown-linux-gnu"
522-
&& target == builder.config.build
523518
{
524519
// Find the name of the LLVM shared library that we just built.
525520
let lib_name = find_llvm_lib_name("so");
526521

527522
// If the shared library exists in LLVM's `/build/lib/` or `/lib/` folders, strip its
528-
// debuginfo. Note: `output` will propagate any errors here.
529-
let strip_if_possible = |path: PathBuf| {
530-
if path.exists() {
531-
output(Command::new("strip").arg("--strip-debug").arg(path));
532-
}
533-
};
534-
strip_if_possible(out_dir.join("lib").join(&lib_name));
535-
strip_if_possible(out_dir.join("build").join("lib").join(&lib_name));
523+
// debuginfo.
524+
crate::compile::strip_debug(builder, target, &out_dir.join("lib").join(&lib_name));
525+
crate::compile::strip_debug(
526+
builder,
527+
target,
528+
&out_dir.join("build").join("lib").join(&lib_name),
529+
);
536530
}
537531

538532
t!(stamp.write());

0 commit comments

Comments
 (0)