bootstrap: add bootstrap step to run intrinsic-test in CI#156356
Conversation
|
|
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.
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.
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.
| fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { | ||
| run.path("library/stdarch/crates/intrinsic-test") | ||
| } |
There was a problem hiding this comment.
the intrinsic tests should also run when any of the source in core_arch changes (unless I'm misinterpreting what this function does).
There was a problem hiding this comment.
No you are not misinterpreting :) . You're so right I have added the core_arch path so it triggers . Thanks for the review.
| RUN curl -L http://ci-mirrors.rust-lang.org/sde-external-10.8.0-2026-03-15-lin.tar.xz -o /tmp/sde.tar.xz \ | ||
| && mkdir -p /intel-sde \ | ||
| && tar -xJf /tmp/sde.tar.xz --strip-components=1 -C /intel-sde \ | ||
| && rm /tmp/sde.tar.xz |
There was a problem hiding this comment.
this doesn't need an instant fix, but we bump the version of this tool occasionally, so ideally we'd sync that version between the repositories.
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thank you!
I wonder what to do about the required dependencies. GCC/Clang is fine, but the SDE emulator seems problematic. We have a lot of CI jobs that will run the intrinsics test and I don't think it is great if we install it on all of them. Of course it is also not great if it is required when running tests locally.
I think that we should make this test be non-default for now, and run it explicitly on selected CI jobs.
|
|
||
| let crates_link = out_dir.join("crates"); | ||
| if !crates_link.exists() { | ||
| std::os::unix::fs::symlink(builder.src.join("library/stdarch/crates"), &crates_link) |
There was a problem hiding this comment.
This won't work on non-Linux OSes. Why do we need this symlink?
There are existing functions in bootstrap to create symlinks, see symlink_dir.
There was a problem hiding this comment.
we need symlink because the generated Cargo.toml files depend on core_arch via a relative path. We generate into the build directory to avoid writing into the submodule so that path doesn't exist there. The symlink points back at the real stdarch crates so it resolves.
Yes , non-default and run it explicitly on selected CI jobs is better way to handle instead on every CI jobs . |
This comment has been minimized.
This comment has been minimized.
9383a8f to
f10bf02
Compare
|
Let's run this test explicitly in the |
@Kobzol As I added the x86_64-gnu Dockerfile invocation you suggested and ran ./x test library/stdarch/crates/intrinsic-test locally . It failed giving this error. warning: hidden lifetime parameters in types are deprecated error: |
|
You can just fix the warning, the change will be later synced back to |
This comment has been minimized.
This comment has been minimized.
|
Looks like it works! Thank you! Let's try. @bors r=kobzol,folkertdev |
This comment has been minimized.
This comment has been minimized.
bootstrap: add bootstrap step to run intrinsic-test in CI This PR ports `library/stdarch/crates/intrinsic-test` crate into the bootstrap test runner as a step, so that we can run the intrinsic-test suite via x.py test. Changes -: Ports `intrinsic-test` from the stdarch crate into the bootstrap system as a new test step so it runs inside the existing `x86_64-gnu` and `aarch64-gnu` CI jobs . Added `intrinsic-test` in `src/bootstrap/src/core/build_steps/test.rs` . Registers `intrinsic-test` as a bootstrap tool in `tool.rs` . Installs `clang`, `lld`, and Intel SDE in the respective Dockerfiles. r? @Kobzol try-job: x86_64-gnu
|
💔 Test for 5697a1f failed: CI. Failed job:
|
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
@bors retry |
…ol,folkertdev bootstrap: add bootstrap step to run intrinsic-test in CI This PR ports `library/stdarch/crates/intrinsic-test` crate into the bootstrap test runner as a step, so that we can run the intrinsic-test suite via x.py test. Changes -: Ports `intrinsic-test` from the stdarch crate into the bootstrap system as a new test step so it runs inside the existing `x86_64-gnu` and `aarch64-gnu` CI jobs . Added `intrinsic-test` in `src/bootstrap/src/core/build_steps/test.rs` . Registers `intrinsic-test` as a bootstrap tool in `tool.rs` . Installs `clang`, `lld`, and Intel SDE in the respective Dockerfiles. r? @Kobzol try-job: x86_64-gnu
…uwer Rollup of 9 pull requests Successful merges: - #156356 (bootstrap: add bootstrap step to run intrinsic-test in CI) - #157711 (Move proc-macro dlopen from proc-macro-srv to rustc_metadata) - #157836 (rebuild LLVM when `bootstrap.toml` config changes) - #158214 (Don't try to remove assignments in SimplifyComparisonIntegral) - #158226 (miri subtree update) - #158108 (Update actions/download-artifact action to v8.0.1) - #158150 (Update backtrace submodule to pick up AIX related fixes.) - #158178 (Use the target checking infrastructure for the diagnostic attributes) - #158195 (Add me to some rotation groups)
Rollup merge of #156356 - xonx4l:port-intrinsic-test, r=kobzol,folkertdev bootstrap: add bootstrap step to run intrinsic-test in CI This PR ports `library/stdarch/crates/intrinsic-test` crate into the bootstrap test runner as a step, so that we can run the intrinsic-test suite via x.py test. Changes -: Ports `intrinsic-test` from the stdarch crate into the bootstrap system as a new test step so it runs inside the existing `x86_64-gnu` and `aarch64-gnu` CI jobs . Added `intrinsic-test` in `src/bootstrap/src/core/build_steps/test.rs` . Registers `intrinsic-test` as a bootstrap tool in `tool.rs` . Installs `clang`, `lld`, and Intel SDE in the respective Dockerfiles. r? @Kobzol try-job: x86_64-gnu
…uwer Rollup of 9 pull requests Successful merges: - rust-lang/rust#156356 (bootstrap: add bootstrap step to run intrinsic-test in CI) - rust-lang/rust#157711 (Move proc-macro dlopen from proc-macro-srv to rustc_metadata) - rust-lang/rust#157836 (rebuild LLVM when `bootstrap.toml` config changes) - rust-lang/rust#158214 (Don't try to remove assignments in SimplifyComparisonIntegral) - rust-lang/rust#158226 (miri subtree update) - rust-lang/rust#158108 (Update actions/download-artifact action to v8.0.1) - rust-lang/rust#158150 (Update backtrace submodule to pick up AIX related fixes.) - rust-lang/rust#158178 (Use the target checking infrastructure for the diagnostic attributes) - rust-lang/rust#158195 (Add me to some rotation groups)
…uwer Rollup of 9 pull requests Successful merges: - rust-lang/rust#156356 (bootstrap: add bootstrap step to run intrinsic-test in CI) - rust-lang/rust#157711 (Move proc-macro dlopen from proc-macro-srv to rustc_metadata) - rust-lang/rust#157836 (rebuild LLVM when `bootstrap.toml` config changes) - rust-lang/rust#158214 (Don't try to remove assignments in SimplifyComparisonIntegral) - rust-lang/rust#158226 (miri subtree update) - rust-lang/rust#158108 (Update actions/download-artifact action to v8.0.1) - rust-lang/rust#158150 (Update backtrace submodule to pick up AIX related fixes.) - rust-lang/rust#158178 (Use the target checking infrastructure for the diagnostic attributes) - rust-lang/rust#158195 (Add me to some rotation groups)
| // intrinsic-test shells out to `cargo` and `rustfmt` make bootstrap's | ||
| // managed binaries findable by prepending their dirs to PATH. | ||
| let rustfmt_path = builder.config.initial_rustfmt.clone().unwrap_or_else(|| { | ||
| eprintln!("intrinsic-test: rustfmt is required but not available on this channel"); |
There was a problem hiding this comment.
This is going to break on beta/stable channels (as it says) -- is the intent to not run this test in CI on those channels? I'll drop it temporarily but we should land something on main that avoids breaking CI (as seen here - #159072 (comment)).
There was a problem hiding this comment.
Good catch. I think we can simply print a warning and then skip the test (just return) from this function. @xonx4l Can you please send a PR that does that? Thanks!
There was a problem hiding this comment.
Okay got it , OTW. Thanks @Mark-Simulacrum for the catch!
I just ran into a failure while locally running I think this test command would be the typical recommendation for standard library contributors, so do we need to do something more explicit to exclude |
|
Oh, I didn't realize that this would trigger during @xonx4l, could you please change |
|
That other PR is going to make all |
|
Yes, that's the plan. But it will be executed only at the very end, and I don't know anyone who actually runs the whole But if you think it's problematic, we can always just skip running the test if SDE isn't available. |
|
Actually, I think it would be indeed better to follow a scheme that we already use elsewhere in bootstrap.
What do you think? |
It's not exactly local, but I do run that in our Fedora/RHEL rpmbuild, and I don't think it's feasible to get SDE in there. The opt-in |
|
I do also run |
View all comments
This PR ports
library/stdarch/crates/intrinsic-testcrate into the bootstrap test runner as a step, so that we can run the intrinsic-test suite via x.py test.Changes -:
Ports
intrinsic-testfrom the stdarch crate into the bootstrap system as a new test step so it runs inside the existingx86_64-gnuandaarch64-gnuCI jobs .Added
intrinsic-testinsrc/bootstrap/src/core/build_steps/test.rs.Registers
intrinsic-testas a bootstrap tool intool.rs.Installs
clang,lld, and Intel SDE in the respective Dockerfiles.r? @Kobzol
try-job: x86_64-gnu