Skip to content

Commit

Permalink
Rollup merge of #110701 - jyn514:test-core, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Fix `x test core` when download-rustc is enabled

Fix `x test --stage 2 core` when download-rustc is enabled

This works by building std from source instead of downloading it, for library tests only.

This was somewhat complicated because of the following requirements:
1. Unconditionally downloading libstd breaks `x test core`, 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, to avoid having both versions of std in `stage2-test-sysroot`.

Fixes #110352.
  • Loading branch information
matthiaskrgr authored Jun 4, 2023
2 parents 7ee2505 + a80d69a commit f2f0e6c
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,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 @@ -271,11 +271,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 @@ -332,6 +329,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 @@ -382,11 +390,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
26 changes: 24 additions & 2 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2204,7 +2204,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 @@ -2218,6 +2219,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 @@ -2269,6 +2280,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 @@ -2320,7 +2336,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 f2f0e6c

Please sign in to comment.