Skip to content

Commit

Permalink
Fix x test --stage 2 core when download-rustc is enabled
Browse files Browse the repository at this point in the history
This works by building std from source unconditionally instead of downloading it, for library tests only.

This was somewhat complicated because of the following requirements:
1. Unconditionally downloading libstd breaks `x test std`, because `coretests` requires the std loaded from the sysroot to match the std that's currently being tested.
2. Unconditionally rebuilding libstd breaks `x test ui-fulldeps librustdoc`, because anything loading `rustc_private` needs to use the same libstd that rustc was built with.

Break the knot by introducing a new `stage2-test-sysroot`, used only for testing `std` itself. This
holds a freshly compiled std, while `stage2` and `ci-rustc-sysroot` still hold the downloaded std.

This also extends the existing `cp_filtered` in Sysroot to apply to the `rust-std` component, not just the `rustc-dev` component.
  • Loading branch information
jyn514 committed May 31, 2023
1 parent 2f0a266 commit a80d69a
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ impl<'a> Builder<'a> {
}

pub fn sysroot(&self, compiler: Compiler) -> Interned<PathBuf> {
self.ensure(compile::Sysroot { compiler })
self.ensure(compile::Sysroot::new(compiler))
}

/// Returns the libdir where the standard library and other artifacts are
Expand Down
120 changes: 92 additions & 28 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,18 @@ pub struct Std {
///
/// This shouldn't be used from other steps; see the comment on [`Rustc`].
crates: Interned<Vec<String>>,
/// When using download-rustc, we need to use a new build of `std` for running unit tests of Std itself,
/// but we need to use the downloaded copy of std for linking to rustdoc. Allow this to be overriden by `builder.ensure` from other steps.
force_recompile: bool,
}

impl Std {
pub fn new(compiler: Compiler, target: TargetSelection) -> Self {
Self { target, compiler, crates: Default::default() }
Self { target, compiler, crates: Default::default(), force_recompile: false }
}

pub fn force_recompile(compiler: Compiler, target: TargetSelection) -> Self {
Self { target, compiler, crates: Default::default(), force_recompile: true }
}
}

Expand Down Expand Up @@ -77,6 +84,7 @@ impl Step for Std {
compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
target: run.target,
crates: make_run_crates(&run, "library"),
force_recompile: false,
});
}

Expand All @@ -89,11 +97,20 @@ impl Step for Std {
let target = self.target;
let compiler = self.compiler;

// When using `download-rustc`, we already have artifacts for the host available
// (they were copied in `impl Step for Sysroot`). Don't recompile them.
// NOTE: the ABI of the beta compiler is different from the ABI of the downloaded compiler,
// so its artifacts can't be reused.
if builder.download_rustc() && compiler.stage != 0 && target == builder.build.build {
// When using `download-rustc`, we already have artifacts for the host available. Don't
// recompile them.
if builder.download_rustc() && target == builder.build.build
// NOTE: the beta compiler may generate different artifacts than the downloaded compiler, so
// its artifacts can't be reused.
&& compiler.stage != 0
// This check is specific to testing std itself; see `test::Std` for more details.
&& !self.force_recompile
{
cp_rustc_component_to_ci_sysroot(
builder,
compiler,
builder.config.ci_rust_std_contents(),
);
return;
}

Expand Down Expand Up @@ -428,6 +445,8 @@ struct StdLink {
pub target: TargetSelection,
/// Not actually used; only present to make sure the cache invalidation is correct.
crates: Interned<Vec<String>>,
/// See [`Std::force_recompile`].
force_recompile: bool,
}

impl StdLink {
Expand All @@ -437,6 +456,7 @@ impl StdLink {
target_compiler: std.compiler,
target: std.target,
crates: std.crates,
force_recompile: std.force_recompile,
}
}
}
Expand All @@ -460,8 +480,24 @@ impl Step for StdLink {
let compiler = self.compiler;
let target_compiler = self.target_compiler;
let target = self.target;
let libdir = builder.sysroot_libdir(target_compiler, target);
let hostdir = builder.sysroot_libdir(target_compiler, compiler.host);

// NOTE: intentionally does *not* check `target == builder.build` to avoid having to add the same check in `test::Crate`.
let (libdir, hostdir) = if self.force_recompile && builder.download_rustc() {
// NOTE: copies part of `sysroot_libdir` to avoid having to add a new `force_recompile` argument there too
let lib = builder.sysroot_libdir_relative(self.compiler);
let sysroot = builder.ensure(crate::compile::Sysroot {
compiler: self.compiler,
force_recompile: self.force_recompile,
});
let libdir = sysroot.join(lib).join("rustlib").join(target.triple).join("lib");
let hostdir = sysroot.join(lib).join("rustlib").join(compiler.host.triple).join("lib");
(INTERNER.intern_path(libdir), INTERNER.intern_path(hostdir))
} else {
let libdir = builder.sysroot_libdir(target_compiler, target);
let hostdir = builder.sysroot_libdir(target_compiler, compiler.host);
(libdir, hostdir)
};

add_to_sysroot(builder, &libdir, &hostdir, &libstd_stamp(builder, compiler, target));
}
}
Expand Down Expand Up @@ -594,6 +630,25 @@ impl Step for StartupObjects {
}
}

fn cp_rustc_component_to_ci_sysroot(
builder: &Builder<'_>,
compiler: Compiler,
contents: Vec<String>,
) {
let sysroot = builder.ensure(Sysroot { compiler, force_recompile: false });

let ci_rustc_dir = builder.out.join(&*builder.build.build.triple).join("ci-rustc");
for file in 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);
}
}
}

#[derive(Debug, PartialOrd, Ord, Copy, Clone, PartialEq, Eq, Hash)]
pub struct Rustc {
pub target: TargetSelection,
Expand Down Expand Up @@ -653,18 +708,11 @@ impl Step for Rustc {
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 (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);
}
}
cp_rustc_component_to_ci_sysroot(
builder,
compiler,
builder.config.ci_rustc_dev_contents(),
);
return;
}

Expand Down Expand Up @@ -1225,6 +1273,14 @@ pub fn compiler_file(
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct Sysroot {
pub compiler: Compiler,
/// See [`Std::force_recompile`].
force_recompile: bool,
}

impl Sysroot {
pub(crate) fn new(compiler: Compiler) -> Self {
Sysroot { compiler, force_recompile: false }
}
}

impl Step for Sysroot {
Expand All @@ -1247,6 +1303,8 @@ impl Step for Sysroot {
let sysroot_dir = |stage| {
if stage == 0 {
host_dir.join("stage0-sysroot")
} else if self.force_recompile && stage == compiler.stage {
host_dir.join(format!("stage{stage}-test-sysroot"))
} else if builder.download_rustc() && compiler.stage != builder.top_stage {
host_dir.join("ci-rustc-sysroot")
} else {
Expand Down Expand Up @@ -1286,14 +1344,19 @@ impl Step for Sysroot {
// 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 mut add_filtered_files = |suffix, contents| {
for path in 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 suffix = format!("lib/rustlib/{}/lib", compiler.host);
add_filtered_files(suffix.as_str(), builder.config.ci_rustc_dev_contents());
// NOTE: we can't copy std eagerly because `stage2-test-sysroot` needs to have only the
// newly compiled std, not the downloaded std.
add_filtered_files("lib", builder.config.ci_rust_std_contents());

let filtered_extensions = [OsStr::new("rmeta"), OsStr::new("rlib"), OsStr::new("so")];
let ci_rustc_dir = builder.ci_rustc_dir(builder.config.build);
Expand Down Expand Up @@ -1411,7 +1474,8 @@ impl Step for Assemble {

// If we're downloading a compiler from CI, we can use the same compiler for all stages other than 0.
if builder.download_rustc() {
let sysroot = builder.ensure(Sysroot { compiler: target_compiler });
let sysroot =
builder.ensure(Sysroot { compiler: target_compiler, force_recompile: false });
// Ensure that `libLLVM.so` ends up in the newly created target directory,
// so that tools using `rustc_private` can use it.
dist::maybe_install_llvm_target(builder, target_compiler.host, &sysroot);
Expand Down
33 changes: 25 additions & 8 deletions src/bootstrap/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,8 @@ impl Config {
// `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
};
let mut recorded_entries =
if dst.ends_with("ci-rustc") { recorded_entries(dst, pattern) } else { None };

for member in t!(tar.entries()) {
let mut member = t!(member);
Expand Down Expand Up @@ -331,6 +328,17 @@ impl Config {
}
}

fn recorded_entries(dst: &Path, pattern: &str) -> Option<BufWriter<File>> {
let name = if pattern == "rustc-dev" {
".rustc-dev-contents"
} else if pattern.starts_with("rust-std") {
".rust-std-contents"
} else {
return None;
};
Some(BufWriter::new(t!(File::create(dst.join(name)))))
}

enum DownloadSource {
CI,
Dist,
Expand Down Expand Up @@ -381,11 +389,20 @@ impl Config {
Some(rustfmt_path)
}

pub(crate) fn rustc_dev_contents(&self) -> Vec<String> {
pub(crate) fn ci_rust_std_contents(&self) -> Vec<String> {
self.ci_component_contents(".rust-std-contents")
}

pub(crate) fn ci_rustc_dev_contents(&self) -> Vec<String> {
self.ci_component_contents(".rustc-dev-contents")
}

fn ci_component_contents(&self, stamp_file: &str) -> 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())
let stamp_file = ci_rustc_dir.join(stamp_file);
let contents_file = t!(File::open(&stamp_file), stamp_file.display().to_string());
t!(BufReader::new(contents_file).lines().collect())
}

pub(crate) fn download_ci_rustc(&self, commit: &str) {
Expand Down
34 changes: 31 additions & 3 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1777,7 +1777,13 @@ impl Step for BookTest {
/// This uses the `rustdoc` that sits next to `compiler`.
fn run(self, builder: &Builder<'_>) {
let host = self.compiler.host;
let _guard = builder.msg(Kind::Test, self.compiler.stage, &format!("book {}", self.name), host, host);
let _guard = builder.msg(
Kind::Test,
self.compiler.stage,
&format!("book {}", self.name),
host,
host,
);
// External docs are different from local because:
// - Some books need pre-processing by mdbook before being tested.
// - They need to save their state to toolstate.
Expand Down Expand Up @@ -2202,7 +2208,8 @@ impl Step for Crate {
let target = self.target;
let mode = self.mode;

builder.ensure(compile::Std::new(compiler, target));
// See [field@compile::Std::force_recompile].
builder.ensure(compile::Std::force_recompile(compiler, target));
builder.ensure(RemoteCopyLibs { compiler, target });

// If we're not doing a full bootstrap but we're testing a stage2
Expand All @@ -2216,6 +2223,16 @@ impl Step for Crate {
match mode {
Mode::Std => {
compile::std_cargo(builder, target, compiler.stage, &mut cargo);
// `std_cargo` actually does the wrong thing: it passes `--sysroot build/host/stage2`,
// but we want to use the force-recompile std we just built in `build/host/stage2-test-sysroot`.
// Override it.
if builder.download_rustc() {
let sysroot = builder
.out
.join(compiler.host.triple)
.join(format!("stage{}-test-sysroot", compiler.stage));
cargo.env("RUSTC_SYSROOT", sysroot);
}
}
Mode::Rustc => {
compile::rustc_cargo(builder, &mut cargo, target, compiler.stage);
Expand Down Expand Up @@ -2267,6 +2284,11 @@ impl Step for CrateRustdoc {
// isn't really necessary.
builder.compiler_for(builder.top_stage, target, target)
};
// NOTE: normally `ensure(Rustc)` automatically runs `ensure(Std)` for us. However, when
// using `download-rustc`, the rustc_private artifacts may be in a *different sysroot* from
// the target rustdoc (`ci-rustc-sysroot` vs `stage2`). In that case, we need to ensure this
// explicitly to make sure it ends up in the stage2 sysroot.
builder.ensure(compile::Std::new(compiler, target));
builder.ensure(compile::Rustc::new(compiler, target));

let mut cargo = tool::prepare_tool_cargo(
Expand Down Expand Up @@ -2318,7 +2340,13 @@ impl Step for CrateRustdoc {
dylib_path.insert(0, PathBuf::from(&*libdir));
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());

let _guard = builder.msg(builder.kind, compiler.stage, "rustdoc", compiler.host, target);
let _guard = builder.msg_sysroot_tool(
builder.kind,
compiler.stage,
"rustdoc",
compiler.host,
target,
);
run_cargo_test(
cargo,
&[],
Expand Down

0 comments on commit a80d69a

Please sign in to comment.