-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Don't release Miri if its tests only failed on Windows #81666
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
…n't be shipped on all targets
LGTM, but I don't know this code that well and I cannot comment on the CI config changes, so let's see what @Mark-Simulacrum says. |
The implementation looks reasonable, though I didn't review in detail. I'm not sure if this is the right decision to make here. From one aspect, it does make sense to not release miri with tests failing on a subset of platforms (we do the same with rustc, in some sense). On the other hand, it does seem likely that cross-compiling is a rare use case and not shipping it to linux when it fails on windows is unfortunate. Can you say more about whether this is common, for the tests to fail on one platform but not the other? I would have expected it to be exceedingly rare, personally, but maybe there's more platform-specific code in miri than I know of. |
For example:
Not quite common indeed. @rustbot label -S-waiting-on-author S-waiting-on-review |
Yeah, most failures are cross-platform. But when the platform-specific part of libstd changes, that can require changes in Miri's emulation of low-level OS functions, and those failures are typically platform-dependent. |
Ok - let's try this. We can alter the specific behavior later if it turns out this is causing a lot of pain for folks, but I expect it to be largely neutral, and it seems to match what we do for rustc itself (in the sense of not shipping if tests aren't passing on at least one platform). @bors r+ rollup=iffy |
📌 Commit f87afe5 has been approved by |
…ark-Simulacrum Don't release Miri if its tests only failed on Windows Extends rust-lang#66053 to Windows, so the released Miri won't be broken if its tests only fail on Windows. Relevant Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/269128-miri/topic/Miri.20is.20still.20available.20in.20rustup.20today.3F
⌛ Testing commit f87afe5 with merge 78bd41e4a9a6869895733e33c5d8e00b95f855b8... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Looks spurious (#81890). |
@bors retry |
☀️ Test successful - checks-actions |
Extends #66053 to Windows, so the released Miri won't be broken if its tests only fail on Windows.
Relevant Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/269128-miri/topic/Miri.20is.20still.20available.20in.20rustup.20today.3F