Skip to content

Commit

Permalink
compiletest: introduce and use --src-root and --src-test-suite-root
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jieyouxu committed Feb 4, 2025
1 parent bb908fe commit 4c627ec
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 97 deletions.
6 changes: 4 additions & 2 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 1 addition & 14 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
64 changes: 39 additions & 25 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -300,6 +300,10 @@ 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));

Config {
bless: matches.opt_present("bless"),
compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")),
Expand All @@ -314,7 +318,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"),

Expand Down Expand Up @@ -422,7 +429,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));
Expand Down Expand Up @@ -620,20 +630,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;

Expand All @@ -655,7 +674,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);

Expand All @@ -670,17 +689,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").join("etc").join("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,
Expand All @@ -689,10 +708,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
}
Expand Down Expand Up @@ -933,10 +952,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
}
Expand Down Expand Up @@ -1026,10 +1042,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(),
Expand Down
6 changes: 3 additions & 3 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
));
}
Expand Down
29 changes: 9 additions & 20 deletions src/tools/compiletest/src/runtest/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions src/tools/compiletest/src/runtest/js_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
35 changes: 7 additions & 28 deletions src/tools/compiletest/src/runtest/run_make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
Loading

0 comments on commit 4c627ec

Please sign in to comment.