Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot write tests that need the csky backend #125697

Closed
RalfJung opened this issue May 29, 2024 · 5 comments · Fixed by #125702
Closed

Cannot write tests that need the csky backend #125697

RalfJung opened this issue May 29, 2024 · 5 comments · Fixed by #125702
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-csky Target: glaCSKY above covers over me~ T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@RalfJung
Copy link
Member

In tests/ui/abi/compatibility.rs we're trying to test the ABI adjustment logic of all targets. This generally works as the test is no_core and LLVM has backends for all targets available -- except it does not seem possible to test the csky target that way. Trying to add the usual needs-llvm-components: csky leads to an error from tidy:

/checkout/tests/ui/abi/compatibility.rs: revision csky specifies unknown LLVM component `csky`

The test passes just fine locally, so it seems like at least the downloaded LLVM has the csky component. I don't know why it was not added to tidy's KNOWN_LLVM_COMPONENTS list.

Cc @Dirreke

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 29, 2024
@workingjubilee
Copy link
Contributor

cc @erikdesjardins but also I think this was just an oversight.

@RalfJung
Copy link
Member Author

FWIW if one just adds this to a test it does pass on CI

//@revisions: csky
//@[csky] compile-flags: --target csky-unknown-linux-gnuabiv2
//@[csky] needs-llvm-components: csky

but of course it may just be silently ignored, hard to say whether the test actually runs... this needs-llvm-components stuff makes tests quite fragile. :/

@workingjubilee
Copy link
Contributor

Yeah, the tidy check was added to reduce the fragility because a lot of people were giving names other than the LLVM component's name, like riscv64 instead of riscv, etc.

@RalfJung
Copy link
Member Author

The remaining question then is, how sure are we that the components accepted by tidy are all available on some CI runner?

@RalfJung
Copy link
Member Author

Also, something else must have been going on... I disabled this test before that sanity check was added to tidy.

Sadly the logs for this failure are already gone.

@jieyouxu jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 29, 2024
@workingjubilee workingjubilee added the O-csky Target: glaCSKY above covers over me~ label May 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue May 29, 2024
… r=Kobzol,lqd

Give tidy the good news about C-SKY

It seems this was overlooked in rust-lang#125472 because we don't test C-SKY much yet.

Fixes rust-lang#125697

r? `@erikdesjardins`
@bors bors closed this as completed in 23ea77b May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-csky Target: glaCSKY above covers over me~ T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants