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

Add missing needs-llvm-components directives for run-make tests that need target-specific codegen #129605

Merged
merged 2 commits into from
Aug 31, 2024

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Aug 26, 2024

Without suitable needs-llvm-components directives, some run-make tests exercising target-specific codegen can fail if the LLVM used is built without the necessary components. Currently, the list is:

tests\run-make\print-target-list
tests\run-make\print-to-output
tests\run-make\print-cfg
tests\run-make\target-without-atomic-cas

This PR also skips tidy checks for revisions and needs-llvm-components for run-make tests since revisions are not supported.

Fixes #129390.
Fixes #127895.

cc @petrochenkov who noticed this, thanks! Would be great if you could confirm that this fixes the test errors for you locally.

Since run-make tests do not support revisions.
Without suitable `needs-llvm-components` directives, these tests that
rely on target-specific codegen can fail if used with a LLVM that is
built without the required components.
@rustbot
Copy link
Collaborator

rustbot commented Aug 26, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 26, 2024
Comment on lines +5 to +7
//@ needs-llvm-components: aarch64 arm avr bpf csky hexagon loongarch m68k mips msp430 nvptx powerpc riscv sparc systemz webassembly x86
// FIXME(jieyouxu): there has to be a better way to do this, without the needs-llvm-components it
// will fail on LLVM built without all of the components listed above.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of this, but I also am not sure if there's a better way

Copy link
Member

@cuviper cuviper Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rustc should be filtering this before printing targets anyway? Or else it could do a better job of avoiding LLVM initialization when it's not really needed -- e.g. I see no reason why --print sysroot should care.

That might also help coreos/cargo-vendor-filterer#87

@petrochenkov
Copy link
Contributor

Would be great if you could confirm that this fixes the test errors for you locally.

I confirm, there are no errors in run-make tests on my setup with this change.

@jieyouxu
Copy link
Member Author

Thanks!

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 31, 2024

📌 Commit beaf9d1 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 31, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2024
…Mark-Simulacrum

Add missing `needs-llvm-components` directives for run-make tests that need target-specific codegen

Without suitable `needs-llvm-components` directives, some run-make tests exercising target-specific codegen can fail if the LLVM used is built without the necessary components. Currently, the list is:

```
tests\run-make\print-target-list
tests\run-make\print-to-output
tests\run-make\print-cfg
tests\run-make\target-without-atomic-cas
```

This PR also skips tidy checks for revisions and `needs-llvm-components` for run-make tests since revisions are not supported.

Fixes rust-lang#129390.
Fixes rust-lang#127895.

cc `@petrochenkov` who noticed this, thanks! Would be great if you could confirm that this fixes the test errors for you locally.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#128523 (Add release notes for 1.81.0)
 - rust-lang#129605 (Add missing `needs-llvm-components` directives for run-make tests that need target-specific codegen)
 - rust-lang#129650 (Clean up `library/profiler_builtins/build.rs`)
 - rust-lang#129651 (skip stage 0 target check if `BOOTSTRAP_SKIP_TARGET_SANITY` is set)
 - rust-lang#129684 (Enable Miri to pass pointers through FFI)
 - rust-lang#129762 (Update the `wasm-component-ld` binary dependency)
 - rust-lang#129782 (couple more crash tests)
 - rust-lang#129816 (tidy: say which feature gate has a stability issue mismatch)
 - rust-lang#129818 (make the const-unstable-in-stable error more clear)
 - rust-lang#129824 (Fix code examples buttons not appearing on click on mobile)
 - rust-lang#129826 (library: Fix typo in `core::mem`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#128523 (Add release notes for 1.81.0)
 - rust-lang#129605 (Add missing `needs-llvm-components` directives for run-make tests that need target-specific codegen)
 - rust-lang#129650 (Clean up `library/profiler_builtins/build.rs`)
 - rust-lang#129651 (skip stage 0 target check if `BOOTSTRAP_SKIP_TARGET_SANITY` is set)
 - rust-lang#129684 (Enable Miri to pass pointers through FFI)
 - rust-lang#129762 (Update the `wasm-component-ld` binary dependency)
 - rust-lang#129782 (couple more crash tests)
 - rust-lang#129816 (tidy: say which feature gate has a stability issue mismatch)
 - rust-lang#129818 (make the const-unstable-in-stable error more clear)
 - rust-lang#129824 (Fix code examples buttons not appearing on click on mobile)
 - rust-lang#129826 (library: Fix typo in `core::mem`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d354d4d into rust-lang:master Aug 31, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 31, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2024
Rollup merge of rust-lang#129605 - jieyouxu:needs-llvm-components, r=Mark-Simulacrum

Add missing `needs-llvm-components` directives for run-make tests that need target-specific codegen

Without suitable `needs-llvm-components` directives, some run-make tests exercising target-specific codegen can fail if the LLVM used is built without the necessary components. Currently, the list is:

```
tests\run-make\print-target-list
tests\run-make\print-to-output
tests\run-make\print-cfg
tests\run-make\target-without-atomic-cas
```

This PR also skips tidy checks for revisions and `needs-llvm-components` for run-make tests since revisions are not supported.

Fixes rust-lang#129390.
Fixes rust-lang#127895.

cc ``@petrochenkov`` who noticed this, thanks! Would be great if you could confirm that this fixes the test errors for you locally.
@jieyouxu jieyouxu deleted the needs-llvm-components branch October 21, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Status: Done
6 participants