From 1b4972bc28f4b4658cd011e159b5aac7c98e9944 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, 19 Jul 2024 08:07:40 +0000 Subject: [PATCH 01/16] compiletest/rmake: adjust docs for rmake.rs setup and add FIXMEs --- src/tools/compiletest/src/runtest.rs | 34 ++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 0122886961737..140fc04eb89dc 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3433,14 +3433,19 @@ impl<'test> TestCx<'test> { fn run_rmake_v2_test(&self) { // For `run-make` V2, we need to perform 2 steps to build and run a `run-make` V2 recipe - // (`rmake.rs`) to run the actual tests. The support library is already built as a tool - // dylib and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`. + // (`rmake.rs`) to run the actual tests. The support library is already built as a tool rust + // library and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`. // - // 1. We need to build the recipe `rmake.rs` and link in the support library. - // 2. We need to run the recipe to build and run the tests. + // 1. We need to build the recipe `rmake.rs` as a binary and link in the `run_make_support` + // library. + // 2. We need to run the recipe binary. + + // FIXME(jieyouxu): hm, cwd doesn't look right here? let cwd = env::current_dir().unwrap(); + // FIXME(jieyouxu): is there a better way to get `src_root`? let src_root = self.config.src_base.parent().unwrap().parent().unwrap(); let src_root = cwd.join(&src_root); + // FIXME(jieyouxu): is there a better way to get `build_root`? let build_root = self.config.build_base.parent().unwrap().parent().unwrap(); let build_root = cwd.join(&build_root); @@ -3450,11 +3455,13 @@ impl<'test> TestCx<'test> { // rmake.exe // rmake_out/ // ``` - // having the executable separate from the output artifacts directory allows the recipes to - // `remove_dir_all($TMPDIR)` without running into permission denied issues because - // the executable is not under the `rmake_out/` directory. + // having the recipe executable separate from the output artifacts directory allows the + // recipes to `remove_dir_all($TMPDIR)` without running into issues related trying to remove + // a currently running executable because the recipe executable is not under the + // `rmake_out/` directory. // // This setup intentionally diverges from legacy Makefile run-make tests. + // FIXME(jieyouxu): is there a better way to compute `base_dir`? let base_dir = cwd.join(self.output_base_name()); if base_dir.exists() { self.aggressive_rm_rf(&base_dir).unwrap(); @@ -3477,10 +3484,13 @@ impl<'test> TestCx<'test> { } } + // FIXME(jieyouxu): is there a better way to get the stage number or otherwise compute the + // required stage-specific build directories? // HACK: assume stageN-target, we only want stageN. let stage = self.config.stage_id.split('-').next().unwrap(); // First, we construct the path to the built support library. + // FIXME(jieyouxu): explain what the hecc we are doing here. let mut support_lib_path = PathBuf::new(); support_lib_path.push(&build_root); support_lib_path.push(format!("{}-tools-bin", stage)); @@ -3492,12 +3502,14 @@ impl<'test> TestCx<'test> { stage_std_path.push("lib"); // Then, we need to build the recipe `rmake.rs` and link in the support library. + // FIXME(jieyouxu): use `std::env::consts::EXE_EXTENSION`. let recipe_bin = base_dir.join(if self.config.target.contains("windows") { "rmake.exe" } else { "rmake" }); + // FIXME(jieyouxu): explain what the hecc we are doing here. let mut support_lib_deps = PathBuf::new(); support_lib_deps.push(&build_root); support_lib_deps.push(format!("{}-tools", stage)); @@ -3505,6 +3517,7 @@ impl<'test> TestCx<'test> { support_lib_deps.push("release"); support_lib_deps.push("deps"); + // FIXME(jieyouxu): explain what the hecc we are doing here. let mut support_lib_deps_deps = PathBuf::new(); support_lib_deps_deps.push(&build_root); support_lib_deps_deps.push(format!("{}-tools", stage)); @@ -3514,6 +3527,7 @@ impl<'test> TestCx<'test> { debug!(?support_lib_deps); debug!(?support_lib_deps_deps); + // FIXME(jieyouxu): explain what the hecc we are doing here. let orig_dylib_env_paths = Vec::from_iter(env::split_paths(&env::var(dylib_env_var()).unwrap())); @@ -3522,6 +3536,8 @@ impl<'test> TestCx<'test> { host_dylib_env_paths.extend(orig_dylib_env_paths.iter().cloned()); let host_dylib_env_paths = env::join_paths(host_dylib_env_paths).unwrap(); + // FIXME(jieyouxu): explain what the hecc we are doing here. + // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! let mut cmd = Command::new(&self.config.rustc_path); cmd.arg("-o") .arg(&recipe_bin) @@ -3563,16 +3579,20 @@ impl<'test> TestCx<'test> { // Finally, we need to run the recipe binary to build and run the actual tests. debug!(?recipe_bin); + // FIXME(jieyouxu): explain what the hecc we are doing here. let mut dylib_env_paths = orig_dylib_env_paths.clone(); dylib_env_paths.push(support_lib_path.parent().unwrap().to_path_buf()); dylib_env_paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib")); let dylib_env_paths = env::join_paths(dylib_env_paths).unwrap(); + // FIXME(jieyouxu): explain what the hecc we are doing here. let mut target_rpath_env_path = Vec::new(); 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(); + // FIXME(jieyouxu): explain what the hecc we are doing here. + // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! let mut cmd = Command::new(&recipe_bin); cmd.current_dir(&rmake_out_dir) .stdout(Stdio::piped()) From 23f32f44ddda29bf469cb273d228c5dae384bde8 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, 19 Jul 2024 08:42:49 +0000 Subject: [PATCH 02/16] compiletest/rmake: make `{source,build}_root` path calculation more robust for rmake.rs setup --- src/tools/compiletest/src/runtest.rs | 65 ++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 140fc04eb89dc..5bc0ecd1fb49b 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3440,14 +3440,42 @@ impl<'test> TestCx<'test> { // library. // 2. We need to run the recipe binary. - // FIXME(jieyouxu): hm, cwd doesn't look right here? - let cwd = env::current_dir().unwrap(); - // FIXME(jieyouxu): is there a better way to get `src_root`? - let src_root = self.config.src_base.parent().unwrap().parent().unwrap(); - let src_root = cwd.join(&src_root); - // FIXME(jieyouxu): is there a better way to get `build_root`? - let build_root = self.config.build_base.parent().unwrap().parent().unwrap(); - let build_root = cwd.join(&build_root); + // FIXME(jieyouxu): path examples + // source_root="/home/gh-jieyouxu/rust" + // src_root="/home/gh-jieyouxu/rust" + // build_root="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu" + // self.config.build_base="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/test/run-make" + // support_lib_deps="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/stage1-tools/aarch64-unknown-linux-gnu/release/deps" + // support_lib_deps_deps="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/stage1-tools/release/deps" + // recipe_bin="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/test/run-make/a-b-a-linker-guard/a-b-a-linker-guard/rmake" + + // So we assume the rust-lang/rust project setup looks like (our `.` is the top-level + // directory, irrelevant entries to our purposes omitted): + // + // ``` + // . // <- `source_root` + // ├── build/ // <- `build_root` + // ├── compiler/ + // ├── library/ + // ├── src/ + // │ └── tools/ + // │ └── run_make_support/ + // └── tests + // └── run-make/ + // ``` + + // `source_root` is the top-level directory containing the rust-lang/rust checkout. + let source_root = + self.config.find_rust_src_root().expect("could not determine rust source root"); + debug!(?source_root); + // `self.config.build_base` is actually the build base folder + "test" + test suite name, it + // looks like `build//test/run-make`. But we want `build//`. Note + // that the `build` directory does not need to be called `build`, nor does it need to be + // under `source_root`, so we must compute it based off of `self.config.build_base`. + debug!(?self.config.build_base); + let build_root = + self.config.build_base.parent().and_then(Path::parent).unwrap().to_path_buf(); + debug!(?build_root); // We construct the following directory tree for each rmake.rs test: // ``` @@ -3462,7 +3490,7 @@ impl<'test> TestCx<'test> { // // This setup intentionally diverges from legacy Makefile run-make tests. // FIXME(jieyouxu): is there a better way to compute `base_dir`? - let base_dir = cwd.join(self.output_base_name()); + let base_dir = self.output_base_name(); if base_dir.exists() { self.aggressive_rm_rf(&base_dir).unwrap(); } @@ -3532,7 +3560,7 @@ impl<'test> TestCx<'test> { Vec::from_iter(env::split_paths(&env::var(dylib_env_var()).unwrap())); let mut host_dylib_env_paths = Vec::new(); - host_dylib_env_paths.push(cwd.join(&self.config.compile_lib_path)); + host_dylib_env_paths.push(self.config.compile_lib_path.clone()); host_dylib_env_paths.extend(orig_dylib_env_paths.iter().cloned()); let host_dylib_env_paths = env::join_paths(host_dylib_env_paths).unwrap(); @@ -3551,17 +3579,18 @@ impl<'test> TestCx<'test> { .env("TARGET", &self.config.target) .env("PYTHON", &self.config.python) .env("RUST_BUILD_STAGE", &self.config.stage_id) - .env("RUSTC", cwd.join(&self.config.rustc_path)) + .env("RUSTC", &self.config.rustc_path) .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)) - .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path)) + .env("HOST_RPATH_DIR", &self.config.compile_lib_path) + .env("TARGET_RPATH_DIR", &self.config.run_lib_path) .env("LLVM_COMPONENTS", &self.config.llvm_components); // In test code we want to be very pedantic about values being silently discarded that are // annotated with `#[must_use]`. cmd.arg("-Dunused_must_use"); + // FIXME(jieyouxu): explain this! if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() { let mut stage0_sysroot = build_root.clone(); stage0_sysroot.push("stage0-sysroot"); @@ -3602,15 +3631,15 @@ impl<'test> TestCx<'test> { .env(dylib_env_var(), &dylib_env_paths) .env("TARGET", &self.config.target) .env("PYTHON", &self.config.python) - .env("SOURCE_ROOT", &src_root) + .env("SOURCE_ROOT", &source_root) .env("RUST_BUILD_STAGE", &self.config.stage_id) - .env("RUSTC", cwd.join(&self.config.rustc_path)) - .env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path)) - .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path)) + .env("RUSTC", &self.config.rustc_path) + .env("HOST_RPATH_DIR", &self.config.compile_lib_path) + .env("TARGET_RPATH_DIR", &self.config.run_lib_path) .env("LLVM_COMPONENTS", &self.config.llvm_components); if let Some(ref rustdoc) = self.config.rustdoc_path { - cmd.env("RUSTDOC", cwd.join(rustdoc)); + cmd.env("RUSTDOC", source_root.join(rustdoc)); } if let Some(ref node) = self.config.nodejs { From 6ca31099e3cde3c91698e693c98dde84df225bd5 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, 19 Jul 2024 09:10:58 +0000 Subject: [PATCH 03/16] compiletest/rmake: improve `stage` explanation --- src/tools/compiletest/src/runtest.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 5bc0ecd1fb49b..c8fc84ef3fb1a 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3489,7 +3489,6 @@ impl<'test> TestCx<'test> { // `rmake_out/` directory. // // This setup intentionally diverges from legacy Makefile run-make tests. - // FIXME(jieyouxu): is there a better way to compute `base_dir`? let base_dir = self.output_base_name(); if base_dir.exists() { self.aggressive_rm_rf(&base_dir).unwrap(); @@ -3512,9 +3511,29 @@ impl<'test> TestCx<'test> { } } - // FIXME(jieyouxu): is there a better way to get the stage number or otherwise compute the - // required stage-specific build directories? - // HACK: assume stageN-target, we only want stageN. + // `self.config.stage_id` looks like `stage1-`, but we only want + // the `stage1` part as that is what the output directories of bootstrap are prefixed with. + // Note that this *assumes* build layout from bootstrap is produced as: + // + // ``` + // build// // <- this is `build_root` + // ├── stage0 + // ├── stage0-bootstrap-tools + // ├── stage0-codegen + // ├── stage0-rustc + // ├── stage0-std + // ├── stage0-sysroot + // ├── stage0-tools + // ├── stage0-tools-bin + // ├── stage1 + // ├── stage1-std + // ├── stage1-tools + // ├── stage1-tools-bin + // └── test + // ``` + // FIXME(jieyouxu): improve the communication between bootstrap and compiletest here so + // we don't have to hack out a `stageN`. + debug!(?self.config.stage_id); let stage = self.config.stage_id.split('-').next().unwrap(); // First, we construct the path to the built support library. From 2eb8810d322470d2b7f72ac08c10824cc147a9b9 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, 19 Jul 2024 09:19:44 +0000 Subject: [PATCH 04/16] compiletest/rmake: move `stage_std_path` and `recipe_bin` closer to use site --- src/tools/compiletest/src/runtest.rs | 29 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c8fc84ef3fb1a..2efd9820172b6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3543,19 +3543,6 @@ impl<'test> TestCx<'test> { support_lib_path.push(format!("{}-tools-bin", stage)); support_lib_path.push("librun_make_support.rlib"); - let mut stage_std_path = PathBuf::new(); - stage_std_path.push(&build_root); - stage_std_path.push(&stage); - stage_std_path.push("lib"); - - // Then, we need to build the recipe `rmake.rs` and link in the support library. - // FIXME(jieyouxu): use `std::env::consts::EXE_EXTENSION`. - let recipe_bin = base_dir.join(if self.config.target.contains("windows") { - "rmake.exe" - } else { - "rmake" - }); - // FIXME(jieyouxu): explain what the hecc we are doing here. let mut support_lib_deps = PathBuf::new(); support_lib_deps.push(&build_root); @@ -3583,6 +3570,15 @@ impl<'test> TestCx<'test> { host_dylib_env_paths.extend(orig_dylib_env_paths.iter().cloned()); let host_dylib_env_paths = env::join_paths(host_dylib_env_paths).unwrap(); + // Finally, we need to run the recipe binary to build and run the actual tests. + // FIXME(jieyouxu): use `std::env::consts::EXE_EXTENSION`. + let recipe_bin = base_dir.join(if self.config.target.contains("windows") { + "rmake.exe" + } else { + "rmake" + }); + debug!(?recipe_bin); + // FIXME(jieyouxu): explain what the hecc we are doing here. // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! let mut cmd = Command::new(&self.config.rustc_path); @@ -3624,8 +3620,11 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec("run-make test failed: could not build `rmake.rs` recipe", &res); } - // Finally, we need to run the recipe binary to build and run the actual tests. - debug!(?recipe_bin); + // FIXME(jieyouxu): explain what the hecc we are doing here. + let mut stage_std_path = PathBuf::new(); + stage_std_path.push(&build_root); + stage_std_path.push(&stage); + stage_std_path.push("lib"); // FIXME(jieyouxu): explain what the hecc we are doing here. let mut dylib_env_paths = orig_dylib_env_paths.clone(); From 01ed951f8b3769d04782edaedc186feba3257122 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, 19 Jul 2024 09:21:37 +0000 Subject: [PATCH 05/16] compiletest/rmake: improve clarity of `support_lib_{path,deps,deps_deps}` calculations --- src/tools/compiletest/src/runtest.rs | 63 ++++++++++++++++++---------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 2efd9820172b6..bfeeb1ba1d8d1 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3536,29 +3536,50 @@ impl<'test> TestCx<'test> { debug!(?self.config.stage_id); let stage = self.config.stage_id.split('-').next().unwrap(); - // First, we construct the path to the built support library. - // FIXME(jieyouxu): explain what the hecc we are doing here. - let mut support_lib_path = PathBuf::new(); - support_lib_path.push(&build_root); - support_lib_path.push(format!("{}-tools-bin", stage)); - support_lib_path.push("librun_make_support.rlib"); - - // FIXME(jieyouxu): explain what the hecc we are doing here. - let mut support_lib_deps = PathBuf::new(); - support_lib_deps.push(&build_root); - support_lib_deps.push(format!("{}-tools", stage)); - support_lib_deps.push(&self.config.host); - support_lib_deps.push("release"); - support_lib_deps.push("deps"); - - // FIXME(jieyouxu): explain what the hecc we are doing here. - let mut support_lib_deps_deps = PathBuf::new(); - support_lib_deps_deps.push(&build_root); - support_lib_deps_deps.push(format!("{}-tools", stage)); - support_lib_deps_deps.push("release"); - support_lib_deps_deps.push("deps"); + // In order to link in the support library as a rlib when compiling recipes, we need three + // paths: + // 1. Path of the built support library rlib itself. + // 2. Path of the built support library's dependencies directory. + // 3. Path of the built support library's dependencies' dependencies directory. + // + // The paths look like + // + // ``` + // build// + // ├── stageN-tools-bin/ + // │ └── librun_make_support.rlib // <- support rlib itself + // ├── stageN-tools/ + // │ ├── release/deps/ // <- deps of deps + // │ └── /release/deps/ // <- deps + // ``` + // + // There almost certainly is a better way to do this, but this seems to work for now. + let support_lib_path = { + let mut p = build_root.clone(); + p.push(format!("{}-tools-bin", stage)); + p.push("librun_make_support.rlib"); + p + }; + debug!(?support_lib_path); + + let support_lib_deps = { + let mut p = build_root.clone(); + p.push(format!("{}-tools", stage)); + p.push(&self.config.host); + p.push("release"); + p.push("deps"); + p + }; debug!(?support_lib_deps); + + let support_lib_deps_deps = { + let mut p = build_root.clone(); + p.push(format!("{}-tools", stage)); + p.push("release"); + p.push("deps"); + p + }; debug!(?support_lib_deps_deps); // FIXME(jieyouxu): explain what the hecc we are doing here. From 49ca9ff0ac892acf6aee695ee3c6f4d7c412eaea 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, 19 Jul 2024 09:37:09 +0000 Subject: [PATCH 06/16] compiletest/rmake: cleanup dylib search paths related calculations --- src/tools/compiletest/src/runtest.rs | 40 +++++++++++++++++----------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index bfeeb1ba1d8d1..07ffbc2e9dc6d 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3583,13 +3583,18 @@ impl<'test> TestCx<'test> { debug!(?support_lib_deps_deps); // FIXME(jieyouxu): explain what the hecc we are doing here. - let orig_dylib_env_paths = + + // This is the base dynamic library search paths that was made available to compiletest. + let base_dylib_search_paths = Vec::from_iter(env::split_paths(&env::var(dylib_env_var()).unwrap())); - let mut host_dylib_env_paths = Vec::new(); - host_dylib_env_paths.push(self.config.compile_lib_path.clone()); - host_dylib_env_paths.extend(orig_dylib_env_paths.iter().cloned()); - let host_dylib_env_paths = env::join_paths(host_dylib_env_paths).unwrap(); + // We add in `self.config.compile_lib_path` which are the libraries needed to run the + // host compiler. + let host_dylib_search_paths = { + let mut paths = vec![self.config.compile_lib_path.clone()]; + paths.extend(base_dylib_search_paths.iter().cloned()); + paths + }; // Finally, we need to run the recipe binary to build and run the actual tests. // FIXME(jieyouxu): use `std::env::consts::EXE_EXTENSION`. @@ -3617,7 +3622,7 @@ impl<'test> TestCx<'test> { .env("RUST_BUILD_STAGE", &self.config.stage_id) .env("RUSTC", &self.config.rustc_path) .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env(dylib_env_var(), &host_dylib_env_paths) + .env(dylib_env_var(), &env::join_paths(host_dylib_search_paths).unwrap()) .env("HOST_RPATH_DIR", &self.config.compile_lib_path) .env("TARGET_RPATH_DIR", &self.config.run_lib_path) .env("LLVM_COMPONENTS", &self.config.llvm_components); @@ -3648,16 +3653,19 @@ impl<'test> TestCx<'test> { stage_std_path.push("lib"); // FIXME(jieyouxu): explain what the hecc we are doing here. - let mut dylib_env_paths = orig_dylib_env_paths.clone(); - dylib_env_paths.push(support_lib_path.parent().unwrap().to_path_buf()); - dylib_env_paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib")); - let dylib_env_paths = env::join_paths(dylib_env_paths).unwrap(); + let recipe_dylib_search_paths = { + let mut paths = base_dylib_search_paths.clone(); + paths.push(support_lib_path.parent().unwrap().to_path_buf()); + paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib")); + paths + }; // FIXME(jieyouxu): explain what the hecc we are doing here. - let mut target_rpath_env_path = Vec::new(); - 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(); + let target_rpaths = { + let mut paths = vec![rmake_out_dir.clone()]; + paths.extend(base_dylib_search_paths.iter().cloned()); + paths + }; // FIXME(jieyouxu): explain what the hecc we are doing here. // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! @@ -3666,8 +3674,8 @@ impl<'test> TestCx<'test> { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env("TARGET_RPATH_ENV", &target_rpath_env_path) - .env(dylib_env_var(), &dylib_env_paths) + .env("TARGET_RPATH_ENV", &env::join_paths(target_rpaths).unwrap()) + .env(dylib_env_var(), &env::join_paths(recipe_dylib_search_paths).unwrap()) .env("TARGET", &self.config.target) .env("PYTHON", &self.config.python) .env("SOURCE_ROOT", &source_root) From 9f2a660f86bc88112445cea38bcd22e0f30cbd89 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, 19 Jul 2024 09:44:22 +0000 Subject: [PATCH 07/16] compiletest/rmake: cleanup library search paths --- src/tools/compiletest/src/runtest.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 07ffbc2e9dc6d..a38a900b3d820 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3582,14 +3582,15 @@ impl<'test> TestCx<'test> { }; debug!(?support_lib_deps_deps); - // FIXME(jieyouxu): explain what the hecc we are doing here. + // To compile the recipe with rustc, we need to provide suitable dynamic library search + // paths to rustc. This includes both: + // 1. The "base" dylib search paths that was provided to compiletest, e.g. `LD_LIBRARY_PATH` + // on some linux distros. + // 2. Specific library paths in `self.config.compile_lib_path` needed for running rustc. - // This is the base dynamic library search paths that was made available to compiletest. let base_dylib_search_paths = Vec::from_iter(env::split_paths(&env::var(dylib_env_var()).unwrap())); - // We add in `self.config.compile_lib_path` which are the libraries needed to run the - // host compiler. let host_dylib_search_paths = { let mut paths = vec![self.config.compile_lib_path.clone()]; paths.extend(base_dylib_search_paths.iter().cloned()); From cf5edfe3f92e7b40f984c3e6336f441869901eec 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, 19 Jul 2024 09:45:11 +0000 Subject: [PATCH 08/16] compiletest/rmake: cleanup rmake exe extension calculation --- src/tools/compiletest/src/runtest.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a38a900b3d820..79c6371007043 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3479,7 +3479,7 @@ impl<'test> TestCx<'test> { // We construct the following directory tree for each rmake.rs test: // ``` - // base_dir/ + // / // rmake.exe // rmake_out/ // ``` @@ -3597,13 +3597,14 @@ impl<'test> TestCx<'test> { paths }; - // Finally, we need to run the recipe binary to build and run the actual tests. - // FIXME(jieyouxu): use `std::env::consts::EXE_EXTENSION`. - let recipe_bin = base_dir.join(if self.config.target.contains("windows") { - "rmake.exe" - } else { - "rmake" - }); + // Calculate the paths of the recipe binary. As previously discussed, this is placed at + // `/` with `bin_name` being `rmake` or `rmake.exe` dependending on + // platform. + let recipe_bin = { + let mut p = base_dir.join("rmake"); + p.set_extension(env::consts::EXE_EXTENSION); + p + }; debug!(?recipe_bin); // FIXME(jieyouxu): explain what the hecc we are doing here. From f5488f0c4b12aa56744da0cb212df7b7152ea580 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, 19 Jul 2024 09:49:32 +0000 Subject: [PATCH 09/16] compiletest/rmake: rename `cmd` to `rustc` --- src/tools/compiletest/src/runtest.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 79c6371007043..ba7bac18c9da7 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3609,8 +3609,9 @@ impl<'test> TestCx<'test> { // FIXME(jieyouxu): explain what the hecc we are doing here. // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! - let mut cmd = Command::new(&self.config.rustc_path); - cmd.arg("-o") + let mut rustc = Command::new(&self.config.rustc_path); + rustc + .arg("-o") .arg(&recipe_bin) .arg(format!("-Ldependency={}", &support_lib_path.parent().unwrap().to_string_lossy())) .arg(format!("-Ldependency={}", &support_lib_deps.to_string_lossy())) @@ -3631,7 +3632,7 @@ impl<'test> TestCx<'test> { // In test code we want to be very pedantic about values being silently discarded that are // annotated with `#[must_use]`. - cmd.arg("-Dunused_must_use"); + rustc.arg("-Dunused_must_use"); // FIXME(jieyouxu): explain this! if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() { @@ -3640,10 +3641,10 @@ impl<'test> TestCx<'test> { debug!(?stage0_sysroot); debug!(exists = stage0_sysroot.exists()); - cmd.arg("--sysroot").arg(&stage0_sysroot); + rustc.arg("--sysroot").arg(&stage0_sysroot); } - let res = self.run_command_to_procres(&mut cmd); + let res = self.run_command_to_procres(&mut rustc); if !res.status.success() { self.fatal_proc_rec("run-make test failed: could not build `rmake.rs` recipe", &res); } From 2383e9dd4f61d9a6f9f4334ec8a10ae0116ad107 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, 19 Jul 2024 09:56:53 +0000 Subject: [PATCH 10/16] compiletest/rmake: prune useless env vars and explain passed rustc options and env vars --- src/tools/compiletest/src/runtest.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ba7bac18c9da7..ea9b928a29ca8 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3611,24 +3611,23 @@ impl<'test> TestCx<'test> { // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! let mut rustc = Command::new(&self.config.rustc_path); rustc + // Specify output path .arg("-o") .arg(&recipe_bin) + // Specify library search paths for `run_make_support`. .arg(format!("-Ldependency={}", &support_lib_path.parent().unwrap().to_string_lossy())) .arg(format!("-Ldependency={}", &support_lib_deps.to_string_lossy())) .arg(format!("-Ldependency={}", &support_lib_deps_deps.to_string_lossy())) + // Provide `run_make_support` as extern prelude, so test writers don't need to write + // `extern run_make_support;`. .arg("--extern") .arg(format!("run_make_support={}", &support_lib_path.to_string_lossy())) + // Default to Edition 2021. .arg("--edition=2021") + // The recipe file itself. .arg(&self.testpaths.file.join("rmake.rs")) - .env("TARGET", &self.config.target) - .env("PYTHON", &self.config.python) - .env("RUST_BUILD_STAGE", &self.config.stage_id) - .env("RUSTC", &self.config.rustc_path) - .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env(dylib_env_var(), &env::join_paths(host_dylib_search_paths).unwrap()) - .env("HOST_RPATH_DIR", &self.config.compile_lib_path) - .env("TARGET_RPATH_DIR", &self.config.run_lib_path) - .env("LLVM_COMPONENTS", &self.config.llvm_components); + // Provide necessary library search paths for rustc. + .env(dylib_env_var(), &env::join_paths(host_dylib_search_paths).unwrap()); // In test code we want to be very pedantic about values being silently discarded that are // annotated with `#[must_use]`. From aa22102f2f52f35f0e48688bcc6f90de1ee48b20 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, 19 Jul 2024 10:01:44 +0000 Subject: [PATCH 11/16] compiletest/rmake: better explain why stage0 sysroot is needed if forced stage0 --- src/tools/compiletest/src/runtest.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ea9b928a29ca8..4dffef36af745 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3633,12 +3633,29 @@ impl<'test> TestCx<'test> { // annotated with `#[must_use]`. rustc.arg("-Dunused_must_use"); - // FIXME(jieyouxu): explain this! + // > `cg_clif` uses `COMPILETEST_FORCE_STAGE0=1 ./x.py test --stage 0` for running the rustc + // > test suite. With the introduction of rmake.rs this broke. `librun_make_support.rlib` is + // > compiled using the bootstrap rustc wrapper which sets `--sysroot + // > build/aarch64-unknown-linux-gnu/stage0-sysroot`, but then compiletest will compile + // > `rmake.rs` using the sysroot of the bootstrap compiler causing it to not find the + // > `libstd.rlib` against which `librun_make_support.rlib` is compiled. + // + // The gist here is that we have to pass the proper stage0 sysroot if we want + // + // ``` + // $ COMPILETEST_FORCE_STAGE0=1 ./x test run-make --stage 0 + // ``` + // + // to work correctly. + // + // See for more background. if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() { - let mut stage0_sysroot = build_root.clone(); - stage0_sysroot.push("stage0-sysroot"); + let stage0_sysroot = { + let mut p = build_root.clone(); + p.push("stage0-sysroot"); + p + }; debug!(?stage0_sysroot); - debug!(exists = stage0_sysroot.exists()); rustc.arg("--sysroot").arg(&stage0_sysroot); } From 993d83af71e133695c5b8378443bf3d49bc422ac 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, 19 Jul 2024 10:22:31 +0000 Subject: [PATCH 12/16] compiletest/rmake: cleanup `stage_std_path` and `recipe_dylib_search_paths` handling --- src/tools/compiletest/src/runtest.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 4dffef36af745..9d7585de57718 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3660,27 +3660,35 @@ impl<'test> TestCx<'test> { rustc.arg("--sysroot").arg(&stage0_sysroot); } + // Now run rustc to build the recipe. let res = self.run_command_to_procres(&mut rustc); if !res.status.success() { self.fatal_proc_rec("run-make test failed: could not build `rmake.rs` recipe", &res); } - // FIXME(jieyouxu): explain what the hecc we are doing here. - let mut stage_std_path = PathBuf::new(); - stage_std_path.push(&build_root); - stage_std_path.push(&stage); - stage_std_path.push("lib"); + // To actually run the recipe, we have to provide the recipe with a bunch of information + // provided through env vars. - // FIXME(jieyouxu): explain what the hecc we are doing here. + // Compute stage-specific standard library paths. + let stage_std_path = { + let mut p = build_root.clone(); + p.push(&stage); + p.push("lib"); + p + }; + debug!(?stage_std_path); + + // Compute dynamic library search paths for recipes. let recipe_dylib_search_paths = { let mut paths = base_dylib_search_paths.clone(); paths.push(support_lib_path.parent().unwrap().to_path_buf()); paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib")); paths }; + debug!(?recipe_dylib_search_paths); - // FIXME(jieyouxu): explain what the hecc we are doing here. - let target_rpaths = { + // Compute runtime library search paths for recipes. This is target-specific. + let target_runtime_dylib_search_paths = { let mut paths = vec![rmake_out_dir.clone()]; paths.extend(base_dylib_search_paths.iter().cloned()); paths @@ -3693,7 +3701,7 @@ impl<'test> TestCx<'test> { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env("TARGET_RPATH_ENV", &env::join_paths(target_rpaths).unwrap()) + .env("TARGET_RPATH_ENV", &env::join_paths(target_runtime_dylib_search_paths).unwrap()) .env(dylib_env_var(), &env::join_paths(recipe_dylib_search_paths).unwrap()) .env("TARGET", &self.config.target) .env("PYTHON", &self.config.python) From 3e77f7c9a54fd40d90f892ad9ecc3f53051c0e26 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, 19 Jul 2024 10:24:25 +0000 Subject: [PATCH 13/16] compiletest/rmake: prune unused `RUST_BUILD_STAGE` and explain env vars passed to recipes --- src/tools/compiletest/src/runtest.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 9d7585de57718..0b0415a5de492 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3598,7 +3598,7 @@ impl<'test> TestCx<'test> { }; // Calculate the paths of the recipe binary. As previously discussed, this is placed at - // `/` with `bin_name` being `rmake` or `rmake.exe` dependending on + // `/` with `bin_name` being `rmake` or `rmake.exe` depending on // platform. let recipe_bin = { let mut p = base_dir.join("rmake"); @@ -3611,7 +3611,6 @@ impl<'test> TestCx<'test> { // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! let mut rustc = Command::new(&self.config.rustc_path); rustc - // Specify output path .arg("-o") .arg(&recipe_bin) // Specify library search paths for `run_make_support`. @@ -3622,9 +3621,7 @@ impl<'test> TestCx<'test> { // `extern run_make_support;`. .arg("--extern") .arg(format!("run_make_support={}", &support_lib_path.to_string_lossy())) - // Default to Edition 2021. .arg("--edition=2021") - // The recipe file itself. .arg(&self.testpaths.file.join("rmake.rs")) // Provide necessary library search paths for rustc. .env(dylib_env_var(), &env::join_paths(host_dylib_search_paths).unwrap()); @@ -3696,20 +3693,37 @@ impl<'test> TestCx<'test> { // FIXME(jieyouxu): explain what the hecc we are doing here. // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! + // FIXME(jieyouxu): please rename `TARGET_RPATH_ENV`, `HOST_RPATH_DIR` and + // `TARGET_RPATH_DIR`, it is **extremely** confusing! let mut cmd = Command::new(&recipe_bin); cmd.current_dir(&rmake_out_dir) .stdout(Stdio::piped()) .stderr(Stdio::piped()) + // Provide the target-specific env var that is used to record dylib search paths. For + // example, this could be `LD_LIBRARY_PATH` on some linux distros but `PATH` on Windows. .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env("TARGET_RPATH_ENV", &env::join_paths(target_runtime_dylib_search_paths).unwrap()) + // Provide the dylib search paths. .env(dylib_env_var(), &env::join_paths(recipe_dylib_search_paths).unwrap()) + // Provide runtime dylib search paths. + .env("TARGET_RPATH_ENV", &env::join_paths(target_runtime_dylib_search_paths).unwrap()) + // Provide the target. .env("TARGET", &self.config.target) + // Some tests unfortunately still need Python, so provide path to a Python interpreter. .env("PYTHON", &self.config.python) + // Provide path to checkout root. This is the top-level directory containing + // rust-lang/rust checkout. .env("SOURCE_ROOT", &source_root) - .env("RUST_BUILD_STAGE", &self.config.stage_id) + // Provide path to stage-corresponding rustc. .env("RUSTC", &self.config.rustc_path) + // Provide the directory to libraries that are needed to run the *compiler*. This is not + // to be confused with `TARGET_RPATH_ENV` or `TARGET_RPATH_DIR`. This is needed if the + // recipe wants to invoke rustc. .env("HOST_RPATH_DIR", &self.config.compile_lib_path) + // Provide the directory to libraries that might be needed to run compiled binaries + // (further compiled by the recipe!). .env("TARGET_RPATH_DIR", &self.config.run_lib_path) + // Provide which LLVM components are available (e.g. which LLVM components are provided + // through a specific CI runner). .env("LLVM_COMPONENTS", &self.config.llvm_components); if let Some(ref rustdoc) = self.config.rustdoc_path { From c863525374a599fa52aa3093d4c83935f516d85e 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, 19 Jul 2024 10:28:35 +0000 Subject: [PATCH 14/16] compiletest/rmake: improve comments --- src/tools/compiletest/src/runtest.rs | 37 ++++++---------------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 0b0415a5de492..9ff9dbba5dfbb 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3440,17 +3440,8 @@ impl<'test> TestCx<'test> { // library. // 2. We need to run the recipe binary. - // FIXME(jieyouxu): path examples - // source_root="/home/gh-jieyouxu/rust" - // src_root="/home/gh-jieyouxu/rust" - // build_root="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu" - // self.config.build_base="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/test/run-make" - // support_lib_deps="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/stage1-tools/aarch64-unknown-linux-gnu/release/deps" - // support_lib_deps_deps="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/stage1-tools/release/deps" - // recipe_bin="/home/gh-jieyouxu/rust/build/aarch64-unknown-linux-gnu/test/run-make/a-b-a-linker-guard/a-b-a-linker-guard/rmake" - - // So we assume the rust-lang/rust project setup looks like (our `.` is the top-level - // directory, irrelevant entries to our purposes omitted): + // So we assume the rust-lang/rust project setup looks like the following (our `.` is the + // top-level directory, irrelevant entries to our purposes omitted): // // ``` // . // <- `source_root` @@ -3467,15 +3458,12 @@ impl<'test> TestCx<'test> { // `source_root` is the top-level directory containing the rust-lang/rust checkout. let source_root = self.config.find_rust_src_root().expect("could not determine rust source root"); - debug!(?source_root); // `self.config.build_base` is actually the build base folder + "test" + test suite name, it - // looks like `build//test/run-make`. But we want `build//`. Note + // looks like `build//test/run-make`. But we want `build//`. Note // that the `build` directory does not need to be called `build`, nor does it need to be // under `source_root`, so we must compute it based off of `self.config.build_base`. - debug!(?self.config.build_base); let build_root = self.config.build_base.parent().and_then(Path::parent).unwrap().to_path_buf(); - debug!(?build_root); // We construct the following directory tree for each rmake.rs test: // ``` @@ -3511,12 +3499,12 @@ impl<'test> TestCx<'test> { } } - // `self.config.stage_id` looks like `stage1-`, but we only want + // `self.config.stage_id` looks like `stage1-`, but we only want // the `stage1` part as that is what the output directories of bootstrap are prefixed with. // Note that this *assumes* build layout from bootstrap is produced as: // // ``` - // build// // <- this is `build_root` + // build// // <- this is `build_root` // ├── stage0 // ├── stage0-bootstrap-tools // ├── stage0-codegen @@ -3533,7 +3521,6 @@ impl<'test> TestCx<'test> { // ``` // FIXME(jieyouxu): improve the communication between bootstrap and compiletest here so // we don't have to hack out a `stageN`. - debug!(?self.config.stage_id); let stage = self.config.stage_id.split('-').next().unwrap(); // In order to link in the support library as a rlib when compiling recipes, we need three @@ -3545,12 +3532,12 @@ impl<'test> TestCx<'test> { // The paths look like // // ``` - // build// + // build// // ├── stageN-tools-bin/ // │ └── librun_make_support.rlib // <- support rlib itself // ├── stageN-tools/ // │ ├── release/deps/ // <- deps of deps - // │ └── /release/deps/ // <- deps + // │ └── /release/deps/ // <- deps // ``` // // There almost certainly is a better way to do this, but this seems to work for now. @@ -3561,7 +3548,6 @@ impl<'test> TestCx<'test> { p.push("librun_make_support.rlib"); p }; - debug!(?support_lib_path); let support_lib_deps = { let mut p = build_root.clone(); @@ -3571,7 +3557,6 @@ impl<'test> TestCx<'test> { p.push("deps"); p }; - debug!(?support_lib_deps); let support_lib_deps_deps = { let mut p = build_root.clone(); @@ -3580,7 +3565,6 @@ impl<'test> TestCx<'test> { p.push("deps"); p }; - debug!(?support_lib_deps_deps); // To compile the recipe with rustc, we need to provide suitable dynamic library search // paths to rustc. This includes both: @@ -3605,10 +3589,7 @@ impl<'test> TestCx<'test> { p.set_extension(env::consts::EXE_EXTENSION); p }; - debug!(?recipe_bin); - // FIXME(jieyouxu): explain what the hecc we are doing here. - // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! let mut rustc = Command::new(&self.config.rustc_path); rustc .arg("-o") @@ -3673,7 +3654,6 @@ impl<'test> TestCx<'test> { p.push("lib"); p }; - debug!(?stage_std_path); // Compute dynamic library search paths for recipes. let recipe_dylib_search_paths = { @@ -3682,7 +3662,6 @@ impl<'test> TestCx<'test> { paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib")); paths }; - debug!(?recipe_dylib_search_paths); // Compute runtime library search paths for recipes. This is target-specific. let target_runtime_dylib_search_paths = { @@ -3691,8 +3670,6 @@ impl<'test> TestCx<'test> { paths }; - // FIXME(jieyouxu): explain what the hecc we are doing here. - // FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc! // FIXME(jieyouxu): please rename `TARGET_RPATH_ENV`, `HOST_RPATH_DIR` and // `TARGET_RPATH_DIR`, it is **extremely** confusing! let mut cmd = Command::new(&recipe_bin); From 2c867d0b5f630a1e0dea48da93abe806a1816400 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: Sat, 20 Jul 2024 09:05:38 +0000 Subject: [PATCH 15/16] compiletest/rmake: avoid double test directory for rmake.rs tests This is important for other tests that have various things like modes, revisions and the like. These features are not supported in run-make tests, so we don't need the double layering. --- src/tools/compiletest/src/common.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index da7f03441e72f..bc66dcbfb944e 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -760,8 +760,14 @@ pub fn output_testname_unique( /// test/revision should reside. Example: /// /path/to/build/host-triple/test/ui/relative/testname.revision.mode/ pub fn output_base_dir(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf { - output_relative_path(config, &testpaths.relative_dir) - .join(output_testname_unique(config, testpaths, revision)) + // In run-make tests, constructing a relative path + unique testname causes a double layering + // since revisions are not supported, causing unnecessary nesting. + if config.mode == Mode::RunMake { + output_relative_path(config, &testpaths.relative_dir) + } else { + output_relative_path(config, &testpaths.relative_dir) + .join(output_testname_unique(config, testpaths, revision)) + } } /// Absolute path to the base filename used as output for the given From a8463bea91658163c18119b29233505741cc73aa 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: Sat, 20 Jul 2024 09:08:19 +0000 Subject: [PATCH 16/16] compiletest/rmake: simplify path calculations --- src/tools/compiletest/src/runtest.rs | 46 ++++++---------------------- 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 9ff9dbba5dfbb..5398820313648 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3540,31 +3540,16 @@ impl<'test> TestCx<'test> { // │ └── /release/deps/ // <- deps // ``` // - // There almost certainly is a better way to do this, but this seems to work for now. + // FIXME(jieyouxu): there almost certainly is a better way to do this (specifically how the + // support lib and its deps are organized, can't we copy them to the tools-bin dir as + // well?), but this seems to work for now. - let support_lib_path = { - let mut p = build_root.clone(); - p.push(format!("{}-tools-bin", stage)); - p.push("librun_make_support.rlib"); - p - }; + let stage_tools_bin = build_root.join(format!("{stage}-tools-bin")); + let support_lib_path = stage_tools_bin.join("librun_make_support.rlib"); - let support_lib_deps = { - let mut p = build_root.clone(); - p.push(format!("{}-tools", stage)); - p.push(&self.config.host); - p.push("release"); - p.push("deps"); - p - }; - - let support_lib_deps_deps = { - let mut p = build_root.clone(); - p.push(format!("{}-tools", stage)); - p.push("release"); - p.push("deps"); - p - }; + let stage_tools = build_root.join(format!("{stage}-tools")); + let support_lib_deps = stage_tools.join(&self.config.host).join("release").join("deps"); + let support_lib_deps_deps = stage_tools.join("release").join("deps"); // To compile the recipe with rustc, we need to provide suitable dynamic library search // paths to rustc. This includes both: @@ -3628,13 +3613,7 @@ impl<'test> TestCx<'test> { // // See for more background. if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() { - let stage0_sysroot = { - let mut p = build_root.clone(); - p.push("stage0-sysroot"); - p - }; - debug!(?stage0_sysroot); - + let stage0_sysroot = build_root.join("stage0-sysroot"); rustc.arg("--sysroot").arg(&stage0_sysroot); } @@ -3648,12 +3627,7 @@ impl<'test> TestCx<'test> { // provided through env vars. // Compute stage-specific standard library paths. - let stage_std_path = { - let mut p = build_root.clone(); - p.push(&stage); - p.push("lib"); - p - }; + let stage_std_path = build_root.join(&stage).join("lib"); // Compute dynamic library search paths for recipes. let recipe_dylib_search_paths = {