Unrolled build for rust-lang#136474

Rollup merge of rust-lang#136474 - jieyouxu:src-base, r=clubby789

[`compiletest`-related cleanups 3/7] Make the distinction between sources root vs test suite sources root in compiletest less confusing

Reference for overall changes: rust-lang#136437
Part **3** of **7** of the *`compiletest`-related cleanups* PR series.

### Summary

- Remove `--src-base` compiletest in favor of new flags `--src-root` and `--src-test-suite-root` which more accurately conveys the intent. `--src-base` previously actually meant `--src-test-suite-root` and has caused multiple confusions.
- Use `--src-root` to have bootstrap directly feed source root path to compiletest, instead of doing a hacky directory parent search heuristic (`find_rust_src_root`) that somehow returns an `Option<PathBuf>`.

### Review advice

Best reviewed commit-by-commit.

r? bootstrap

Author: rust-timer

src/bootstrap/src/core/build_steps/test.rs

@@ -1762,7 +1762,9 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
             cmd.arg("--coverage-dump-path").arg(coverage_dump);
         }
 
-        cmd.arg("--src-base").arg(builder.src.join("tests").join(suite));
+        cmd.arg("--src-root").arg(&builder.src);
+        cmd.arg("--src-test-suite-root").arg(builder.src.join("tests").join(suite));
+
         cmd.arg("--build-base").arg(testdir(builder, compiler.host).join(suite));
 
         // When top stage is 0, that means that we're testing an externally provided compiler.

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")