compiletest: split rmake executable from scratch dir

jieyouxu · jieyouxu · commit 45622450f8bc · 2024-06-02T06:32:15.000Z
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 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 sratch
directory:

```
base_dir/
    rmake.exe
    scratch/
```

We construct a base directory, unique to each run-make test, under
which we place rmake.exe alongside a `scratch/` directory. This
`scratch/` 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`.

Oops. This was a fun one to try figure out.
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
@@ -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();
@@ -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);
@@ -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))
@@ -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();
 
@@ -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)
