Toolstate: Don't block beta week on already broken tools. #69624
Conversation
rust-highfive
added
the
S-waiting-on-review
|
No PRs will pass starting Wednesday without this if all tools aren't fixed by then. I'll try to get the books fixed before then, but that is cutting it close. I think Clippy should be good to update, but there is some risk because it will require a coordinated update with Cargo. |
|
This looks correct to me, and does seem like the better behavior: realistically, we have 6 weeks to get tools working on beta branch. @bors r+ |
|
📌 Commit 22d840e has been approved by |
bors
added
S-waiting-on-bors
Rollup of 6 pull requests Successful merges: - #68682 (Add documentation to compiler intrinsics) - #69544 (Unrevert "Remove `checked_add` in `Layout::repeat`") - #69617 (constify mem::forget) - #69622 (Rename `syntax` in librustc_ast/README.md) - #69623 (stash API: remove panic to fix ICE.) - #69624 (Toolstate: Don't block beta week on already broken tools.) Failed merges: - #69626 (Toolstate: don't duplicate nightly tool list.) r? @ghost
Was this meant to link to something else? That PR doesn't touch |
|
Yea, it was supposed to be #66681. GitHub's auto-complete for issue numbers is very annoying, and too often "helpfully" picks a random number. |
| did_error = true; | ||
| eprintln!( | ||
| "error: Tool `{}` has regressed from {} to {} during beta week.", | ||
| tool, old_state, state | ||
| ); |
There was a problem hiding this comment.
Reading this code, something very odd is going on here... there are two places where the code does "if regressed and in_beta_weak, then fail": it does that here, and it does that again and seemingly independently in change_toolstate.
What is going on here? Seems like duplication of the same logic, which usually shouldn't happen.
There was a problem hiding this comment.
Yea, I had noticed that, too. I didn't want to make too many changes in one PR. The check in change_toolstate can be removed, that only runs on master. I'm not even sure if anyone monitors if master is failing?
I'll submit a PR to clean it up.
EDIT: I was wrong about master-only.
There was a problem hiding this comment.
Posted #69705 with the fix.
…ta, r=Mark-Simulacrum Toolstate: remove redundant beta-week check. I made a bit of a mistake in rust-lang#69624. The "beta regression" doesn't need to be checked twice. I also rolled up rust-lang#69693 to avoid merge conflicts.
…ta, r=Mark-Simulacrum Toolstate: remove redundant beta-week check. I made a bit of a mistake in rust-lang#69624. The "beta regression" doesn't need to be checked twice. I also rolled up rust-lang#69693 to avoid merge conflicts.
…ta, r=Mark-Simulacrum Toolstate: remove redundant beta-week check. I made a bit of a mistake in rust-lang#69624. The "beta regression" doesn't need to be checked twice. I also rolled up rust-lang#69693 to avoid merge conflicts.
…ta, r=Mark-Simulacrum Toolstate: remove redundant beta-week check. I made a bit of a mistake in rust-lang#69624. The "beta regression" doesn't need to be checked twice. I also rolled up rust-lang#69693 to avoid merge conflicts.
…ta, r=Mark-Simulacrum Toolstate: remove redundant beta-week check. I made a bit of a mistake in rust-lang#69624. The "beta regression" doesn't need to be checked twice. I also rolled up rust-lang#69693 to avoid merge conflicts.
This changes it so that tools are allowed to be broken entering the beta week if they are already broken. This restores the original behavior before the changes in #66681.
Closes #68458