compiletest: introduce and use --src-root and --src-test-suite-root

Instead of only having `--src-base` and `src_base` which *actually*
refers to the directory containing the test suite and not the sources
root. More importantly, kill off `find_rust_src_root` when we can simply
pass that info from bootstrap.

Author: jieyouxu

src/tools/compiletest/src/common.rs

@@ -215,8 +215,10 @@ pub struct Config {
     /// `None` then these tests will be ignored.
     pub run_clang_based_tests_with: Option<String>,
 
-    /// The directory containing the tests to run
-    pub src_base: PathBuf,
+    /// The directory containing the sources.
+    pub src_root: PathBuf,
+    /// The directory containing the test suite sources. Must be a subdirectory of `src_root`.
+    pub src_test_suite_root: PathBuf,
 
     /// The directory where programs should be built
     pub build_base: PathBuf,

src/tools/compiletest/src/header.rs

@@ -1026,19 +1026,6 @@ impl Config {
         }
     }
 
-    pub fn find_rust_src_root(&self) -> Option<PathBuf> {
-        let mut path = self.src_base.clone();
-        let path_postfix = Path::new("src/etc/lldb_batchmode.py");
-
-        while path.pop() {
-            if path.join(&path_postfix).is_file() {
-                return Some(path);
-            }
-        }
-
-        None
-    }
-
     fn parse_edition(&self, line: &str) -> Option<String> {
         self.parse_name_value_directive(line, "edition")
     }
@@ -1098,7 +1085,7 @@ fn expand_variables(mut value: String, config: &Config) -> String {
     }
 
     if value.contains(SRC_BASE) {
-        value = value.replace(SRC_BASE, &config.src_base.to_string_lossy());
+        value = value.replace(SRC_BASE, &config.src_test_suite_root.to_str().unwrap());
     }
 
     if value.contains(BUILD_BASE) {

src/tools/compiletest/src/header/tests.rs

@@ -153,7 +153,8 @@ impl ConfigBuilder {
             "--run-lib-path=",
             "--python=",
             "--jsondocck-path=",
-            "--src-base=",
+            "--src-root=",
+            "--src-test-suite-root=",
             "--build-base=",
             "--sysroot-base=",
             "--cc=c",

src/tools/compiletest/src/lib.rs

@@ -61,7 +61,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
         .optopt("", "jsondoclint-path", "path to jsondoclint to use for doc tests", "PATH")
         .optopt("", "run-clang-based-tests-with", "path to Clang executable", "PATH")
         .optopt("", "llvm-filecheck", "path to LLVM's FileCheck binary", "DIR")
-        .reqopt("", "src-base", "directory to scan for test files", "PATH")
+        .reqopt("", "src-root", "directory containing sources", "PATH")
+        .reqopt("", "src-test-suite-root", "directory containing test suite sources", "PATH")
         .reqopt("", "build-base", "directory to deposit test outputs", "PATH")
         .reqopt("", "sysroot-base", "directory containing the compiler sysroot", "PATH")
         .reqopt("", "stage", "stage number under test", "N")
@@ -243,7 +244,6 @@ pub fn parse_config(args: Vec<String>) -> Config {
             || header::extract_llvm_version_from_binary(&matches.opt_str("llvm-filecheck")?),
         );
 
-    let src_base = opt_path(matches, "src-base");
     let run_ignored = matches.opt_present("ignored");
     let with_rustc_debug_assertions = matches.opt_present("with-rustc-debug-assertions");
     let with_std_debug_assertions = matches.opt_present("with-std-debug-assertions");
@@ -300,6 +300,15 @@ pub fn parse_config(args: Vec<String>) -> Config {
         None => panic!("`--stage` is required"),
     };
 
+    let src_root = opt_path(matches, "src-root");
+    let src_test_suite_root = opt_path(matches, "src-test-suite-root");
+    assert!(
+        src_test_suite_root.starts_with(&src_root),
+        "`src-root` must be a parent of `src-test-suite-root`: `src-root`=`{}`, `src-test-suite-root` = `{}`",
+        src_root.display(),
+        src_test_suite_root.display()
+    );
+
     Config {
         bless: matches.opt_present("bless"),
         compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")),
@@ -314,7 +323,10 @@ pub fn parse_config(args: Vec<String>) -> Config {
         run_clang_based_tests_with: matches.opt_str("run-clang-based-tests-with"),
         llvm_filecheck: matches.opt_str("llvm-filecheck").map(PathBuf::from),
         llvm_bin_dir: matches.opt_str("llvm-bin-dir").map(PathBuf::from),
-        src_base,
+
+        src_root,
+        src_test_suite_root,
+
         build_base: opt_path(matches, "build-base"),
         sysroot_base: opt_path(matches, "sysroot-base"),
 
@@ -422,7 +434,10 @@ pub fn log_config(config: &Config) {
     logv(c, format!("rustc_path: {:?}", config.rustc_path.display()));
     logv(c, format!("cargo_path: {:?}", config.cargo_path));
     logv(c, format!("rustdoc_path: {:?}", config.rustdoc_path));
-    logv(c, format!("src_base: {:?}", config.src_base.display()));
+
+    logv(c, format!("src_root: {}", config.src_root.display()));
+    logv(c, format!("src_test_suite_root: {}", config.src_test_suite_root.display()));
+
     logv(c, format!("build_base: {:?}", config.build_base.display()));
     logv(c, format!("stage: {}", config.stage));
     logv(c, format!("stage_id: {}", config.stage_id));
@@ -620,20 +635,29 @@ struct TestCollector {
 /// regardless of whether any filters/tests were specified on the command-line,
 /// because filtering is handled later by libtest.
 pub fn collect_and_make_tests(config: Arc<Config>) -> Vec<test::TestDescAndFn> {
-    debug!("making tests from {:?}", config.src_base.display());
+    debug!("making tests from {}", config.src_test_suite_root.display());
     let common_inputs_stamp = common_inputs_stamp(&config);
-    let modified_tests = modified_tests(&config, &config.src_base).unwrap_or_else(|err| {
-        panic!("modified_tests got error from dir: {}, error: {}", config.src_base.display(), err)
-    });
+    let modified_tests =
+        modified_tests(&config, &config.src_test_suite_root).unwrap_or_else(|err| {
+            panic!(
+                "modified_tests got error from dir: {}, error: {}",
+                config.src_test_suite_root.display(),
+                err
+            )
+        });
     let cache = HeadersCache::load(&config);
 
     let cx = TestCollectorCx { config, cache, common_inputs_stamp, modified_tests };
     let mut collector =
         TestCollector { tests: vec![], found_path_stems: HashSet::new(), poisoned: false };
 
-    collect_tests_from_dir(&cx, &mut collector, &cx.config.src_base, Path::new("")).unwrap_or_else(
-        |reason| panic!("Could not read tests from {}: {reason}", cx.config.src_base.display()),
-    );
+    collect_tests_from_dir(&cx, &mut collector, &cx.config.src_test_suite_root, Path::new(""))
+        .unwrap_or_else(|reason| {
+            panic!(
+                "Could not read tests from {}: {reason}",
+                cx.config.src_test_suite_root.display()
+            )
+        });
 
     let TestCollector { tests, found_path_stems, poisoned } = collector;
 
@@ -655,7 +679,7 @@ pub fn collect_and_make_tests(config: Arc<Config>) -> Vec<test::TestDescAndFn> {
 /// common to some subset of tests, and are hopefully unlikely to be modified
 /// while working on other tests.)
 fn common_inputs_stamp(config: &Config) -> Stamp {
-    let rust_src_dir = config.find_rust_src_root().expect("Could not find Rust source root");
+    let src_root = &config.src_root;
 
     let mut stamp = Stamp::from_path(&config.rustc_path);
 
@@ -670,17 +694,17 @@ fn common_inputs_stamp(config: &Config) -> Stamp {
         "src/etc/lldb_providers.py",
     ];
     for file in &pretty_printer_files {
-        let path = rust_src_dir.join(file);
+        let path = src_root.join(file);
         stamp.add_path(&path);
     }
 
-    stamp.add_dir(&rust_src_dir.join("src/etc/natvis"));
+    stamp.add_dir(&src_root.join("src/etc/natvis"));
 
     stamp.add_dir(&config.run_lib_path);
 
     if let Some(ref rustdoc_path) = config.rustdoc_path {
         stamp.add_path(&rustdoc_path);
-        stamp.add_path(&rust_src_dir.join("src/etc/htmldocck.py"));
+        stamp.add_path(&src_root.join("src/etc/htmldocck.py"));
     }
 
     // Re-run coverage tests if the `coverage-dump` tool was modified,
@@ -689,10 +713,10 @@ fn common_inputs_stamp(config: &Config) -> Stamp {
         stamp.add_path(coverage_dump_path)
     }
 
-    stamp.add_dir(&rust_src_dir.join("src/tools/run-make-support"));
+    stamp.add_dir(&src_root.join("src/tools/run-make-support"));
 
     // Compiletest itself.
-    stamp.add_dir(&rust_src_dir.join("src/tools/compiletest/"));
+    stamp.add_dir(&src_root.join("src/tools/compiletest"));
 
     stamp
 }
@@ -933,10 +957,7 @@ fn files_related_to_test(
     }
 
     // `minicore.rs` test auxiliary: we need to make sure tests get rerun if this changes.
-    //
-    // FIXME(jieyouxu): untangle these paths, we should provide both a path to root `tests/` or
-    // `tests/auxiliary/` and the test suite in question. `src_base` is also a terrible name.
-    related.push(config.src_base.parent().unwrap().join("auxiliary").join("minicore.rs"));
+    related.push(config.src_root.join("tests").join("auxiliary").join("minicore.rs"));
 
     related
 }
@@ -1026,10 +1047,8 @@ fn make_test_name(
     testpaths: &TestPaths,
     revision: Option<&str>,
 ) -> test::TestName {
-    // Print the name of the file, relative to the repository root.
-    // `src_base` looks like `/path/to/rust/tests/ui`
-    let root_directory = config.src_base.parent().unwrap().parent().unwrap();
-    let path = testpaths.file.strip_prefix(root_directory).unwrap();
+    // Print the name of the file, relative to the sources root.
+    let path = testpaths.file.strip_prefix(&config.src_root).unwrap();
     let debugger = match config.debugger {
         Some(d) => format!("-{}", d),
         None => String::new(),

src/tools/compiletest/src/runtest.rs

@@ -1365,7 +1365,7 @@ impl<'test> TestCx<'test> {
         //
         // Note: avoid adding a subdirectory of an already filtered directory here, otherwise the
         // same slice of text will be double counted and the truncation might not happen.
-        add_path(&self.config.src_base);
+        add_path(&self.config.src_test_suite_root);
         add_path(&self.config.build_base);
 
         read2_abbreviated(child, &filter_paths_from_len).expect("failed to read output")
@@ -1471,7 +1471,7 @@ impl<'test> TestCx<'test> {
         // Similarly, vendored sources shouldn't be shown when running from a dist tarball.
         rustc.arg("-Z").arg(format!(
             "ignore-directory-in-diagnostics-source-blocks={}",
-            self.config.find_rust_src_root().unwrap().join("vendor").display(),
+            self.config.src_root.join("vendor").to_str().unwrap(),
         ));
 
         // Optionally prevent default --sysroot if specified in test compile-flags.
@@ -1632,7 +1632,7 @@ impl<'test> TestCx<'test> {
         if self.props.remap_src_base {
             rustc.arg(format!(
                 "--remap-path-prefix={}={}",
-                self.config.src_base.display(),
+                self.config.src_test_suite_root.to_str().unwrap(),
                 FAKE_SRC_BASE,
             ));
         }

src/tools/compiletest/src/runtest/debuginfo.rs

@@ -257,11 +257,8 @@ impl TestCx<'_> {
                 println!("Adb process is already finished.");
             }
         } else {
-            let rust_src_root =
-                self.config.find_rust_src_root().expect("Could not find Rust source root");
-            let rust_pp_module_rel_path = Path::new("./src/etc");
-            let rust_pp_module_abs_path =
-                rust_src_root.join(rust_pp_module_rel_path).to_str().unwrap().to_owned();
+            let rust_pp_module_abs_path = self.config.src_root.join("src").join("etc");
+            let rust_pp_module_abs_path = rust_pp_module_abs_path.to_str().unwrap();
             // write debugger script
             let mut script_str = String::with_capacity(2048);
             script_str.push_str(&format!("set charset {}\n", Self::charset()));
@@ -338,7 +335,7 @@ impl TestCx<'_> {
             let pythonpath = if let Ok(pp) = std::env::var("PYTHONPATH") {
                 format!("{pp}:{rust_pp_module_abs_path}")
             } else {
-                rust_pp_module_abs_path
+                rust_pp_module_abs_path.to_string()
             };
             gdb.args(debugger_opts).env("PYTHONPATH", pythonpath);
 
@@ -407,11 +404,8 @@ impl TestCx<'_> {
         // Make LLDB emit its version, so we have it documented in the test output
         script_str.push_str("version\n");
 
-        // Switch LLDB into "Rust mode"
-        let rust_src_root =
-            self.config.find_rust_src_root().expect("Could not find Rust source root");
-        let rust_pp_module_rel_path = Path::new("./src/etc");
-        let rust_pp_module_abs_path = rust_src_root.join(rust_pp_module_rel_path);
+        // Switch LLDB into "Rust mode".
+        let rust_pp_module_abs_path = self.config.src_root.join("src/etc");
 
         script_str.push_str(&format!(
             "command script import {}/lldb_lookup.py\n",
@@ -445,7 +439,7 @@ impl TestCx<'_> {
         let debugger_script = self.make_out_name("debugger.script");
 
         // Let LLDB execute the script via lldb_batchmode.py
-        let debugger_run_result = self.run_lldb(&exe_file, &debugger_script, &rust_src_root);
+        let debugger_run_result = self.run_lldb(&exe_file, &debugger_script);
 
         if !debugger_run_result.status.success() {
             self.fatal_proc_rec("Error while running LLDB", &debugger_run_result);
@@ -456,18 +450,13 @@ impl TestCx<'_> {
         }
     }
 
-    fn run_lldb(
-        &self,
-        test_executable: &Path,
-        debugger_script: &Path,
-        rust_src_root: &Path,
-    ) -> ProcRes {
+    fn run_lldb(&self, test_executable: &Path, debugger_script: &Path) -> ProcRes {
         // Prepare the lldb_batchmode which executes the debugger script
-        let lldb_script_path = rust_src_root.join("src/etc/lldb_batchmode.py");
+        let lldb_script_path = self.config.src_root.join("src/etc/lldb_batchmode.py");
         let pythonpath = if let Ok(pp) = std::env::var("PYTHONPATH") {
             format!("{pp}:{}", self.config.lldb_python_dir.as_ref().unwrap())
         } else {
-            self.config.lldb_python_dir.as_ref().unwrap().to_string()
+            self.config.lldb_python_dir.clone().unwrap()
         };
         self.run_command_to_procres(
             Command::new(&self.config.python)

src/tools/compiletest/src/runtest/js_doc.rs

@@ -9,12 +9,11 @@ impl TestCx<'_> {
 
             self.document(&out_dir, &self.testpaths);
 
-            let root = self.config.find_rust_src_root().unwrap();
             let file_stem =
                 self.testpaths.file.file_stem().and_then(|f| f.to_str()).expect("no file stem");
             let res = self.run_command_to_procres(
                 Command::new(&nodejs)
-                    .arg(root.join("src/tools/rustdoc-js/tester.js"))
+                    .arg(self.config.src_root.join("src/tools/rustdoc-js/tester.js"))
                     .arg("--doc-folder")
                     .arg(out_dir)
                     .arg("--crate-name")

src/tools/compiletest/src/runtest/run_make.rs

@@ -21,8 +21,6 @@ impl TestCx<'_> {
 
     fn run_rmake_legacy_test(&self) {
         let cwd = env::current_dir().unwrap();
-        let src_root = self.config.src_base.parent().unwrap().parent().unwrap();
-        let src_root = cwd.join(&src_root);
 
         // FIXME(Zalathar): This should probably be `output_base_dir` to avoid
         // an unnecessary extra subdirectory, but since legacy Makefile tests
@@ -51,7 +49,7 @@ impl TestCx<'_> {
             .stderr(Stdio::piped())
             .env("TARGET", &self.config.target)
             .env("PYTHON", &self.config.python)
-            .env("S", src_root)
+            .env("S", &self.config.src_root)
             .env("RUST_BUILD_STAGE", &self.config.stage_id)
             .env("RUSTC", cwd.join(&self.config.rustc_path))
             .env("TMPDIR", &tmpdir)
@@ -181,28 +179,10 @@ impl TestCx<'_> {
         //    library.
         // 2. We need to run the recipe binary.
 
-        // 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`
-        // ├── 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");
         // `self.config.build_base` is actually the build base folder + "test" + test suite name, it
         // looks like `build/<host_triple>/test/run-make`. But we want `build/<host_triple>/`. 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`.
+        // under `src_root`, so we must compute it based off of `self.config.build_base`.
         let build_root =
             self.config.build_base.parent().and_then(Path::parent).unwrap().to_path_buf();
 
@@ -389,10 +369,9 @@ impl TestCx<'_> {
             .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)
-            // Path to the build directory. This is usually the same as `source_root.join("build").join("host")`.
+            // Provide path to sources root.
+            .env("SOURCE_ROOT", &self.config.src_root)
+            // Path to the host build directory.
             .env("BUILD_ROOT", &build_root)
             // Provide path to stage-corresponding rustc.
             .env("RUSTC", &self.config.rustc_path)
@@ -408,11 +387,11 @@ impl TestCx<'_> {
             .env("LLVM_COMPONENTS", &self.config.llvm_components);
 
         if let Some(ref cargo) = self.config.cargo_path {
-            cmd.env("CARGO", source_root.join(cargo));
+            cmd.env("CARGO", cargo);
         }
 
         if let Some(ref rustdoc) = self.config.rustdoc_path {
-            cmd.env("RUSTDOC", source_root.join(rustdoc));
+            cmd.env("RUSTDOC", rustdoc);
         }
 
         if let Some(ref node) = self.config.nodejs {