feat(dist): stage fresh toolchains before atomic publication - #4965
feat(dist): stage fresh toolchains before atomic publication#4965cachebag wants to merge 4 commits into
Conversation
6189268 to
0611f4c
Compare
02a3bd7 to
109729d
Compare
This comment has been minimized.
This comment has been minimized.
109729d to
acc04b6
Compare
…g dir Fresh dist installs now stage under `toolchains/+rustup-staging-<name>`, so interrupted installs of the same toolchain reuse a single staging directory instead of accumulating abandoned ones. The stage is guarded by an exclusive advisory file lock held for its lifetime: a dead owner's stage is reclaimed on the next attempt, while a live owner makes the contending install fail fast with a clear error. Blocking instead of failing, and serializing whole operations, is left to the locking work tracked in rust-lang#988. Publication failures are classified: if the destination appeared while the install was staging, the error now says so instead of surfacing a bare rename failure. The staged dispatch moves into `InstallMethod::run`, checkpoint names become exported constants co-located with their checkpoint sites, and the stray-hash removal in `install_into` is dropped as unreachable: staged installs never read the alias-scoped hash and rewrite it after publication.
Interrupted installs are asserted to reuse one deterministic staging path; a stale stage left by a dead process is reclaimed without leaking its contents into the published toolchain; and a stray update hash is ignored during a staged install and refreshed after publication.
acc04b6 to
5553723
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
There was a problem hiding this comment.
Thanks!
I have a question though: If I understand it correctly, we are performing the final stage of the installation in an isolated path under toolchain/ in this case (so no indirection WRT the actual toolchain), how can this evolve into the A/B partitioning scheme as we have previously discussed?
| assert_nightly_is_complete(&cx).await; | ||
| } | ||
|
|
||
| const BEFORE_METADATA: &str = "manifestation-update-before-metadata"; |
There was a problem hiding this comment.
Nit: It looks like those constants are not fully reused. Try moving them to the specific module instead of just under ./tests (there is an existing example already).
| #[cfg(feature = "test")] | ||
| use crate::test::checkpoint; | ||
|
|
||
| impl StagedToolchain { |
There was a problem hiding this comment.
Nit: Logically impl blocks should follow the type declaration.
| } | ||
| } | ||
|
|
||
| async fn run_staged_dist( |
| assert_nightly_is_complete(&cx).await; | ||
| } | ||
|
|
||
| const BEFORE_METADATA: &str = "manifestation-update-before-metadata"; |
There was a problem hiding this comment.
Prevent back-and-forth in a single PR; reorganize the history so that each new change are as close to the place at the end of this PR as possible.
| let name = destination | ||
| .file_name() | ||
| .and_then(|name| name.to_str()) | ||
| .expect("toolchain destination must have a UTF-8 base name"); |
There was a problem hiding this comment.
I think here name being a path segment can safely stay as OsStr[ing], there seems to be no need to convert it to a native string?.
3 participants
This PR implements the second step of the plan discussed in #4663 (details in my design notes), addressing #4400 and #4724 for fresh installs.
The idea is that a fresh toolchain now builds in its own "staging" directory and is then published to
toolchains/<name>with a same-filesystem rename, with the update hash only being written after publication. In the case of an interrupted install, this means no partial toolchain is left behind and a retry always works. The stage itself lives at a deterministic per-toolchain path guarded by an OS-released file lock, so an abandoned stage gets reclaimed by the next install instead of accumulating, and a concurrent install of the same toolchain fails fast instead of racing.Updates still modify toolchains in place, and blocking instead of failing is deferred to the locking work in #988.
cc @rami3l