Skip to content

Commit 611e7b9

Browse files
committed
Auto merge of rust-lang#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 rust-lang#96803. I also took the liberty of fixing `x dist llvm-tools` to work even if you don't call `x build` previously.
2 parents bb8c2f4 + 057eab7 commit 611e7b9

File tree

5 files changed

+47
-20
lines changed

5 files changed

+47
-20
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ dependencies = [
236236
"sysinfo",
237237
"tar",
238238
"toml",
239+
"walkdir",
239240
"winapi",
240241
"xz2",
241242
]

src/bootstrap/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ ignore = "0.4.10"
5151
opener = "0.5"
5252
once_cell = "1.7.2"
5353
xz2 = "0.1"
54+
walkdir = "2"
5455

5556
# Dependencies needed by the build-metrics feature
5657
sysinfo = { version = "0.24.1", optional = true }

src/bootstrap/dist.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,12 @@ impl Step for PlainSourceTarball {
845845

846846
/// Creates the plain source tarball
847847
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
848-
let tarball = Tarball::new(builder, "rustc", "src");
848+
// NOTE: This is a strange component in a lot of ways. It uses `src` as the target, which
849+
// means neither rustup nor rustup-toolchain-install-master know how to download it.
850+
// It also contains symbolic links, unlike other any other dist tarball.
851+
// It's used for distros building rustc from source in a pre-vendored environment.
852+
let mut tarball = Tarball::new(builder, "rustc", "src");
853+
tarball.permit_symlinks(true);
849854
let plain_dst_src = tarball.image_dir();
850855

851856
// 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) {
18471852

18481853
/// Maybe add LLVM object files to the given destination lib-dir. Allows either static or dynamic linking.
18491854
///
1850-
18511855
/// Returns whether the files were actually copied.
18521856
fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir: &Path) -> bool {
18531857
if let Some(config) = builder.config.target_config.get(&target) {
@@ -1957,6 +1961,8 @@ impl Step for LlvmTools {
19571961
}
19581962
}
19591963

1964+
builder.ensure(crate::native::Llvm { target });
1965+
19601966
let mut tarball = Tarball::new(builder, "llvm-tools", &target.triple);
19611967
tarball.set_overlay(OverlayKind::LLVM);
19621968
tarball.is_preview(true);

src/bootstrap/lib.rs

+18-17
Original file line numberDiff line numberDiff line change
@@ -1427,6 +1427,10 @@ impl Build {
14271427

14281428
/// Copies a file from `src` to `dst`
14291429
pub fn copy(&self, src: &Path, dst: &Path) {
1430+
self.copy_internal(src, dst, false);
1431+
}
1432+
1433+
fn copy_internal(&self, src: &Path, dst: &Path, dereference_symlinks: bool) {
14301434
if self.config.dry_run {
14311435
return;
14321436
}
@@ -1436,15 +1440,22 @@ impl Build {
14361440
}
14371441
let _ = fs::remove_file(&dst);
14381442
let metadata = t!(src.symlink_metadata());
1443+
let mut src = src.to_path_buf();
14391444
if metadata.file_type().is_symlink() {
1440-
let link = t!(fs::read_link(src));
1441-
t!(symlink_file(link, dst));
1442-
} else if let Ok(()) = fs::hard_link(src, dst) {
1445+
if dereference_symlinks {
1446+
src = t!(fs::canonicalize(src));
1447+
} else {
1448+
let link = t!(fs::read_link(src));
1449+
t!(symlink_file(link, dst));
1450+
return;
1451+
}
1452+
}
1453+
if let Ok(()) = fs::hard_link(&src, dst) {
14431454
// Attempt to "easy copy" by creating a hard link
14441455
// (symlinks don't work on windows), but if that fails
14451456
// just fall back to a slow `copy` operation.
14461457
} else {
1447-
if let Err(e) = fs::copy(src, dst) {
1458+
if let Err(e) = fs::copy(&src, dst) {
14481459
panic!("failed to copy `{}` to `{}`: {}", src.display(), dst.display(), e)
14491460
}
14501461
t!(fs::set_permissions(dst, metadata.permissions()));
@@ -1516,20 +1527,10 @@ impl Build {
15161527
let dst = dstdir.join(src.file_name().unwrap());
15171528
self.verbose_than(1, &format!("Install {:?} to {:?}", src, dst));
15181529
t!(fs::create_dir_all(dstdir));
1519-
drop(fs::remove_file(&dst));
1520-
{
1521-
if !src.exists() {
1522-
panic!("Error: File \"{}\" not found!", src.display());
1523-
}
1524-
let metadata = t!(src.symlink_metadata());
1525-
if let Err(e) = fs::copy(&src, &dst) {
1526-
panic!("failed to copy `{}` to `{}`: {}", src.display(), dst.display(), e)
1527-
}
1528-
t!(fs::set_permissions(&dst, metadata.permissions()));
1529-
let atime = FileTime::from_last_access_time(&metadata);
1530-
let mtime = FileTime::from_last_modification_time(&metadata);
1531-
t!(filetime::set_file_times(&dst, atime, mtime));
1530+
if !src.exists() {
1531+
panic!("Error: File \"{}\" not found!", src.display());
15321532
}
1533+
self.copy_internal(src, &dst, true);
15331534
chmod(&dst, perms);
15341535
}
15351536

src/bootstrap/tarball.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ pub(crate) struct Tarball<'a> {
102102

103103
include_target_in_component_name: bool,
104104
is_preview: bool,
105+
permit_symlinks: bool,
105106
}
106107

107108
impl<'a> Tarball<'a> {
@@ -141,6 +142,7 @@ impl<'a> Tarball<'a> {
141142

142143
include_target_in_component_name: false,
143144
is_preview: false,
145+
permit_symlinks: false,
144146
}
145147
}
146148

@@ -160,6 +162,10 @@ impl<'a> Tarball<'a> {
160162
self.is_preview = is;
161163
}
162164

165+
pub(crate) fn permit_symlinks(&mut self, flag: bool) {
166+
self.permit_symlinks = flag;
167+
}
168+
163169
pub(crate) fn image_dir(&self) -> &Path {
164170
t!(std::fs::create_dir_all(&self.image_dir));
165171
&self.image_dir
@@ -316,6 +322,18 @@ impl<'a> Tarball<'a> {
316322
}
317323
self.builder.run(&mut cmd);
318324

325+
// Ensure there are no symbolic links in the tarball. In particular,
326+
// rustup-toolchain-install-master and most versions of Windows can't handle symbolic links.
327+
let decompressed_output = self.temp_dir.join(&package_name);
328+
if !self.builder.config.dry_run && !self.permit_symlinks {
329+
for entry in walkdir::WalkDir::new(&decompressed_output) {
330+
let entry = t!(entry);
331+
if entry.path_is_symlink() {
332+
panic!("generated a symlink in a tarball: {}", entry.path().display());
333+
}
334+
}
335+
}
336+
319337
// Use either the first compression format defined, or "gz" as the default.
320338
let ext = self
321339
.builder
@@ -328,7 +346,7 @@ impl<'a> Tarball<'a> {
328346

329347
GeneratedTarball {
330348
path: crate::dist::distdir(self.builder).join(format!("{}.tar.{}", package_name, ext)),
331-
decompressed_output: self.temp_dir.join(package_name),
349+
decompressed_output,
332350
work: self.temp_dir,
333351
}
334352
}

0 commit comments

Comments
 (0)