Skip to content

Commit 8e7849e

Browse files
committed
rustbuild: Use Cargo's "target runner"
This commit leverages a relatively new feature in Cargo to execute cross-compiled tests, the `target.$target.runner` configuration. We configure it through environment variables in rustbuild and this avoids the need for us to locate and run tests after-the-fact, instead relying on Cargo to do all that execution for us.
1 parent eba9d7f commit 8e7849e

File tree

2 files changed

+26
-69
lines changed

2 files changed

+26
-69
lines changed

Diff for: src/bootstrap/check.rs

+13-67
Original file line numberDiff line numberDiff line change
@@ -1050,11 +1050,8 @@ impl Step for Crate {
10501050
dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
10511051
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
10521052

1053-
if target.contains("emscripten") || build.remote_tested(target) {
1054-
cargo.arg("--no-run");
1055-
}
1056-
10571053
cargo.arg("--");
1054+
cargo.args(&build.flags.cmd.test_args());
10581055

10591056
if build.config.quiet_tests {
10601057
cargo.arg("--quiet");
@@ -1063,75 +1060,24 @@ impl Step for Crate {
10631060
let _time = util::timeit();
10641061

10651062
if target.contains("emscripten") {
1066-
build.run(&mut cargo);
1067-
krate_emscripten(build, compiler, target, mode);
1063+
cargo.env(format!("CARGO_TARGET_{}_RUNNER", envify(&target)),
1064+
build.config.nodejs.as_ref().expect("nodejs not configured"));
10681065
} else if build.remote_tested(target) {
1069-
build.run(&mut cargo);
1070-
krate_remote(builder, compiler, target, mode);
1071-
} else {
1072-
cargo.args(&build.flags.cmd.test_args());
1073-
try_run(build, &mut cargo);
1074-
}
1075-
}
1076-
}
1077-
1078-
fn krate_emscripten(build: &Build,
1079-
compiler: Compiler,
1080-
target: Interned<String>,
1081-
mode: Mode) {
1082-
let out_dir = build.cargo_out(compiler, mode, target);
1083-
let tests = find_tests(&out_dir.join("deps"), target);
1084-
1085-
let nodejs = build.config.nodejs.as_ref().expect("nodejs not configured");
1086-
for test in tests {
1087-
println!("running {}", test.display());
1088-
let mut cmd = Command::new(nodejs);
1089-
cmd.arg(&test);
1090-
if build.config.quiet_tests {
1091-
cmd.arg("--quiet");
1092-
}
1093-
try_run(build, &mut cmd);
1094-
}
1095-
}
1096-
1097-
fn krate_remote(builder: &Builder,
1098-
compiler: Compiler,
1099-
target: Interned<String>,
1100-
mode: Mode) {
1101-
let build = builder.build;
1102-
let out_dir = build.cargo_out(compiler, mode, target);
1103-
let tests = find_tests(&out_dir.join("deps"), target);
1104-
1105-
let tool = builder.tool_exe(Tool::RemoteTestClient);
1106-
for test in tests {
1107-
let mut cmd = Command::new(&tool);
1108-
cmd.arg("run")
1109-
.arg(&test);
1110-
if build.config.quiet_tests {
1111-
cmd.arg("--quiet");
1066+
cargo.env(format!("CARGO_TARGET_{}_RUNNER", envify(&target)),
1067+
format!("{} run",
1068+
builder.tool_exe(Tool::RemoteTestClient).display()));
11121069
}
1113-
cmd.args(&build.flags.cmd.test_args());
1114-
try_run(build, &mut cmd);
1070+
try_run(build, &mut cargo);
11151071
}
11161072
}
11171073

1118-
fn find_tests(dir: &Path, target: Interned<String>) -> Vec<PathBuf> {
1119-
let mut dst = Vec::new();
1120-
for e in t!(dir.read_dir()).map(|e| t!(e)) {
1121-
let file_type = t!(e.file_type());
1122-
if !file_type.is_file() {
1123-
continue
1124-
}
1125-
let filename = e.file_name().into_string().unwrap();
1126-
if (target.contains("windows") && filename.ends_with(".exe")) ||
1127-
(!target.contains("windows") && !filename.contains(".")) ||
1128-
(target.contains("emscripten") &&
1129-
filename.ends_with(".js") &&
1130-
!filename.ends_with(".asm.js")) {
1131-
dst.push(e.path());
1074+
fn envify(s: &str) -> String {
1075+
s.chars().map(|c| {
1076+
match c {
1077+
'-' => '_',
1078+
c => c,
11321079
}
1133-
}
1134-
dst
1080+
}).flat_map(|c| c.to_uppercase()).collect()
11351081
}
11361082

11371083
/// Some test suites are run inside emulators or on remote devices, and most

Diff for: src/libstd/process.rs

+13-2
Original file line numberDiff line numberDiff line change
@@ -1417,8 +1417,19 @@ mod tests {
14171417
let output = String::from_utf8(result.stdout).unwrap();
14181418

14191419
for (ref k, ref v) in env::vars() {
1420-
// don't check android RANDOM variables
1421-
if cfg!(target_os = "android") && *k == "RANDOM" {
1420+
// Don't check android RANDOM variable which seems to change
1421+
// whenever the shell runs, and our `env_cmd` is indeed running a
1422+
// shell which means it'll get a different RANDOM than we probably
1423+
// have.
1424+
//
1425+
// Also skip env vars with `-` in the name on android because, well,
1426+
// I'm not sure. It appears though that the `set` command above does
1427+
// not print env vars with `-` in the name, so we just skip them
1428+
// here as we won't find them in the output. Note that most env vars
1429+
// use `_` instead of `-`, but our build system sets a few env vars
1430+
// with `-` in the name.
1431+
if cfg!(target_os = "android") &&
1432+
(*k == "RANDOM" || k.contains("-")) {
14221433
continue
14231434
}
14241435

0 commit comments

Comments
 (0)