Skip to content

Commit

Permalink
Auto merge of #125827 - jieyouxu:rmake-separate-tmp-dir, r=onur-ozkan
Browse files Browse the repository at this point in the history
compiletest: split rmake.rs executable from scratch directory

When implementing support for rmake.rs, I copied over the `$TMPDIR` directory logic from the legacy Makefile setup. In doing so, I also compiled recipe `rmake.rs` into executables which unfortunately are placed into `$TMPDIR` as well.

This causes a problem on Windows (as observed in PRs like #125752 (comment)) where:

- The `rmake.exe` executable is placed in `$TMPDIR`.
- We run the `rmake.exe` as a process.
- The process uses `rmake.exe` inside `$TMPDIR`.
- Windows prevents the .exe file from being deleted when the process is still alive.
- The recipe test code tries to `remove_dir_all($TMPDIR)`, which fails with access denied because `rmake.exe` is still being used.

We fix this by separating the recipe executable and the output artifacts directory:

```
base_dir/
    rmake.exe
    rmake_out/
```

We construct a base directory, unique to each run-make test, under which we place rmake.exe alongside a `rmake_out/` directory. This `rmake_out/` directory is what is passed to rmake.rs tests as `$TMPDIR`, so now `remove_dir_all($TMPDIR)` has a chance to succeed because it no longer contains `rmake.exe`.

This wasn't a problem for Makefile tests because there's no exe file under `$TMPDIR` whose process is still running when `rm -rf $TMPDIR` is called.

try-job: x86_64-msvc
  • Loading branch information
bors committed Jun 2, 2024
2 parents a83cf56 + 4562245 commit 13e2d72
Showing 1 changed file with 24 additions and 9 deletions.
33 changes: 24 additions & 9 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3431,11 +3431,23 @@ impl<'test> TestCx<'test> {
let build_root = self.config.build_base.parent().unwrap().parent().unwrap();
let build_root = cwd.join(&build_root);

let tmpdir = cwd.join(self.output_base_name());
if tmpdir.exists() {
self.aggressive_rm_rf(&tmpdir).unwrap();
// We construct the following directory tree for each rmake.rs test:
// ```
// base_dir/
// rmake.exe
// scratch/
// ```
// having the executable separate from the scratch directory allows the recipes to
// `remove_dir_all(scratch)` without running into permission denied issues because
// the executable is not under the `scratch/` directory.
//
// This setup diverges from legacy Makefile run-make tests.
let base_dir = cwd.join(self.output_base_name());
if base_dir.exists() {
self.aggressive_rm_rf(&base_dir).unwrap();
}
create_dir_all(&tmpdir).unwrap();
let rmake_out_dir = base_dir.join("rmake_out");
create_dir_all(&rmake_out_dir).unwrap();

// HACK: assume stageN-target, we only want stageN.
let stage = self.config.stage_id.split('-').next().unwrap();
Expand All @@ -3452,8 +3464,11 @@ impl<'test> TestCx<'test> {
stage_std_path.push("lib");

// Then, we need to build the recipe `rmake.rs` and link in the support library.
let recipe_bin =
tmpdir.join(if self.config.target.contains("windows") { "rmake.exe" } else { "rmake" });
let recipe_bin = base_dir.join(if self.config.target.contains("windows") {
"rmake.exe"
} else {
"rmake"
});

let mut support_lib_deps = PathBuf::new();
support_lib_deps.push(&build_root);
Expand Down Expand Up @@ -3494,7 +3509,7 @@ impl<'test> TestCx<'test> {
.env("S", &src_root)
.env("RUST_BUILD_STAGE", &self.config.stage_id)
.env("RUSTC", cwd.join(&self.config.rustc_path))
.env("TMPDIR", &tmpdir)
.env("TMPDIR", &rmake_out_dir)
.env("LD_LIB_PATH_ENVVAR", dylib_env_var())
.env(dylib_env_var(), &host_dylib_env_paths)
.env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path))
Expand Down Expand Up @@ -3530,7 +3545,7 @@ impl<'test> TestCx<'test> {
let dylib_env_paths = env::join_paths(dylib_env_paths).unwrap();

let mut target_rpath_env_path = Vec::new();
target_rpath_env_path.push(&tmpdir);
target_rpath_env_path.push(&rmake_out_dir);
target_rpath_env_path.extend(&orig_dylib_env_paths);
let target_rpath_env_path = env::join_paths(target_rpath_env_path).unwrap();

Expand All @@ -3546,7 +3561,7 @@ impl<'test> TestCx<'test> {
.env("S", &src_root)
.env("RUST_BUILD_STAGE", &self.config.stage_id)
.env("RUSTC", cwd.join(&self.config.rustc_path))
.env("TMPDIR", &tmpdir)
.env("TMPDIR", &rmake_out_dir)
.env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path))
.env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path))
.env("LLVM_COMPONENTS", &self.config.llvm_components)
Expand Down

0 comments on commit 13e2d72

Please sign in to comment.