From 45622450f8bcf4e39238ade6830873a30438e173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 31 May 2024 17:52:41 +0000 Subject: [PATCH] compiletest: split rmake executable from scratch dir 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. --- src/tools/compiletest/src/runtest.rs | 33 ++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 79e158992d4..1c2e3c40671 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3431,11 +3431,23 @@ fn run_rmake_v2_test(&self) { 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 @@ fn run_rmake_v2_test(&self) { 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 @@ fn run_rmake_v2_test(&self) { .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 @@ fn run_rmake_v2_test(&self) { 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 @@ fn run_rmake_v2_test(&self) { .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)