-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Add a CI job that runs a subset of UI tests with the GCC backend #146414
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
base: master
Are you sure you want to change the base?
Add a CI job that runs a subset of UI tests with the GCC backend #146414
Conversation
We can also specify the rust/src/ci/github-actions/jobs.yml Line 345 in 5d41f7c
|
e2d0466
to
2c2dac0
Compare
This comment has been minimized.
This comment has been minimized.
dfd72e4
to
bbdbe70
Compare
This comment has been minimized.
This comment has been minimized.
6bcf347
to
3a3bcf7
Compare
This comment has been minimized.
This comment has been minimized.
3a3bcf7
to
0efa10e
Compare
Error in ui tests:
Seems like something went wrong with the build/install of libgccjit. Any idea @Kobzol? I thought libgccjit was built with bootstrap if |
Is the .so actually in the load path? For some reason the GCC backend works differently than clif for me locally and I often find I need to manually append |
Arf, it's the usual case of "it works locally for me". :-/ Gonna try to investigate if it's something wrong with paths. |
Seems like we don't copy the |
…eGomez Fix `libgccjit` symlink when we build GCC locally Unblocks rust-lang#146414. r? `@GuillaumeGomez`
…eGomez Fix `libgccjit` symlink when we build GCC locally Unblocks rust-lang#146414. r? ``@GuillaumeGomez``
…eGomez Fix `libgccjit` symlink when we build GCC locally Unblocks rust-lang#146414. r? ```@GuillaumeGomez```
0efa10e
to
7d861fe
Compare
This comment has been minimized.
This comment has been minimized.
Fix `libgccjit` symlink when we build GCC locally Unblocks rust-lang/rust#146414. r? ```@GuillaumeGomez```
0e66bc1
to
fad2ab4
Compare
This comment has been minimized.
This comment has been minimized.
…nding from inline asm
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3403655
to
2ae5737
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Initialize llvm submodule if not already the case to run citool While working on rust-lang/rust#146414, I ran the following command (to run CI docker locally): ``` cargo run --manifest-path src/ci/citool/Cargo.toml run-local --type pr x86_64-gnu-gcc ``` However, since I didn't have `src/llvm` submodule initialized, it failed. Apparently it's a common issue for people using this tool so this PR removes this small inconvenience. r? ``@Kobzol``
This comment has been minimized.
This comment has been minimized.
9ee26a6
to
042451c
Compare
@Kobzol It's ready to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GCC job spends a bunch of time documenting rustc and other things. Is the GCC backend actually used for anything when documenting (doctests?). If not, we could skip that.
Could you remove the temporary comment of the GCC download? I want to check that the job downloads GCC properly and I want to see how long it takes.
let target = run.target; | ||
|
||
// GCC cannot run coverage tests. | ||
if let Some(codegen_backend) = run.builder.config.cmd.test_codegen_backend() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's at least print some warning when these tests are skipped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think that it will be better to just ignore these tests in the CI config. Let's use ./x test --stage 2 tests --skip tests/coverage --skip tests/coverage-run-rustdoc
in the Dockerfile.
<<: *job-linux-4c | ||
- name: x86_64-gnu-miri | ||
<<: *job-linux-4c | ||
- name: x86_64-gnu-gcc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: remove before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want this to run on each PR or did I misunderstand when this one is run?
src/ci/run.sh
Outdated
|
||
# Download GCC from CI on test builders | ||
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set gcc.download-ci-gcc=true" | ||
# FIXME: Temporarily commented out until the GCC build/download strategy is cleared out or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: remove before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
return; | ||
} | ||
|
||
// It we were running with GCC backend tests, no need to run these tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit unobvious, why should the GCC codegen tests be ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They run tests/ui
, which has already been run. Gonna update the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That assumes a specific execution from our CI. Someone could run x test <cg_gcc>
with the GCC backend, which should work.
In any case, we should modify the CI test command in the Dockerfile to not run this test at all. So I would also revert this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good point.
let target = run.target; | ||
|
||
// GCC cannot run coverage tests. | ||
if let Some(codegen_backend) = run.builder.config.cmd.test_codegen_backend() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think that it will be better to just ignore these tests in the CI config. Let's use ./x test --stage 2 tests --skip tests/coverage --skip tests/coverage-run-rustdoc
in the Dockerfile.
Good idea.
Sure. |
042451c
to
63d5352
Compare
This comment has been minimized.
This comment has been minimized.
63d5352
to
5b36236
Compare
@@ -1,4 +1,5 @@ | |||
//@ aux-build:doctest_crate.rs | |||
//@ ignore-backends: gcc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Since we're now skipping the whole suite in the Dockerfile, this test doesn't need an explicit ignore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks!
5b36236
to
fbf75ff
Compare
This comment has been minimized.
This comment has been minimized.
fbf75ff
to
bf2b742
Compare
Part of rust-lang/compiler-team#891.
r? @Kobzol