Detect cyclic auxiliary

[Shunpoco]

This PR adds a function which detects a cyclic auxiliaries.
It builds a graph from files in the given auxiliary dir, then search if there is a cycle in the graph using DFS.
The function runs when compiletest collects tests from directories, so a cycle is detected before actual tests are executed.

Fixes #133580

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Kobzol (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[Shunpoco]

I'm not sure if we can put a test for compiletest which fails in preparation time, but I ran a manual test on my local:

1. Put a test file aux-cyclic.rs under /tests/ui/compiletest-self-test/

//@ aux-build:cycle1.rs
//@ should-fail

extern crate cycle1;

2. Put an aux file cycle1.rs under /tests/ui/compiletest-self-test/auxiliary/

//@aux-build:cycle2.rs

extern crate cycle2;

3. Put an aux file cycle2.rs under /tests/ui/compiletest-self-test/auxiliary/

//@aux-build:cycle1.rs

extern crate cycle1;

So there is a cycle between cycle1.rs and cycle2.rs.

4. Run test

$ ./x test ./tests/ui/compiletest-self-test/
...
Testing stage1 compiletest suite=ui mode=ui (aarch64-apple-darwin)
thread 'main' panicked at src/tools/compiletest/src/lib.rs:604:13:
Could not read tests from /rust/tests/ui: detect cyclic auxiliary: ["cycle1.rs", "cycle2.rs"]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:00:02

[rustbot]

Some changes occurred in src/tools/compiletest

cc @jieyouxu

[jieyouxu]

Thanks for the PR. I'll try to do some manual testing tmrw.

[jieyouxu]

I'm busy this weekend as I'm going to the CS major, I'll revisit this next Monday.

Remark (for myself mostly): I took a cursory glance, and the filesystem ops looks potentially expensive, will take a closer look.

[jieyouxu]

Problem: so I thought about this a bit. build_graph during test collection that does directory traversal and file reading and prop parsing is quite expensive (especially on slower filesystems like native Windows), and I don't think this is necessarily the right place to do the check.

I would've expected that compiletest does something like:

1. First, it collects the tests based on the path filter (or was it test name filter) that was passed from bootstrap.
2. Then, compiletest will do some "early props" collection and handling. At this point is when I believe compiletest establishes the basic test conditions and relationships between main test files and their auxiliaries (i.e. if a test will be run or ignored, if it has auxiliaries and such).
3. Then, compiletest will further collect and handle "late props".

I would've expected that aux cycle detection happens after early prop collection after we establish the relationship between test files and auxiliaries, and not during test discovery and collection.

Alternatively, we could also just declare that we're not going to handle cyclic auxiliaries and "just don't do that", and doing cyclic auxiliaries is considered PEBKAC as "compiletest did exactly what the user instructed it to do".

cc @oli-obk: how does ui_test handle infinitely-recursive auxiliaries?

[oli-obk]

It doesn't: oli-obk/ui_test#123

But it should be fixable now that I have a build manager that handles all of these in one place.

[Shunpoco]

@jieyouxu
Thanks for your review.

2. Then, compiletest will do some "early props" collection and handling. At this point is when I believe compiletest establishes the basic test conditions and relationships between main test files and their auxiliaries (i.e. if a test will be run or ignored, if it has auxiliaries and such).

Now I understood that cycle detection should be run around here, is that right?

FYI: EarlyProps gathers auxiliaries only which are referred from the test file. For example from my manual test files above, cycle1.rs is included in the early_props for aux-cyclic.rs but cycle2.rs is not because only cycle1.rs has a dependency to cycle2.rs. So reading files to make a dependency graph is inevitable, but it should be lighter than reading files by entire directory traversal since we can check the cycle only against test files which contains auxs.

[jieyouxu]

Good question, I'll have to revisit this.

[jieyouxu]

@rustbot review (for revisit)

EDIT: I'll revisit this early next week.

[jieyouxu]

@Shunpoco I thought about this some more, I'd like to postpone adding this detection logic and revisit this during https://rust-lang.github.io/rust-project-goals/2025h1/compiletest-directive-rework.html?highlight=compiletest#making-compiletest-more-maintainable-reworking-directive-handling work, because:

• I'll likely redo the aux build graph logic, then coalesce your changes for aux build cycle detection (with proper attribution, of course) when I get to that.
• In particular, I'm going to have to do some experimentation to see how it fits into test discovery logic, because I likely will not stick with the current EarlyProps/LateProps distinction.
• Also, the current compiletest does not have the self-test infra to make this non-trivial logic easily self-testable, and I'd like to revisit how to properly self-test this.

I'm going to close this PR for the time being, thanks for your PR!