From d7569b08db1735271f3e532b8a0fea062df69b15 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Mon, 3 Feb 2025 17:22:13 +0800 Subject: [PATCH 01/13] 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. --- src/tools/compiletest/src/common.rs | 6 +- src/tools/compiletest/src/header.rs | 15 +---- src/tools/compiletest/src/header/tests.rs | 3 +- src/tools/compiletest/src/lib.rs | 64 +++++++++++-------- src/tools/compiletest/src/runtest.rs | 6 +- .../compiletest/src/runtest/debuginfo.rs | 29 +++------ src/tools/compiletest/src/runtest/js_doc.rs | 3 +- src/tools/compiletest/src/runtest/run_make.rs | 35 ++-------- src/tools/compiletest/src/runtest/rustdoc.rs | 5 +- 9 files changed, 69 insertions(+), 97 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index cde4f7a665cbf..aaa706f9c8f62 100644 --- a/src/tools/compiletest/src/common.rs +++ b/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, - /// 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, diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 452a2e9a9d518..3bdf37a1f294a 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1026,19 +1026,6 @@ impl Config { } } - pub fn find_rust_src_root(&self) -> Option { - 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 { 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) { diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 522d340b678f9..b2659983763e6 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/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", diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index d0a83cab9cd99..ef4168e81fb4f 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -61,7 +61,8 @@ pub fn parse_config(args: Vec) -> 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) -> 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,10 @@ pub fn parse_config(args: Vec) -> 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")), @@ -314,7 +318,10 @@ pub fn parse_config(args: Vec) -> 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 +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)); @@ -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) -> Vec { - 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 +674,7 @@ pub fn collect_and_make_tests(config: Arc) -> Vec { /// 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 +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/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 +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 } @@ -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 } @@ -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(), diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 0e2da2b02ca0f..536e19bc4933e 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/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, )); } diff --git a/src/tools/compiletest/src/runtest/debuginfo.rs b/src/tools/compiletest/src/runtest/debuginfo.rs index b236b06756903..170b8a8099687 100644 --- a/src/tools/compiletest/src/runtest/debuginfo.rs +++ b/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) diff --git a/src/tools/compiletest/src/runtest/js_doc.rs b/src/tools/compiletest/src/runtest/js_doc.rs index a83bcd70c8718..d630affbec104 100644 --- a/src/tools/compiletest/src/runtest/js_doc.rs +++ b/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") diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 16c46fc13390f..74e6af36ea1dd 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/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//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`. + // 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 { diff --git a/src/tools/compiletest/src/runtest/rustdoc.rs b/src/tools/compiletest/src/runtest/rustdoc.rs index 3f33862d2cfe2..2583ae96a6788 100644 --- a/src/tools/compiletest/src/runtest/rustdoc.rs +++ b/src/tools/compiletest/src/runtest/rustdoc.rs @@ -17,9 +17,10 @@ impl TestCx<'_> { if self.props.check_test_line_numbers_match { self.check_rustdoc_test_option(proc_res); } else { - let root = self.config.find_rust_src_root().unwrap(); let mut cmd = Command::new(&self.config.python); - cmd.arg(root.join("src/etc/htmldocck.py")).arg(&out_dir).arg(&self.testpaths.file); + cmd.arg(self.config.src_root.join("src/etc/htmldocck.py")) + .arg(&out_dir) + .arg(&self.testpaths.file); if self.config.bless { cmd.arg("--bless"); } From 71817b9002427ba8d0b4ee2e8f23f1c2737ac9ba 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Mon, 3 Feb 2025 16:28:32 +0800 Subject: [PATCH 02/13] bootstrap: pass `--src-root` and `--src-test-suite-root` instead of `--src-base` --- src/bootstrap/src/core/build_steps/test.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index b0181410be124..d5bb1d6db07f8 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1761,7 +1761,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. From 11d6660b741817316f5c46a7303e7dea7b250167 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Tue, 4 Feb 2025 17:48:24 +0800 Subject: [PATCH 03/13] [DO NOT MERGE] ----- commit stack separator ----- From 6f7bc0648936cd5da29c90433acfe7b029f26243 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Tue, 4 Feb 2025 21:54:15 +0800 Subject: [PATCH 04/13] compiletest: disambiguate between root build dir vs test suite specific build dir - Introduce and use `--build-{root,test-suite-root}` over `--build-base`. - A few minor cleanups. --- src/tools/compiletest/src/common.rs | 20 +++++++++++------- src/tools/compiletest/src/header.rs | 10 +++++---- src/tools/compiletest/src/header/tests.rs | 3 ++- src/tools/compiletest/src/lib.rs | 21 ++++++++++++++----- src/tools/compiletest/src/runtest.rs | 16 +++++++------- src/tools/compiletest/src/runtest/run_make.rs | 17 ++++++--------- src/tools/compiletest/src/runtest/ui.rs | 2 +- 7 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index aaa706f9c8f62..07d8962b3a77d 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -220,8 +220,10 @@ pub struct Config { /// 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, + /// Root build directory (e.g. `build/`). + pub build_root: PathBuf, + /// Test suite specific build directory (e.g. `build/host/test/ui/`). + pub build_test_suite_root: PathBuf, /// The directory containing the compiler sysroot pub sysroot_base: PathBuf, @@ -347,7 +349,7 @@ pub struct Config { /// If true, this will generate a coverage file with UI test files that run `MachineApplicable` /// diagnostics but are missing `run-rustfix` annotations. The generated coverage file is - /// created in `//rustfix_missing_coverage.txt` + /// created in `/rustfix_missing_coverage.txt` pub rustfix_coverage: bool, /// whether to run `tidy` (html-tidy) when a rustdoc test fails @@ -801,12 +803,16 @@ pub const UI_STDERR_16: &str = "16bit.stderr"; pub const UI_COVERAGE: &str = "coverage"; pub const UI_COVERAGE_MAP: &str = "cov-map"; -/// Absolute path to the directory where all output for all tests in the given -/// `relative_dir` group should reside. Example: -/// /path/to/build/host-tuple/test/ui/relative/ +/// Absolute path to the directory where all output for all tests in the given `relative_dir` group +/// should reside. Example: +/// +/// ```text +/// /path/to/build/host-tuple/test/ui/relative/ +/// ``` +/// /// This is created early when tests are collected to avoid race conditions. pub fn output_relative_path(config: &Config, relative_dir: &Path) -> PathBuf { - config.build_base.join(relative_dir) + config.build_test_suite_root.join(relative_dir) } /// Generates a unique name for the test, such as `testname.revision.mode`. diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 3bdf37a1f294a..53ee901b8bc48 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1070,10 +1070,11 @@ impl Config { } } +// FIXME(jieyouxu): fix some of these variable names to more accurately reflect what they do. fn expand_variables(mut value: String, config: &Config) -> String { const CWD: &str = "{{cwd}}"; const SRC_BASE: &str = "{{src-base}}"; - const BUILD_BASE: &str = "{{build-base}}"; + const TEST_SUITE_BUILD_BASE: &str = "{{build-base}}"; const RUST_SRC_BASE: &str = "{{rust-src-base}}"; const SYSROOT_BASE: &str = "{{sysroot-base}}"; const TARGET_LINKER: &str = "{{target-linker}}"; @@ -1088,12 +1089,13 @@ fn expand_variables(mut value: String, config: &Config) -> String { value = value.replace(SRC_BASE, &config.src_test_suite_root.to_str().unwrap()); } - if value.contains(BUILD_BASE) { - value = value.replace(BUILD_BASE, &config.build_base.to_string_lossy()); + if value.contains(TEST_SUITE_BUILD_BASE) { + value = + value.replace(TEST_SUITE_BUILD_BASE, &config.build_test_suite_root.to_str().unwrap()); } if value.contains(SYSROOT_BASE) { - value = value.replace(SYSROOT_BASE, &config.sysroot_base.to_string_lossy()); + value = value.replace(SYSROOT_BASE, &config.sysroot_base.to_str().unwrap()); } if value.contains(TARGET_LINKER) { diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index b2659983763e6..bf8c924c30db5 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -155,7 +155,8 @@ impl ConfigBuilder { "--jsondocck-path=", "--src-root=", "--src-test-suite-root=", - "--build-base=", + "--build-root=", + "--build-test-suite-root=", "--sysroot-base=", "--cc=c", "--cxx=c++", diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index ef4168e81fb4f..be6b62fe3c24f 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -63,7 +63,8 @@ pub fn parse_config(args: Vec) -> Config { .optopt("", "llvm-filecheck", "path to LLVM's FileCheck binary", "DIR") .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("", "build-root", "path to root build directory", "PATH") + .reqopt("", "build-test-suite-root", "path to test suite specific build directory", "PATH") .reqopt("", "sysroot-base", "directory containing the compiler sysroot", "PATH") .reqopt("", "stage", "stage number under test", "N") .reqopt("", "stage-id", "the target-stage identifier", "stageN-TARGET") @@ -157,7 +158,7 @@ pub fn parse_config(args: Vec) -> Config { "", "rustfix-coverage", "enable this to generate a Rustfix coverage file, which is saved in \ - `.//rustfix_missing_coverage.txt`", + `.//rustfix_missing_coverage.txt`", ) .optflag("", "force-rerun", "rerun tests even if the inputs are unchanged") .optflag("", "only-modified", "only run tests that result been modified") @@ -304,6 +305,10 @@ pub fn parse_config(args: Vec) -> Config { let src_test_suite_root = opt_path(matches, "src-test-suite-root"); assert!(src_test_suite_root.starts_with(&src_root)); + let build_root = opt_path(matches, "build-root"); + let build_test_suite_root = opt_path(matches, "build-test-suite-root"); + assert!(build_test_suite_root.starts_with(&build_root)); + Config { bless: matches.opt_present("bless"), compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")), @@ -322,7 +327,9 @@ pub fn parse_config(args: Vec) -> Config { src_root, src_test_suite_root, - build_base: opt_path(matches, "build-base"), + build_root, + build_test_suite_root, + sysroot_base: opt_path(matches, "sysroot-base"), stage, @@ -433,7 +440,11 @@ pub fn log_config(config: &Config) { 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!("build_root: {}", config.build_root.display())); + logv(c, format!("build_test_suite_root: {}", config.build_test_suite_root.display())); + + logv(c, format!("sysroot_base: {}", config.sysroot_base.display())); + logv(c, format!("stage: {}", config.stage)); logv(c, format!("stage_id: {}", config.stage_id)); logv(c, format!("mode: {}", config.mode)); @@ -483,7 +494,7 @@ pub fn run_tests(config: Arc) { // we first make sure that the coverage file does not exist. // It will be created later on. if config.rustfix_coverage { - let mut coverage_file_path = config.build_base.clone(); + let mut coverage_file_path = config.build_test_suite_root.clone(); coverage_file_path.push("rustfix_missing_coverage.txt"); if coverage_file_path.exists() { if let Err(e) = fs::remove_file(&coverage_file_path) { diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 536e19bc4933e..f36d43b211fff 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -535,7 +535,9 @@ impl<'test> TestCx<'test> { .arg(&out_dir) .arg(&format!("--target={}", target)) .arg("-L") - .arg(&self.config.build_base) + // FIXME(jieyouxu): this search path seems questionable. Is this intended for + // `rust_test_helpers` in ui tests? + .arg(&self.config.build_test_suite_root) .arg("-L") .arg(aux_dir) .arg("-A") @@ -1366,7 +1368,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_test_suite_root); - add_path(&self.config.build_base); + add_path(&self.config.build_test_suite_root); read2_abbreviated(child, &filter_paths_from_len).expect("failed to read output") } @@ -1421,7 +1423,7 @@ impl<'test> TestCx<'test> { } fn get_mir_dump_dir(&self) -> PathBuf { - let mut mir_dump_dir = PathBuf::from(self.config.build_base.as_path()); + let mut mir_dump_dir = self.config.build_test_suite_root.clone(); debug!("input_file: {:?}", self.testpaths.file); mir_dump_dir.push(&self.testpaths.relative_dir); mir_dump_dir.push(self.testpaths.file.file_stem().unwrap()); @@ -2410,14 +2412,10 @@ impl<'test> TestCx<'test> { let rust_src_dir = rust_src_dir.read_link().unwrap_or(rust_src_dir.to_path_buf()); normalize_path(&rust_src_dir.join("library"), "$SRC_DIR_REAL"); - // Paths into the build directory - let test_build_dir = &self.config.build_base; - let parent_build_dir = test_build_dir.parent().unwrap().parent().unwrap().parent().unwrap(); - // eg. /home/user/rust/build/x86_64-unknown-linux-gnu/test/ui - normalize_path(test_build_dir, "$TEST_BUILD_DIR"); + normalize_path(&self.config.build_test_suite_root, "$TEST_BUILD_DIR"); // eg. /home/user/rust/build - normalize_path(parent_build_dir, "$BUILD_DIR"); + normalize_path(&self.config.build_root, "$BUILD_DIR"); if json { // escaped newlines in json strings should be readable diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 74e6af36ea1dd..9bb3993223e21 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/src/tools/compiletest/src/runtest/run_make.rs @@ -179,12 +179,7 @@ impl TestCx<'_> { // library. // 2. We need to run the recipe binary. - // `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 `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(); + let host_build_root = self.config.build_root.join(&self.config.host); // We construct the following directory tree for each rmake.rs test: // ``` @@ -242,10 +237,10 @@ impl TestCx<'_> { let stage_number = self.config.stage; - let stage_tools_bin = build_root.join(format!("stage{stage_number}-tools-bin")); + let stage_tools_bin = host_build_root.join(format!("stage{stage_number}-tools-bin")); let support_lib_path = stage_tools_bin.join("librun_make_support.rlib"); - let stage_tools = build_root.join(format!("stage{stage_number}-tools")); + let stage_tools = host_build_root.join(format!("stage{stage_number}-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"); @@ -311,7 +306,7 @@ impl TestCx<'_> { // to work correctly. // // See for more background. - let stage0_sysroot = build_root.join("stage0-sysroot"); + let stage0_sysroot = host_build_root.join("stage0-sysroot"); if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() { rustc.arg("--sysroot").arg(&stage0_sysroot); } @@ -326,7 +321,7 @@ impl TestCx<'_> { // provided through env vars. // Compute stage-specific standard library paths. - let stage_std_path = build_root.join(format!("stage{stage_number}")).join("lib"); + let stage_std_path = host_build_root.join(format!("stage{stage_number}")).join("lib"); // Compute dynamic library search paths for recipes. let recipe_dylib_search_paths = { @@ -372,7 +367,7 @@ impl TestCx<'_> { // Provide path to sources root. .env("SOURCE_ROOT", &self.config.src_root) // Path to the host build directory. - .env("BUILD_ROOT", &build_root) + .env("BUILD_ROOT", &host_build_root) // 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 diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index 0c6d46188e6f4..3329e10745f8d 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -66,7 +66,7 @@ impl TestCx<'_> { && !self.props.run_rustfix && !self.props.rustfix_only_machine_applicable { - let mut coverage_file_path = self.config.build_base.clone(); + let mut coverage_file_path = self.config.build_test_suite_root.clone(); coverage_file_path.push("rustfix_missing_coverage.txt"); debug!("coverage_file_path: {}", coverage_file_path.display()); From de82c54525b2e4ee9b263aa6421c5706f3cb8410 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Tue, 4 Feb 2025 21:28:32 +0800 Subject: [PATCH 05/13] bootstrap: remove `--build-base` and use `--build-root` and `--build-test-suite-root` instead --- src/bootstrap/src/core/build_steps/test.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index d5bb1d6db07f8..d9afbb9d87b81 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1764,7 +1764,11 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the 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)); + // N.B. it's important to distinguish between the *root* build directory, the *host* build + // directory immediately under the root build directory, and the test-suite-specific build + // directory. + cmd.arg("--build-root").arg(&builder.out); + cmd.arg("--build-test-suite-root").arg(testdir(builder, compiler.host).join(suite)); // When top stage is 0, that means that we're testing an externally provided compiler. // In that case we need to use its specific sysroot for tests to pass. From c77dcfc0ca7b1c26d1ef6f58f1db9163f916b5c9 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Tue, 4 Feb 2025 21:30:36 +0800 Subject: [PATCH 06/13] bootstrap: remove redundant `to_path_buf()` --- src/bootstrap/src/core/build_steps/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index d9afbb9d87b81..9a2eb39a77904 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1775,7 +1775,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let sysroot = if builder.top_stage == 0 { builder.initial_sysroot.clone() } else { - builder.sysroot(compiler).to_path_buf() + builder.sysroot(compiler) }; cmd.arg("--sysroot-base").arg(sysroot); From 0671c762f938c53d309d789a31838631b6c6e947 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:10:16 +0800 Subject: [PATCH 07/13] [DO NOT MERGE] ----- commit stack separator ----- From 15f2ad2a972b4966896ec53a7d2c1f58546510c9 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:15:04 +0800 Subject: [PATCH 08/13] compiletest: remove legacy `Makefile`-based `run-make` support --- src/tools/compiletest/src/header.rs | 16 +- src/tools/compiletest/src/header/tests.rs | 9 - src/tools/compiletest/src/lib.rs | 43 ++--- src/tools/compiletest/src/runtest/run_make.rs | 164 ------------------ 4 files changed, 18 insertions(+), 214 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 53ee901b8bc48..7675e13990d6c 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -709,11 +709,11 @@ impl TestProps { /// returns a struct containing various parts of the directive. fn line_directive<'line>( line_number: usize, - comment: &str, original_line: &'line str, ) -> Option> { // Ignore lines that don't start with the comment prefix. - let after_comment = original_line.trim_start().strip_prefix(comment)?.trim_start(); + let after_comment = + original_line.trim_start().strip_prefix(COMPILETEST_DIRECTIVE_PREFIX)?.trim_start(); let revision; let raw_directive; @@ -722,7 +722,7 @@ fn line_directive<'line>( // A comment like `//@[foo]` only applies to revision `foo`. let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else { panic!( - "malformed condition directive: expected `{comment}[foo]`, found `{original_line}`" + "malformed condition directive: expected `{COMPILETEST_DIRECTIVE_PREFIX}[foo]`, found `{original_line}`" ) }; @@ -836,6 +836,8 @@ pub(crate) fn check_directive<'a>( CheckDirectiveResult { is_known_directive: is_known(&directive_name), trailing_directive } } +const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; + fn iter_header( mode: Mode, _suite: &str, @@ -849,8 +851,7 @@ fn iter_header( } // Coverage tests in coverage-run mode always have these extra directives, without needing to - // specify them manually in every test file. (Some of the comments below have been copied over - // from the old `tests/run-make/coverage-reports/Makefile`, which no longer exists.) + // specify them manually in every test file. // // FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later. if mode == Mode::CoverageRun { @@ -867,9 +868,6 @@ fn iter_header( } } - // NOTE(jieyouxu): once we get rid of `Makefile`s we can unconditionally check for `//@`. - let comment = if testfile.extension().is_some_and(|e| e == "rs") { "//@" } else { "#" }; - let mut rdr = BufReader::with_capacity(1024, rdr); let mut ln = String::new(); let mut line_number = 0; @@ -882,7 +880,7 @@ fn iter_header( } let ln = ln.trim(); - let Some(directive_line) = line_directive(line_number, comment, ln) else { + let Some(directive_line) = line_directive(line_number, ln) else { continue; }; diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index bf8c924c30db5..beb91dfc8acb1 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -239,11 +239,6 @@ fn check_ignore(config: &Config, contents: &str) -> bool { d.ignore } -fn parse_makefile(config: &Config, contents: &str) -> EarlyProps { - let bytes = contents.as_bytes(); - EarlyProps::from_reader(config, Path::new("Makefile"), bytes) -} - #[test] fn should_fail() { let config: Config = cfg().build(); @@ -261,10 +256,6 @@ fn revisions() { let config: Config = cfg().build(); assert_eq!(parse_rs(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],); - assert_eq!( - parse_makefile(&config, "# revisions: hello there").revisions, - vec!["hello", "there"], - ); } #[test] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index be6b62fe3c24f..41364ce9ffe7e 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -21,7 +21,7 @@ pub mod util; use core::panic; use std::collections::HashSet; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::io::{self, ErrorKind}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -268,12 +268,8 @@ pub fn parse_config(args: Vec) -> Config { let path = Path::new(f); let mut iter = path.iter().skip(1); - // We skip the test folder and check if the user passed `rmake.rs` or `Makefile`. - if iter - .next() - .is_some_and(|s| s == OsStr::new("rmake.rs") || s == OsStr::new("Makefile")) - && iter.next().is_none() - { + // We skip the test folder and check if the user passed `rmake.rs`. + if iter.next().is_some_and(|s| s == "rmake.rs") && iter.next().is_none() { path.parent().unwrap().to_str().unwrap().to_string() } else { f.to_string() @@ -771,16 +767,9 @@ fn collect_tests_from_dir( return Ok(()); } - // For run-make tests, a "test file" is actually a directory that contains - // an `rmake.rs` or `Makefile`" + // For run-make tests, a "test file" is actually a directory that contains an `rmake.rs`. if cx.config.mode == Mode::RunMake { - if dir.join("Makefile").exists() && dir.join("rmake.rs").exists() { - return Err(io::Error::other( - "run-make tests cannot have both `Makefile` and `rmake.rs`", - )); - } - - if dir.join("Makefile").exists() || dir.join("rmake.rs").exists() { + if dir.join("rmake.rs").exists() { let paths = TestPaths { file: dir.to_path_buf(), relative_dir: relative_dir_path.parent().unwrap().to_path_buf(), @@ -849,24 +838,14 @@ pub fn is_test(file_name: &OsString) -> bool { !invalid_prefixes.iter().any(|p| file_name.starts_with(p)) } -/// For a single test file, creates one or more test structures (one per revision) -/// that can be handed over to libtest to run, possibly in parallel. +/// For a single test file, creates one or more test structures (one per revision) that can be +/// handed over to libtest to run, possibly in parallel. fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &TestPaths) { - // For run-make tests, each "test file" is actually a _directory_ containing - // an `rmake.rs` or `Makefile`. But for the purposes of directive parsing, - // we want to look at that recipe file, not the directory itself. + // For run-make tests, each "test file" is actually a _directory_ containing an `rmake.rs`. But + // for the purposes of directive parsing, we want to look at that recipe file, not the directory + // itself. let test_path = if cx.config.mode == Mode::RunMake { - if testpaths.file.join("rmake.rs").exists() && testpaths.file.join("Makefile").exists() { - panic!("run-make tests cannot have both `rmake.rs` and `Makefile`"); - } - - if testpaths.file.join("rmake.rs").exists() { - // Parse directives in rmake.rs. - testpaths.file.join("rmake.rs") - } else { - // Parse directives in the Makefile. - testpaths.file.join("Makefile") - } + testpaths.file.join("rmake.rs") } else { PathBuf::from(&testpaths.file) }; diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 9bb3993223e21..b237fac38f6b7 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/src/tools/compiletest/src/runtest/run_make.rs @@ -9,168 +9,6 @@ use crate::util::{copy_dir_all, dylib_env_var}; impl TestCx<'_> { pub(super) fn run_rmake_test(&self) { - let test_dir = &self.testpaths.file; - if test_dir.join("rmake.rs").exists() { - self.run_rmake_v2_test(); - } else if test_dir.join("Makefile").exists() { - self.run_rmake_legacy_test(); - } else { - self.fatal("failed to find either `rmake.rs` or `Makefile`") - } - } - - fn run_rmake_legacy_test(&self) { - let cwd = env::current_dir().unwrap(); - - // FIXME(Zalathar): This should probably be `output_base_dir` to avoid - // an unnecessary extra subdirectory, but since legacy Makefile tests - // are hopefully going away, it seems safer to leave this perilous code - // as-is until it can all be deleted. - let tmpdir = cwd.join(self.output_base_name()); - ignore_not_found(|| recursive_remove(&tmpdir)).unwrap(); - - fs::create_dir_all(&tmpdir).unwrap(); - - let host = &self.config.host; - let make = if host.contains("dragonfly") - || host.contains("freebsd") - || host.contains("netbsd") - || host.contains("openbsd") - || host.contains("aix") - { - "gmake" - } else { - "make" - }; - - let mut cmd = Command::new(make); - cmd.current_dir(&self.testpaths.file) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .env("TARGET", &self.config.target) - .env("PYTHON", &self.config.python) - .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) - .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path)) - .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path)) - .env("LLVM_COMPONENTS", &self.config.llvm_components) - // We for sure don't want these tests to run in parallel, so make - // sure they don't have access to these vars if we run via `make` - // at the top level - .env_remove("MAKEFLAGS") - .env_remove("MFLAGS") - .env_remove("CARGO_MAKEFLAGS"); - - if let Some(ref cargo) = self.config.cargo_path { - cmd.env("CARGO", cwd.join(cargo)); - } - - if let Some(ref rustdoc) = self.config.rustdoc_path { - cmd.env("RUSTDOC", cwd.join(rustdoc)); - } - - if let Some(ref node) = self.config.nodejs { - cmd.env("NODE", node); - } - - if let Some(ref linker) = self.config.target_linker { - cmd.env("RUSTC_LINKER", linker); - } - - if let Some(ref clang) = self.config.run_clang_based_tests_with { - cmd.env("CLANG", clang); - } - - if let Some(ref filecheck) = self.config.llvm_filecheck { - cmd.env("LLVM_FILECHECK", filecheck); - } - - if let Some(ref llvm_bin_dir) = self.config.llvm_bin_dir { - cmd.env("LLVM_BIN_DIR", llvm_bin_dir); - } - - if let Some(ref remote_test_client) = self.config.remote_test_client { - cmd.env("REMOTE_TEST_CLIENT", remote_test_client); - } - - // We don't want RUSTFLAGS set from the outside to interfere with - // compiler flags set in the test cases: - cmd.env_remove("RUSTFLAGS"); - - // Use dynamic musl for tests because static doesn't allow creating dylibs - if self.config.host.contains("musl") { - cmd.env("RUSTFLAGS", "-Ctarget-feature=-crt-static").env("IS_MUSL_HOST", "1"); - } - - if self.config.bless { - cmd.env("RUSTC_BLESS_TEST", "--bless"); - // Assume this option is active if the environment variable is "defined", with _any_ value. - // As an example, a `Makefile` can use this option by: - // - // ifdef RUSTC_BLESS_TEST - // cp "$(TMPDIR)"/actual_something.ext expected_something.ext - // else - // $(DIFF) expected_something.ext "$(TMPDIR)"/actual_something.ext - // endif - } - - if self.config.target.contains("msvc") && !self.config.cc.is_empty() { - // We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe` - // and that `lib.exe` lives next to it. - let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe"); - - // MSYS doesn't like passing flags of the form `/foo` as it thinks it's - // a path and instead passes `C:\msys64\foo`, so convert all - // `/`-arguments to MSVC here to `-` arguments. - let cflags = self - .config - .cflags - .split(' ') - .map(|s| s.replace("/", "-")) - .collect::>() - .join(" "); - let cxxflags = self - .config - .cxxflags - .split(' ') - .map(|s| s.replace("/", "-")) - .collect::>() - .join(" "); - - cmd.env("IS_MSVC", "1") - .env("IS_WINDOWS", "1") - .env("MSVC_LIB", format!("'{}' -nologo", lib.display())) - .env("MSVC_LIB_PATH", format!("{}", lib.display())) - .env("CC", format!("'{}' {}", self.config.cc, cflags)) - .env("CXX", format!("'{}' {}", &self.config.cxx, cxxflags)); - } else { - cmd.env("CC", format!("{} {}", self.config.cc, self.config.cflags)) - .env("CXX", format!("{} {}", self.config.cxx, self.config.cxxflags)) - .env("AR", &self.config.ar); - - if self.config.target.contains("windows") { - cmd.env("IS_WINDOWS", "1"); - } - } - - let (output, truncated) = - self.read2_abbreviated(cmd.spawn().expect("failed to spawn `make`")); - if !output.status.success() { - let res = ProcRes { - status: output.status, - stdout: String::from_utf8_lossy(&output.stdout).into_owned(), - stderr: String::from_utf8_lossy(&output.stderr).into_owned(), - truncated, - cmdline: format!("{:?}", cmd), - }; - self.fatal_proc_rec("make failed", &res); - } - } - - 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 rust // library and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`. @@ -191,8 +29,6 @@ impl TestCx<'_> { // 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. let base_dir = self.output_base_dir(); ignore_not_found(|| recursive_remove(&base_dir)).unwrap(); From c8819afd4d85219c398dd633678d7c41d558cd1f 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:43:20 +0800 Subject: [PATCH 09/13] run-make: remove `tools.mk` It has served us well, but it's time to retire the `Makefile` support file since this is no longer needed. --- tests/run-make/tools.mk | 209 ---------------------------------------- 1 file changed, 209 deletions(-) delete mode 100644 tests/run-make/tools.mk diff --git a/tests/run-make/tools.mk b/tests/run-make/tools.mk deleted file mode 100644 index b1e872a202af5..0000000000000 --- a/tests/run-make/tools.mk +++ /dev/null @@ -1,209 +0,0 @@ -# These deliberately use `=` and not `:=` so that client makefiles can -# augment HOST_RPATH_DIR / TARGET_RPATH_DIR. -HOST_RPATH_ENV = \ - $(LD_LIB_PATH_ENVVAR)="$(TMPDIR):$(HOST_RPATH_DIR):$($(LD_LIB_PATH_ENVVAR))" -TARGET_RPATH_ENV = \ - $(LD_LIB_PATH_ENVVAR)="$(TMPDIR):$(TARGET_RPATH_DIR):$($(LD_LIB_PATH_ENVVAR))" - -RUSTC_ORIGINAL := $(RUSTC) -BARE_RUSTC := $(HOST_RPATH_ENV) '$(RUSTC)' -BARE_RUSTDOC := $(HOST_RPATH_ENV) '$(RUSTDOC)' -RUSTC := $(BARE_RUSTC) --out-dir $(TMPDIR) -L $(TMPDIR) $(RUSTFLAGS) -Ainternal_features -RUSTDOC := $(BARE_RUSTDOC) -L $(TARGET_RPATH_DIR) -ifdef RUSTC_LINKER -RUSTC := $(RUSTC) -Clinker='$(RUSTC_LINKER)' -RUSTDOC := $(RUSTDOC) -Clinker='$(RUSTC_LINKER)' -endif -#CC := $(CC) -L $(TMPDIR) -HTMLDOCCK := '$(PYTHON)' '$(S)/src/etc/htmldocck.py' -CGREP := "$(S)/src/etc/cat-and-grep.sh" - -# diff with common flags for multi-platform diffs against text output -DIFF := diff -u --strip-trailing-cr - -# With RUSTC_TEST_OP you can elegantly support blessing of run-make tests. Do -# like this in a Makefile recipe: -# -# "$(TMPDIR)"/your-test > "$(TMPDIR)"/your-test.run.stdout -# $(RUSTC_TEST_OP) "$(TMPDIR)"/your-test.run.stdout your-test.run.stdout -# -# When running the test normally with -# -# ./x test tests/run-make/your-test -# -# the actual output will be diffed against the expected output. When running in -# bless-mode with -# -# ./x test --bless tests/run-make/your-test -# -# the actual output will be blessed as the expected output. -ifdef RUSTC_BLESS_TEST - RUSTC_TEST_OP = cp -else - RUSTC_TEST_OP = $(DIFF) -endif - -# Some of the Rust CI platforms use `/bin/dash` to run `shell` script in -# Makefiles. Other platforms, including many developer platforms, default to -# `/bin/bash`. (In many cases, `make` is actually using `/bin/sh`, but `sh` -# is configured to execute one or the other shell binary). `dash` features -# support only a small subset of `bash` features, so `dash` can be thought of as -# the lowest common denominator, and tests should be validated against `dash` -# whenever possible. Most developer platforms include `/bin/dash`, but to ensure -# tests still work when `/bin/dash`, if not available, this `SHELL` override is -# conditional: -ifndef IS_WINDOWS # dash interprets backslashes in executable paths incorrectly -ifneq (,$(wildcard /bin/dash)) -SHELL := /bin/dash -endif -endif - -# This is the name of the binary we will generate and run; use this -# e.g. for `$(CC) -o $(RUN_BINFILE)`. -RUN_BINFILE = $(TMPDIR)/$(1) - -# Invoke the generated binary on the remote machine if compiletest was -# configured to use a remote test device, otherwise run it on the current host. -ifdef REMOTE_TEST_CLIENT -# FIXME: if a test requires additional files, this will need to be changed to -# also push them (by changing the 0 to the number of additional files, and -# providing the path of the additional files as the last arguments). -EXECUTE = $(REMOTE_TEST_CLIENT) run 0 $(RUN_BINFILE) -else -EXECUTE = $(RUN_BINFILE) -endif - -# RUN and FAIL are basic way we will invoke the generated binary. On -# non-windows platforms, they set the LD_LIBRARY_PATH environment -# variable before running the binary. - -RLIB_GLOB = lib$(1)*.rlib -BIN = $(1) - -UNAME = $(shell uname) - -ifeq ($(UNAME),Darwin) -RUN = $(TARGET_RPATH_ENV) $(EXECUTE) -FAIL = $(TARGET_RPATH_ENV) $(EXECUTE) && exit 1 || exit 0 -DYLIB_GLOB = lib$(1)*.dylib -DYLIB = $(TMPDIR)/lib$(1).dylib -STATICLIB = $(TMPDIR)/lib$(1).a -STATICLIB_GLOB = lib$(1)*.a -else -ifdef IS_WINDOWS -RUN = PATH="$(PATH):$(TARGET_RPATH_DIR)" $(EXECUTE) -FAIL = PATH="$(PATH):$(TARGET_RPATH_DIR)" $(EXECUTE) && exit 1 || exit 0 -DYLIB_GLOB = $(1)*.dll -DYLIB = $(TMPDIR)/$(1).dll -ifdef IS_MSVC -STATICLIB = $(TMPDIR)/$(1).lib -STATICLIB_GLOB = $(1)*.lib -else -IMPLIB = $(TMPDIR)/lib$(1).dll.a -STATICLIB = $(TMPDIR)/lib$(1).a -STATICLIB_GLOB = lib$(1)*.a -endif -BIN = $(1).exe -LLVM_FILECHECK := $(shell cygpath -u "$(LLVM_FILECHECK)") -else -RUN = $(TARGET_RPATH_ENV) $(EXECUTE) -FAIL = $(TARGET_RPATH_ENV) $(EXECUTE) && exit 1 || exit 0 -DYLIB_GLOB = lib$(1)*.so -DYLIB = $(TMPDIR)/lib$(1).so -STATICLIB = $(TMPDIR)/lib$(1).a -STATICLIB_GLOB = lib$(1)*.a -endif -endif - -ifdef IS_MSVC -COMPILE_OBJ = $(CC) -c -Fo:`cygpath -w $(1)` $(2) -COMPILE_OBJ_CXX = $(CXX) -EHs -c -Fo:`cygpath -w $(1)` $(2) -NATIVE_STATICLIB_FILE = $(1).lib -NATIVE_STATICLIB = $(TMPDIR)/$(call NATIVE_STATICLIB_FILE,$(1)) -OUT_EXE=-Fe:`cygpath -w $(TMPDIR)/$(call BIN,$(1))` \ - -Fo:`cygpath -w $(TMPDIR)/$(1).obj` -else -COMPILE_OBJ = $(CC) -v -c -o $(1) $(2) -COMPILE_OBJ_CXX = $(CXX) -c -o $(1) $(2) -NATIVE_STATICLIB_FILE = lib$(1).a -NATIVE_STATICLIB = $(call STATICLIB,$(1)) -OUT_EXE=-o $(TMPDIR)/$(1) -endif - - -# Extra flags needed to compile a working executable with the standard library -ifdef IS_WINDOWS -ifdef IS_MSVC - EXTRACFLAGS := ws2_32.lib userenv.lib advapi32.lib bcrypt.lib ntdll.lib synchronization.lib -else - EXTRACFLAGS := -lws2_32 -luserenv -lbcrypt -lntdll -lsynchronization - EXTRACXXFLAGS := -lstdc++ - # So this is a bit hacky: we can't use the DLL version of libstdc++ because - # it pulls in the DLL version of libgcc, which means that we end up with 2 - # instances of the DW2 unwinding implementation. This is a problem on - # i686-pc-windows-gnu because each module (DLL/EXE) needs to register its - # unwind information with the unwinding implementation, and libstdc++'s - # __cxa_throw won't see the unwinding info we registered with our statically - # linked libgcc. - # - # Now, simply statically linking libstdc++ would fix this problem, except - # that it is compiled with the expectation that pthreads is dynamically - # linked as a DLL and will fail to link with a statically linked libpthread. - # - # So we end up with the following hack: we link use static:-bundle to only - # link the parts of libstdc++ that we actually use, which doesn't include - # the dependency on the pthreads DLL. - EXTRARSCXXFLAGS := -l static:-bundle=stdc++ -endif -else -ifeq ($(UNAME),Darwin) - EXTRACFLAGS := -lresolv - EXTRACXXFLAGS := -lc++ - EXTRARSCXXFLAGS := -lc++ -else -ifeq ($(UNAME),FreeBSD) - EXTRACFLAGS := -lm -lpthread -lgcc_s -else -ifeq ($(UNAME),SunOS) - EXTRACFLAGS := -lm -lpthread -lposix4 -lsocket -lresolv -else -ifeq ($(UNAME),OpenBSD) - EXTRACFLAGS := -lm -lpthread -lc++abi - RUSTC := $(RUSTC) -C linker="$(word 1,$(CC:ccache=))" -else - EXTRACFLAGS := -lm -lrt -ldl -lpthread - EXTRACXXFLAGS := -lstdc++ - EXTRARSCXXFLAGS := -lstdc++ -endif -endif -endif -endif -endif - -REMOVE_DYLIBS = rm $(TMPDIR)/$(call DYLIB_GLOB,$(1)) -REMOVE_RLIBS = rm $(TMPDIR)/$(call RLIB_GLOB,$(1)) - -%.a: %.o - $(AR) crus $@ $< -ifdef IS_MSVC -%.lib: lib%.o - $(MSVC_LIB) -out:`cygpath -w $@` $< -else -%.lib: lib%.o - $(AR) crus $@ $< -endif -%.dylib: %.o - $(CC) -dynamiclib -Wl,-dylib -o $@ $< -%.so: %.o - $(CC) -o $@ $< -shared - -ifdef IS_MSVC -%.dll: lib%.o - $(CC) $< -link -dll -out:`cygpath -w $@` -else -%.dll: lib%.o - $(CC) -o $@ $< -shared -Wl,--out-implib=$@.a -endif - -$(TMPDIR)/lib%.o: %.c - $(call COMPILE_OBJ,$@,$<) From 7c60e78d1d4e84ce82d23e1889a9ba94697f3e9b 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:35:25 +0800 Subject: [PATCH 10/13] run-make: update test suite README --- tests/run-make/README.md | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/tests/run-make/README.md b/tests/run-make/README.md index 4035990347389..66698eab93620 100644 --- a/tests/run-make/README.md +++ b/tests/run-make/README.md @@ -1,32 +1,17 @@ # The `run-make` test suite -The `run-make` test suite contains tests which are the most flexible out of all -the [rust-lang/rust](https://github.com/rust-lang/rust) test suites. `run-make` -tests can basically contain arbitrary code, and are supported by the -[`run_make_support`] library. +The `run-make` test suite contains tests which are the most flexible out of all the [rust-lang/rust](https://github.com/rust-lang/rust) test suites. `run-make` tests can basically contain arbitrary code, and are supported by the [`run_make_support`] library. ## Infrastructure -There are two kinds of run-make tests: +A `run-make` test is a test recipe source file `rmake.rs` accompanied by its parent directory (e.g. `tests/run-make/foo/rmake.rs` is the `foo` `run-make` test). -1. The new `rmake.rs` version: this allows run-make tests to be written in Rust - (with `rmake.rs` as the main test file). -2. The legacy `Makefile` version: this is what run-make tests were written with - before support for `rmake.rs` was introduced. +The implementation for collecting and building the `rmake.rs` recipes are in [`src/tools/compiletest/src/runtest.rs`](../../src/tools/compiletest/src/runtest.rs), in `run_rmake_v2_test` and `run_rmake_legacy_test`. -The implementation for collecting and building the `rmake.rs` recipes (or -`Makefile`s) are in -[`src/tools/compiletest/src/runtest.rs`](../../src/tools/compiletest/src/runtest.rs), -in `run_rmake_v2_test` and `run_rmake_legacy_test`. - -### Rust-based `run-make` tests: `rmake.rs` - -The setup for the `rmake.rs` version is a 3-stage process: +The setup for the `rmake.rs` can be summarized as a 3-stage process: 1. First, we build the [`run_make_support`] library in bootstrap as a tool lib. -2. Then, we compile the `rmake.rs` "recipe" linking the support library and its - dependencies in, and provide a bunch of env vars. We setup a directory - structure within `build//test/run-make/` +2. Then, we compile the `rmake.rs` "recipe" linking the support library and its dependencies in, and provide a bunch of env vars. We setup a directory structure within `build//test/run-make/` ``` / @@ -34,15 +19,8 @@ The setup for the `rmake.rs` version is a 3-stage process: rmake_out/ # sources from test sources copied over ``` - and copy non-`rmake.rs` input support files over to `rmake_out/`. The - support library is made available as an [*extern prelude*][extern_prelude]. -3. Finally, we run the recipe binary and set `rmake_out/` as the working - directory. + and copy non-`rmake.rs` input support files over to `rmake_out/`. The support library is made available as an [*extern prelude*][extern_prelude]. +3. Finally, we run the recipe binary and set `rmake_out/` as the working directory. [`run_make_support`]: ../../src/tools/run-make-support [extern_prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude - -### Formatting - -Note that files under `tests/` are not formatted by `./x fmt`, -use `rustfmt tests/path/to/file.rs` to format a specific file if desired. From 393c7aabe88df1a09d3697f153ba13b37cc9ba20 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:48:48 +0800 Subject: [PATCH 11/13] tidy: remove legacy `Makefile` checks --- .../tidy/src/allowed_run_make_makefiles.txt | 0 src/tools/tidy/src/lib.rs | 1 - src/tools/tidy/src/main.rs | 2 - src/tools/tidy/src/run_make_tests.rs | 104 ------------------ 4 files changed, 107 deletions(-) delete mode 100644 src/tools/tidy/src/allowed_run_make_makefiles.txt delete mode 100644 src/tools/tidy/src/run_make_tests.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 1e7eb82b83e98..9f6d563166e2b 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -79,7 +79,6 @@ pub(crate) mod iter_header; pub mod known_bug; pub mod mir_opt_tests; pub mod pal; -pub mod run_make_tests; pub mod rustdoc_css_themes; pub mod rustdoc_gui_tests; pub mod rustdoc_templates; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 13a558fea48ea..1d8514ef4c9c4 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -103,8 +103,6 @@ fn main() { check!(tests_revision_unpaired_stdout_stderr, &tests_path); check!(debug_artifacts, &tests_path); check!(ui_tests, &root_path, bless); - // FIXME(jieyouxu): remove this check once all run-make tests are ported over to rmake.rs. - check!(run_make_tests, &tests_path, &src_path, bless); check!(mir_opt_tests, &tests_path, bless); check!(rustdoc_gui_tests, &tests_path); check!(rustdoc_css_themes, &librustdoc_path); diff --git a/src/tools/tidy/src/run_make_tests.rs b/src/tools/tidy/src/run_make_tests.rs deleted file mode 100644 index 8d1767878378c..0000000000000 --- a/src/tools/tidy/src/run_make_tests.rs +++ /dev/null @@ -1,104 +0,0 @@ -//! Tidy check to ensure that no new Makefiles are added under `tests/run-make/`. - -use std::collections::BTreeSet; -use std::fs::File; -use std::io::Write; -use std::path::{Path, PathBuf}; - -pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { - let mut is_sorted = true; - - let allowed_makefiles = { - let mut total_lines = 0; - let mut prev_line = ""; - let allowed_makefiles: BTreeSet<&str> = include_str!("allowed_run_make_makefiles.txt") - .lines() - .map(|line| { - total_lines += 1; - - if prev_line > line { - is_sorted = false; - } - - prev_line = line; - - line - }) - .collect(); - - if !is_sorted && !bless { - tidy_error!( - bad, - "`src/tools/tidy/src/allowed_run_make_makefiles.txt` is not in order, likely \ - because you modified it manually, please only update it with command \ - `x test tidy --bless`" - ); - } - if allowed_makefiles.len() != total_lines { - tidy_error!( - bad, - "`src/tools/tidy/src/allowed_run_make_makefiles.txt` contains duplicate entries, \ - likely because you modified it manually, please only update it with command \ - `x test tidy --bless`" - ); - } - - allowed_makefiles - }; - - let mut remaining_makefiles = allowed_makefiles.clone(); - - crate::walk::walk_no_read( - &[tests_path.join("run-make").as_ref()], - |_, _| false, - &mut |entry| { - if entry.file_type().map_or(true, |t| t.is_dir()) { - return; - } - - if entry.file_name().to_str().map_or(true, |f| f != "Makefile") { - return; - } - - let makefile_path = entry.path().strip_prefix(&tests_path).unwrap(); - let makefile_path = makefile_path.to_str().unwrap().replace('\\', "/"); - - if !remaining_makefiles.remove(makefile_path.as_str()) { - tidy_error!( - bad, - "found run-make Makefile not permitted in \ - `src/tools/tidy/src/allowed_run_make_makefiles.txt`, please write new run-make \ - tests with `rmake.rs` instead: {}", - entry.path().display() - ); - } - }, - ); - - // If there are any expected Makefiles remaining, they were moved or deleted. - // Our data must remain up to date, so they must be removed from - // `src/tools/tidy/src/allowed_run_make_makefiles.txt`. - // This can be done automatically on --bless, or else a tidy error will be issued. - if bless && (!remaining_makefiles.is_empty() || !is_sorted) { - let tidy_src = src_path.join("tools").join("tidy").join("src"); - let org_file_path = tidy_src.join("allowed_run_make_makefiles.txt"); - let temp_file_path = tidy_src.join("blessed_allowed_run_make_makefiles.txt"); - let mut temp_file = t!(File::create_new(&temp_file_path)); - for file in allowed_makefiles.difference(&remaining_makefiles) { - t!(writeln!(temp_file, "{file}")); - } - t!(std::fs::rename(&temp_file_path, &org_file_path)); - } else { - for file in remaining_makefiles { - let mut p = PathBuf::from(tests_path); - p.push(file); - tidy_error!( - bad, - "Makefile `{}` no longer exists and should be removed from the exclusions in \ - `src/tools/tidy/src/allowed_run_make_makefiles.txt`, you can run `x test tidy --bless` to update \ - the allow list", - p.display() - ); - } - } -} From 47c269c7b9933b8c5980544e1596752b56a50d5b 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:40:57 +0800 Subject: [PATCH 12/13] rustc-dev-guide: remove mentions of legacy `Makefile` run-make infra And remove outdated requirements to run `run-make` tests on Windows. --- .../rustc-dev-guide/src/tests/compiletest.md | 30 +------------------ .../rustc-dev-guide/src/tests/directives.md | 7 +---- src/doc/rustc-dev-guide/src/tests/running.md | 23 -------------- 3 files changed, 2 insertions(+), 58 deletions(-) diff --git a/src/doc/rustc-dev-guide/src/tests/compiletest.md b/src/doc/rustc-dev-guide/src/tests/compiletest.md index 459c082906eba..2905e470fabd3 100644 --- a/src/doc/rustc-dev-guide/src/tests/compiletest.md +++ b/src/doc/rustc-dev-guide/src/tests/compiletest.md @@ -74,8 +74,7 @@ The following test suites are available, with links for more information: ### General purpose test suite -[`run-make`](#run-make-tests) are general purpose tests using Rust programs (or -Makefiles (legacy)). +[`run-make`](#run-make-tests) are general purpose tests using Rust programs. ### Rustdoc test suites @@ -396,14 +395,6 @@ your test, causing separate files to be generated for 32bit and 64bit systems. ### `run-make` tests -> **Note on phasing out `Makefile`s** -> -> We are planning to migrate all existing Makefile-based `run-make` tests -> to Rust programs. You should not be adding new Makefile-based `run-make` -> tests. -> -> See . - The tests in [`tests/run-make`] are general-purpose tests using Rust *recipes*, which are small programs (`rmake.rs`) allowing arbitrary Rust code such as `rustc` invocations, and is supported by a [`run_make_support`] library. Using @@ -424,11 +415,6 @@ Compiletest directives like `//@ only-` or `//@ ignore-` are supported in `rmake.rs`, like in UI tests. However, revisions or building auxiliary via directives are not currently supported. -Two `run-make` tests are ported over to Rust recipes as examples: - -- -- - #### Quickly check if `rmake.rs` tests can be compiled You can quickly check if `rmake.rs` tests can be compiled without having to @@ -481,20 +467,6 @@ Then add a corresponding entry to `"rust-analyzer.linkedProjects"` ], ``` -#### Using Makefiles (legacy) - -
-You should avoid writing new Makefile-based `run-make` tests. -
- -Each test should be in a separate directory with a `Makefile` indicating the -commands to run. - -There is a [`tools.mk`] Makefile which you can include which provides a bunch of -utilities to make it easier to run commands and compare outputs. Take a look at -some of the other tests for some examples on how to get started. - -[`tools.mk`]: https://github.com/rust-lang/rust/blob/master/tests/run-make/tools.mk [`tests/run-make`]: https://github.com/rust-lang/rust/tree/master/tests/run-make [`run_make_support`]: https://github.com/rust-lang/rust/tree/master/src/tools/run-make-support diff --git a/src/doc/rustc-dev-guide/src/tests/directives.md b/src/doc/rustc-dev-guide/src/tests/directives.md index 9e0f8f9c279c7..9f734a28ed1f7 100644 --- a/src/doc/rustc-dev-guide/src/tests/directives.md +++ b/src/doc/rustc-dev-guide/src/tests/directives.md @@ -6,10 +6,7 @@ FIXME(jieyouxu) completely revise this chapter. --> -Directives are special comments that tell compiletest how to build and interpret -a test. They must appear before the Rust source in the test. They may also -appear in `rmake.rs` or legacy Makefiles for [run-make -tests](compiletest.md#run-make-tests). +Directives are special comments that tell compiletest how to build and interpret a test. They must appear before the Rust source in the test. They may also appear in `rmake.rs` [run-make tests](compiletest.md#run-make-tests). They are normally put after the short comment that explains the point of this test. Compiletest test suites use `//@` to signal that a comment is a directive. @@ -218,8 +215,6 @@ The following directives will check LLVM support: [`aarch64-gnu-debug`]), which only runs a subset of `run-make` tests. Other tests with this directive will not run at all, which is usually not what you want. - - Notably, the [`aarch64-gnu-debug`] CI job *currently* only runs `run-make` - tests which additionally contain `clang` in their test name. See also [Debuginfo tests](compiletest.md#debuginfo-tests) for directives for ignoring debuggers. diff --git a/src/doc/rustc-dev-guide/src/tests/running.md b/src/doc/rustc-dev-guide/src/tests/running.md index 6ce65092389bd..591cca3067b56 100644 --- a/src/doc/rustc-dev-guide/src/tests/running.md +++ b/src/doc/rustc-dev-guide/src/tests/running.md @@ -240,28 +240,6 @@ and they may not run the same without those options. ## Running `run-make` tests -### Windows - -Running the `run-make` test suite on Windows is a currently bit more involved. -There are numerous prerequisites and environmental requirements: - -- Install msys2: -- Specify `MSYS2_PATH_TYPE=inherit` in `msys2.ini` in the msys2 installation directory, run the - following with `MSYS2 MSYS`: - - `pacman -Syuu` - - `pacman -S make` - - `pacman -S diffutils` - - `pacman -S binutils` - - `./x test run-make` (`./x test tests/run-make` doesn't work) - -There is [on-going work][port-run-make] to not rely on `Makefile`s in the -run-make test suite. Once this work is completed, you can run the entire -`run-make` test suite on native Windows inside `cmd` or `PowerShell` without -needing to install and use MSYS2. As of Oct 2024, it is -already possible to run the vast majority of the `run-make` test suite outside -of MSYS2, but there will be failures for the tests that still use `Makefile`s -due to not finding `make`. - ## Running tests on a remote machine Tests may be run on a remote machine (e.g. to test builds for a different @@ -406,4 +384,3 @@ If you encounter bugs or problems, don't hesitate to open issues on the repository](https://github.com/rust-lang/rustc_codegen_gcc/). [`tests/ui`]: https://github.com/rust-lang/rust/tree/master/tests/ui -[port-run-make]: https://github.com/rust-lang/rust/issues/121876 From 45c0df1d7b7e0cbfacc05ae8b42398e6cf4939e8 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?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:52:30 +0800 Subject: [PATCH 13/13] triagebot: stop backlinking to the test porting tracking issue No longer needed. --- triagebot.toml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index efd694994f9f7..a2c6c9860add5 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -797,13 +797,7 @@ message = "Some changes occurred in GUI tests." cc = ["@GuillaumeGomez"] [mentions."tests/run-make/"] -message = """ -This PR modifies `tests/run-make/`. If this PR is trying to port a Makefile -run-make test to use rmake.rs, please update the -[run-make port tracking issue](https://github.com/rust-lang/rust/issues/121876) -so we can track our progress. You can either modify the tracking issue -directly, or you can comment on the tracking issue and link this PR. -""" +message = "This PR modifies `run-make` tests." cc = ["@jieyouxu"] [mentions."src/rustdoc-json-types"]