Skip to content

Fix dist overlay: persistent toolchain dirs and sysroot packaging - #2849

Closed
justdoGIT wants to merge 2 commits into
mozilla:mainfrom
justdoGIT:fix/dist-overlay-persistent-dirs-and-sysroot
Closed

justdoGIT wants to merge 2 commits into
mozilla:mainfrom
justdoGIT:fix/dist-overlay-persistent-dirs-and-sysroot

Conversation

@justdoGIT

Copy link
Copy Markdown

Summary

Two independent fixes for sccache-dist that affect Void Linux (and potentially other distros with non-standard sysroot layouts).

Fix 1: OverlayBuilder crash on persistent toolchain dirs

When sccache-dist restarts, the in-memory toolchain_dir_map is wiped, but the on-disk toolchain directories survive. On the next build, prepare_overlay_dirs hits the else branch (toolchain not in the map) and calls fs::create_dir on the existing directory, which returns an AlreadyExists error and fails the build.

Guard create_dir with an exists() check. The archive_id is a content hash, so a pre-existing directory for the same id already holds the correct unpacked toolchain and is safe to reuse.

Fix 2: Package rustlib/ instead of whole sysroot lib dir

On distros where rustc --print sysroot reports /usr (e.g. Void Linux), the ToolchainPackager joins LIBS_DIR ("lib") to the sysroot and packages the entire /usr/lib tree. This drags the full system library set (several GiB) into every distributed toolchain archive, blowing past the toolchain cache size and making toolchain packaging prohibitively slow.

Package the rustlib/ subtree instead, which is the actual Rust standard library. Fall back to the old behavior (whole lib dir) if rustlib/ does not exist, preserving compatibility with non-standard sysroot layouts.

Testing

  • Built and deployed sccache-dist with both fixes on a 4-node LAN cluster (Void Linux)
  • Verified toolchain packaging completes in seconds instead of timing out
  • Verified sccache-dist restarts no longer fail with AlreadyExists errors
  • All existing unit tests pass

When sccache-dist restarts, the in-memory toolchain_dir_map is
wiped but the on-disk toolchain directories survive. On the next
build, prepare_overlay_dirs hits the else branch (toolchain not
in the map) and calls fs::create_dir on the existing directory,
which returns an AlreadyExists error and fails the build.

Guard create_dir with an exists() check. The archive_id is a
content hash, so a pre-existing directory for the same id already
holds the correct unpacked toolchain and is safe to reuse.

Signed-off-by: KK <pandeykamal13526@gmail.com>
On distros where rustc --print sysroot reports /usr (e.g. Void
Linux), the ToolchainPackager joins LIBS_DIR ("lib") to the
sysroot and packages the entire /usr/lib tree. This drags the full
system library set (several GiB) into every distributed toolchain
archive, blowing past the toolchain cache size and making toolchain
packaging prohibitively slow.

Package the rustlib/ subtree instead, which is the actual Rust
standard library. Fall back to the old behavior (whole lib dir) if
rustlib/ does not exist, preserving compatibility with non-standard
sysroot layouts.

Signed-off-by: KK <pandeykamal13526@gmail.com>
Copilot AI lite review requested due to automatic review settings September 11, 2026 10:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Missing regression tests and unresolved overlay-directory persistence prevent approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR reduces Rust toolchain archive sizes and handles pre-existing overlay toolchain directories.

Changes:

  • Packages lib/rustlib with a compatibility fallback.
  • Avoids errors when toolchain directories already exist.
File summaries
File Summary
src/compiler/rust.rs Narrows Rust packaging to rustlib; regression tests for selection and fallback are needed.
src/bin/sccache-dist/build.rs Guards existing directories; persistence behavior remains unresolved because cleanup removes them, and regression coverage is needed.
Review details

Suppressed comments (2)

src/bin/sccache-dist/build.rs:197

  • This branch is not reached for the described server-restart scenario: OverlayBuilder::new calls cleanup() before creating these directories, and cleanup() recursively removes self.dir. A normal restart therefore deletes toolchains/<archive_id> before prepare_overlay_dirs runs, so this change does not provide the claimed restart persistence. Preserve the toolchain directory while cleaning only transient build data, or otherwise add the actual persistence path before relying on this guard.
                if !toolchain_dir.exists() {
                    fs::create_dir(&toolchain_dir)?;

src/compiler/rust.rs:2296

  • Because this changes the bytes produced for an unchanged compiler, the toolchain key also needs invalidation. RustHasher builds weak_toolchain_key from CACHE_VERSION and direct sysroot shared-library digests, while ClientToolchains::put_toolchain reuses the persistent weak_map without calling write_pkg; existing clients will therefore keep selecting the old multi-GiB archive after upgrading. Bump the Rust cache/package version or add a packaging-version component to the weak key so this branch runs for existing caches.
            let rustlib_path = libs_path.join("rustlib");
            if rustlib_path.is_dir() {
                package_builder.add_dir_contents(&rustlib_path)?;
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +196 to +197
if !toolchain_dir.exists() {
fs::create_dir(&toolchain_dir)?;
Comment thread src/compiler/rust.rs
Comment on lines +2294 to +2298
let rustlib_path = libs_path.join("rustlib");
if rustlib_path.is_dir() {
package_builder.add_dir_contents(&rustlib_path)?;
} else {
package_builder.add_dir_contents(&libs_path)?;
@justdoGIT

Copy link
Copy Markdown
Author

Superseded by #2850 which consolidates all fixes onto a single branch.

@justdoGIT justdoGIT closed this Sep 11, 2026
@justdoGIT
justdoGIT deleted the fix/dist-overlay-persistent-dirs-and-sysroot branch September 11, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants