Skip to content

Commit 6aa7115

Browse files
committed
Auto merge of rust-lang#127958 - jieyouxu:compiletest-rmake-cleanup, r=<try>
Cleanup rmake.rs setup in compiletest While debugging rmake.rs tests I realized that the rmake.rs setup itself in compiletest is very messy and confused. Now that I know some of the bootstrap steps and the rmake.rs tests themselves better, I realized there are cleanups that are possible: - Rework how `source_root` and `build_root` are calculated. They should now be less fragile then before. - Shuffle around path calculations to make them more logically grouped and closer to eventual use site(s). - Cleanup executable extension calculation with `std::env::consts::EXE_EXTENSION`. - Cleanup various dylib search path handling: renamed variables to better reflect their purpose, minimized mutability scope of said variables. - Prune useless env vars passed to both `rustc` and recipe binary commands. - Vastly improve the documentation for the setup of rmake.rs tests, including assumed bootstrap-provided build layouts, rmake.rs test layout, dylib search paths, intended purpose of passed env vars and the `COMPILETEST_FORCE_STAGE0=1 ./x test run-make --stage 0` stage0 sysroot special handling. r? bootstrap (or Kobzol, or Mark, or T-compiler) try-job: aarch64-apple try-job: armhf-gnu try-job: dist-x86_64-linux try-job: test-various try-job: x86_64-mingw try-job: x86_64-msvc try-job: x86_64-gnu-llvm-18
2 parents 3d68afc + bf8ad90 commit 6aa7115

File tree

1 file changed

+214
-86
lines changed

1 file changed

+214
-86
lines changed

src/tools/compiletest/src/runtest.rs

+214-86
Original file line numberDiff line numberDiff line change
@@ -3431,29 +3431,52 @@ impl<'test> TestCx<'test> {
34313431

34323432
fn run_rmake_v2_test(&self) {
34333433
// For `run-make` V2, we need to perform 2 steps to build and run a `run-make` V2 recipe
3434-
// (`rmake.rs`) to run the actual tests. The support library is already built as a tool
3435-
// dylib and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`.
3434+
// (`rmake.rs`) to run the actual tests. The support library is already built as a tool rust
3435+
// library and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`.
34363436
//
3437-
// 1. We need to build the recipe `rmake.rs` and link in the support library.
3438-
// 2. We need to run the recipe to build and run the tests.
3439-
let cwd = env::current_dir().unwrap();
3440-
let src_root = self.config.src_base.parent().unwrap().parent().unwrap();
3441-
let src_root = cwd.join(&src_root);
3442-
let build_root = self.config.build_base.parent().unwrap().parent().unwrap();
3443-
let build_root = cwd.join(&build_root);
3437+
// 1. We need to build the recipe `rmake.rs` as a binary and link in the `run_make_support`
3438+
// library.
3439+
// 2. We need to run the recipe binary.
3440+
3441+
// So we assume the rust-lang/rust project setup looks like (our `.` is the top-level
3442+
// directory, irrelevant entries to our purposes omitted):
3443+
//
3444+
// ```
3445+
// . // <- `source_root`
3446+
// ├── build/ // <- `build_root`
3447+
// ├── compiler/
3448+
// ├── library/
3449+
// ├── src/
3450+
// │ └── tools/
3451+
// │ └── run_make_support/
3452+
// └── tests
3453+
// └── run-make/
3454+
// ```
3455+
3456+
// `source_root` is the top-level directory containing the rust-lang/rust checkout.
3457+
let source_root =
3458+
self.config.find_rust_src_root().expect("could not determine rust source root");
3459+
debug!(?source_root);
3460+
// `self.config.build_base` looks like `build/<target_tuplet>/test/run-make`. We
3461+
// could try to `.parent()` twice, or we could just reconstruct the `build/<target_tuplet>`
3462+
// starting from `source_root`.
3463+
debug!(?self.config.target);
3464+
let build_root = source_root.join("build").join(&self.config.target);
3465+
debug!(?build_root);
34443466

34453467
// We construct the following directory tree for each rmake.rs test:
34463468
// ```
3447-
// base_dir/
3469+
// <base_dir>/
34483470
// rmake.exe
34493471
// rmake_out/
34503472
// ```
3451-
// having the executable separate from the output artifacts directory allows the recipes to
3452-
// `remove_dir_all($TMPDIR)` without running into permission denied issues because
3453-
// the executable is not under the `rmake_out/` directory.
3473+
// having the recipe executable separate from the output artifacts directory allows the
3474+
// recipes to `remove_dir_all($TMPDIR)` without running into issues related trying to remove
3475+
// a currently running executable because the recipe executable is not under the
3476+
// `rmake_out/` directory.
34543477
//
34553478
// This setup intentionally diverges from legacy Makefile run-make tests.
3456-
let base_dir = cwd.join(self.output_base_name());
3479+
let base_dir = self.output_base_name();
34573480
if base_dir.exists() {
34583481
self.aggressive_rm_rf(&base_dir).unwrap();
34593482
}
@@ -3475,120 +3498,225 @@ impl<'test> TestCx<'test> {
34753498
}
34763499
}
34773500

3478-
// HACK: assume stageN-target, we only want stageN.
3501+
// `self.config.stage_id` looks like `stage1-<target_tuplet>`, but we only want
3502+
// the `stage1` part as that is what the output directories of bootstrap are prefixed with.
3503+
// Note that this *assumes* build layout from bootstrap is produced as:
3504+
//
3505+
// ```
3506+
// build/<target_tuplet>/ // <- this is `build_root`
3507+
// ├── stage0
3508+
// ├── stage0-bootstrap-tools
3509+
// ├── stage0-codegen
3510+
// ├── stage0-rustc
3511+
// ├── stage0-std
3512+
// ├── stage0-sysroot
3513+
// ├── stage0-tools
3514+
// ├── stage0-tools-bin
3515+
// ├── stage1
3516+
// ├── stage1-std
3517+
// ├── stage1-tools
3518+
// ├── stage1-tools-bin
3519+
// └── test
3520+
// ```
3521+
// FIXME(jieyouxu): improve the communication between bootstrap and compiletest here so
3522+
// we don't have to hack out a `stageN`.
3523+
debug!(?self.config.stage_id);
34793524
let stage = self.config.stage_id.split('-').next().unwrap();
34803525

3481-
// First, we construct the path to the built support library.
3482-
let mut support_lib_path = PathBuf::new();
3483-
support_lib_path.push(&build_root);
3484-
support_lib_path.push(format!("{}-tools-bin", stage));
3485-
support_lib_path.push("librun_make_support.rlib");
3486-
3487-
let mut stage_std_path = PathBuf::new();
3488-
stage_std_path.push(&build_root);
3489-
stage_std_path.push(&stage);
3490-
stage_std_path.push("lib");
3491-
3492-
// Then, we need to build the recipe `rmake.rs` and link in the support library.
3493-
let recipe_bin = base_dir.join(if self.config.target.contains("windows") {
3494-
"rmake.exe"
3495-
} else {
3496-
"rmake"
3497-
});
3498-
3499-
let mut support_lib_deps = PathBuf::new();
3500-
support_lib_deps.push(&build_root);
3501-
support_lib_deps.push(format!("{}-tools", stage));
3502-
support_lib_deps.push(&self.config.host);
3503-
support_lib_deps.push("release");
3504-
support_lib_deps.push("deps");
3505-
3506-
let mut support_lib_deps_deps = PathBuf::new();
3507-
support_lib_deps_deps.push(&build_root);
3508-
support_lib_deps_deps.push(format!("{}-tools", stage));
3509-
support_lib_deps_deps.push("release");
3510-
support_lib_deps_deps.push("deps");
3526+
// In order to link in the support library as a rlib when compiling recipes, we need three
3527+
// paths:
3528+
// 1. Path of the built support library rlib itself.
3529+
// 2. Path of the built support library's dependencies directory.
3530+
// 3. Path of the built support library's dependencies' dependencies directory.
3531+
//
3532+
// The paths look like
3533+
//
3534+
// ```
3535+
// build/<target_tuplet>/
3536+
// ├── stageN-tools-bin/
3537+
// │ └── librun_make_support.rlib // <- support rlib itself
3538+
// ├── stageN-tools/
3539+
// │ ├── release/deps/ // <- deps of deps
3540+
// │ └── <host_tuplet>/release/deps/ // <- deps
3541+
// ```
3542+
//
3543+
// There almost certainly is a better way to do this, but this seems to work for now.
35113544

3545+
let support_lib_path = {
3546+
let mut p = build_root.clone();
3547+
p.push(format!("{}-tools-bin", stage));
3548+
p.push("librun_make_support.rlib");
3549+
p
3550+
};
3551+
debug!(?support_lib_path);
3552+
3553+
let support_lib_deps = {
3554+
let mut p = build_root.clone();
3555+
p.push(format!("{}-tools", stage));
3556+
p.push(&self.config.host);
3557+
p.push("release");
3558+
p.push("deps");
3559+
p
3560+
};
35123561
debug!(?support_lib_deps);
3562+
3563+
let support_lib_deps_deps = {
3564+
let mut p = build_root.clone();
3565+
p.push(format!("{}-tools", stage));
3566+
p.push("release");
3567+
p.push("deps");
3568+
p
3569+
};
35133570
debug!(?support_lib_deps_deps);
35143571

3515-
let orig_dylib_env_paths =
3572+
// To compile the recipe with rustc, we need to provide suitable dynamic library search
3573+
// paths to rustc. This includes both:
3574+
// 1. The "base" dylib search paths that was provided to compiletest, e.g. `LD_LIBRARY_PATH`
3575+
// on some linux distros.
3576+
// 2. Specific library paths in `self.config.compile_lib_path` needed for running rustc.
3577+
3578+
let base_dylib_search_paths =
35163579
Vec::from_iter(env::split_paths(&env::var(dylib_env_var()).unwrap()));
35173580

3518-
let mut host_dylib_env_paths = Vec::new();
3519-
host_dylib_env_paths.push(cwd.join(&self.config.compile_lib_path));
3520-
host_dylib_env_paths.extend(orig_dylib_env_paths.iter().cloned());
3521-
let host_dylib_env_paths = env::join_paths(host_dylib_env_paths).unwrap();
3581+
let host_dylib_search_paths = {
3582+
let mut paths = vec![self.config.compile_lib_path.clone()];
3583+
paths.extend(base_dylib_search_paths.iter().cloned());
3584+
paths
3585+
};
3586+
3587+
// Calculate the paths of the recipe binary. As previously discussed, this is placed at
3588+
// `<base_dir>/<bin_name>` with `bin_name` being `rmake` or `rmake.exe` dependending on
3589+
// platform.
3590+
let recipe_bin = {
3591+
let mut p = base_dir.join("rmake");
3592+
p.set_extension(env::consts::EXE_EXTENSION);
3593+
p
3594+
};
3595+
debug!(?recipe_bin);
35223596

3523-
let mut cmd = Command::new(&self.config.rustc_path);
3524-
cmd.arg("-o")
3597+
let mut rustc = Command::new(&self.config.rustc_path);
3598+
rustc
3599+
// Specify output path
3600+
.arg("-o")
35253601
.arg(&recipe_bin)
3602+
// Specify library search paths for `run_make_support`.
35263603
.arg(format!("-Ldependency={}", &support_lib_path.parent().unwrap().to_string_lossy()))
35273604
.arg(format!("-Ldependency={}", &support_lib_deps.to_string_lossy()))
35283605
.arg(format!("-Ldependency={}", &support_lib_deps_deps.to_string_lossy()))
3606+
// Provide `run_make_support` as extern prelude, so test writers don't need to write
3607+
// `extern run_make_support;`.
35293608
.arg("--extern")
35303609
.arg(format!("run_make_support={}", &support_lib_path.to_string_lossy()))
3610+
// Default to Edition 2021.
35313611
.arg("--edition=2021")
3612+
// Specify compilation target.
3613+
.arg("--target")
3614+
.arg(&self.config.target)
3615+
// The recipe file itself.
35323616
.arg(&self.testpaths.file.join("rmake.rs"))
3533-
.env("TARGET", &self.config.target)
3534-
.env("PYTHON", &self.config.python)
3535-
.env("RUST_BUILD_STAGE", &self.config.stage_id)
3536-
.env("RUSTC", cwd.join(&self.config.rustc_path))
3537-
.env("LD_LIB_PATH_ENVVAR", dylib_env_var())
3538-
.env(dylib_env_var(), &host_dylib_env_paths)
3539-
.env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path))
3540-
.env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path))
3541-
.env("LLVM_COMPONENTS", &self.config.llvm_components);
3617+
// Provide necessary library search paths for rustc.
3618+
.env(dylib_env_var(), &env::join_paths(host_dylib_search_paths).unwrap());
35423619

35433620
// In test code we want to be very pedantic about values being silently discarded that are
35443621
// annotated with `#[must_use]`.
3545-
cmd.arg("-Dunused_must_use");
3546-
3622+
rustc.arg("-Dunused_must_use");
3623+
3624+
// > `cg_clif` uses `COMPILETEST_FORCE_STAGE0=1 ./x.py test --stage 0` for running the rustc
3625+
// > test suite. With the introduction of rmake.rs this broke. `librun_make_support.rlib` is
3626+
// > compiled using the bootstrap rustc wrapper which sets `--sysroot
3627+
// > build/aarch64-unknown-linux-gnu/stage0-sysroot`, but then compiletest will compile
3628+
// > `rmake.rs` using the sysroot of the bootstrap compiler causing it to not find the
3629+
// > `libstd.rlib` against which `librun_make_support.rlib` is compiled.
3630+
//
3631+
// The gist here is that we have to pass the proper stage0 sysroot if we want
3632+
//
3633+
// ```
3634+
// $ COMPILETEST_FORCE_STAGE0=1 ./x test run-make --stage 0
3635+
// ```
3636+
//
3637+
// to work correctly.
3638+
//
3639+
// See <https://github.com/rust-lang/rust/pull/122248> for more background.
35473640
if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() {
3548-
let mut stage0_sysroot = build_root.clone();
3549-
stage0_sysroot.push("stage0-sysroot");
3641+
let stage0_sysroot = {
3642+
let mut p = build_root.clone();
3643+
p.push("stage0-sysroot");
3644+
p
3645+
};
35503646
debug!(?stage0_sysroot);
3551-
debug!(exists = stage0_sysroot.exists());
35523647

3553-
cmd.arg("--sysroot").arg(&stage0_sysroot);
3648+
rustc.arg("--sysroot").arg(&stage0_sysroot);
35543649
}
35553650

3556-
let res = self.run_command_to_procres(&mut cmd);
3651+
// Now run rustc to build the recipe.
3652+
let res = self.run_command_to_procres(&mut rustc);
35573653
if !res.status.success() {
35583654
self.fatal_proc_rec("run-make test failed: could not build `rmake.rs` recipe", &res);
35593655
}
35603656

3561-
// Finally, we need to run the recipe binary to build and run the actual tests.
3562-
debug!(?recipe_bin);
3657+
// To actually run the recipe, we have to provide the recipe with a bunch of information
3658+
// provided through env vars.
35633659

3564-
let mut dylib_env_paths = orig_dylib_env_paths.clone();
3565-
dylib_env_paths.push(support_lib_path.parent().unwrap().to_path_buf());
3566-
dylib_env_paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib"));
3567-
let dylib_env_paths = env::join_paths(dylib_env_paths).unwrap();
3660+
// Compute stage-specific standard library paths.
3661+
let stage_std_path = {
3662+
let mut p = build_root.clone();
3663+
p.push(&stage);
3664+
p.push("lib");
3665+
p
3666+
};
3667+
debug!(?stage_std_path);
3668+
3669+
// Compute dynamic library search paths for recipes.
3670+
let recipe_dylib_search_paths = {
3671+
let mut paths = base_dylib_search_paths.clone();
3672+
paths.push(support_lib_path.parent().unwrap().to_path_buf());
3673+
paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib"));
3674+
paths
3675+
};
3676+
debug!(?recipe_dylib_search_paths);
35683677

3569-
let mut target_rpath_env_path = Vec::new();
3570-
target_rpath_env_path.push(&rmake_out_dir);
3571-
target_rpath_env_path.extend(&orig_dylib_env_paths);
3572-
let target_rpath_env_path = env::join_paths(target_rpath_env_path).unwrap();
3678+
// Compute runtime library search paths for recipes. This is target-specific.
3679+
let target_runtime_dylib_search_paths = {
3680+
let mut paths = vec![rmake_out_dir.clone()];
3681+
paths.extend(base_dylib_search_paths.iter().cloned());
3682+
paths
3683+
};
35733684

3685+
// FIXME(jieyouxu): please rename `TARGET_RPATH_ENV`, `HOST_RPATH_DIR` and
3686+
// `TARGET_RPATH_DIR`, it is **extremely** confusing!
35743687
let mut cmd = Command::new(&recipe_bin);
35753688
cmd.current_dir(&rmake_out_dir)
35763689
.stdout(Stdio::piped())
35773690
.stderr(Stdio::piped())
3691+
// Provide the target-specific env var that is used to record dylib search paths. For
3692+
// example, this could be `LD_LIBRARY_PATH` on some linux distros but `PATH` on Windows.
35783693
.env("LD_LIB_PATH_ENVVAR", dylib_env_var())
3579-
.env("TARGET_RPATH_ENV", &target_rpath_env_path)
3580-
.env(dylib_env_var(), &dylib_env_paths)
3694+
// Provide the dylib search paths.
3695+
.env(dylib_env_var(), &env::join_paths(recipe_dylib_search_paths).unwrap())
3696+
// Provide runtime dylib search paths.
3697+
.env("TARGET_RPATH_ENV", &env::join_paths(target_runtime_dylib_search_paths).unwrap())
3698+
// Provide the target.
35813699
.env("TARGET", &self.config.target)
3700+
// Some tests unfortunately still need Python, so provide path to a Python interpreter.
35823701
.env("PYTHON", &self.config.python)
3583-
.env("SOURCE_ROOT", &src_root)
3584-
.env("RUST_BUILD_STAGE", &self.config.stage_id)
3585-
.env("RUSTC", cwd.join(&self.config.rustc_path))
3586-
.env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path))
3587-
.env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path))
3702+
// Provide path to checkout root. This is the top-level directory containing
3703+
// rust-lang/rust checkout.
3704+
.env("SOURCE_ROOT", &source_root)
3705+
// Provide path to stage-corresponding rustc.
3706+
.env("RUSTC", &self.config.rustc_path)
3707+
// Provide the directory to libraries that are needed to run the *compiler*. This is not
3708+
// to be confused with `TARGET_RPATH_ENV` or `TARGET_RPATH_DIR`. This is needed if the
3709+
// recipe wants to invoke rustc.
3710+
.env("HOST_RPATH_DIR", &self.config.compile_lib_path)
3711+
// Provide the directory to libraries that might be needed to run compiled binaries
3712+
// (further compiled by the recipe!).
3713+
.env("TARGET_RPATH_DIR", &self.config.run_lib_path)
3714+
// Provide which LLVM components are available (e.g. which LLVM components are provided
3715+
// through a specific CI runner).
35883716
.env("LLVM_COMPONENTS", &self.config.llvm_components);
35893717

35903718
if let Some(ref rustdoc) = self.config.rustdoc_path {
3591-
cmd.env("RUSTDOC", cwd.join(rustdoc));
3719+
cmd.env("RUSTDOC", source_root.join(rustdoc));
35923720
}
35933721

35943722
if let Some(ref node) = self.config.nodejs {

0 commit comments

Comments
 (0)