tidy: validate LLVM component names in tests #125472
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LLVM component names are not immediately obvious (they usually omit any suffixes on the target arch name), and if they're incorrect, the test will silently never run.
|
r? @clubby789 rustbot has assigned @clubby789. Use |
rustbot
added
A-testsuite
erikdesjardins
commented
May 24, 2024
Comment on lines
+13
to
+30
| const KNOWN_LLVM_COMPONENTS: &[&str] = &[ | ||
| "aarch64", | ||
| "arm", | ||
| "avr", | ||
| "bpf", | ||
| "hexagon", | ||
| "loongarch", | ||
| "m68k", | ||
| "mips", | ||
| "msp430", | ||
| "nvptx", | ||
| "powerpc", | ||
| "riscv", | ||
| "sparc", | ||
| "systemz", | ||
| "webassembly", | ||
| "x86", | ||
| ]; |
There was a problem hiding this comment.
I considered invoking llvm-config --components to get this list, but I think that would be a bit awkward to do within tidy.
I think hardcoding it is fine. It should be very rare that this needs to be changed--only when adding support for a brand new architecture.
There was a problem hiding this comment.
It would mean tidy fails if it can't run llvm-config, and I'm not sure we actually haul that binary around...
There was a problem hiding this comment.
Would be good to give people some guidance on how to figure out the correct current list.
There was a problem hiding this comment.
FWIW this works for me
build/x86_64-unknown-linux-gnu/ci-llvm/bin/llvm-config --components
but it lists a lot more than this PR.
There was a problem hiding this comment.
many of them are actually the same thing, like this:
- hexagon
- hexagonasmparser
- hexagoncodegen
- hexagondesc
- hexagondisassembler
- hexagoninfo
for our purposes we only really care about having "hexagon"
|
@bors r+ |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 25, 2024
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#121377 (Stabilize `LazyCell` and `LazyLock`) - rust-lang#122986 (Fix c_char on AIX) - rust-lang#123803 (Fix `VecDeque::shrink_to` UB when `handle_alloc_error` unwinds.) - rust-lang#124080 (Some unstable changes to where opaque types get defined) - rust-lang#124667 (Stabilize `div_duration`) - rust-lang#125472 (tidy: validate LLVM component names in tests) - rust-lang#125523 (Exit the process a short time after entering our ctrl-c handler) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 26, 2024
Rollup merge of rust-lang#125472 - erikdesjardins:component, r=clubby789 tidy: validate LLVM component names in tests LLVM component names are not immediately obvious (they usually omit any suffixes on the target arch name), and if they're incorrect, the test will silently never run. This happened [here](rust-lang#125220 (comment)), and it would be nice to prevent it.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
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
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 29, 2024
… r=nikic 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
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 17, 2024
Revert "tidy: validate LLVM component names in tests" This reverts rust-lang#125472. This has already caused a [bit](rust-lang#125702) of [trouble](rust-lang#125710), and I was mistaken about the original motivation--incorrect component names [_will_](rust-lang#125702 (comment)) be detected by a full CI run. I no longer think it pulls its weight. r? `@workingjubilee`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LLVM component names are not immediately obvious (they usually omit any suffixes on the target arch name), and if they're incorrect, the test will silently never run.
This happened here, and it would be nice to prevent it.