Revert "Try_run must only be used if toolstate is populated" #73275
Conversation
This reverts commit 6f01576.
rust-highfive
added
the
S-waiting-on-review
|
@bors p=1 |
|
I didn't realize this was in |
|
I don't get the connection either, but maybe clippy being left over in toolstate but not participating in it somehow broke the toolstate logic? We've never removed something from toolstate before. |
Yeah I think that's what happened: what likely happened is that a clippy test failed, but for some reason, instead of aborting CI, that lead to all tools being marked as test-pass... |
|
@bors rollup- let's not get this into another rollup confusing things even more |
|
I'll take a look at logs and such first thing today, I think that the patch we landed was correct, but may mean that we need to drop clippy from the toolstate builder or move it around a bit. |
|
I'm going to go ahead and close this as we have an actual fix up (and approved) now. |
…ng,oli-obk Avoid prematurely recording toolstates When we're running with dry_run enabled (i.e. all builds do this initially), we're guaranteed to save of a toolstate of TestFail for tools that aren't tested. In practice, we do test tools as well, so for those tools we would initially record them as being TestPass, and then later on re-record the correct state after actually testing them. However, this would not work well if the build failed for whatever reason (e.g. panicking in bootstrap, or as was the case in rust-lang#73097, clippy failing to test successfully), we would just go on believing that things passed when they in practice did not. This commit also adjusts saving toolstate to never record clippy explicitly (otherwise, it would be recorded when building it); eventually that'll likely move to other tools as well but not yet. This is deemed simpler than checking everywhere we generically save toolstate. We also move clippy out of the "toolstate" no-fail-fast build into a separate x.py invocation; this should no longer be technically required but provides the nice state of letting us check toolstate for all tools and only then check clippy (giving full results on every build). r? @oli-obk Supercedes rust-lang#73275, also fixes rust-lang#73274
This reverts commit 6f01576.
#73097 made a change that appears to have broken toolstate although I can't tell what happened. Let's revert it for now so we get toolstate tracking back and don't ship broken tools.
r? @Mark-Simulacrum @RalfJung @rust-lang/infra
fixes #73274