Auto merge of #110229 - jyn514:download-rustc-tests, r=albertlarsan68

Fix no_std tests that load libc from the sysroot when download-rustc is enabled

There were a series of unfortunate interactions here. Here's an MCVE of the test this fixes (committed as `tests/ui/meta/no_std-extern-libc.rs`):
```rust
#![crate_type = "lib"]
#![no_std]
#![feature(rustc_private)]
extern crate libc;
```

Before, this would give an error about duplicate versions of libc:
```
error[E0464]: multiple candidates for `rlib` dependency `libc` found
  --> fake-test-src-base/allocator/no_std-alloc-error-handler-default.rs:15:1
   |
LL | extern crate libc;
   | ^^^^^^^^^^^^^^^^^^
   |
   = note: candidate #1: /home/gh-jyn514/rust/build/aarch64-unknown-linux-gnu/stage2/lib/rustlib/aarch64-unknown-linux-gnu/lib/liblibc-358db1024b7d9957.rlib
   = note: candidate #2: /home/gh-jyn514/rust/build/aarch64-unknown-linux-gnu/stage2/lib/rustlib/aarch64-unknown-linux-gnu/lib/liblibc-ebc478710122a279.rmeta
```
Both these versions were downloaded from CI, but one came from the `rust-std` component and one came from `rustc-dev`:
```
; tar -tf build/cache/f2d9a3d0771504f1ae776226a5799dcb4408a91a/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz | grep liblibc
rust-std-nightly-x86_64-unknown-linux-gnu/rust-std-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-68a2d9e195dd6ed2.rlib
; tar -tf build/cache/f2d9a3d0771504f1ae776226a5799dcb4408a91a/rustc-dev-nightly-x86_64-unknown-linux-gnu.tar.xz | grep liblibc
rustc-dev-nightly-x86_64-unknown-linux-gnu/rustc-dev/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-f226c9fbdd92a0fd.rmeta
```
The fix was to only copy files from `rust-std` unless a Step explicitly requests for the `rustc-dev` components to be available by calling `builder.ensure(compile::Rustc)`.

To avoid having to re-parse the `rustc-dev.tar.xz` tarball every time, which is quite slow, this adds a new `build/host/ci-rustc/.rustc-dev-contents` cache file which stores only the names of files we need to copy into the sysroot.

This also allows reverting the hack in https://github.com/rust-lang/rust/pull/110121; now that we only copy rustc-dev on-demand, we can correctly add the `Rustc` check artifacts into the sysroot, so that this works correctly even when `download-rustc` is forced to `true` and some tool depends on a local change to `compiler`.

---

See https://github.com/rust-lang/rust/issues/108767#issuecomment-1501217657 for why `no_std` is required for the MCVE test to fail; it's complicated and not particularly important.

Fixes https://github.com/rust-lang/rust/issues/108767.

src/bootstrap/check.rs

@@ -271,17 +271,9 @@ impl Step for Rustc {
             false,
         );
 
-        // HACK: This avoids putting the newly built artifacts in the sysroot if we're using
-        // `download-rustc`, to avoid "multiple candidates for `rmeta`" errors. Technically, that's
-        // not quite right: people can set `download-rustc = true` to download even if there are
-        // changes to the compiler, and in that case ideally we would put the *new* artifacts in the
-        // sysroot, in case there are API changes that should be used by tools.  In practice,
-        // though, that should be very uncommon, and people can still disable download-rustc.
-        if !builder.download_rustc() {
-            let libdir = builder.sysroot_libdir(compiler, target);
-            let hostdir = builder.sysroot_libdir(compiler, compiler.host);
-            add_to_sysroot(&builder, &libdir, &hostdir, &librustc_stamp(builder, compiler, target));
-        }
+        let libdir = builder.sysroot_libdir(compiler, target);
+        let hostdir = builder.sysroot_libdir(compiler, compiler.host);
+        add_to_sysroot(&builder, &libdir, &hostdir, &librustc_stamp(builder, compiler, target));
     }
 }
 

src/bootstrap/compile.rs

@@ -9,6 +9,7 @@
 use std::borrow::Cow;
 use std::collections::HashSet;
 use std::env;
+use std::ffi::OsStr;
 use std::fs;
 use std::io::prelude::*;
 use std::io::BufReader;
@@ -652,8 +653,19 @@ impl Step for Rustc {
         // so its artifacts can't be reused.
         if builder.download_rustc() && compiler.stage != 0 {
             // Copy the existing artifacts instead of rebuilding them.
-            // NOTE: this path is only taken for tools linking to rustc-dev.
-            builder.ensure(Sysroot { compiler });
+            // NOTE: this path is only taken for tools linking to rustc-dev (including ui-fulldeps tests).
+            let sysroot = builder.ensure(Sysroot { compiler });
+
+            let ci_rustc_dir = builder.out.join(&*builder.build.build.triple).join("ci-rustc");
+            for file in builder.config.rustc_dev_contents() {
+                let src = ci_rustc_dir.join(&file);
+                let dst = sysroot.join(file);
+                if src.is_dir() {
+                    t!(fs::create_dir_all(dst));
+                } else {
+                    builder.copy(&src, &dst);
+                }
+            }
             return;
         }
 
@@ -1282,7 +1294,40 @@ impl Step for Sysroot {
             }
 
             // Copy the compiler into the correct sysroot.
-            builder.cp_r(&builder.ci_rustc_dir(builder.build.build), &sysroot);
+            // NOTE(#108767): We intentionally don't copy `rustc-dev` artifacts until they're requested with `builder.ensure(Rustc)`.
+            // This fixes an issue where we'd have multiple copies of libc in the sysroot with no way to tell which to load.
+            // There are a few quirks of bootstrap that interact to make this reliable:
+            // 1. The order `Step`s are run is hard-coded in `builder.rs` and not configurable. This
+            //    avoids e.g. reordering `test::UiFulldeps` before `test::Ui` and causing the latter to
+            //    fail because of duplicate metadata.
+            // 2. The sysroot is deleted and recreated between each invocation, so running `x test
+            //    ui-fulldeps && x test ui` can't cause failures.
+            let mut filtered_files = Vec::new();
+            // Don't trim directories or files that aren't loaded per-target; they can't cause conflicts.
+            let suffix = format!("lib/rustlib/{}/lib", compiler.host);
+            for path in builder.config.rustc_dev_contents() {
+                let path = Path::new(&path);
+                if path.parent().map_or(false, |parent| parent.ends_with(&suffix)) {
+                    filtered_files.push(path.file_name().unwrap().to_owned());
+                }
+            }
+
+            let filtered_extensions = [OsStr::new("rmeta"), OsStr::new("rlib"), OsStr::new("so")];
+            let ci_rustc_dir = builder.ci_rustc_dir(builder.config.build);
+            builder.cp_filtered(&ci_rustc_dir, &sysroot, &|path| {
+                if path.extension().map_or(true, |ext| !filtered_extensions.contains(&ext)) {
+                    return true;
+                }
+                if !path.parent().map_or(true, |p| p.ends_with(&suffix)) {
+                    return true;
+                }
+                if !filtered_files.iter().all(|f| f != path.file_name().unwrap()) {
+                    builder.verbose_than(1, &format!("ignoring {}", path.display()));
+                    false
+                } else {
+                    true
+                }
+            });
             return INTERNER.intern_path(sysroot);
         }
 

src/bootstrap/download.rs

@@ -2,7 +2,7 @@ use std::{
     env,
     ffi::{OsStr, OsString},
     fs::{self, File},
-    io::{BufRead, BufReader, ErrorKind},
+    io::{BufRead, BufReader, BufWriter, ErrorKind, Write},
     path::{Path, PathBuf},
     process::{Command, Stdio},
 };
@@ -262,10 +262,20 @@ impl Config {
         let directory_prefix = Path::new(Path::new(uncompressed_filename).file_stem().unwrap());
 
         // decompress the file
-        let data = t!(File::open(tarball));
+        let data = t!(File::open(tarball), format!("file {} not found", tarball.display()));
         let decompressor = XzDecoder::new(BufReader::new(data));
 
         let mut tar = tar::Archive::new(decompressor);
+
+        // `compile::Sysroot` needs to know the contents of the `rustc-dev` tarball to avoid adding
+        // it to the sysroot unless it was explicitly requested. But parsing the 100 MB tarball is slow.
+        // Cache the entries when we extract it so we only have to read it once.
+        let mut recorded_entries = if dst.ends_with("ci-rustc") && pattern == "rustc-dev" {
+            Some(BufWriter::new(t!(File::create(dst.join(".rustc-dev-contents")))))
+        } else {
+            None
+        };
+
         for member in t!(tar.entries()) {
             let mut member = t!(member);
             let original_path = t!(member.path()).into_owned();
@@ -283,13 +293,19 @@ impl Config {
             if !t!(member.unpack_in(dst)) {
                 panic!("path traversal attack ??");
             }
+            if let Some(record) = &mut recorded_entries {
+                t!(writeln!(record, "{}", short_path.to_str().unwrap()));
+            }
             let src_path = dst.join(original_path);
             if src_path.is_dir() && dst_path.exists() {
                 continue;
             }
             t!(fs::rename(src_path, dst_path));
         }
-        t!(fs::remove_dir_all(dst.join(directory_prefix)));
+        let dst_dir = dst.join(directory_prefix);
+        if dst_dir.exists() {
+            t!(fs::remove_dir_all(&dst_dir), format!("failed to remove {}", dst_dir.display()));
+        }
     }
 
     /// Returns whether the SHA256 checksum of `path` matches `expected`.
@@ -365,6 +381,13 @@ impl Config {
         Some(rustfmt_path)
     }
 
+    pub(crate) fn rustc_dev_contents(&self) -> Vec<String> {
+        assert!(self.download_rustc());
+        let ci_rustc_dir = self.out.join(&*self.build.triple).join("ci-rustc");
+        let rustc_dev_contents_file = t!(File::open(ci_rustc_dir.join(".rustc-dev-contents")));
+        t!(BufReader::new(rustc_dev_contents_file).lines().collect())
+    }
+
     pub(crate) fn download_ci_rustc(&self, commit: &str) {
         self.verbose(&format!("using downloaded stage2 artifacts from CI (commit {commit})"));
 

tests/ui/meta/no_std-extern-libc.rs

@@ -0,0 +1,7 @@
+// Test that `download-rustc` doesn't put duplicate copies of libc in the sysroot.
+// check-pass
+#![crate_type = "lib"]
+#![no_std]
+#![feature(rustc_private)]
+
+extern crate libc;