Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiletest-related cleanups 2/7] Feed stage number to compiletest directly #136472

Merged
merged 2 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1648,16 +1648,17 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// bootstrap compiler.
// NOTE: Only stage 1 is special cased because we need the rustc_private artifacts to match the
// running compiler in stage 2 when plugins run.
let stage_id = if suite == "ui-fulldeps" && compiler.stage == 1 {
// At stage 0 (stage - 1) we are using the beta compiler. Using `self.target` can lead finding
// an incorrect compiler path on cross-targets, as the stage 0 beta compiler is always equal
// to `build.build` in the configuration.
let (stage, stage_id) = if suite == "ui-fulldeps" && compiler.stage == 1 {
// At stage 0 (stage - 1) we are using the beta compiler. Using `self.target` can lead
// finding an incorrect compiler path on cross-targets, as the stage 0 beta compiler is
// always equal to `build.build` in the configuration.
let build = builder.build.build;

compiler = builder.compiler(compiler.stage - 1, build);
format!("stage{}-{}", compiler.stage + 1, build)
let test_stage = compiler.stage + 1;
(test_stage, format!("stage{}-{}", test_stage, build))
} else {
format!("stage{}-{}", compiler.stage, target)
let stage = compiler.stage;
(stage, format!("stage{}-{}", stage, target))
};

if suite.ends_with("fulldeps") {
Expand Down Expand Up @@ -1699,6 +1700,9 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// compiletest currently has... a lot of arguments, so let's just pass all
// of them!

cmd.arg("--stage").arg(stage.to_string());
cmd.arg("--stage-id").arg(stage_id);

cmd.arg("--compile-lib-path").arg(builder.rustc_libdir(compiler));
cmd.arg("--run-lib-path").arg(builder.sysroot_target_libdir(compiler, target));
cmd.arg("--rustc-path").arg(builder.rustc(compiler));
Expand Down Expand Up @@ -1767,8 +1771,9 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
} else {
builder.sysroot(compiler).to_path_buf()
};

cmd.arg("--sysroot-base").arg(sysroot);
cmd.arg("--stage-id").arg(stage_id);

cmd.arg("--suite").arg(suite);
cmd.arg("--mode").arg(mode);
cmd.arg("--target").arg(target.rustc_target_arg());
Expand Down
4 changes: 3 additions & 1 deletion src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ pub struct Config {
/// The directory containing the compiler sysroot
pub sysroot_base: PathBuf,

/// The name of the stage being built (stage1, etc)
/// The number of the stage under test.
pub stage: u32,
/// The id of the stage under test (stage1-xxx, etc).
pub stage_id: String,

/// The test mode, e.g. ui or debuginfo.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/header/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ fn parse_cfg_name_directive<'a>(
message: "on big-endian targets",
}
condition! {
name: config.stage_id.split('-').next().unwrap(),
name: format!("stage{}", config.stage).as_str(),
allowed_names: &["stage0", "stage1", "stage2"],
message: "when the bootstrapping stage is {name}",
}
Expand Down
10 changes: 9 additions & 1 deletion src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ struct ConfigBuilder {
channel: Option<String>,
host: Option<String>,
target: Option<String>,
stage: Option<u32>,
stage_id: Option<String>,
llvm_version: Option<String>,
git_hash: bool,
Expand Down Expand Up @@ -102,6 +103,11 @@ impl ConfigBuilder {
self
}

fn stage(&mut self, n: u32) -> &mut Self {
self.stage = Some(n);
self
}

fn stage_id(&mut self, s: &str) -> &mut Self {
self.stage_id = Some(s.to_owned());
self
Expand Down Expand Up @@ -156,6 +162,8 @@ impl ConfigBuilder {
"--cxxflags=",
"--llvm-components=",
"--android-cross-path=",
"--stage",
&self.stage.unwrap_or(2).to_string(),
"--stage-id",
self.stage_id.as_deref().unwrap_or("stage2-x86_64-unknown-linux-gnu"),
"--channel",
Expand Down Expand Up @@ -387,7 +395,7 @@ fn std_debug_assertions() {

#[test]
fn stage() {
let config: Config = cfg().stage_id("stage1-x86_64-unknown-linux-gnu").build();
let config: Config = cfg().stage(1).stage_id("stage1-x86_64-unknown-linux-gnu").build();

assert!(check_ignore(&config, "//@ ignore-stage1"));
assert!(!check_ignore(&config, "//@ ignore-stage2"));
Expand Down
10 changes: 10 additions & 0 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
.reqopt("", "src-base", "directory to scan for test files", "PATH")
.reqopt("", "build-base", "directory to deposit test outputs", "PATH")
.reqopt("", "sysroot-base", "directory containing the compiler sysroot", "PATH")
.reqopt("", "stage", "stage number under test", "N")
.reqopt("", "stage-id", "the target-stage identifier", "stageN-TARGET")
.reqopt(
"",
Expand Down Expand Up @@ -294,6 +295,11 @@ pub fn parse_config(args: Vec<String>) -> Config {
panic!("`--nocapture` is deprecated; please use `--no-capture`");
}

let stage = match matches.opt_str("stage") {
Some(stage) => stage.parse::<u32>().expect("expected `--stage` to be an unsigned integer"),
None => panic!("`--stage` is required"),
};

Config {
bless: matches.opt_present("bless"),
compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")),
Expand All @@ -311,7 +317,10 @@ pub fn parse_config(args: Vec<String>) -> Config {
src_base,
build_base: opt_path(matches, "build-base"),
sysroot_base: opt_path(matches, "sysroot-base"),

stage,
stage_id: matches.opt_str("stage-id").unwrap(),

mode,
suite: matches.opt_str("suite").unwrap(),
debugger: matches.opt_str("debugger").map(|debugger| {
Expand Down Expand Up @@ -415,6 +424,7 @@ pub fn log_config(config: &Config) {
logv(c, format!("rustdoc_path: {:?}", config.rustdoc_path));
logv(c, format!("src_base: {:?}", config.src_base.display()));
logv(c, format!("build_base: {:?}", config.build_base.display()));
logv(c, format!("stage: {}", config.stage));
logv(c, format!("stage_id: {}", config.stage_id));
logv(c, format!("mode: {}", config.mode));
logv(c, format!("run_ignored: {}", config.run_ignored));
Expand Down
32 changes: 5 additions & 27 deletions src/tools/compiletest/src/runtest/run_make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,30 +239,6 @@ impl TestCx<'_> {
}
}

// `self.config.stage_id` looks like `stage1-<target_triple>`, but we only want
// the `stage1` part as that is what the output directories of bootstrap are prefixed with.
// Note that this *assumes* build layout from bootstrap is produced as:
//
// ```
// build/<target_triple>/ // <- this is `build_root`
// ├── stage0
// ├── stage0-bootstrap-tools
// ├── stage0-codegen
// ├── stage0-rustc
// ├── stage0-std
// ├── stage0-sysroot
// ├── stage0-tools
// ├── stage0-tools-bin
// ├── stage1
// ├── stage1-std
// ├── stage1-tools
// ├── stage1-tools-bin
// └── test
// ```
// FIXME(jieyouxu): improve the communication between bootstrap and compiletest here so
// we don't have to hack out a `stageN`.
let stage = self.config.stage_id.split('-').next().unwrap();
Comment on lines -262 to -264
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly to fix this


// In order to link in the support library as a rlib when compiling recipes, we need three
// paths:
// 1. Path of the built support library rlib itself.
Expand All @@ -284,10 +260,12 @@ impl TestCx<'_> {
// support lib and its deps are organized, can't we copy them to the tools-bin dir as
// well?), but this seems to work for now.

let stage_tools_bin = build_root.join(format!("{stage}-tools-bin"));
let stage_number = self.config.stage;

let stage_tools_bin = build_root.join(format!("stage{stage_number}-tools-bin"));
let support_lib_path = stage_tools_bin.join("librun_make_support.rlib");

let stage_tools = build_root.join(format!("{stage}-tools"));
let stage_tools = build_root.join(format!("stage{stage_number}-tools"));
let support_lib_deps = stage_tools.join(&self.config.host).join("release").join("deps");
let support_lib_deps_deps = stage_tools.join("release").join("deps");

Expand Down Expand Up @@ -368,7 +346,7 @@ impl TestCx<'_> {
// provided through env vars.

// Compute stage-specific standard library paths.
let stage_std_path = build_root.join(&stage).join("lib");
let stage_std_path = build_root.join(format!("stage{stage_number}")).join("lib");

// Compute dynamic library search paths for recipes.
let recipe_dylib_search_paths = {
Expand Down
Loading