-
Notifications
You must be signed in to change notification settings - Fork 13k
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
miri: make --stage 0 testing work #99599
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
// The stdlib we need might be at a different stage. And just asking for the | ||
// sysroot does not seem to populate it, so we do that first. | ||
builder.ensure(compile::Std::new(compiler_std, host)); | ||
let sysroot = builder.sysroot(compiler_std); |
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.
Is it expected that I need to call ensure
and sysroot
, i.e., that calling sysroot
does not implicitly ensure
anything?
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.
It ensures the directory exists. It doesn't build the standard library, a) because it doesn't know what target you want and b) because this is also called when building Std so we can copy the newly built artifacts into the sysroot, so building Std at that time would lead to a dependency cycle
Locally this seems to work. (Miri's tests then fail but that is due to another issue: #99605) |
support MIRI_HOST_SYSROOT env var for stage 0 builds Together with a [patch on the rustc side](rust-lang/rust#99599), this makes `./x.py test src/tools/miri --stage 0` work again. :) r? `@oli-obk`
The Miri submodule was changed cc @rust-lang/miri |
The Miri side of this has landed, so this is ready. |
Oh funny, so we test this with debug assertions in the PR but not in main CI? This is not a new issue in this PR, it already exists on master and will be fixed by #99607. |
@bors r+ p=1 |
⌛ Testing commit 2564a08 with merge 0fc54ebf0a6dfe2c92ae6ebdc4ce00f63802b5ab... |
💔 Test failed - checks-actions |
Uh... what? But this already fails on master, the PR doesn't make the assertions any worse... ... whatever, let's land #99607 first I guess. |
☀️ Test successful - checks-actions |
Tested on commit rust-lang/rust@7d0a55b. Direct link to PR: <rust-lang/rust#99599> 🎉 miri on windows: build-fail → test-pass (cc @RalfJung @oli-obk). 🎉 miri on linux: build-fail → test-pass (cc @RalfJung @oli-obk).
Finished benchmarking commit (7d0a55b): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
This needs rust-lang/miri#2415 or it'll break Miri entirely.
also fixes #99589