about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-06-19 22:22:07 +0000
committerbors <bors@rust-lang.org>2022-06-19 22:22:07 +0000
commit611e7b9cea2b982b63de7f6697b2a9079b0bf188 (patch)
treec3448ee402d72d6b595bea149096065dd82d996c
parentbb8c2f41174caceec00c28bc6c5c20ae9f9a175c (diff)
parent057eab7ae96ee491f8939e52722b6c29ce3cc445 (diff)
downloadrust-611e7b9cea2b982b63de7f6697b2a9079b0bf188.tar.gz
rust-611e7b9cea2b982b63de7f6697b2a9079b0bf188.zip
Auto merge of #97268 - jyn514:faster-assemble, r=Mark-Simulacrum
Make "Assemble stage1 compiler" orders of magnitude faster (take 2)

This used to take upwards of 5 seconds for me locally. I found that the culprit was copying the downloaded LLVM shared object:
```
[22:28:03] Install "/home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/ci-llvm/lib/libLLVM-14-rust-1.62.0-nightly.so" to "/home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libLLVM-14-rust-1.62.0-nightly.so"
[22:28:09]   c Sysroot { compiler: Compiler { stage: 1, host: x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu) } }
```

It turned out that `install()` used full copies unconditionally. Change it to try using a hard-link before falling back to copying.

- Panic if we generate a symbolic link in a tarball
- Change install to use copy internally, like in my previous PR
- Change copy to dereference symbolic links, which avoids the previous regression in #96803.

I also took the liberty of fixing `x dist llvm-tools` to work even if you don't call `x build` previously.
-rw-r--r--Cargo.lock1
-rw-r--r--src/bootstrap/Cargo.toml1
-rw-r--r--src/bootstrap/dist.rs10
-rw-r--r--src/bootstrap/lib.rs35
-rw-r--r--src/bootstrap/tarball.rs20
5 files changed, 47 insertions, 20 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 671f85ca3b6..1ca6ce25ba1 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -236,6 +236,7 @@ dependencies = [
  "sysinfo",
  "tar",
  "toml",
+ "walkdir",
  "winapi",
  "xz2",
 ]
diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml
index f5d9e46f9e2..ed5c59a2595 100644
--- a/src/bootstrap/Cargo.toml
+++ b/src/bootstrap/Cargo.toml
@@ -51,6 +51,7 @@ ignore = "0.4.10"
 opener = "0.5"
 once_cell = "1.7.2"
 xz2 = "0.1"
+walkdir = "2"
 
 # Dependencies needed by the build-metrics feature
 sysinfo = { version = "0.24.1", optional = true }
diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs
index b5901ce6f74..b1fae356d89 100644
--- a/src/bootstrap/dist.rs
+++ b/src/bootstrap/dist.rs
@@ -845,7 +845,12 @@ impl Step for PlainSourceTarball {
 
     /// Creates the plain source tarball
     fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
-        let tarball = Tarball::new(builder, "rustc", "src");
+        // NOTE: This is a strange component in a lot of ways. It uses `src` as the target, which
+        // means neither rustup nor rustup-toolchain-install-master know how to download it.
+        // It also contains symbolic links, unlike other any other dist tarball.
+        // It's used for distros building rustc from source in a pre-vendored environment.
+        let mut tarball = Tarball::new(builder, "rustc", "src");
+        tarball.permit_symlinks(true);
         let plain_dst_src = tarball.image_dir();
 
         // This is the set of root paths which will become part of the source package
@@ -1847,7 +1852,6 @@ fn add_env(builder: &Builder<'_>, cmd: &mut Command, target: TargetSelection) {
 
 /// Maybe add LLVM object files to the given destination lib-dir. Allows either static or dynamic linking.
 ///
-
 /// Returns whether the files were actually copied.
 fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir: &Path) -> bool {
     if let Some(config) = builder.config.target_config.get(&target) {
@@ -1957,6 +1961,8 @@ impl Step for LlvmTools {
             }
         }
 
+        builder.ensure(crate::native::Llvm { target });
+
         let mut tarball = Tarball::new(builder, "llvm-tools", &target.triple);
         tarball.set_overlay(OverlayKind::LLVM);
         tarball.is_preview(true);
diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs
index b3ebc991653..4ac857b470e 100644
--- a/src/bootstrap/lib.rs
+++ b/src/bootstrap/lib.rs
@@ -1427,6 +1427,10 @@ impl Build {
 
     /// Copies a file from `src` to `dst`
     pub fn copy(&self, src: &Path, dst: &Path) {
+        self.copy_internal(src, dst, false);
+    }
+
+    fn copy_internal(&self, src: &Path, dst: &Path, dereference_symlinks: bool) {
         if self.config.dry_run {
             return;
         }
@@ -1436,15 +1440,22 @@ impl Build {
         }
         let _ = fs::remove_file(&dst);
         let metadata = t!(src.symlink_metadata());
+        let mut src = src.to_path_buf();
         if metadata.file_type().is_symlink() {
-            let link = t!(fs::read_link(src));
-            t!(symlink_file(link, dst));
-        } else if let Ok(()) = fs::hard_link(src, dst) {
+            if dereference_symlinks {
+                src = t!(fs::canonicalize(src));
+            } else {
+                let link = t!(fs::read_link(src));
+                t!(symlink_file(link, dst));
+                return;
+            }
+        }
+        if let Ok(()) = fs::hard_link(&src, dst) {
             // Attempt to "easy copy" by creating a hard link
             // (symlinks don't work on windows), but if that fails
             // just fall back to a slow `copy` operation.
         } else {
-            if let Err(e) = fs::copy(src, dst) {
+            if let Err(e) = fs::copy(&src, dst) {
                 panic!("failed to copy `{}` to `{}`: {}", src.display(), dst.display(), e)
             }
             t!(fs::set_permissions(dst, metadata.permissions()));
@@ -1516,20 +1527,10 @@ impl Build {
         let dst = dstdir.join(src.file_name().unwrap());
         self.verbose_than(1, &format!("Install {:?} to {:?}", src, dst));
         t!(fs::create_dir_all(dstdir));
-        drop(fs::remove_file(&dst));
-        {
-            if !src.exists() {
-                panic!("Error: File \"{}\" not found!", src.display());
-            }
-            let metadata = t!(src.symlink_metadata());
-            if let Err(e) = fs::copy(&src, &dst) {
-                panic!("failed to copy `{}` to `{}`: {}", src.display(), dst.display(), e)
-            }
-            t!(fs::set_permissions(&dst, metadata.permissions()));
-            let atime = FileTime::from_last_access_time(&metadata);
-            let mtime = FileTime::from_last_modification_time(&metadata);
-            t!(filetime::set_file_times(&dst, atime, mtime));
+        if !src.exists() {
+            panic!("Error: File \"{}\" not found!", src.display());
         }
+        self.copy_internal(src, &dst, true);
         chmod(&dst, perms);
     }
 
diff --git a/src/bootstrap/tarball.rs b/src/bootstrap/tarball.rs
index 689b4819cdd..7b0c029c191 100644
--- a/src/bootstrap/tarball.rs
+++ b/src/bootstrap/tarball.rs
@@ -102,6 +102,7 @@ pub(crate) struct Tarball<'a> {
 
     include_target_in_component_name: bool,
     is_preview: bool,
+    permit_symlinks: bool,
 }
 
 impl<'a> Tarball<'a> {
@@ -141,6 +142,7 @@ impl<'a> Tarball<'a> {
 
             include_target_in_component_name: false,
             is_preview: false,
+            permit_symlinks: false,
         }
     }
 
@@ -160,6 +162,10 @@ impl<'a> Tarball<'a> {
         self.is_preview = is;
     }
 
+    pub(crate) fn permit_symlinks(&mut self, flag: bool) {
+        self.permit_symlinks = flag;
+    }
+
     pub(crate) fn image_dir(&self) -> &Path {
         t!(std::fs::create_dir_all(&self.image_dir));
         &self.image_dir
@@ -316,6 +322,18 @@ impl<'a> Tarball<'a> {
         }
         self.builder.run(&mut cmd);
 
+        // Ensure there are no symbolic links in the tarball. In particular,
+        // rustup-toolchain-install-master and most versions of Windows can't handle symbolic links.
+        let decompressed_output = self.temp_dir.join(&package_name);
+        if !self.builder.config.dry_run && !self.permit_symlinks {
+            for entry in walkdir::WalkDir::new(&decompressed_output) {
+                let entry = t!(entry);
+                if entry.path_is_symlink() {
+                    panic!("generated a symlink in a tarball: {}", entry.path().display());
+                }
+            }
+        }
+
         // Use either the first compression format defined, or "gz" as the default.
         let ext = self
             .builder
@@ -328,7 +346,7 @@ impl<'a> Tarball<'a> {
 
         GeneratedTarball {
             path: crate::dist::distdir(self.builder).join(format!("{}.tar.{}", package_name, ext)),
-            decompressed_output: self.temp_dir.join(package_name),
+            decompressed_output,
             work: self.temp_dir,
         }
     }