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

improvements on build_steps::test implementation #135887

Merged
merged 3 commits into from
Jan 23, 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
96 changes: 32 additions & 64 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl Step for CrateBootstrap {
&[],
);
let crate_name = path.rsplit_once('/').unwrap().1;
run_cargo_test(cargo, &[], &[], crate_name, crate_name, compiler, bootstrap_host, builder);
run_cargo_test(cargo, &[], &[], crate_name, crate_name, bootstrap_host, builder);
}
}

Expand Down Expand Up @@ -143,7 +143,6 @@ You can skip linkcheck with --skip src/tools/linkchecker"
&[],
"linkchecker",
"linkchecker self tests",
compiler,
bootstrap_host,
builder,
);
Expand Down Expand Up @@ -312,7 +311,7 @@ impl Step for Cargo {
);

// NOTE: can't use `run_cargo_test` because we need to overwrite `PATH`
let mut cargo = prepare_cargo_test(cargo, &[], &[], "cargo", compiler, self.host, builder);
let mut cargo = prepare_cargo_test(cargo, &[], &[], "cargo", self.host, builder);

// Don't run cross-compile tests, we may not have cross-compiled libstd libs
// available.
Expand Down Expand Up @@ -397,7 +396,7 @@ impl Step for RustAnalyzer {
cargo.env("SKIP_SLOW_TESTS", "1");

cargo.add_rustc_lib_path(builder);
run_cargo_test(cargo, &[], &[], "rust-analyzer", "rust-analyzer", compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rust-analyzer", "rust-analyzer", host, builder);
}
}

Expand Down Expand Up @@ -445,7 +444,7 @@ impl Step for Rustfmt {

cargo.add_rustc_lib_path(builder);

run_cargo_test(cargo, &[], &[], "rustfmt", "rustfmt", compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rustfmt", "rustfmt", host, builder);
}
}

Expand Down Expand Up @@ -565,7 +564,7 @@ impl Step for Miri {

// We can NOT use `run_cargo_test` since Miri's integration tests do not use the usual test
// harness and therefore do not understand the flags added by `add_flags_and_try_run_test`.
let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", host_compiler, host, builder);
let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", host, builder);

// miri tests need to know about the stage sysroot
cargo.env("MIRI_SYSROOT", &miri_sysroot);
Expand Down Expand Up @@ -713,16 +712,7 @@ impl Step for CompiletestTest {
&[],
);
cargo.allow_features("test");
run_cargo_test(
cargo,
&[],
&[],
"compiletest",
"compiletest self test",
compiler,
host,
builder,
);
run_cargo_test(cargo, &[], &[], "compiletest", "compiletest self test", host, builder);
}
}

Expand Down Expand Up @@ -769,7 +759,7 @@ impl Step for Clippy {
cargo.env("HOST_LIBS", host_libs);

cargo.add_rustc_lib_path(builder);
let cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder);
let cargo = prepare_cargo_test(cargo, &[], &[], "clippy", host, builder);

let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host);

Expand Down Expand Up @@ -1294,7 +1284,6 @@ impl Step for CrateRunMakeSupport {
&[],
"run-make-support",
"run-make-support self test",
compiler,
host,
builder,
);
Expand Down Expand Up @@ -1334,16 +1323,7 @@ impl Step for CrateBuildHelper {
&[],
);
cargo.allow_features("test");
run_cargo_test(
cargo,
&[],
&[],
"build_helper",
"build_helper self test",
compiler,
host,
builder,
);
run_cargo_test(cargo, &[], &[], "build_helper", "build_helper self test", host, builder);
}
}

Expand Down Expand Up @@ -2540,19 +2520,17 @@ impl Step for CrateLibrustc {
/// Given a `cargo test` subcommand, add the appropriate flags and run it.
///
/// Returns whether the test succeeded.
#[allow(clippy::too_many_arguments)] // FIXME: reduce the number of args and remove this.
fn run_cargo_test<'a>(
cargo: impl Into<BootstrapCommand>,
cargo: builder::Cargo,
libtest_args: &[&str],
crates: &[String],
primary_crate: &str,
description: impl Into<Option<&'a str>>,
compiler: Compiler,
target: TargetSelection,
builder: &Builder<'_>,
) -> bool {
let mut cargo =
prepare_cargo_test(cargo, libtest_args, crates, primary_crate, compiler, target, builder);
let compiler = cargo.compiler();
let mut cargo = prepare_cargo_test(cargo, libtest_args, crates, primary_crate, target, builder);
let _time = helpers::timeit(builder);
let _group = description.into().and_then(|what| {
builder.msg_sysroot_tool(Kind::Test, compiler.stage, what, compiler.host, target)
Expand All @@ -2573,15 +2551,15 @@ fn run_cargo_test<'a>(

/// Given a `cargo test` subcommand, pass it the appropriate test flags given a `builder`.
fn prepare_cargo_test(
cargo: impl Into<BootstrapCommand>,
cargo: builder::Cargo,
libtest_args: &[&str],
crates: &[String],
primary_crate: &str,
compiler: Compiler,
target: TargetSelection,
builder: &Builder<'_>,
) -> BootstrapCommand {
let mut cargo = cargo.into();
let compiler = cargo.compiler();
let mut cargo: BootstrapCommand = cargo.into();

// Propagate `--bless` if it has not already been set/unset
// Any tools that want to use this should bless if `RUSTC_BLESS` is set to
Expand Down Expand Up @@ -2793,7 +2771,6 @@ impl Step for Crate {
&self.crates,
&self.crates[0],
&*crate_description(&self.crates),
compiler,
target,
builder,
);
Expand Down Expand Up @@ -2895,7 +2872,6 @@ impl Step for CrateRustdoc {
&["rustdoc:0.0.0".to_string()],
"rustdoc",
"rustdoc",
compiler,
target,
builder,
);
Expand Down Expand Up @@ -2956,7 +2932,6 @@ impl Step for CrateRustdocJsonTypes {
&["rustdoc-json-types".to_string()],
"rustdoc-json-types",
"rustdoc-json-types",
compiler,
target,
builder,
);
Expand Down Expand Up @@ -3113,23 +3088,25 @@ impl Step for Bootstrap {
// Use `python -m unittest` manually if you want to pass arguments.
check_bootstrap.delay_failure().run(builder);

let mut cmd = command(&builder.initial_cargo);
cmd.arg("test")
.current_dir(builder.src.join("src/bootstrap"))
.env("RUSTFLAGS", "--cfg test -Cdebuginfo=2")
let mut cargo = tool::prepare_tool_cargo(
builder,
compiler,
Mode::ToolBootstrap,
host,
Kind::Test,
"src/bootstrap",
SourceType::InTree,
&[],
);

cargo
.rustflag("-Cdebuginfo=2")
.env("CARGO_TARGET_DIR", builder.out.join("bootstrap"))
.env("RUSTC_BOOTSTRAP", "1")
.env("RUSTDOC", builder.rustdoc(compiler))
.env("RUSTC", &builder.initial_rustc);
if let Some(flags) = option_env!("RUSTFLAGS") {
// Use the same rustc flags for testing as for "normal" compilation,
// so that Cargo doesn’t recompile the entire dependency graph every time:
// /~https://github.com/rust-lang/rust/issues/49215
cmd.env("RUSTFLAGS", flags);
}
Comment on lines -3124 to -3129
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this is already handled from Cargo with

fn new(target: TargetSelection) -> Rustflags {
let mut ret = Rustflags(String::new(), target);
ret.propagate_cargo_env("RUSTFLAGS");
ret
}

.env("RUSTC_BOOTSTRAP", "1");

// bootstrap tests are racy on directory creation so just run them one at a time.
// Since there's not many this shouldn't be a problem.
run_cargo_test(cmd, &["--test-threads=1"], &[], "bootstrap", None, compiler, host, builder);
run_cargo_test(cargo, &["--test-threads=1"], &[], "bootstrap", None, host, builder);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down Expand Up @@ -3254,7 +3231,7 @@ impl Step for RustInstaller {
bootstrap_host,
bootstrap_host,
);
run_cargo_test(cargo, &[], &[], "installer", None, compiler, bootstrap_host, builder);
run_cargo_test(cargo, &[], &[], "installer", None, bootstrap_host, builder);

// We currently don't support running the test.sh script outside linux(?) environments.
// Eventually this should likely migrate to #[test]s in rust-installer proper rather than a
Expand Down Expand Up @@ -3639,16 +3616,7 @@ impl Step for TestFloatParse {
&[],
);

run_cargo_test(
cargo_test,
&[],
&[],
crate_name,
crate_name,
compiler,
bootstrap_host,
builder,
);
run_cargo_test(cargo_test, &[], &[], crate_name, crate_name, bootstrap_host, builder);

// Run the actual parse tests.
let mut cargo_run = tool::prepare_tool_cargo(
Expand Down
4 changes: 4 additions & 0 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ impl Cargo {
cargo
}

pub fn compiler(&self) -> Compiler {
self.compiler
}

pub fn into_cmd(self) -> BootstrapCommand {
self.into()
}
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ fn detect_src_and_out() {
// `{build-dir}/bootstrap/debug/deps/bootstrap-c7ee91d5661e2804`
// `{build-dir}` can be anywhere, not just in the rust project directory.
let dep = Path::new(args.first().unwrap());
let expected_out = dep.ancestors().nth(4).unwrap();
let expected_out = dep.ancestors().nth(5).unwrap();
Copy link
Member Author

Choose a reason for hiding this comment

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

Output path now also includes the host triple as bootstrap passes that regardless even for non-cross builds

if cmd_kind != Kind::Install {
cargo.arg("--target").arg(target.rustc_target_arg());


assert_eq!(&cfg.out, expected_out);
}
Expand Down
Loading