[DO NOT LOOK] a bunch of compiletest-related changes by jieyouxu · Pull Request #136437 · rust-lang/rust

jieyouxu · 2025-02-02T13:01:52Z

Some exploratory changes while trying to figure out why cg_clif's tests were failing.

Not intended for review, this PR needs to be broken up into reviewable PRs with logical commits. Only posted for reference purposes.

-        cmd.arg("--src-base").arg(builder.src.join("tests").join(suite));
-        cmd.arg("--build-base").arg(testdir(builder, compiler.host).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-root").arg(&builder.out);
+        cmd.arg("--build-test-suite-root").arg(testdir(builder, compiler.host).join(suite));

$ find tests -type f -exec sed -i 's/{{src-base}}/{{test-suite-src-base}}/g' {} \;
$ find tests -type f -exec sed -i 's/{{rust-src-base}}/{{sysroot-rust-src-base}}/g' {} \;
$ find tests -type f -exec sed -i 's/{{build-base}}/{{test-suite-build-base}}/g' {} \;

Planned PR series

Cleanup is_rustdoc matching logic and remove a useless rustdoc-json Path::join. [compiletest-related cleanups 1/7] Cleanup is_rustdoc logic and remove a useless path join in rustdoc-json runtest logic #136441
Pass --stage as a compiletest flag to not have to do stage-id gymnastics in run-make runtest logic. [compiletest-related cleanups 2/7] Feed stage number to compiletest directly #136472
Rectify src_base handling: split --src-base into two flags, --src-root and --src-test-suite-root, and tidy up logic that exercises src_base previously. Remove find_rust_src_root. [compiletest-related cleanups 3/7] Make the distinction between sources root vs test suite sources root in compiletest less confusing #136474
Rectify build_base handling: split --build-base into two flags, --build-root and --build-test-suite-root, and tidy up logic that exercises build_base previously. [compiletest-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing #136542
Rename {{src-base}} -> {{test-suite-src-base}}. Rebless tests. Update rustc-dev-guide.
Rename {{rust-src-base}} -> {{sysroot-rust-src-base}}. Rebless tests. Update rustc-dev-guide.
Rename {{build-base}} -> {{test-suite-build-base}}. Rebless tests. Update rustc-dev-guide.

jieyouxu · 2025-02-02T13:02:36Z

Sorry, forgor to write ghost.

jieyouxu · 2025-02-02T13:09:45Z

src/tools/compiletest/src/runtest/rustdoc_json.rs

-        let root = self.config.find_rust_src_root().unwrap();
        let mut json_out = out_dir.join(self.testpaths.file.file_stem().unwrap());
        json_out.set_extension("json");
        let res = self.run_command_to_procres(
            Command::new(self.config.jsondocck_path.as_ref().unwrap())
                .arg("--doc-dir")
-                .arg(root.join(&out_dir))
+                .arg(&out_dir)


This was simply wrong, and only happens to work because PathBuf::join when the arg is an absolute path out_dir will simply became a replacement operation.

jieyouxu · 2025-02-02T13:11:03Z

tests/ui/errors/remap-path-prefix-reverse.rs


 //@ revisions: local-self remapped-self
-// [local-self] no-remap-src-base: The hack should work regardless of remapping.
+// The hack should work regardless of remapping.


This is kinda-wrong, in the sense that no-remap-src-base is not a directive yet this line looks like a revisioned directive, but also, because this isn't recognized as a revisioned compiletest directive (not //@), it ends up functioning as intended.

jieyouxu · 2025-02-02T13:12:01Z

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

-            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();


Beautiful incantation :ferrisClueless:

jieyouxu · 2025-02-02T13:12:39Z

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

-        // FIXME(jieyouxu): improve the communication between bootstrap and compiletest here so
-        // we don't have to hack out a `stageN`.
-        let stage = self.config.stage_id.split('-').next().unwrap();


Yeah, for example, by actually passing the stage number through a flag like --stage

jieyouxu · 2025-02-02T13:13:08Z

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

+            // Path to `$build_dir/$host_triple/`.
+            .env("BUILD_ROOT", &host_build_root)


Cheeky, not build/, but build/$host/.

jieyouxu · 2025-02-02T13:13:25Z

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

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


jieyouxu · 2025-02-02T13:13:54Z

src/tools/compiletest/src/runtest.rs

-        self.config.src_base.ends_with("rustdoc-ui")
-            || self.config.src_base.ends_with("rustdoc-js")
-            || self.config.src_base.ends_with("rustdoc-json")
+        matches!(self.config.suite.as_str(), "rustdoc-ui" | "rustdoc-js" | "rustdoc-json")


Not sure what's happening here lol

jieyouxu · 2025-02-02T13:14:37Z

src/tools/compiletest/src/header.rs

-    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
-    }


No need to use a heuristic to find source root, when we can just provide it through bootstrap.

jieyouxu · 2025-02-02T13:16:38Z

src/tools/compiletest/src/common.rs

-    /// The directory containing the tests to run
-    pub src_base: PathBuf,
-
-    /// The directory where programs should be built
-    pub build_base: PathBuf,
-
-    /// The directory containing the compiler sysroot
-    pub sysroot_base: PathBuf,
-
+    /// Path to directory containing rust-lang/rust sources.
+    pub src_root: PathBuf,
+    /// Path to the directory containg the test suite sources. Expected to be a subdirectory of
+    /// `src_root`.
+    pub src_test_suite_root: PathBuf,
+
+    /// Bootstrap build directory.
+    pub build_root: PathBuf,
+    /// Bootstrap test suite build directory. Expected to be a subdirectory of `build_root`.
+    pub build_test_suite_root: PathBuf,


These are terribly named, and looking at the runtest impls, it has confused multiple people. src_base is not sources root, it's the directory $SRC_ROOT/tests/<test-suite>/. build_base is not build directory root, it's $BUILD/<host-triple>/test/<test-suite>.

jieyouxu · 2025-02-02T13:49:43Z

src/tools/compiletest/src/runtest.rs

        // 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");
+        eprintln!("build_test_suite_root = {:?}", self.config.build_test_suite_root.display());


Reminder: delete this

…piler-errors [`compiletest`-related cleanups 1/7] Cleanup `is_rustdoc` logic and remove a useless path join in rustdoc-json runtest logic Reference for overall changes: rust-lang#136437 Part **1** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Don't match on path when we already have test suite names. - Remove a useless path join. r? bootstrap (or compiler)

Rollup merge of rust-lang#136441 - jieyouxu:cleanup-is-rustdoc, r=compiler-errors [`compiletest`-related cleanups 1/7] Cleanup `is_rustdoc` logic and remove a useless path join in rustdoc-json runtest logic Reference for overall changes: rust-lang#136437 Part **1** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Don't match on path when we already have test suite names. - Remove a useless path join. r? bootstrap (or compiler)

[`compiletest`-related cleanups 1/7] Cleanup `is_rustdoc` logic and remove a useless path join in rustdoc-json runtest logic Reference for overall changes: rust-lang/rust#136437 Part **1** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Don't match on path when we already have test suite names. - Remove a useless path join. r? bootstrap (or compiler)

…crum [`compiletest`-related cleanups 2/7] Feed stage number to compiletest directly Reference for overall changes: rust-lang#136437 Part **2** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Pass stage number via new `--stage` compiletest flag directly from bootstrap, instead of deriving that info in compiletest by doing gymnastics on `--stage-id`. - Just a cleanup, should have no functional changes. r? bootstrap

Rollup merge of rust-lang#136472 - jieyouxu:pass-stage, r=Mark-Simulacrum [`compiletest`-related cleanups 2/7] Feed stage number to compiletest directly Reference for overall changes: rust-lang#136437 Part **2** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Pass stage number via new `--stage` compiletest flag directly from bootstrap, instead of deriving that info in compiletest by doing gymnastics on `--stage-id`. - Just a cleanup, should have no functional changes. r? bootstrap

bors · 2025-02-10T15:32:51Z

☔ The latest upstream changes (presumably #136809) made this pull request unmergeable. Please resolve the merge conflicts.

[`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

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

[`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing Reference for overall changes: rust-lang#136437 Part **4** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Remove `--build-base` compiletest flag, and introduce `--build-{root,test-suite-root}` flags instead. `--build-base` previously actually is test suite specific build directory, not the root `build/` directory. - Feed the root build directory directly from bootstrap to compiletest via `--build-root` instead of doing multiple layers of parent unwrapping[^parent] based on the test suite specific build directory. - Remove a redundant `to_path_buf()`. [^parent]: Please do not unwrap the parents. r? bootstrap

Rollup merge of rust-lang#136542 - jieyouxu:build-base, r=onur-ozkan [`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing Reference for overall changes: rust-lang#136437 Part **4** of **7** of the *`compiletest`-related cleanups* PR series. ### Summary - Remove `--build-base` compiletest flag, and introduce `--build-{root,test-suite-root}` flags instead. `--build-base` previously actually is test suite specific build directory, not the root `build/` directory. - Feed the root build directory directly from bootstrap to compiletest via `--build-root` instead of doing multiple layers of parent unwrapping[^parent] based on the test suite specific build directory. - Remove a redundant `to_path_buf()`. [^parent]: Please do not unwrap the parents. r? bootstrap

jieyouxu · 2025-03-20T08:14:45Z

Closing because I'm not sure if the substitution renaming is worth the churn.

a bunch of changes

ab3a013

rustbot assigned oli-obk Feb 2, 2025

This comment was marked as off-topic.

Sign in to view

jieyouxu unassigned oli-obk Feb 2, 2025

jieyouxu commented Feb 2, 2025

View reviewed changes

jieyouxu mentioned this pull request Feb 4, 2025

[compiletest-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing #136542

Merged

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 10, 2025

jieyouxu closed this Mar 20, 2025

jieyouxu deleted the compiletest-rust-src-root branch March 20, 2025 08:14

		// Path to `$build_dir/$host_triple/`.
		.env("BUILD_ROOT", &host_build_root)

Uh oh!

Conversation

jieyouxu commented Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Planned PR series

Uh oh!

This comment was marked as off-topic.

jieyouxu commented Feb 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Feb 10, 2025

Uh oh!

jieyouxu commented Mar 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jieyouxu commented Feb 2, 2025 •

edited

Loading

jieyouxu Feb 2, 2025 •

edited

Loading