about summary refs log tree commit diff
path: root/src/bootstrap
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-08-08 21:44:45 +0200
committerGitHub <noreply@github.com>2023-08-08 21:44:45 +0200
commit4f82fb81f53464729ed836c5f06a0fe048d5c777 (patch)
treecd50ae3a5ead50e68a02b945f8eb4b40678790eb /src/bootstrap
parent54a9c2cb2a97f5d8532d6322d4b1144c6b8a2b6e (diff)
parentda00356e5570e66dc90e6f403345bbbab255b95a (diff)
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```
Diffstat (limited to 'src/bootstrap')
-rw-r--r--src/bootstrap/compile.rs37
-rw-r--r--src/bootstrap/llvm.rs20
2 files changed, 36 insertions, 21 deletions
diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
index 2c63ec80c3e..62c7b96b5be 100644
--- a/src/bootstrap/compile.rs
+++ b/src/bootstrap/compile.rs
@@ -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 {
@@ -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(
@@ -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));
+}
diff --git a/src/bootstrap/llvm.rs b/src/bootstrap/llvm.rs
index 14ee5659ed5..4943f93fa9a 100644
--- a/src/bootstrap/llvm.rs
+++ b/src/bootstrap/llvm.rs
@@ -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());