Skip to content

Commit

Permalink
Rollup merge of rust-lang#114613 - ferrocene:pa-fix-rebuild, r=lqd
Browse files Browse the repository at this point in the history
Prevent constant rebuilds of `rustc-main` (and thus everything else)

PR rust-lang#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`
  • Loading branch information
Dylan-DPC authored Aug 8, 2023
2 parents 58e651f + da00356 commit a3fb831
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 21 deletions.
37 changes: 29 additions & 8 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use crate::util::get_clang_cl_resource_dir;
use crate::util::{exe, is_debug_info, is_dylib, output, symlink_dir, t, up_to_date};
use crate::LLVM_TOOLS;
use crate::{CLang, Compiler, DependencyType, GitRepo, Mode};
use filetime::FileTime;

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

builder.ensure(RustcLink::from_rustc(
Expand Down Expand Up @@ -1974,3 +1968,30 @@ pub enum CargoMessage<'a> {
success: bool,
},
}

pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) {
// FIXME: to make things simpler for now, limit this to the host and target where we know
// `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not
// cross-compiling. Expand this to other appropriate targets in the future.
if target != "x86_64-unknown-linux-gnu" || target != builder.config.build || !path.exists() {
return;
}

let previous_mtime = FileTime::from_last_modification_time(&path.metadata().unwrap());
// Note: `output` will propagate any errors here.
output(Command::new("strip").arg("--strip-debug").arg(path));

// After running `strip`, we have to set the file modification time to what it was before,
// otherwise we risk Cargo invalidating its fingerprint and rebuilding the world next time
// bootstrap is invoked.
//
// An example of this is if we run this on librustc_driver.so. In the first invocation:
// - Cargo will build librustc_driver.so (mtime of 1)
// - Cargo will build rustc-main (mtime of 2)
// - Bootstrap will strip librustc_driver.so (changing the mtime to 3).
//
// In the second invocation of bootstrap, Cargo will see that the mtime of librustc_driver.so
// is greater than the mtime of rustc-main, and will rebuild rustc-main. That will then cause
// everything else (standard library, future stages...) to be rebuilt.
t!(filetime::set_file_mtime(path, previous_mtime));
}
20 changes: 7 additions & 13 deletions src/bootstrap/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,27 +512,21 @@ impl Step for Llvm {
// When building LLVM as a shared library on linux, it can contain unexpected debuginfo:
// some can come from the C++ standard library. Unless we're explicitly requesting LLVM to
// be built with debuginfo, strip it away after the fact, to make dist artifacts smaller.
// FIXME: to make things simpler for now, limit this to the host and target where we know
// `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not
// cross-compiling. Expand this to other appropriate targets in the future.
if builder.llvm_link_shared()
&& builder.config.llvm_optimize
&& !builder.config.llvm_release_debuginfo
&& target == "x86_64-unknown-linux-gnu"
&& target == builder.config.build
{
// Find the name of the LLVM shared library that we just built.
let lib_name = find_llvm_lib_name("so");

// If the shared library exists in LLVM's `/build/lib/` or `/lib/` folders, strip its
// debuginfo. Note: `output` will propagate any errors here.
let strip_if_possible = |path: PathBuf| {
if path.exists() {
output(Command::new("strip").arg("--strip-debug").arg(path));
}
};
strip_if_possible(out_dir.join("lib").join(&lib_name));
strip_if_possible(out_dir.join("build").join("lib").join(&lib_name));
// debuginfo.
crate::compile::strip_debug(builder, target, &out_dir.join("lib").join(&lib_name));
crate::compile::strip_debug(
builder,
target,
&out_dir.join("build").join("lib").join(&lib_name),
);
}

t!(stamp.write());
Expand Down

0 comments on commit a3fb831

Please sign in to comment.