fix(npm): ship static archives zstd-compressed to clear npm's size limit (E413)#5145
Merged
Conversation
The per-platform npm packages (@perryts/perry-linux-*, win32) fail to publish: the bundled static archives total ~750 MB unpacked / 236.8 MB packed, and npm rejects the upload with HTTP 413 (Payload Too Large). npm linux/windows have been stuck at 0.5.1129 as a result (darwin, ~523 MB, still squeaks under). Compress the archives in the npm packages and decompress them transparently on first use: - scripts/stage-npm.sh: zstd -19 each lib/*.a (and .lib) into *.zst, dropping the raw archive. The perry binary in bin/ stays raw (exec'd directly). Skippable via PERRY_NPM_NO_COMPRESS=1 for local staging. - compressed_libs.rs: when find_library_with_candidates sees a candidate .a missing but a sibling .a.zst present, decompress it once into a per-user cache (keyed on the .zst size+mtime) and link the cached .a. Atomic temp+rename; honors PERRY_LIB_CACHE_DIR. Purely additive — installs that ship raw .a (Homebrew, apt, dev) never hit this path. - zstd is statically vendored into perry, so this adds no system-library dependency on the user's machine. Measured: runtime 83.5 MB -> 22.6 MB, stdlib 281 MB -> 76.9 MB (~3.7x). Verified end-to-end: a compile against a compressed-only lib dir decompresses, links, runs, and reuses the cache on the second run.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds end-to-end zstd compression support for npm-published static libraries. The staging script gains optional Changeszstd Compression Pipeline for npm Static Libraries
Sequence Diagram(s)sequenceDiagram
participant StagingScript as stage-npm.sh
participant NpmPkg as pkg_dir/lib/*.a
participant NpmPkgZst as pkg_dir/lib/*.a.zst
StagingScript->>NpmPkg: iterate lib files (skip .zst)
StagingScript->>NpmPkgZst: zstd -19 -T0 --rm: compress .a → .a.zst
participant Compiler as find_library_with_candidates
participant CompressedLibs as compressed_libs
participant Cache as perry/libs cache dir
Compiler->>NpmPkgZst: compressed_sibling(.a) → .a.zst path
Compiler->>CompressedLibs: decompressed_archive(.a.zst, lib_name)
CompressedLibs->>NpmPkgZst: stat for size+mtime cache key
CompressedLibs->>Cache: check existing decompressed .a
alt cache hit
Cache-->>CompressedLibs: return cached .a path
else cache miss
CompressedLibs->>NpmPkgZst: stream-decompress via zstd decoder → tempfile
CompressedLibs->>Cache: atomic rename tempfile → cache slot
Cache-->>CompressedLibs: return new cached .a path
end
CompressedLibs-->>Compiler: PathBuf to decompressed .a
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
204-204:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winIncrement patch version per coding guidelines.
Adding a dependency to
[workspace.dependencies]requires incrementing the patch version. Line 204 showsversion = "0.5.1167", but per the coding guidelines ("Increment patch version in[workspace.package].versioninCargo.tomlfor every change that lands onmain"), this must be incremented to0.5.1168.🔧 Proposed fix
-version = "0.5.1167" +version = "0.5.1168"Also applies to: 265-267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Cargo.toml` at line 204, The patch version in the Cargo.toml file needs to be incremented following the coding guidelines that require incrementing the patch version for every change that lands on main. Update the version field in the [workspace.package] section from "0.5.1167" to "0.5.1168". This same version increment applies at multiple locations in the file (the main anchor point and additional sibling locations), so ensure all version references are updated consistently to maintain version alignment across the workspace configuration.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry/src/commands/compile/compressed_libs.rs`:
- Around line 73-89: The call to out_file.sync_all() in the decompression
closure is using .ok() to silently discard any errors that occur during the
fsync operation. This is problematic because if the sync fails (due to disk
full, I/O error, etc.), the incompletely written temp file will still be
processed further and potentially cached with corrupted data. Replace the
out_file.sync_all().ok() call with a properly error-handled version using
.with_context() (consistent with the pattern used for other file operations in
this block) to ensure that any fsync failures are propagated as errors, allowing
the error handling logic to properly clean up the temporary file.
In `@crates/perry/src/commands/compile/library_search.rs`:
- Around line 649-660: The decompression error from
super::compressed_libs::decompressed_archive is being silently swallowed by the
if let Ok() pattern at line 655, which masks the actual failure (corrupted
archive, disk full, etc.) and produces a misleading "library not found" error
instead. Since npm packages ship only .zst files with no fallback, a
decompression failure should be immediately propagated to the user. You need to
replace the if let Ok() with error handling that either changes the function's
return type to distinguish between decompression failures and missing libraries
(using a richer error enum), or immediately returns/propagates the error from
decompressed_archive with a user-facing message that explains the decompression
failure rather than continuing to check other candidates.
---
Outside diff comments:
In `@Cargo.toml`:
- Line 204: The patch version in the Cargo.toml file needs to be incremented
following the coding guidelines that require incrementing the patch version for
every change that lands on main. Update the version field in the
[workspace.package] section from "0.5.1167" to "0.5.1168". This same version
increment applies at multiple locations in the file (the main anchor point and
additional sibling locations), so ensure all version references are updated
consistently to maintain version alignment across the workspace configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52859369-f068-4fc6-8f30-e9ae92eae1cb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.tomlcrates/perry/Cargo.tomlcrates/perry/src/commands/compile.rscrates/perry/src/commands/compile/compressed_libs.rscrates/perry/src/commands/compile/library_search.rsscripts/stage-npm.sh
Comment on lines
+649
to
+660
| // npm per-platform packages ship `*.a.zst` (the raw archives exceed | ||
| // npm's tarball upload limit). When only the compressed sibling is | ||
| // present, decompress it once into a per-user cache and link that. | ||
| let compressed = super::compressed_libs::compressed_sibling(path); | ||
| if compressed.exists() { | ||
| let lib_name = path.file_name().and_then(|s| s.to_str()).unwrap_or(name); | ||
| if let Ok(decompressed) = | ||
| super::compressed_libs::decompressed_archive(&compressed, lib_name) | ||
| { | ||
| return Ok(decompressed); | ||
| } | ||
| } |
There was a problem hiding this comment.
Propagate decompression errors instead of silently treating them as missing candidates.
When a .zst sibling exists but decompressed_archive fails (corrupted archive, disk full, zstd decoder error, etc.), the if let Ok() at line 655 silently swallows the error and continues checking other candidates. Since npm packages ship only .zst files (the staging script uses --rm to delete raw archives), a decompression failure leaves the user with no fallback. The function then returns Err(candidates) and the caller reports "library not found," masking the real issue.
Users need to see "decompression failed: " to diagnose corrupted packages or system issues, not a misleading missing-library error.
🔧 Proposed fix
let compressed = super::compressed_libs::compressed_sibling(path);
if compressed.exists() {
let lib_name = path.file_name().and_then(|s| s.to_str()).unwrap_or(name);
- if let Ok(decompressed) =
- super::compressed_libs::decompressed_archive(&compressed, lib_name)
- {
- return Ok(decompressed);
- }
+ // If a .zst exists, decompression failure is a real error (not just
+ // a missing candidate) — npm packages ship only .zst, so there's no
+ // fallback. Propagate the error so users see the root cause.
+ return super::compressed_libs::decompressed_archive(&compressed, lib_name)
+ .map_err(|e| {
+ // Convert the decompression error into the outer Err type by
+ // wrapping it. Callers can then surface a meaningful diagnostic.
+ // (Alternatively, if you want to preserve the candidate list for
+ // the outer error, you could log/eprintln the decompression error
+ // and continue, but that still hides the root cause from --format json.)
+ vec![compressed] // or return the error directly if you change the return type
+ });
}Note: The current function signature returns Result<PathBuf, Vec<PathBuf>>, where Err contains the list of candidates. To properly propagate decompression errors, you may need to change the error type to a richer enum (e.g., LibraryNotFound(Vec<PathBuf>) vs. DecompressionFailed(anyhow::Error)), or convert the anyhow::Error to a user-facing message and fail immediately. The key point is to surface the decompression failure, not treat it as a missing candidate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry/src/commands/compile/library_search.rs` around lines 649 - 660,
The decompression error from super::compressed_libs::decompressed_archive is
being silently swallowed by the if let Ok() pattern at line 655, which masks the
actual failure (corrupted archive, disk full, etc.) and produces a misleading
"library not found" error instead. Since npm packages ship only .zst files with
no fallback, a decompression failure should be immediately propagated to the
user. You need to replace the if let Ok() with error handling that either
changes the function's return type to distinguish between decompression failures
and missing libraries (using a richer error enum), or immediately
returns/propagates the error from decompressed_archive with a user-facing
message that explains the decompression failure rather than continuing to check
other candidates.
- compressed_libs: propagate out_file.sync_all() failure (was .ok()) so a truncated temp archive is cleaned up instead of cached. - library_search: surface a decompression failure loudly instead of swallowing it into a misleading 'library not found' error.
Contributor
Author
|
Addressed CodeRabbit's review in 720a8d4:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The per-platform npm packages (
@perryts/perry-linux-arm64,-linux-x64, the musl variants,-win32-x64) can't publish. Each bundles the prebuilt static archives (libperry_runtime.a,libperry_runtime_abort.a,libperry_stdlib.a, the UI lib) so out-of-treeperry compilecan link user programs. Those total ~750 MB unpacked / 236.8 MB packed for linux-arm64, and npm rejects the upload withnpm error code E413 — 413 Payload Too Large.As a result npm's linux/windows packages have been stuck at 0.5.1129 (darwin, ~523 MB unpacked, still squeaks under), and every recent release run is red on
npm-publish(v0.5.1158, 1159, 1166, 1167 …). The archives are full-featured static libs the linker only dead-strips at final link time, so they can't simply be trimmed without dropping functionality.Fix
Ship the archives zstd-compressed in the npm packages and decompress them transparently on first use.
scripts/stage-npm.sh—zstd -19eachlib/*.a/*.libinto*.zst, dropping the raw archive. The perry binary inbin/stays raw (it's exec'd directly).PERRY_NPM_NO_COMPRESS=1skips compression for local staging.compressed_libs.rs(new) —find_library_with_candidatesalready probes a list of candidate.apaths. When a candidate.ais absent but a sibling.a.zstexists, decompress it once into a per-user cache (keyed on the.zstsize+mtime so a new release lands in a fresh slot) and link the cached.a. Atomic temp-write + rename; honorsPERRY_LIB_CACHE_DIR. Purely additive — installs that ship raw.a(Homebrew, apt, in-tree dev builds) match a.acandidate first and never reach this path.zstdis statically vendored into the perry binary, so this adds no system-library dependency on the user's machine.Why zstd (not xz)
zstd's crate always statically vendors libzstd (clean cross-platform build incl. Windows-MSVC, no
liblzma.soruntime dep that could break minimal Linux installs) and decompresses far faster for the one-time cost. Swappable if xz is preferred.Measured
~3.7×. The dominant file (stdlib) drops from ~281 MB to ~77 MB shipped, bringing the linux tarball well under npm's ~200 MB limit.
Validation
End-to-end against an npm-style layout with only compressed archives (
PERRY_LIB_DIR→ a dir of*.a.zst, no raw.a):DECOMP_OK 2,4,6 {"a":1}cargo fmt --all --checkclean. No version/changelog changes (left for maintainer at merge).Note
Doesn't retroactively fix already-published tags; the next release that runs
stage-npm.shproduces compressed packages that publish.Summary by CodeRabbit
Release Notes
New Features
Chores