mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-28 08:27:30 -04:00
node-runtime-fix
11458 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Zoltan Kochan
|
3b6f2ecbc7 |
fix(cli): honour --frozen-lockfile over config.lockfile (#257)
* fix(cli): honour --frozen-lockfile over config.lockfile
`pacquet install --frozen-lockfile` was silently downgrading to the
no-lockfile registry-resolving install path whenever `config.lockfile`
was `false`. That's the default since the new yaml-first config model
(commit
|
||
|
Minha Kang
|
f74114a039 |
fix: upgrade peer dependencies with pnpm upgrade --latest (#9928)
The `--latest` flag triggers the `installSome` code path, which built `currentBareSpecifiers` via `getAllDependenciesFromManifest()` — a function that excluded peer dependencies. The non-`--latest` path uses `getWantedDependencies()`, which honors `autoInstallPeers` and includes them. Pass `autoInstallPeers` through `getAllDependenciesFromManifest` so both paths agree. Closes #9900 Co-authored-by: Zoltan Kochan <z@kochan.io> |
||
|
Zoltan Kochan
|
ed56eacf7b |
feat(package-manager): hardlink CAS files into node_modules (#256)
* feat(package-manager): hardlink CAS files into node_modules Implements all `PackageImportMethod` variants in `link_file`, which until now always used `reflink_copy::reflink_or_copy` and ignored the user's setting (`create_cas_files` hard-asserted `Auto`). Dispatch is: * `Auto` — pnpm's documented chain: clone → hardlink → copy. Each step broad-catches because "operation not supported" surfaces as different `io::ErrorKind`s across platform and filesystem (`EOPNOTSUPP`, `EXDEV`, `EPERM`, …) and pnpm itself doesn't enumerate them. * `Hardlink` — `fs::hard_link`, errors propagate. * `Clone` — `reflink_copy::reflink`, errors propagate. * `CloneOrCopy` — `reflink_copy::reflink_or_copy`. * `Copy` — `fs::copy`. The existing `exists()` short-circuit stays; both `hard_link` and `reflink` would error on an existing target otherwise. Hardlinking means any package that edits its own files at runtime mutates the shared store copy. Pnpm falls back to copy for packages with a postinstall script; pacquet doesn't run postinstall yet, so there's nothing to gate on here — revisit when script execution lands. The old `// NOTE: do not hardlink packages with postinstall` TODO is replaced by an inline comment to that effect. Drive-by: the `Copy` / `Clone` / `CloneOrCopy` doc comments in the npmrc enum were pairwise swapped. Those variants were previously unreachable thanks to the `assert_eq!(.., Auto)` in `create_cas_files`, so the mismatch never mattered — it does now. Tests cover Copy (independent inodes), Hardlink (shared inode / nlink bump on unix), Auto (always succeeds via fallback), the no-op path when the target already exists across all five methods, and explicit Hardlink error propagation when the source is missing. Closes #174. * perf(package-manager): cache Auto's link-method decision across files The prior implementation chained `reflink → hard_link → copy` via `.or_else(…)` on every `link_file` call. On a filesystem that doesn't support reflink, installing a package with hundreds of files paid the "try reflink, fail" cost hundreds of times — once per file. Reflink and hardlink support are properties of the (source fs, target fs) pair, not of individual files. Cache the downgrade decision in a process-wide `AtomicU8` (`CLONE → HARDLINK → COPY`) and advance it with `fetch_max` so rayon workers racing on the first failure all converge to the same downgraded state without a lock. Worst-case startup cost is `N` stale attempts per tier, where `N` is the rayon thread count — bounded, not per-file. Factor the chain out as `auto_link(state, src, dst)` so the downgrade semantics can be exercised directly in tests with a caller-supplied `AtomicU8`; the production path uses a `static AtomicU8` declared inside `link_file`. Three new tests cover the cache: * `auto_state_downgrades_monotonically_on_failure` — drive a non-existent source through the full state machine, assert terminal state is `COPY`. * `auto_respects_cached_copy_state` — pre-seed `COPY`, observe independent inodes (copy semantics). * `auto_respects_cached_hardlink_state` — pre-seed `HARDLINK`, observe a shared inode (hardlink semantics) and that the state didn't drift. * fix(package-manager): address Copilot review on hardlink PR Three review points from pnpm/pacquet#256: 1. **Dangling-symlink short-circuit.** `target_link.exists()` follows symlinks, so a dangling symlink left behind by an interrupted prior install would slip past the "already exists → no-op" check and then collide with `hard_link` / `reflink` as `AlreadyExists`. Switched to `fs::symlink_metadata(target_link).is_ok()` which asks about the directory entry itself — covers files, directories, and symlinks (broken or otherwise). Added `dangling_symlink_is_treated_as_already_present` to pin the behaviour. 2. **Misleading `explicit_hardlink_surfaces_errors` doc.** The comment framed the test as simulating `EXDEV` (cross-device hardlink) but the setup actually uses a missing source, which surfaces as `NotFound`. Reworded to describe what the test actually exercises: deterministic error propagation on any platform / filesystem. 3. **Untested Clone / CloneOrCopy match arms.** The `existing_target_is_preserved` loop short-circuits before those arms run, so they had no coverage of their real code paths. Added `clone_or_copy_materializes_the_file_contents` (CloneOrCopy falls back to copy on any FS) and `explicit_clone_surfaces_errors` (Clone propagates errors on missing source). All 11 `link_file` tests pass. * style(package-manager): cargo fmt the explicit_clone test * feat(package-manager): match pnpm's import-method logic across the board Review of `render-peer/fs/indexed-pkg-importer/src/index.ts` surfaced two gaps vs pnpm's implementation: 1. **CloneOrCopy didn't cache its downgrade.** pnpm's `createCloneOrCopyImporter` keeps a `let auto = initialAuto` closure and reassigns it to `copyPkg` on the first reflink failure, so every subsequent file skips the reflink probe entirely. Pacquet was calling `reflink_copy::reflink_or_copy` per file, which re-probes each time on a non-reflink FS. Added a second `static AtomicU8` and a `clone_or_copy_link` helper that mirrors `auto_link`'s state machine, minus the hardlink tier (pnpm's CloneOrCopy has no hardlink fallback). 2. **Explicit Hardlink was fail-hard.** pnpm's `hardlink` method binds to `hardlinkPkg(linkOrCopy)`, and `linkOrCopy` falls back to copy on `EXDEV` (cross-device link not permitted). A user who sets `package-import-method=hardlink` with their store on a different device from `node_modules` gets a working install on pnpm; pacquet was surfacing the raw EXDEV error. Match pnpm: on `EXDEV` only, copy. Every other error still propagates, so missing-source and permission-denied failures are still loud. Renamed the state constants from `AUTO_STATE_*` to `LINK_STATE_*` now that `auto_link` and `clone_or_copy_link` both use them. Added an `is_cross_device(io::Error)` helper — raw errno 18 on Linux/macOS/BSD, 17 (ERROR_NOT_SAME_DEVICE) on Windows. Tighter than pnpm's message-prefix check (`err.message.startsWith('EXDEV:')`). Tests: two new cases for CloneOrCopy caching (downgrade-on-failure and respects-cached-state), plus the existing coverage — 13 link_file tests all passing. Cross-device EXDEV fallback for explicit Hardlink is intentionally not unit-tested: there's no portable way to force EXDEV from a Rust test on all CI hosts (would need a bind-mount or a separate loopback FS). The code path is straightforward enough that code review is the intended verification. * fix(package-manager): address Copilot round 2 on hardlink PR Three review points from pnpm/pacquet#256: 1. **Dangling symlink silently treated as success.** The previous `symlink_metadata().is_ok()` short-circuit returned `Ok(())` on broken symlinks, leaving the package with a silently-missing file while the install reported success. New logic splits the check: * `fs::metadata` follows symlinks — success means the target is live (regular file or symlink-to-file); short-circuit. * If `metadata` fails but `symlink_metadata` succeeds and the dirent is a symlink, the link is dangling — remove it and fall through so the configured import tier can re-materialize the file. * Non-symlink dirent that still fails `metadata` (permissions, stale NFS handle) is left for the link/copy below to surface the real error. Tests updated: `dangling_symlink_is_replaced` asserts the re-materialize behaviour; `live_symlink_short_circuits` covers the "legitimate user state, don't touch" case; old `dangling_symlink_is_treated_as_already_present` removed. 2. **`LinkFileError::CreateLink` message wrong for Copy/CloneOrCopy.** `link_file` now dispatches to copy / reflink / hardlink depending on `PackageImportMethod`, so "fail to create a link" was misleading when the user configured `copy`. Renamed the variant to `Import` (pnpm's term, see `createPackageImporter`) with message `"failed to import {from:?} to {to:?}: {error}"`. Also added a new `RemoveStale` variant for the dangling-symlink scrub path above. Tests updated for the rename; no external consumers reference the variant name. 3. **Caching comment claimed per-fs-pair scope it doesn't have.** The `AUTO_STATE` / `CLONE_OR_COPY_STATE` statics are one process-global AtomicU8 each, not keyed by `(source_dev, target_dev)`. A failure on one path can downgrade later calls on a different pair. Reworded the block comment per Copilot's suggestion: explicit that the cache is process-global and a coarse optimization, with a note that pnpm's per-importer closure has the same coarseness once an install has picked an import direction. * feat(package-manager): narrow downgrade triggers + import-method log Two review points from pnpm/pacquet#256 plus a traceability ask: 1. **Downgrade only on capability errors.** The previous implementation downgraded the `auto_link` / `clone_or_copy_link` cache on any `Err(_)`, so a one-off `NotFound` / `PermissionDenied` / `AlreadyExists` on a single file permanently disabled reflink / hardlink for the rest of the process — even on a filesystem that fully supports the higher tier. Added two classification helpers: * `is_reflink_fallback_error` — `Unsupported`, `EXDEV`, and raw errnos `ENOSYS` (38), `EOPNOTSUPP` / `ENOTSUP` (95 / 102 / 45), `ENOTTY` (25). `ENOTTY` is the CI-critical code: ext4 returns it for `ioctl_ficlone` when the filesystem doesn't implement reflink, so without it the downgrade would never fire on GHA ubuntu runners. * `is_hardlink_fallback_error` — `EXDEV` + `Unsupported`, matching pnpm's `linkOrCopy` narrow catch. Non-fallback errors propagate immediately. New test `is_reflink_fallback_error_classifies_capability_codes` pins the classification for the important codes; the two `*_call_errors_propagate_without_downgrading` tests replace the old `*_state_downgrades_monotonically_on_failure` tests, which relied on broad catch and no longer hold. 2. **Stale doc comment.** `clone_or_copy_materializes_the_file_contents` test comment referenced `reflink_or_copy` but the implementation is `clone_or_copy_link` (manual reflink + `fs::copy`). Updated. 3. **One-shot `package-import-method` log.** Matches pnpm's `packageImportMethodLogger.debug({ method: 'clone' | 'hardlink' | 'copy' })` in `fs/indexed-pkg-importer/src/index.ts`. Fires once per (process, method) via a `static AtomicU8` bitfield: the first caller to set a given bit emits. Pnpm uses `debug`; pacquet uses `info` per the ask so the message surfaces without a debug subscriber. Target `pacquet::package_import_method` with `method = "…"` field so the payload shape mirrors pnpm's. All five `PackageImportMethod` variants log on their success path (Auto logs its cached tier, CloneOrCopy logs either clone or copy, Hardlink logs copy on EXDEV fallback). Unlocks "is hardlink actually being used on CI?" by reading the install logs. * fix(package-manager): deny-list downgrade errors instead of allow-list Windows CI was failing: failed to import "…store…" to "…node_modules…": Incorrect function. (os error -2147024895) Raw OS error `1` / HRESULT `0x80070001` = `ERROR_INVALID_FUNCTION`, which is what NTFS returns for the `FSCTL_DUPLICATE_EXTENTS_TO_FILE` ioctl (reflink is ReFS/Dev-Drive-only on Windows). Rust surfaces this as `io::ErrorKind::InvalidInput`, not `Unsupported`, so the allow-list I added in `0d5896a` (`ENOSYS`/`EOPNOTSUPP`/`ENOTSUP`/`ENOTTY` plus `Unsupported`) didn't match it, and the error bubbled up instead of downgrading to hardlink. Switching to a deny-list is both simpler and more robust: `is_call_error` now matches exactly `NotFound`, `PermissionDenied`, and `AlreadyExists` — errors where the call itself is malformed and a different tier won't help, so propagate. Everything else (capability, cross-device, `ERROR_INVALID_FUNCTION`, and anything else a future platform throws at us) downgrades. This is the same classification `reflink-copy`'s `reflink_or_copy` uses internally, so it's already battle-tested across Windows / Linux / macOS. Drops the `is_reflink_fallback_error` and `is_hardlink_fallback_error` helpers along with their platform-specific errno tables. Test `is_reflink_fallback_error_classifies_capability_codes` renamed to `is_call_error_rejects_capability_codes` and expanded to include `ErrorKind::InvalidInput` — pinning the behaviour that previously regressed. * test(package-manager): fix Linux-failing cache-propagation tests `auto_call_errors_propagate_without_downgrading` and its `clone_or_copy_` sibling used `NotFound` (missing source) as the call-error trigger. `reflink_copy::reflink` rewrites that into `ErrorKind::InvalidInput` on non-macOS platforms with the message "the source path is not an existing regular file" (see `reflink-copy/src/lib.rs:64-85`). On macOS the original error passes through, so the tests passed locally but failed on Linux CI: thread 'auto_call_errors_propagate_without_downgrading' panicked: assertion failed: `(left == right)` < left: InvalidInput > right: NotFound Switch the trigger to `AlreadyExists` (pre-populated target file). reflink's `AutoRemovedFile::create_new` calls `OpenOptions::create_new`, which fails with `AlreadyExists` when the target already exists; the wrap-on-non-macOS path leaves it alone because the condition requires the source to NOT be a regular file (our source is fine). `AlreadyExists` is in the `is_call_error` deny-list, so the propagation path gets exercised on every platform. Also added `auto_hardlink_tier_call_errors_propagate`: pre-seeds state to HARDLINK and uses the simpler missing-source trigger. `fs::hard_link` doesn't get the reflink-copy rewrap treatment, so `NotFound` surfaces directly there. 16 `link_file` tests passing locally on macOS. * fix(package-manager): gate cross-device raw-errno 17 to Windows only `is_cross_device` previously matched `raw_os_error == Some(17)` on every platform. On Windows that's `ERROR_NOT_SAME_DEVICE` (correct signal). On Unix it's `EEXIST` — surfaces as `ErrorKind::AlreadyExists` and means a concurrent process created the target between our `fs::metadata` short-circuit and the link / reflink call. Treating that race as cross-device would downgrade the Auto cache and, worse, fall through to `fs::copy` and overwrite the other process's freshly installed file. Split the check by `cfg`: Unix matches only `Some(18)` (EXDEV), Windows matches only `Some(17)`. New test `is_cross_device_distinguishes_unix_eexist_from_windows_not_same_device` pins the behaviour on both platforms. * test(package-manager): gate raw-errno 18 assertion to Unix The unconditional assert that raw 18 is cross-device failed on Windows CI, where raw 18 is ERROR_NO_MORE_FILES and is_cross_device only treats 17 (ERROR_NOT_SAME_DEVICE) as the cross-device signal. Move the raw-18 check under #[cfg(unix)] and add the Windows-side counterpart asserting raw 18 is NOT classified as cross-device. * docs(package-manager): clarify downgrade cache is per-mode, not shared The header comment said "states shared by Auto and CloneOrCopy", which reads as one cache used by both. The actual cache is two separate atomics (AUTO_STATE and CLONE_OR_COPY_STATE) — the state *machine* (CLONE → HARDLINK → COPY) is shared, but each mode keeps its own process-global atomic so an Auto downgrade doesn't affect CloneOrCopy. Reword to match. * fix(package-manager): narrow dangling-symlink cleanup to NotFound only fs::metadata follows symlinks and can fail on a *live* symlink for reasons other than a dangling target — PermissionDenied on the pointed-to file, transient NFS errors, and so on. The previous code treated any metadata failure as "check for dangling symlink and scrub", which could delete a live symlink we merely couldn't stat. Narrow the cleanup to ErrorKind::NotFound. For other stat errors, leave the dirent alone and fall through to the import call so the real error surfaces. * style(package-manager): apply cargo fmt after dangling-symlink fix * fix(package-manager): treat post-stat AlreadyExists as idempotent success There's a TOCTOU window between link_file's fs::metadata short-circuit and the actual import syscall: a concurrent writer (another install process, another rayon worker) can create the target in between, and hard_link / reflink will return AlreadyExists. Bubbling that up as LinkFileError::Import contradicts the docstring's "existing target is a no-op" contract and fails installs under concurrent runs. Treat AlreadyExists from the import call as success when fs::metadata confirms the target is now a live dirent. Metadata follows symlinks and returns NotFound for dangling ones, so a broken symlink that races in doesn't silently pass as success — it surfaces the original error instead. Race is hard to reproduce deterministically without fault injection, so no new test; the change is a small outer-match refactor on the existing result-handling step. * style(package-manager): add message string to import-method log Match the repo's tracing::info! convention (target, field, message) as used in install.rs / tarball/lib.rs. The field-only form grepped fine but rendered as a bare structured line in default formatters. |
||
|
Zoltan Kochan
|
f30b8cca11 |
ci(benchmark): compare against pnpm in Integrated-Benchmark (#253)
* ci(benchmark): compare against pnpm in Integrated-Benchmark Adds `--with-pnpm` to the `Integrated-Benchmark` workflow's hyperfine run, so every PR's results include a `pnpm` row alongside `pacquet@HEAD` / `pacquet@main`. Makes regressions vs. upstream obvious in the PR comment. While wiring that up, noticed that `WorkEnv::benchmark` built the `rm -rf` prepare-command from only `revision_ids()` — if the caller set `with_pnpm`, pnpm's bench dir would survive between iterations and the timed runs after the warmup would hit an already-installed `node_modules` instead of actually reinstalling. Fix that: include the `pnpm` bench dir in the cleanup set when `with_pnpm` is on, so pnpm and pacquet are measured under the same cold-`node_modules` conditions. Pnpm's global store stays warm across iterations, same as pacquet's — both tools get a fair repeat-install benchmark. Supersedes the 2023-era #177, which had the same intent but has bit-rotted beyond trivial rebasing (the store-dir schema, the benchmark task layout, the `just` harness, and the pnpm RC version all moved under it). * ci(benchmark): fix mislabelled BENCHMARK_REPORT copy filename In the commented-out Clean Install block, the JSON copy target was `BENCHMARK_REPORT_FROZEN_LOCKFILE.json` — if that block ever gets un-commented, it would overwrite the Frozen Lockfile report instead of producing a separate Clean Install one. Should be `BENCHMARK_REPORT_CLEAN_INSTALL.json` to match the markdown copy line and the summary section below it. |
||
|
Zoltan Kochan
|
86f17f580c |
fix(store): write index.db rows as msgpackr records for pnpm interop (#252)
## Summary Follow-up to #251. Pacquet's `StoreIndex::set` was writing `index.db` rows via `rmp_serde::to_vec_named` — plain MessagePack maps. pnpm reads the same file with `Packr({useRecords: true, moreTypes: true}).unpack(…)`, and in that mode **every msgpack map at any nesting level decodes as a JS `Map`**. Records are the escape hatch that tells msgpackr "this one's a plain object". So pacquet-written rows came back as a top-level `Map`, `pkgIndex.files` (a property access on that `Map`) was `undefined`, and pnpm's `for (const [f, fstat] of pkgIndex.files)` threw `files is not iterable`. That surfaced in CI as `ERR_PNPM_READ_FROM_STORE` the first time a benchmark run picked up a shared pnpm-v11 cache with pacquet-written rows in it — every downstream job's `pnpm install` crashed trying to decode them. #251 taught pacquet how to **read** pnpm's records; b0bed43 fixed the `checkedAt` encoding (float64 vs uint64-as-BigInt) and called the pnpm-reads-pacquet direction done — but the container shape was still wrong, and nothing in the test suite actually put a pacquet-written row back through msgpackr, so it slipped past. ## Fix New `encode_package_files_index` in `msgpackr_records.rs` that emits msgpackr-records bytes. Behaviour matches what msgpackr 1.11.8 itself emits for the same logical input (verified against the version pinned in pnpm v11's `catalog:`): - **Slot allocation is shape-aware.** Slot `0x40` is reserved for the top-level `PackageFilesIndex`. Inner slots in `0x41..=0x7f` are allocated lazily, one per distinct record shape, in first-seen order. A `CafsFileInfo` carrying `checkedAt` and one without it land in separate slots; same-shape instances downstream collapse to a single bare-slot byte, which is the record-compression win records exist for. - **Optional fields are omitted from the schema when `None`**, not encoded as `nil`. A `SideEffectsDiff { added: Some, deleted: None }` decodes on the pnpm side to `{ added: Map }`, not `{ added: Map, deleted: null }` — the same JS shape msgpackr itself would produce for the same Rust input, so downstream checks like `diff.added != null` / `diff.deleted != null` behave identically regardless of which tool wrote the row. - **`CafsFileInfo.checkedAt`** is written as `float 64` when `Some` (uint 64 would decode to a JS `BigInt` and crash pnpm's `mtimeMs - (checkedAt ?? 0)`), omitted from the schema when `None`. - **`manifest: Some(_)` returns `EncodeError::ManifestNotSupported`.** Pacquet doesn't populate it today; encoding `serde_json::Value` as records is follow-up work. The encoder also **promotes integer values in `0x40..=0x7f` to `uint 8`** instead of positive fixints. Inside a records stream those bytes are slot-reference markers — a bare `0x7b` would be parsed as a ref to slot `0x7b` rather than the integer `123`. msgpackr does the same promotion for the same reason. State is a tight `[Option<u8>; 2]` for `CafsFileInfo` slots and `[Option<u8>; 4]` for `SideEffectsDiff` slots, indexed by a shape bitmask — 2 and 4 are the respective maxima, so lookup is a cache-local array index with no `HashMap` allocation on the hot per-file path. `StoreIndex::set` now calls this encoder. `StoreIndex::get` goes through the existing transcoder which already handled records, so the read path needed no behaviour change — just a doc update to enumerate the three row flavours it now tolerates (pnpm-written records, current pacquet-written records, legacy pacquet plain-msgpack from rows that predate this PR). ## Why not a bespoke format? Considered writing pacquet's rows in a different shape (positional tuple, or keep plain msgpack and switch pnpm's side). Ruled out: the whole point of #244 and #251 is a shared `index.db` the two tools read each other's entries from. Any shape other than "what pnpm's msgpackr emits" breaks that promise. ## Performance Faster **and** smaller than `rmp_serde::to_vec_named`. Measured on a release build, 10k iterations per row-size, with a realistic `CafsFileInfo` shape (128-hex digest, millisecond timestamp): **Serialize time per row** | files | `rmp_serde` (before) | records encoder (after) | speedup | |---|---:|---:|---:| | 1 | 304 ns | 164 ns | **1.9×** | | 10 | 1.23 µs | 838 ns | **1.5×** | | 50 | 3.71 µs | 2.25 µs | **1.6×** | | 200 | 11.9 µs | 8.11 µs | **1.5×** | | 1000 | 55.9 µs | 27.7 µs | **2.0×** | **Row size on disk** | files | `rmp_serde` | records encoder | delta | |---|---:|---:|---:| | 1 | 214 B | 220 B | +6 B | | 10 | 1960 B | 1723 B | **−12%** | | 50 | 9722 B | 8405 B | **−14%** | | 200 | 38822 B | 33455 B | **−14%** | | 1000 | 194022 B | 167055 B | **−14%** | The 6-byte regression at `n=1` is the outer record header (`d4 72 40`) plus a slightly longer field-name array; from `n≥4` records come out smaller because each subsequent `CafsFileInfo` instance collapses to a single slot byte instead of re-emitting `"digest"`, `"mode"`, `"size"`, `"checkedAt"` (~20 bytes of field names). Why it's faster: no serde runtime indirection (no trait dispatch, no generic-over-Serializer code paths), single pre-sized `Vec`, direct byte writes, zero-alloc slot lookup via the fixed-size arrays. `StoreIndex::get` is unchanged, so read cost is identical to before. Tarball writes call `encode` once per package, so this shows up at ~10–50 µs/package — drowned out by the SQLite `INSERT` (~100–500 µs) either way, but the payload-size shrink compounds across thousands of packages in the shared v11 store. ## Test plan - [x] `cargo test -p pacquet-store-dir --lib` — **37 passing, 12 new** in `msgpackr_records::tests`: - `encode_emits_record_header_for_top_level_struct` — outer `0xd4 0x72 0x40` record-def header is present. - `encode_roundtrips_single_file` / `encode_roundtrips_many_files_sharing_one_slot` — single-file round-trip; N uniform-shape `CafsFileInfo`s produce exactly 2 record defs (outer + one inner shape), the rest are bare slot refs. - `encode_handles_fixint_in_slot_range_safely` — `size: 0x7b` round-trips without triggering `UnknownSlot`. - `encode_omits_checked_at_when_none` — `None` `checkedAt` isn't written as `nil`; the string `"checkedAt"` doesn't appear anywhere in the output. - `encode_allocates_separate_slots_for_distinct_cafs_shapes` — mixed `CafsFileInfo` shapes (some with `checkedAt`, some without) land in separate slots; same shape reuses. - `encode_requires_build_when_set` / `encode_omits_requires_build_when_none` — outer-record optional-field handling. - `encode_side_effects_roundtrip` — `SideEffectsDiff` with both fields populated. - `encode_side_effects_with_only_added_omits_deleted_field` — exact shape the reviewer flagged: `deleted: None` must not appear as a field name in the output. - `encode_allocates_separate_slots_for_distinct_side_effects_shapes` — two `SideEffectsDiff`s with distinct shapes land in separate slots. - `encode_rejects_manifest_some` — error path for the currently-unimplemented branch. - [x] `cargo test -p pacquet-cli --test pnpm_compatibility` — new `pnpm_reads_pacquet_written_rows` passes. pacquet installs into a store, `node_modules` is wiped, then `pnpm install --ignore-scripts` reinstalls against the same store. If the record encoding is wrong this reproduces the benchmark's `ERR_PNPM_READ_FROM_STORE`. Complements the existing `same_file_structure` / `same_index_file_contents` which clear the store between the pacquet and pnpm halves and therefore don't exercise this path. - [x] `cargo clippy --workspace --all-targets -- -D warnings` - [x] `cargo fmt --all -- --check` - [x] `cargo doc -p pacquet-store-dir --no-deps` — no warnings. |
||
|
Zoltan Kochan
|
b7367e34f1 |
feat(store): decode pnpm-written msgpackr-records rows in index.db (#251)
* feat(store): decode pnpm-written msgpackr-records rows in index.db Finishes the last open item on #244. pnpm packs every PackageFilesIndex with `new Packr({ useRecords: true, moreTypes: true })`. Records use a msgpackr-private extension (ext type 0x72, "r") that rewrites the string keys of repeated same-shape structs into 1-byte slot references, so the payload `rmp_serde` saw in a pnpm-written index.db row was effectively garbage — the cache-lookup in `StoreIndex::get` would error, pacquet would fall through to a fresh download, and the whole "shared v11 store" promise evaporated for rows pnpm wrote first. - New `crates/store-dir/src/msgpackr_records.rs`: a focused transcoder that walks a msgpackr-records byte stream and emits equivalent plain MessagePack (expanding each record instance into a string-keyed map). Keeping this as a transcoder — rather than a fresh Deserializer — lets us reuse the existing `Deserialize` derive for `PackageFilesIndex`. 270 lines of decoder + 10 unit tests using real fixture bytes captured from msgpackr 1.11.8 (via `store/index/node_modules/msgpackr` in the pnpm v11 worktree). - `StoreIndex::get` now sniffs the leading `d4 72` bytes and routes msgpackr-records rows through the transcoder before handing the result to `rmp_serde`. Pacquet-written rows (plain msgpack) still take the zero-copy fast path — no transcoding, no extra allocation. - The transcoder also narrows integer-valued `float 64`/`float 32` values to `uint 64`. Msgpackr emits JS Number as float whenever the value exceeds int32 range, so millisecond timestamps like `checkedAt = 1_700_000_000_000` arrive as a double even though they're semantically ints — and `rmp_serde` rejects floats for our integer-typed `size: u64` / `checked_at: Option<u128>` fields. Non-integer floats (π and friends) pass through unchanged. - Drops `#[ignore]` on `crates/cli/tests/pnpm_compatibility::same_index_file_contents`. End-to-end: `pnpm install` writes rows, pacquet reads them back, and the snapshot of decoded contents now matches what pacquet itself would have written. Refs #244. * fix(store): make pacquet-written rows decodable by pnpm too Symmetric follow-up to the previous commit: with just msgpackr-records decoding in place, pacquet can read what pnpm wrote, but pnpm could not reliably read what *pacquet* wrote. Verified empirically by feeding pacquet's output to `new Packr({useRecords: true, moreTypes: true})` from the pnpm v11 workspace's msgpackr 1.11.8. Two issues surfaced, both in how pacquet encoded `CafsFileInfo`: 1. `checked_at: Option<u128>` — `rmp_serde` has no native MessagePack integer encoding for `u128` and fell back to writing the value as a `bin 16` (16 raw big-endian bytes). msgpackr decoded the field as a `Uint8Array`, not a timestamp, so pnpm's integrity check would have silently misbehaved. Narrow the type to `u64` — millisecond timestamps fit in `u64` until year ~584M, and `u64` maps cleanly to MessagePack's integer encodings. 2. Even after the u128 → u64 fix, writing as `uint 64` (`cf`) made msgpackr return the value as a JS `BigInt`. pnpm's CAFS integrity check does `mtimeMs - (checkedAt ?? 0)`; mixing `Number` and `BigInt` throws `TypeError: Cannot mix BigInt and other types` at runtime. Pnpm itself sidesteps this because JS Number is a double — msgpackr packs large integer-valued Numbers as `float 64` (`cb`). Match that: add a `serialize_with` that emits `checked_at` as `float 64`. Verified: msgpackr now returns the value as `typeof === 'number'`, bit-identical byte encoding to what pnpm itself produces. Supporting read-side changes: - Add `deserialize_with` on `checked_at` that accepts either integer or integer-valued float. Required because plain msgpack reads bypass the records transcoder (the transcoder would reinterpret legitimate positive fixints in 0x40..=0x7f as record slot references, which is correct under records mode but wrong for pacquet-written rows). - Restore the sniff on `decode_index_value` — only invoke the transcoder when the bytes begin with the msgpackr records marker `d4 72`. An earlier version of this commit tried "always transcode" for simplicity; it broke every pacquet-written row whose `size` or `mode` happened to land in the 0x40..=0x7f byte range. Test updates: - `round_trips_plain_msgpack_through_transcoder` replaces `passes_through_plain_msgpack_unchanged`: the byte-for-byte guarantee no longer holds (the transcoder now narrows integer-valued floats), but the decoded-struct round-trip still does. - Add `plain_msgpack_without_floats_passes_through_unchanged` to pin down the narrower pass-through guarantee for non-float inputs. No changes to `tarball/src/lib.rs` beyond the `as_millis() as u64` cast the u128 → u64 narrowing now requires. * refactor(store): records-mode tracking + tight float upper bound Copilot review round one on #251. - Track whether the transcoder has seen a record definition yet, and only treat `0x40..=0x7f` as slot references once records mode is on. Until then those bytes are positive fixints (64..=127), the same as vanilla MessagePack. Before this change the transcoder couldn't be run on plain-msgpack input without corrupting it — pacquet rows with `size: 123` or similar would be mis-read as a reference to an undefined slot 0x7b. The old code sidestepped the problem with a sniff in `decode_index_value` (transcode only when bytes start with `d4 72`); records-mode tracking makes the transcoder safe for both encodings, so `decode_index_value` drops the sniff and always transcodes (which is necessary anyway for the float-to-uint narrowing on pacquet's own `checkedAt` field). - Replace the `v <= u64::MAX as f64` narrowing bound with a strict `v < 2f64.powi(64)`. `u64::MAX as f64` rounds *up* to 2^64 because 2^64 - 1 isn't exactly representable in `f64`, so the old bound accepted a literal 2^64 and silently saturated it to `u64::MAX` on cast. - Add `plain_positive_fixint_in_slot_range_passes_through` and `float64_equal_to_2_pow_64_passes_through` as regression guards for both fixes. - Drop the now-redundant custom `deserialize_with` on `CafsFileInfo::checked_at` — since all reads transcode first and the transcoder narrows integer-valued floats back to `uint 64`, the value always arrives as an integer for `rmp_serde` to deserialize. Refs #244. * refactor(store): sharper error variants + unreachable-slot guard + micro-tweaks Copilot review round two on #251. - Split `DecodeError::BadRecordDef` into three precise variants — `ExpectedArrayHeader`, `ExpectedStringHeader`, and `InvalidFieldNameUtf8`. The old shared variant carried a meaningless `count: 0 | 1` indicator that made messages like "Expected record-definition header (0 field-name strings in a msgpack array)" actively confusing. - Add `DecodeError::SlotOutOfRange` and reject any record definition whose slot byte isn't in `0x40..=0x7f`. msgpackr never emits one outside that range; accepting them before meant the slot was registered but unreachable, so the first reference-attempt elsewhere in the stream would have surfaced as an unrelated `UnknownSlot` error. - Drop three needless `to_vec()` allocations on the array16, array32, map16, and map32 header-copy paths — use `w.extend_from_slice(r.read_bytes(n)?)` directly. - Switch the `as_millis() as u64` cast in tarball extraction to `u64::try_from(...).ok()` so a hypothetical u128-overflowing timestamp drops to `None` instead of silently wrapping. - Fix a stale doc comment that still referred to an `Option<u128>` deserializer (now `Option<u64>`). - Rewrite the `StoreIndex::get` doc so it matches the current always-transcode implementation — no more "sniff the leading two bytes" language now that records-mode tracking makes unconditional transcoding safe. Refs #244. * refactor(store): Rc<[String]> slot schemas + correct get() doc Respond to two Copilot review notes: - Slot-reference decoding used to `.clone()` the full Vec<String> of field names per record instance. A 200-file row allocated 200 Vec<String>s plus one String per field per clone. Store the slot schema under `Rc<[String]>` so the reference path bumps a refcount instead. Single-thread transcoder, so `Rc` over `Arc`. - `StoreIndex::get` had a doc comment claiming a "sniff the leading two bytes" fast path that didn't exist — the implementation always transcodes. Explain why (pacquet writes `checkedAt` as float64 on purpose for msgpackr/pnpm interop, so every real row needs the transcoder to narrow it back), so the always-transcode behaviour reads as deliberate, not a missing optimisation. * refactor(store): consistent diagnostic codes + format-agnostic EOF msg Two Copilot nits: - Diagnostic codes said `pacquet_store_dir::msgpackr::…` even though the module is `msgpackr_records`. Match the `store_index::…` shape used elsewhere in the crate so codes map 1:1 to module paths. - `UnexpectedEof` said "msgpackr buffer", but the transcoder also runs on plain MessagePack (see the `get_decodes_…` path through `StoreIndex::get`). Reword to "MessagePack buffer" so the error reads correctly in either case. |
||
|
Zoltan Kochan
|
a06a62cf76 |
perf(tarball): skip download when SQLite index already has the entry (#249)
* perf(tarball): skip download when SQLite index already has the entry
On each `DownloadTarballToStore::run_without_mem_cache` call, look up
`{integrity}\t{pkg_id}` in `<store>/v11/index.db` before hitting the
network. If the row exists and every CAFS file it references is still
on disk, return the `{filename → PathBuf}` map straight from the index
and skip the HTTP request entirely — this is the shared-store payoff
of the v11 migration (#244).
The lookup is best-effort: missing db, missing row, decode failure, or
any pruned CAFS blob falls through to a fresh download. Decoding pnpm-
written rows still needs msgpackr record support, which is tracked
separately on #244.
SQLite open + read runs on `tokio::task::spawn_blocking` so the tokio
reactor doesn't stall on the PRAGMA setup / read.
Also adds `StoreDir::cas_file_path_by_mode(hex, mode)` mirroring pnpm's
`getFilePathByModeInCafs`, used to resolve index-recorded digests back
to on-disk paths without re-hashing the blob.
Refs #244.
* test(npmrc): compare PathBuf instead of display string for Windows
`Path::join` uses the platform's separator between components, so on
Windows `/workspace/root`.join(`../shared-store`) renders as
`/workspace/root\../shared-store`, not `/workspace/root/../shared-store`.
The hard-coded Unix-style expected made
`apply_resolves_relative_paths_against_base_dir` fail on windows-latest
in CI.
Rebuild the expected via the same join call and compare `StoreDir`
values directly so the test is portable.
* refactor(tarball): harden cache-lookup against corrupt index rows
Copilot review follow-ups on #249:
- `StoreDir::cas_file_path_by_mode` now returns `Option<PathBuf>` and
validates that the incoming hex digest is at least 2 ASCII-hex chars
long. The `file_path_by_hex_str` call slices `hex[..2]`, which would
otherwise panic on a corrupt / partially-interop row.
- `load_cached_cas_paths` propagates that `None` as a cache miss via
`?` so a bad row falls through to a fresh download instead of
bringing down the install through `spawn_blocking` → `JoinError`.
- Rewrite the `load_cached_cas_paths` doc comment: it claimed every
error path was silent, but the missing-blob branch emits a
`tracing::debug!`. Spell out which paths log and why.
- Fix an inline comment that pointed readers at PR #247 for the v11
store migration — the tracking issue is #244.
- Add unit tests for the new validation (`cas_file_path_by_mode`
invalid-hex inputs) and for the corrupt-digest fall-through through
`run_without_mem_cache`.
Refs #244.
* refactor(tarball): tighten digest validation and reduce cache-key alloc
Copilot review round two on #249:
- `cas_file_path_by_mode` now rejects 2-char digests. A 2-char input
produces an empty `<rest>` component, resolving to the shard
directory `files/XX/` (which usually exists on disk), so a caller
checking `path.exists()` would happily return a directory path as a
file path. Tighten the guard from `hex.len() < 2` to
`hex.len() <= 2` and update the unit test to reflect the new rule.
- `load_cached_cas_paths` now takes `cache_key: String` by value so
the owned key the caller already has can be moved into the
`spawn_blocking` closure without the internal `.to_string()` clone.
The cache-hit log at the call site swaps `?cache_key` for
`?package_id`, which is both cheaper (no extra owned string needed)
and more readable.
Refs #244.
* refactor(store/tarball): set busy_timeout on readonly, reject dir at CAFS path
Copilot review round three on #249:
- `StoreIndex::open_readonly` now sets `busy_timeout=5s` on the
read-only connection. Without it, a concurrent pnpm/pacquet writer
returns `SQLITE_BUSY` immediately and every cache lookup under
contention turns into a spurious miss + full download. The timeout
matches the writer side. `busy_timeout` is a connection-local wait,
not a DB mutation, so it's fine on `SQLITE_OPEN_READ_ONLY`.
- `load_cached_cas_paths` now validates the CAFS path with
`metadata().is_file()` rather than `path.exists()`. A directory
sitting at a CAFS path (store corruption, interrupted write, stray
`mkdir -p`) would otherwise slip through `exists()` and be handed
back to a linker as if it were a file.
- Adds `falls_through_when_cafs_path_is_a_directory` to exercise that
last case end-to-end: plant a directory at the computed CAFS path,
insert the index entry, and assert the fetcher drops through to the
(unreachable) network URL rather than claiming a cache hit.
Refs #244.
* refactor(tarball): reject symlinked blobs, consume file keys, fix error variant
Copilot review round four on #249:
- Swap `metadata()` for `symlink_metadata()` in `load_cached_cas_paths`
and reject symlinks outright. A tampered / corrupted store could
plant a symlink at the CAFS path pointing outside the store; the
previous check followed the link and would have trusted its target
as a valid blob. Now the cache lookup only reuses real regular
files.
- Consume `entry.files` by value so the owned `String` filename keys
move straight into `cas_paths` instead of being cloned on each
iteration. One less allocation per file in the package.
- `StoreIndex::open_readonly` now maps `busy_timeout` failures to
`StoreIndexError::Open` (with the db path) instead of `InitSchema`.
No schema work is happening in this function, so the old variant
was actively misleading in diagnostics.
- New test `falls_through_when_cafs_path_is_a_symlink` plants a
symlink at the computed CAFS path and asserts the fetcher drops
through to the unreachable URL.
Refs #244.
* docs(tarball): match load_cached_cas_paths log-scope doc to implementation
Copilot review round five on #249:
The doc said "only the missing-blob case emits a `debug!` log", but after
widening the CAFS check to reject symlinks, directories, metadata
errors, and other non-regular files, the single log now fires for every
path-validation failure. Rewrite the doc to describe the actual
behaviour: path-validation failures log (with the path, filename, and
cache key so an operator can tell what drifted), earlier checks — db /
row / decode / digest — stay silent because they don't point at a
specific on-disk artifact.
* fix: align pacquet's executable-bit rule with pnpm's modeIsExecutable
Copilot review round six on #249 caught a real bug.
pnpm treats a file as executable when *any* exec bit is set (`mode &
0o111 != 0`); pacquet was using `is_all_exec`, which required *all*
three bits. The two rules agree on `0o755` (which is what npm usually
ships) but disagree on common asymmetric modes like `0o744` or
`0o100` — pacquet would write such a blob without the `-exec` suffix,
pnpm would look for it *with* the suffix, and `DownloadTarballToStore`
would cross-wire its own writes: my new read-side
`cas_file_path_by_mode` used the `any-bit-set` rule, so a pacquet-
written `0o744` file was guaranteed to cache-miss against its own
entry.
- Replace `file_mode::is_all_exec` with `file_mode::is_executable`
(`mode & EXEC_MASK != 0`) and document why.
- Update the tarball write-side call site to use the new helper.
- Have `cas_file_path_by_mode` call `is_executable` too so write + read
can never drift out of sync again.
- Add `cas_file_path_by_mode_suffix_matches_write_side` exercising
`0o744`, `0o100`, and a handful of non-exec modes so the rule can't
silently diverge in the future.
Refs #244.
* test(tarball): use a short-timeout reqwest client for fall-through tests
Copilot review round seven on #249.
The five tests that point at \`http://127.0.0.1:1/unreachable.tgz\` rely
on a fast ECONNREFUSED to prove the cache lookup short-circuited. On a
runner that drops packets instead of refusing them (some firewalled
CIs, some corporate NAT), the default client has no connect / request
timeout so those tests would hang for TCP-retry worth of time.
- Add \`ThrottledClient::from_client\` so callers can wrap a pre-built
\`reqwest::Client\` with custom timeouts without re-implementing the
permit semaphore.
- Add a \`fast_fail_client()\` test helper that builds a client with 1 s
connect and 1 s overall timeout, and use it for all five tests whose
URL is \`127.0.0.1:1\`. The real-network tests
(\`packages_under_orgs_should_work\`,
\`should_throw_error_on_checksum_mismatch\`) still use
\`Default::default()\` since they need a non-capped client.
|
||
|
Zoltan Kochan
|
7dd49048cd |
feat(config)!: match pnpm 11 config — yaml for settings, .npmrc for auth only (#248)
* feat(config): read settings from pnpm-workspace.yaml like pnpm v11 pnpm 10+ moved the bulk of its configuration (\`storeDir\`, \`registry\`, \`lockfile\`, …) out of \`.npmrc\` into \`pnpm-workspace.yaml\`, using camelCase keys. A real pnpm-11-style project may put those overrides only in the yaml, so pacquet has to read them from there to behave correctly — and after the v11 store migration (#247) this was the last place the two tools still disagreed about where config lives. This PR makes pacquet load and layer \`pnpm-workspace.yaml\` on top of \`.npmrc\`, matching pnpm's own precedence: yaml wins over \`.npmrc\` for any key it sets; \`.npmrc\` wins over hard-coded defaults. ## How it works - New \`WorkspaceSettings\` struct in \`pacquet-npmrc\` with camelCase serde derives and \`Option\` for every field mirroring the current \`Npmrc\`: \`hoist\`, \`hoistPattern\`, \`publicHoistPattern\`, \`shamefullyHoist\`, \`storeDir\`, \`modulesDir\`, \`nodeLinker\`, \`symlink\`, \`virtualStoreDir\`, \`packageImportMethod\`, \`modulesCacheMaxAge\`, \`lockfile\`, \`preferFrozenLockfile\`, \`lockfileIncludeTarballUrl\`, \`registry\`, \`autoInstallPeers\`, \`dedupePeerDependents\`, \`strictPeerDependencies\`, \`resolvePeersFromWorkspaceRoot\`. - Non-config keys commonly present in a real pnpm-workspace.yaml (\`packages\`, \`catalog\`, \`catalogs\`, \`onlyBuiltDependencies\`, \`allowBuilds\`, …) are silently ignored — serde drops them since the struct doesn't use \`deny_unknown_fields\`. - \`WorkspaceSettings::find_and_load\` walks up from cwd looking for the nearest \`pnpm-workspace.yaml\` (same algorithm pnpm uses — the first ancestor that has one is the workspace root). - \`WorkspaceSettings::apply_to\` overlays every set field onto the mutable \`Npmrc\`, resolving relative path-valued fields (\`storeDir\`, \`modulesDir\`, \`virtualStoreDir\`) against the workspace root instead of cwd — again matching pnpm. - \`Npmrc::current\` now layers the result on top of whatever \`.npmrc\` (or the default) provided. Missing yaml or unreadable yaml is silently ignored, matching \`.npmrc\`'s best-effort behaviour. 9 new unit tests cover yaml parsing (known + unknown keys), field apply-or-skip, absolute/relative path resolution, upward directory walk, and the full \`Npmrc::current\` integration with both files present. The \`testing-utils::add_mocked_registry\` helper from #247 already writes both \`.npmrc\` and \`pnpm-workspace.yaml\` (so both tools recognise the store-dir override). With this PR, pacquet now actually uses the yaml side of that pair. * feat(config)!: stop reading non-auth settings from .npmrc, like pnpm 11 pnpm 11 narrowed \`.npmrc\` to an auth/network-only allow-list: \`registry\`, \`ca\` / \`cafile\` / \`cert\` / \`key\`, \`_auth\` / \`_authToken\` / \`_password\` / \`email\` / \`keyfile\` / \`username\`, \`https-proxy\` / \`proxy\` / \`no-proxy\` / \`http-proxy\` / \`local-address\` / \`strict-ssl\`, plus the \`@scope:registry\` and \`//host:_authToken\` dynamic patterns. Everything else (\`storeDir\`, \`lockfile\`, hoist patterns, node-linker, …) has to come from \`pnpm-workspace.yaml\` or CLI flags. Pacquet now does the same. Behaviour changes: - Writing project-structural settings to \`.npmrc\` (\`store-dir=…\`, \`lockfile=…\`, \`hoist=…\`, \`node-linker=…\`, …) no longer has any effect on the resolved config. Move them to \`pnpm-workspace.yaml\`. Since pacquet is pre-1.0 and #247 already broke the old store format, there's no migration shim. - \`.npmrc\` is still read for the auth/network subset — today that's just \`registry\`, which is all pacquet actually uses. The other auth keys are tracked in the parser so lighting up auth / proxy / TLS later doesn't require another .npmrc refactor. - Precedence is unchanged in shape: defaults → .npmrc (auth-only) → \`pnpm-workspace.yaml\`. Implementation: - New \`NpmrcAuth\` struct in \`pacquet-npmrc\` with a hand-rolled line parser (drops comments, skips malformed lines, ignores section headers). Picks \`registry\` and normalises the trailing slash — same behaviour the old \`deserialize_registry\` had. - \`Npmrc::current\` no longer runs \`serde_ini::from_str::<Npmrc>\` on \`.npmrc\`. It starts from the caller-supplied default, applies the \`NpmrcAuth\` from the nearest .npmrc (cwd first, home second), then layers the yaml overrides from #248 on top. - Tests that used to write \`symlink=false\` / similar to .npmrc now either assert the opposite (the setting is ignored) or exercise \`registry\` instead. 5 new unit tests in \`npmrc_auth.rs\` cover registry pickup, trailing-slash normalisation, non-auth key ignoring, and malformed-line resilience. The \`Npmrc\` struct still derives \`Deserialize\` so \`Npmrc::new()\` can keep using \`serde_ini::from_str(\"\")\` as its default-producer — cleaning that up to a hand-written \`Default\` impl is separate churn. * test(cli): move store-path test off .npmrc onto pnpm-workspace.yaml `storeDir` is a project-structural setting — after pnpm 11 (and this PR's matching narrowing in pacquet) it's only honoured from `pnpm-workspace.yaml`, not `.npmrc`. The old test wrote `store-dir=foo/bar` to `.npmrc` and expected pacquet to print that as the store path, which is exactly the behaviour we just removed. Switched it to write `storeDir: foo/bar` to `pnpm-workspace.yaml` instead — same assertion, correct config source. * review: make config doc comments match what the code actually does - `NpmrcAuth`'s module doc overstated scope: only `registry` is parsed, but the old wording read as if the full pnpm auth allow-list was implemented. Rewritten to say what's handled today and what's tracked for later. - `Npmrc::current`'s doc listed auth keys (TLS / `_authToken` / scoped-registry / proxy) as if pacquet read them. It doesn't. Narrowed to 'registry only, rest ignored'. - A test comment in `workspace_yaml::tests::swallows_unknown_top_level_keys` still referenced `deny_unknown_fields` and a 'flatten catch-all', both of which I removed in an earlier commit on this branch (serde drops unknown struct fields by default). Updated to describe the actual regression the test guards against — somebody later adding `deny_unknown_fields` to `WorkspaceSettings`. |
||
|
Zoltan Kochan
|
0b0abf8189 |
feat!(store): migrate from v3 JSON index to v11 SQLite+msgpack (#244) (#247)
* feat!(store): migrate from v3 JSON index to v11 SQLite+msgpack layout Pacquet now writes the same on-disk store that pnpm v11 uses, so running pacquet and pnpm on the same machine can share the tarball cache instead of maintaining parallel stores. Closes half of #244. - **Layout:** \`<store>/v11/files/XX/<hex-remainder>[-exec]\` for content-addressed blobs + \`<store>/v11/index.db\` for the per-package file index. The legacy \`v3/files/XX/<hex>-index.json\` JSON layout is gone (we don't need backward compat — pacquet is pre-1.0). - **SQLite schema / PRAGMAs:** byte-identical to pnpm's \`store/index/src/index.ts\` — \`package_index (key TEXT PRIMARY KEY, data BLOB NOT NULL) WITHOUT ROWID\` with WAL journal, 5s busy_timeout, 512 MB mmap, 32 MB page cache, temp_store=MEMORY, wal_autocheckpoint=10000. Two concurrent pacquet tasks (or a pacquet + pnpm pair) contend correctly via SQLite's own locking. - **Value encoding:** msgpack via \`rmp-serde::to_vec_named\` so structs are serialized with named fields, matching pnpm's msgpackr record encoding. \`PackageFilesIndex\` and \`CafsFileInfo\` types mirror pnpm's \`@pnpm/store.cafs\` shape field-for-field (including the hex \`digest\` rather than pacquet's old SSRI \`integrity\` string). - **Key format:** \`"{integrity}\\t{pkg_id}"\` via \`store_index_key()\`, same as pnpm's \`storeIndexKey\`. \`pkg_id\` is \`"{name}@{version}"\` — pulled from \`PkgNameVerPeer::without_peer()\` on the frozen-lockfile path and assembled from \`PackageVersion\` on the registry path. Breaking API changes inside the workspace: - \`pacquet-store-dir\` loses the v3 JSON types (\`PackageFilesIndex\`, \`PackageFileInfo\`, \`WriteIndexFileError\`) and \`StoreDir::v3()\` / \`::write_index_file()\`. In their place: \`StoreIndex\`, \`StoreIndexError\`, the pnpm-shape \`PackageFilesIndex\` + \`CafsFileInfo\`, and \`store_index_key()\`. Unit-tested with 9 new round-trip / re-open / key-format tests. - \`DownloadTarballToStore\` gains a \`package_id\` field. All three callers (two in \`pacquet-package-manager\`, one in the micro-benchmark) pass \`"{name}@{version}"\`. - \`pacquet-testing-utils::add_mocked_registry\` now also writes \`pnpm-workspace.yaml\` with \`storeDir\` / \`cacheDir\` so pnpm 11 (which moved that config out of \`.npmrc\`) respects the test's store override. - \`crates/cli/tests/_utils.rs::index_file_contents\` now walks \`StoreIndex::keys()\` and returns \`BTreeMap<sqlite_key, BTreeMap<filename, CafsFileInfo>>\`. The insta snapshots will need re-accept once the test run against the new mock layout settles. - Dropped the \`#[ignore]\` on \`pnpm_compatibility::{same_file_structure, same_index_file_contents}\` — the v11 cutover is the prerequisite they were waiting on. The install-path snapshots in \`install.rs\` will likely need a re-accept through \`cargo insta test --accept\`; running them requires \`just registry-mock launch\` and is tracked at the top of \`crates/cli/tests/install.rs\`. Dependencies added at the workspace level: \`rusqlite = "0.38"\` (with the \`bundled\` feature so we don't need a system libsqlite3) and \`rmp-serde = "1.3\"\`. Dropped the unused \`base64\` dep from \`tarball\`. Verified end-to-end: \`pnpm install --lockfile-only\` against \`@pnpm/registry-mock\` followed by \`pacquet install --frozen-lockfile\` produces \`store-dir/v11/files/XX/…\` + \`store-dir/v11/index.db\` with 50 rows whose keys match pnpm's \`{integrity}\\t{pkgId}\` format (verified via sqlite3 CLI against the pacquet-written db). Follow-ups (tracked in #244 and this PR's description): - Teach pacquet's config loader to read \`pnpm-workspace.yaml\` in addition to \`.npmrc\` so \`storeDir\` / \`cacheDir\` / \`registry\` overrides resolve without requiring the .npmrc mirror. - Read the index at install-time to skip re-fetching already-cached tarballs. - Decode msgpackr \`useRecords\` entries so pacquet can read back records that pnpm itself wrote (pnpm reads pacquet's named-field writes fine today; the reverse direction is the missing piece). * chore(deny): allow Zlib license for foldhash (transitive via rusqlite) rusqlite pulls in foldhash 0.2, which is Zlib-licensed. Zlib is OSI-approved and widely accepted; add it to the allow list. * test: re-accept v11 store snapshots + scope pnpm_compatibility tests Aftershocks from the v11 store migration: - Three insta snapshots in crates/cli/tests/snapshots/install__*.snap referenced the old \`v3/files/.../*-index.json\` paths and had to be re-accepted via \`cargo insta test --accept\` to reflect the new \`v11/files/XX/<hex>[-exec]\` + \`v11/index.db\` layout. Inner file integrities are unchanged; only the store-dir path prefix differs. - \`pnpm_compatibility::same_file_structure\` now filters out three pnpm-11-specific artifacts that pacquet has no equivalent for, but which don't affect the shared-CAFS invariant the test is there to verify: * \`v11/projects/<hash>\` — pnpm-11 per-project metadata. * \`v11/links/<name>/<version>/<hash>/node_modules/...\` — pnpm 11's hoisted-symlinks layout; pacquet uses its own \`.pnpm/\`-based virtual store. * \`v11/index.db-{wal,shm}\` — SQLite WAL sidecars whose presence at compare-time depends on whether the checkpoint ran. With that filter the two tools' CAFS file listings match byte-for- byte. - \`pnpm_compatibility::same_index_file_contents\` goes back under \`#[ignore]\` with an updated reason. Pacquet-written \`index.db\` values round-trip fine (we serialize via \`rmp-serde::to_vec_named\`, string-keyed maps), but pnpm writes with msgpackr's \`useRecords: true\` extension-typed records that rmp-serde can't decode. Handling those is the last piece of #244 (msgpackr-record decoding for the read side) and will be addressed after this PR lands. * review: address Copilot feedback on the v11 store migration - \`TarballError::WriteStoreIndex\` display message was still "Failed to write tarball index" (a leftover from the per-package JSON era). Rephrased to "Failed to write store index (SQLite index)". - \`entry.header().size().unwrap_or(0)\` silently turned tar header parse errors into 0-byte entries in the index. Propagate the error via \`TarballError::ReadTarballEntries\` instead. - Open + PRAGMA + \`INSERT\` on the SQLite index are blocking (and can stall up to \`busy_timeout=5000\` ms under contention). Moved the write step into \`tokio::task::spawn_blocking\` so it runs on the blocking pool instead of the tokio reactor. The tarball-extraction loop (which holds a \`!Send\` \`tar::Archive\`) is now scoped into a sub-block so it's dropped before the new \`await\`. - Added \`StoreIndex::open_readonly\` / \`open_readonly_in\` that use \`SQLITE_OPEN_READ_ONLY\` and skip the schema-creation PRAGMAs. \`crates/cli/tests/_utils.rs::index_file_contents\` now uses it so the snapshot helper doesn't mutate the store or create WAL / SHM sidecar files during what is supposed to be a read. - Clarified the \`StoreIndex\` module-level doc comment: the DB lives at \`<store-root>/v11/index.db\` when opened via \`open_in\`, not at \`<store_dir>/index.db\` as the old wording suggested. |
||
|
Zoltan Kochan
|
666420d5ad |
ci: split \cargo doc\ into its own ubuntu-only job with --no-deps (#246)
The Doc step was the longest step in the Lint-and-Test matrix on every platform (ubuntu 4:34, macos 5:00, windows 6:13 — longer than clippy and test combined) and ran per-OS even though rustdoc warnings are platform-independent for pure-Rust code. Moves it to a dedicated \`doc\` job: - Runs only on ubuntu-latest — recovers ~11 minutes of wall-clock per CI run (the windows + macos duplicates are gone). - Runs in parallel with the Lint-and-Test matrix instead of blocking the test feedback behind 5-6 minutes of HTML generation. - Uses \`cargo doc --no-deps --workspace\` so only pacquet's own crates get their HTML rendered. Broken intra-doc links and malformed rustdoc markup in workspace code are still caught (\`RUSTDOCFLAGS: '-D warnings'\` is preserved); rendering dep docs that no one reads is not. - Drops the \`docs: true\` flag from the test matrix's rustup step since the matrix no longer needs rust-docs installed. |
||
|
Zoltan Kochan
|
e7005635bd |
chore(deps): bump all dependencies, clear the advisory ignore list (#245)
* chore(deps): bump all dependencies to latest and clear advisory ignores Bumps all workspace deps to the latest compatible versions including the major-version jumps. The `deny.toml` RUSTSEC ignore list that #243 had to carry is now empty — every advisory is resolved by the upstream version bumps rather than by ignore-listing. **Major version bumps (API-breaking):** - `base64` 0.21 → 0.22 - `dashmap` 5.5 → 6.1 - `derive_more` 1.0.0-beta.6 → 2.1.1 (stabilized) - `insta` 1.34 → 1.47 - `itertools` 0.11 → 0.14 - `miette` 5.9 → 7.6 - `reqwest` 0.11 → 0.13 - `split-first-char` 0.0.0 → 2.0.1 - `strum` 0.25 → 0.28 - `sysinfo` 0.29 → 0.38 - `text-block-macros` 0.1 → 0.2 - `which` 4.4 → 8.0 - `criterion` 0.5 → 0.8 **Not bumped:** `sha2` stays on 0.10.9 — 0.11 replaced `GenericArray` with a new `Array<_, _>` type that doesn't impl `LowerHex`, so `format!("{hash:x}")` in `store-dir/src/cas_file.rs` breaks. Revisit once the ecosystem catches up. **Code fallout from the bumps:** - `sysinfo` dropped its `*Ext` traits (methods are now inherent) and renamed `RefreshKind::new()` → `RefreshKind::nothing()` / `ProcessRefreshKind::new()` → `ProcessRefreshKind::nothing()`. Dropped `PidExt` / `ProcessExt` / `SystemExt` from imports in `tasks/registry-mock/src/{kill_verdaccio,mock_instance,registry_info,registry_anchor}.rs`. - `derive_more` 2 fully separates the derive macro from the `std::fmt::Display` trait; unqualified `Display` in trait bounds resolved to the derive. Switched to fully-qualified `std::fmt::Display` in `pkg_name_suffix.rs`'s bounds. - Newer ICU crates (pulled in transitively via `idna`) are licensed `Unicode-3.0` instead of `Unicode-DFS-2016`. Allow the new SPDX id in `deny.toml`. - Dropped two now-unused `std::env` imports that the bumps surfaced. **Verification (Rust 1.95 on macOS):** - `cargo build --workspace` — clean - `cargo clippy --locked --workspace --all-targets -- --deny warnings` — clean - `cargo fmt --all -- --check` — clean - `cargo deny check` — `advisories ok, bans ok, licenses ok, sources ok` - All unit tests on the rewritten crates green (the two pre-existing `pacquet-npmrc` env-pollution failures are identical to main). - End-to-end: `pnpm install --lockfile-only` against `@pnpm/registry-mock` then `pacquet install --frozen-lockfile` still produces the expected `node_modules` + `.pnpm/` layout. * ci: bump pnpm action-setup to v4 and pin pnpm to 11 The Integrated-Benchmark workflow was failing because it pinned pnpm 8 via `pnpm/action-setup@v2`. pnpm 8 writes and reads lockfile v6 only, so the v9 fixture bundled in `tasks/integrated-benchmark` was rejected with "Ignoring not compatible lockfile" followed by ERR_PNPM_NO_LOCKFILE during the proxy-cache population step. - `pnpm/action-setup@v2` → `@v4` across `ci.yml`, `codecov.yml`, `integrated-benchmark.yml`. - `version: 8` / `version: 8.9.2` → `version: 11`. - `actions/cache@v3` → `@v4` in the same three workflows (v3 is already EOL on GitHub-hosted runners). - Cache `path:` from `${PNPM_HOME}/store/v3` to `/store/v11` (pnpm 11 writes to v11 subdir, not v3). - Bust the three pnpm cache keys (`ci-pnpm-` → `ci-pnpm-v11-`, `codecov-pnpm-` → `codecov-pnpm-v11-`, `integrated-benchmark-pnpm` → `integrated-benchmark-pnpm-v11`) to avoid restoring v3 content into the new v11 path. * ci: pin pnpm to 11.0.0-rc.5 (stable 11 not released yet) * ci: bump pnpm/action-setup v4 -> v6 * ci: regenerate tasks/registry-mock/pnpm-lock.yaml as v9 for pnpm 11 * ci: approve esbuild + ignore core-js builds for pnpm 11 strict mode pnpm 11 turns unapproved build scripts into a hard `ERR_PNPM_IGNORED_BUILDS` failure. Mark esbuild (needs its postinstall to fetch the binary) as approved via `pnpm.onlyBuiltDependencies`, and opt-out core-js's ad postinstall via `pnpm.ignoredBuiltDependencies`. * ci: opt out core-js postinstall via pnpm-workspace.yaml allowBuilds Revert the package.json `pnpm.{onlyBuiltDependencies,ignoredBuiltDependencies}` I put in the previous commit — in pnpm 11 this configuration moved to `pnpm-workspace.yaml` as `allowBuilds: { <pkg>: false }`. * chore: apply taplo format + add pre-push hook for fmt checks - The long `sha2` inline comment in `Cargo.toml` bumped the alignment past its column; `taplo format` re-indents it. `cargo fmt` is already green. - Add `.githooks/pre-push` that runs `cargo fmt --all -- --check` and `taplo format --check` before push, so fmt violations don't sneak onto CI. `just install-hooks` wires the repo to it via `git config core.hooksPath .githooks`, and `just init` calls it so fresh clones pick the hook up without extra steps. * review: fix typo, codecov cache key, document pnpm RC pin - \`tasks/registry-mock/src/registry_anchor.rs\`: doc-comment typo "is spawn" → "is spawned". - \`.github/workflows/codecov.yml\`: the \`coverage\` job has no matrix, so \`matrix.os\` was empty in the cache key. Switch to \`runner.os\`. - All three workflows: add an inline comment next to \`version: 11.0.0-rc.5\` noting that stable pnpm 11 isn't released yet and that the pin should move to \`11\` once it ships. * test: update install snapshots + ignore pnpm-compat tests broken by pnpm 11 - \`@pnpm/registry-mock\` bumped 3.16 → 3.50 in my lockfile regen, which repackaged the \`@pnpm.e2e/*\` tarballs. The inner file integrities are unchanged, but the tarball-level sha512 (which drives the index-file location under \`v3/files/XX/…-index.json\`) shifted. Accept the three install.rs insta snapshots so they reflect the new mock tarballs. - \`pnpm_compatibility::{same_file_structure, same_index_file_contents}\` compared pacquet's store layout to pnpm's. Under pnpm 11 both assumptions behind the test are broken: 1. pnpm 11 writes to \`store/v11/…\` with a SQLite \`index.db\`, not the v3 \`files/XX/…-index.json\` layout pacquet still uses. 2. pnpm 11 moved \`store-dir\` config out of \`.npmrc\` into \`pnpm-workspace.yaml\`, so the test's current \`.npmrc\`-based override is silently ignored and pnpm uses the global store. Both are resolvable only by pacquet adopting the v11 store format — tracked in #244. Mark both as \`#[ignore]\` with a pointer to #244; they are expected to un-ignore once pacquet's store migrates. |
||
|
Karl Horky
|
57b785c5a3 |
docs: add Discord link to README (#11166)
Added Discord link to the README. --------- Co-authored-by: Zoltan Kochan <z@kochan.io> |
||
|
Zoltan Kochan
|
aef67087e9 |
feat!: pnpm lockfile v9 support + install error propagation + Rust 1.95 (#243)
* refactor(install): propagate errors through the Install stack
Replace the three `unimplemented!()` panics in the install dispatcher
with proper miette diagnostics, and thread `Result` propagation through
`Install`, `InstallWithoutLockfile`, `InstallFrozenLockfile`, and
`CreateVirtualStore`.
New `InstallError::NoLockfile` (code `pacquet_package_manager::no_lockfile`)
is returned when `--frozen-lockfile` is requested but no `pnpm-lock.yaml`
exists, mirroring pnpm's `ERR_PNPM_NO_LOCKFILE`. The two remaining
writable-lockfile paths surface as `InstallError::UnsupportedLockfileMode`
until lockfile writing lands.
Downstream `.unwrap()`s on `InstallPackageFromRegistryError` and
`InstallPackageBySnapshotError` now become `?`, and the `join_all` calls
switch to `try_join_all` to fail fast.
* feat(lockfile): add Lockfile::save_to_path and save_to_current_dir
Introduces the write side of the lockfile API paired with the existing
`load_lockfile` module. Emits YAML via `serde_yaml::to_string` and
propagates serde/io failures through a new `SaveLockfileError`
(codes `pacquet_lockfile::serialize_yaml`, `pacquet_lockfile::write_file`,
`pacquet_lockfile::current_dir`).
The output is semantically equivalent to the input (parse → save → reparse
yields an equal `Lockfile`), but is NOT yet byte-identical with pnpm's
`sortLockfileKeys`-based output — matching the exact key order and YAML
style pnpm produces is a separate piece and will come with the install
path that consumes this API.
* feat(package-manager): add build_snapshot helpers for PackageVersion -> PackageSnapshot
Introduces `registry_dependency_path` and `build_package_snapshot` in a
new `build_snapshot` module. Given a resolved `PackageVersion` plus an
injected map of resolved sub-dependencies and dev/optional flags, they
produce the `(DependencyPath, PackageSnapshot)` pair that can be dropped
into a `pnpm-lock.yaml`'s `packages` map.
The helpers are pure and unit-tested with synthetic fixtures — wiring
them into `InstallWithoutLockfile` to collect live resolution metadata
and construct the full `Lockfile` is the next step.
Errors propagate via `BuildSnapshotError` (`missing_integrity`,
`parse_name`) rather than panicking when input is malformed.
* feat!(lockfile): replace v6 types with v9 (packages + snapshots + importers)
pnpm v11 writes lockfile v9 exclusively. Pacquet's existing v6 types
cannot read these files. Per-session agreement, no backward
compatibility with v6 is kept.
Type rewrite in `pacquet-lockfile`:
- `Lockfile` now carries `importers: HashMap<String, ProjectSnapshot>`
(with `Lockfile::ROOT_IMPORTER_KEY = "."`) instead of a flattened
`RootProjectSnapshot`, plus separate `packages: HashMap<PackageKey,
PackageMetadata>` and `snapshots: HashMap<PackageKey, SnapshotEntry>`
maps keyed by the v9 `name@version[(peers)]` specifier (no leading
slash).
- `PackageMetadata` (new) holds per-version data (resolution, engines,
cpu/os/libc, deprecated, hasBin, peer metadata). `SnapshotEntry`
(new) holds per-instance data (dependencies, optional_dependencies,
transitive_peer_dependencies, id, patched).
- `PkgNameVerPeer::without_peer` strips the peer suffix so consumers
can look up a snapshot entry's metadata in the `packages:` map.
- Deleted: `dependency_path.rs`, `multi_project_snapshot.rs`,
`package_snapshot.rs`, `package_snapshot_dependency.rs`,
`root_project_snapshot.rs`.
- `LockfileVersion<9>` with the v6 compat test swapped for v9.
Consumer updates in `pacquet-package-manager`:
- `InstallFrozenLockfile` now takes `importers`, `packages`, and
`snapshots` separately.
- `CreateVirtualStore` iterates `snapshots` and resolves each
snapshot's metadata via `PackageKey::without_peer()`, returning
`MissingPackageMetadata` if the lockfile is inconsistent.
- `InstallPackageBySnapshot` takes `(package_key, metadata, snapshot)`
and reads resolution from `PackageMetadata`.
- `CreateVirtualDirBySnapshot` derives the virtual store dir from the
`PackageKey` and reads dependency wiring from the `SnapshotEntry`.
- `SymlinkDirectDependencies` looks up the root project via
`importers[Lockfile::ROOT_IMPORTER_KEY]` instead of pattern-matching
`RootProjectSnapshot::Single`.
- `create_symlink_layout` takes `HashMap<PkgName, PkgVerPeer>` (the v9
snapshot deps shape) instead of the legacy enum.
- `build_snapshot` returns `BuiltSnapshot { package_key, metadata,
snapshot }` matching the v9 decomposition.
Fixture updates:
- `BIG_LOCKFILE` regenerated as v9 via pnpm v11.
- Save-lockfile round-trip fixture rewritten in v9 shape.
- Save-lockfile error fixture uses `lockfileVersion: '9.0'`.
Verified end-to-end: `pnpm install --lockfile-only` generates a v9
lockfile against `@pnpm/registry-mock`, then `pacquet install
--frozen-lockfile` populates node_modules from that lockfile (50
packages, symlinks wired).
Benchmark on the pacquet fixture manifest against the registry mock
(hyperfine, 10 runs + 2 warmup):
- pacquet: 487.9 ms ± 106.6 ms (range 343-656 ms)
- pnpm@11: 739.6 ms ± 9.1 ms (range 725-751 ms)
- pacquet is 1.52x faster; worst-case pacquet still beats best-case pnpm.
* style(install): apply cargo fmt
* chore(deny): rewrite deny.toml for cargo-deny 0.19 schema
The old template predated cargo-deny 0.14. The schema has since moved
the graph/output keys under dedicated sections and changed advisory
level fields. Rewritten to follow the 0.19 shape.
Also adds a scoped ignore list for the RUSTSEC advisories that exist
in the already-pinned transitive deps (bytes, h2, idna, mio, openssl,
tar, tokio, tracing-subscriber) — resolving those requires bumping the
1.73.0 toolchain and a round of coordinated dep updates that is out of
scope for this PR.
* chore(toolchain): bump rust-toolchain.toml to 1.95.0 + fix CI fallout
Bumps the pinned Rust channel from 1.73.0 to 1.95.0 and addresses the
consequences:
- `tasks/registry-mock/src/registry_anchor.rs`: migrate off the
`advisory-lock` crate to Rust 1.89+'s stable `std::fs::File::{lock,
try_lock}` (the crate's trait methods now collide with std's). Drop
the workspace dep.
- New rustc/clippy lints under 1.95:
- `mismatched_lifetime_syntaxes` → add explicit `'_` to
`StoreDir::display`.
- `clippy::map_clone` → `.cloned()` in `PackageManifest::bundle_dependencies`.
- `clippy::manual_pattern_char_comparison` → array pattern in
`PkgVerPeer` parser.
- `clippy::suspicious_open_options` → add `.truncate(false)` to the
persistent lock file in `GuardFile::path`.
- `clippy::useless_into_iter_in_iterator_adapter` →
`InstallWithoutLockfile::run` dependencies call.
- `clippy::manual_unwrap_or` → collapse `if let Some` match in the
`pacquet start` fallback.
- `crates/cli/tests/install.rs`: move `BIG_LOCKFILE` / `BIG_MANIFEST` /
`OpenOptions` / `io::Write` imports behind the same
`cfg(not(macos|windows))` gate as their one consumer so they aren't
flagged as unused on macOS.
Also unblocks the Micro-Benchmark / Code Coverage / release-to-npm
workflows: bump `actions/upload-artifact@v3` → `@v4` (v3 is now
hard-rejected by GitHub Actions).
`cargo build --workspace`, `cargo clippy --locked --workspace
--all-targets -- --deny warnings`, `cargo fmt --all -- --check`, and
`cargo deny check` all pass locally on 1.95.
* chore(ci): bump actions/download-artifact to v4 (v3 is hard-rejected)
* test(cli): disable flaky big-lockfile test on all CI platforms
The test drives the mocked verdaccio with hundreds of concurrent tarball
fetches and reliably trips a connection error on every hosted-runner OS:
ConnectionAborted on Windows, ConnectionReset on macOS, and now
ConnectionClosed on Ubuntu (the regenerated v9 BIG_LOCKFILE has more
packages, so Ubuntu started hitting the same class of issue the other
platforms already gated out).
Align the cfg gate to exclude all three and document the manual run
recipe. The BIG_LOCKFILE / BIG_MANIFEST / OpenOptions / io::Write
imports follow the same gate.
* test(integrated-benchmark): regenerate lockfile fixture as v9
The fixture was v6 and consequently unparseable by HEAD after the v9
rewrite, which failed the Integrated-Benchmark CI run in Benchmark 1
(pacquet@HEAD) before it could even start hyperfine.
With the fixture at v9 the benchmark can at least evaluate pacquet@HEAD
standalone. The HEAD-vs-main hyperfine comparison that this workflow is
designed for is intrinsically broken by this PR (main still speaks v6
only, so it cannot read a v9 fixture); that failure is an expected
consequence of the breaking format change and will resolve once this
PR lands on main.
* review: replace remaining panics/silent-failures with proper diagnostics
Addresses Copilot review feedback on the install path. Every code path
that previously used `panic!` / `expect` / silent `return` now either
surfaces a structured error or — in the test-gating case — uses
`#[ignore]` so the test remains runnable locally.
- `InstallPackageBySnapshotError`: new `MissingTarballIntegrity` and
`UnsupportedResolution` variants; `install_package_by_snapshot` no
longer panics when a tarball resolution lacks an integrity field or
when the lockfile references Directory/Git resolutions that pacquet
does not yet handle.
- `CreateVirtualStoreError`: new `MissingPackagesSection` variant for
the case where a lockfile has `snapshots:` but no `packages:`. The
dispatcher no longer panics in that case.
- `SymlinkDirectDependenciesError`: new `MissingRootImporter` variant.
A lockfile with no `importers."."` entry now produces a clear
diagnostic instead of silently producing an incomplete install.
`InstallFrozenLockfile` gains a transparent variant that wraps it.
- `BuildSnapshotError::ParseVersion`: `registry_package_key` now
propagates `ParsePkgVerPeerError` instead of `.expect`-ing that
`node_semver::Version::to_string` always yields a valid `PkgVerPeer`.
The two grammars agree today but the assumption is no longer encoded
as a panic.
- `frozen_lockfile_should_be_able_to_handle_big_lockfile`: replace the
overly-broad `cfg(not(any(windows, macos, linux)))` gate with
`#[ignore = "flaky on CI…"]`. The test is now runnable locally via
`cargo test -- --ignored` per the updated comment.
- Drive-by: drop `.write(true)` next to `.append(true)` flagged by
`clippy::ineffective_open_options` after the import un-gating above.
Manual verification: `pnpm install --lockfile-only` against
`@pnpm/registry-mock` followed by `pacquet install --frozen-lockfile`
still produces the expected `node_modules` + `.pnpm/` layout.
|
||
|
Zoltan Kochan
|
623c70802f |
chore: remove unused getNodeExecPath helpers (#11336)
Drops `getNodeExecPath`, `getNodeExecPathInBinDir`, and `getNodeExecPathInNodeDir` along with their now-unused `which` dependency. None of these helpers were referenced anywhere in the codebase. |
||
|
Zoltan Kochan
|
f4e32dece4 | ci: run all tests on release branches | ||
|
Zoltan Kochan
|
1beb41652b | ci: run all tests on release branches | ||
|
Zoltan Kochan
|
dd23d19860 |
fix(binary-fetcher): skip zip directory entries during Node.js runtime extraction (#11333)
* fix(binary-fetcher): skip zip directory entries during Node.js extraction When a Node.js Windows zip contains explicit directory entries (which real `node-vX.Y.Z-win-<arch>.zip` archives do), `extractEntryTo` for the top-level directory recurses over every descendant via `getEntryChildren(subfolders: true)`, writing every child file directly and bypassing the `ignoreEntry` filter. That re-materialized the `npm`, `npx`, and `corepack` files stripped in #11325. Skip directory entries in the loop and let file extraction create parent directories implicitly. Add a regression test that constructs a zip with explicit directory entries. Closes the regression on `installing/deps-installer/test/install/nodeRuntime.ts` observed on Windows after #11325. * docs: remove 'subfolders' cspell-flagged word from fix commit |
||
|
Zoltan Kochan
|
73c8482a9d |
fix(binary-fetcher): skip zip directory entries during Node.js runtime extraction (#11333)
* fix(binary-fetcher): skip zip directory entries during Node.js extraction When a Node.js Windows zip contains explicit directory entries (which real `node-vX.Y.Z-win-<arch>.zip` archives do), `extractEntryTo` for the top-level directory recurses over every descendant via `getEntryChildren(subfolders: true)`, writing every child file directly and bypassing the `ignoreEntry` filter. That re-materialized the `npm`, `npx`, and `corepack` files stripped in #11325. Skip directory entries in the loop and let file extraction create parent directories implicitly. Add a regression test that constructs a zip with explicit directory entries. Closes the regression on `installing/deps-installer/test/install/nodeRuntime.ts` observed on Windows after #11325. * docs: remove 'subfolders' cspell-flagged word from fix commit |
||
|
Zoltan Kochan
|
4d7cd56ccc |
chore: upgrade @typescript/native-preview to 7.0.0-dev.20260421.2 (#11332)
* chore: upgrade @typescript/native-preview to 7.0.0-dev.20260421.2
- Add explicit `types: ["node"]` to the shared tsconfig because tsgo
20260421 no longer auto-acquires `@types/*` from `node_modules`.
- Refactor test files to explicitly import jest globals (`describe`,
`it`, `test`, `expect`, `beforeEach`, etc.) from `@jest/globals`
instead of relying on `@types/jest` ambient declarations. Under the
new tsgo build, `import { jest } from '@jest/globals'` shadows the
ambient `jest` namespace, breaking `@types/jest`'s `declare var
describe: jest.Describe;` globals.
- Add `@jest/globals` to each package's devDependencies where tests
now import from it, and add `@types/node` to packages that need it
but were relying on hoisted resolution.
- Replace `fail()` calls with `throw new Error(...)` since `fail` is
no longer globally available.
* chore: fix remaining tsgo type-strictness errors
- Strip `as <PnpmType>` casts on objects passed to toMatchObject /
toStrictEqual / toEqual; @jest/globals rejects the typed objects
(which include AsymmetricMatchers) vs. the repo-specific type.
- Type `jest.fn<...>()` explicitly where the mock's signature matters
for toHaveBeenCalledWith.
- Replace `beforeEach(() => X)` with `beforeEach(() => { X })` so the
return value is void, as the stricter jest typing requires.
- Use `expect.objectContaining({...})` in one place where the full
expected object triggered stricter type resolution.
- Cast `prompt.mock.calls` arg through `as unknown as Record<...>[]`
for patch.test.ts's nested-array matchers.
- Fix off-by-one `<reference path>` in pnpm/test/getConfig.test.ts
that only surfaced now.
- Move `@jest/globals` from devDependencies to dependencies in the
two `__utils__` packages that import it from `src/`.
- Clean up unused imports from the @jest/globals migration.
* chore: address Copilot review on #11332
- Move misplaced `@jest/globals` imports to the top import block in
checkEngine, run.ts, and workspace/root-finder tests where the
script dropped them below executable code.
- Replace `try { await x(); throw new Error('should have thrown') } catch`
in bins/linker, lockfile/fs, and resolving/local-resolver tests with
`await expect(x()).rejects.toMatchObject({...})`. The old pattern
swallowed an unrelated `throw` if the under-test call silently
succeeded, which would fail on the catch-block assertion with a
misleading message.
|
||
|
Zoltan Kochan
|
187049055f |
chore: upgrade @typescript/native-preview to 7.0.0-dev.20260421.2 (#11332)
* chore: upgrade @typescript/native-preview to 7.0.0-dev.20260421.2
- Add explicit `types: ["node"]` to the shared tsconfig because tsgo
20260421 no longer auto-acquires `@types/*` from `node_modules`.
- Refactor test files to explicitly import jest globals (`describe`,
`it`, `test`, `expect`, `beforeEach`, etc.) from `@jest/globals`
instead of relying on `@types/jest` ambient declarations. Under the
new tsgo build, `import { jest } from '@jest/globals'` shadows the
ambient `jest` namespace, breaking `@types/jest`'s `declare var
describe: jest.Describe;` globals.
- Add `@jest/globals` to each package's devDependencies where tests
now import from it, and add `@types/node` to packages that need it
but were relying on hoisted resolution.
- Replace `fail()` calls with `throw new Error(...)` since `fail` is
no longer globally available.
* chore: fix remaining tsgo type-strictness errors
- Strip `as <PnpmType>` casts on objects passed to toMatchObject /
toStrictEqual / toEqual; @jest/globals rejects the typed objects
(which include AsymmetricMatchers) vs. the repo-specific type.
- Type `jest.fn<...>()` explicitly where the mock's signature matters
for toHaveBeenCalledWith.
- Replace `beforeEach(() => X)` with `beforeEach(() => { X })` so the
return value is void, as the stricter jest typing requires.
- Use `expect.objectContaining({...})` in one place where the full
expected object triggered stricter type resolution.
- Cast `prompt.mock.calls` arg through `as unknown as Record<...>[]`
for patch.test.ts's nested-array matchers.
- Fix off-by-one `<reference path>` in pnpm/test/getConfig.test.ts
that only surfaced now.
- Move `@jest/globals` from devDependencies to dependencies in the
two `__utils__` packages that import it from `src/`.
- Clean up unused imports from the @jest/globals migration.
* chore: address Copilot review on #11332
- Move misplaced `@jest/globals` imports to the top import block in
checkEngine, run.ts, and workspace/root-finder tests where the
script dropped them below executable code.
- Replace `try { await x(); throw new Error('should have thrown') } catch`
in bins/linker, lockfile/fs, and resolving/local-resolver tests with
`await expect(x()).rejects.toMatchObject({...})`. The old pattern
swallowed an unrelated `throw` if the under-test call silently
succeeded, which would fail on the catch-block assertion with a
misleading message.
|
||
|
btea
|
a50c5d23f9 |
chore: configure both packageManager and devEngines (#11326)
* chore: configure both `packageManager` and `devEngines` * chore: update pnpm to v11 rc 5 --------- Co-authored-by: Zoltan Kochan <z@kochan.io> |
||
|
Zoltan Kochan
|
08ae491b96 | chore(release): 11.0.0-rc.5 v11.0.0-rc.5 | ||
|
Zoltan Kochan
|
eb7e6ae8ab |
fix(exe): restore @pnpm/exe startup on Node.js v25.7+ (#11330)
## Summary `@pnpm/exe@11.0.0-rc.4` aborts on every invocation with: ``` node::sea::(anonymous namespace)::SeaDeserializer::Read() at ../src/node_sea.cc:174 Assertion failed: (format_value) <= (static_cast<uint8_t>(ModuleFormat::kModule)) ``` Two independent Node.js v25.7+ SEA regressions are responsible, both surfaced by the rc.4 bump of the embedded runtime from 25.6.1 to 25.9.0. This PR fixes both and adds a prepublish smoke test so a broken binary can't reach npm again. ## Root cause **1. SEA blob format changed in Node.js v25.7.0** ([nodejs/node#61813](https://github.com/nodejs/node/pull/61813) added ESM-entry-point support and inserted a new `ModuleFormat` byte into the blob header). SEA blobs carry no version marker, so a blob written by one Node.js version can only be deserialized by a matching one. In rc.4, the CI host Node.js (25.6.1, pre-change) wrote the blob and it was embedded in a 25.9.0 runtime (post-change) — the deserializer reads a misaligned byte as `format_value`, exceeds `kModule`, `CHECK_LE` fires, `SIGABRT`. `resolveBuilderBinary()` was preferring `process.execPath` whenever the running Node supported `--build-sea`, never checking that its version matched the embedded runtime. **2. Node.js v25.7+ replaces the ambient `require` and `import()` inside a CJS SEA entry with embedder hooks** that only resolve built-in module names. The `pnpm.cjs` shim loaded `dist/pnpm.mjs` via `await import(pathToFileURL(...).href)`, which after the fix to (1) reached the CJS entry and then blew up with: ``` ERR_UNKNOWN_BUILTIN_MODULE: No such built-in module: file:///.../dist/pnpm.mjs at loadBuiltinModuleForEmbedder at importModuleDynamicallyForEmbedder ``` ## Changes - **`releasing/commands/src/pack-app/packApp.ts`** — `resolveBuilderBinary` now takes the resolved target runtime version and only reuses `process.execPath` when `process.version` exactly matches; otherwise it downloads a host-arch Node of the target version via the existing `ensureNodeRuntime` path. Added `PACK_APP_RUNTIME_TOO_OLD` for runtimes older than v25.5 (no `--build-sea`). Removed the now-unused `DEFAULT_BUILDER_SPEC` and the stale `fetch`/`nodeDownloadMirrors` args on the builder resolver. Help text / examples refreshed to drop `node@22` / `node@lts` references that would now be rejected. - **`pnpm/pnpm.cjs`** — loads `dist/pnpm.mjs` through `Module.createRequire(process.execPath)` instead of `await import(fileURL)`. `createRequire` returns a regular CJS loader that bypasses the SEA embedder hooks, and the pnpm bundle has no top-level await so synchronous `require` of ESM (Node 22+) loads it cleanly. No build-time paths are baked in — `process.execPath` is evaluated at runtime, verified by relocation-testing the darwin-arm64 SEA under `/tmp/`. - **`pnpm/artifacts/verify-binary.mjs`** (new) + `prepublishOnly` on every platform artifact — replaces the existence-only `test -f pnpm` gate with: 1. A **relocation-sensitivity check**: run the binary without `dist/` staged and confirm the failure mentions a path derived from `process.execPath`, not a build-time constant. Catches any future regression of (2). 2. A **smoke test**: stage a `dist → ../exe/dist` symlink (using `symlink-dir` so Windows junctions are handled transparently), exec `./pnpm -v`, assert the output is a SemVer 2 string. - Cross-platform targets (darwin/win32 artifacts on a Linux CI, or a libc mismatch) skip the exec with a log line and fall back to existence-only, so a musl artifact published from a glibc host still goes through. - Real `dist/` dirs (developer layout) are preserved; stale symlinks from aborted runs are replaced; created symlinks are cleaned up on exit. - **`pnpm/artifacts/exe/test/setup.test.ts`** — new `pnpm -v` execution test gated on both the platform binary and the staged bundle being present, so ordinary `pn compile` test runs skip cleanly instead of failing on a missing `dist/`. |
||
|
Zoltan Kochan
|
fd437ded13 | chore(release): 11.0.0-rc.4 v11.0.0-rc.4 | ||
|
Zoltan Kochan
|
ba791a3a2f | test: skip flaky dlx tests on windows | ||
|
Zoltan Kochan
|
ef4ef7b72d |
fix(exe): restore legacy @pnpm/{macos,win,linux,linuxstatic}-{x64,arm64} names (#11327)
* fix(exe): restore legacy @pnpm/{macos,win,linux,linuxstatic}-{x64,arm64} package names
Reverts the published package names renamed in #11316 back to the legacy
scheme so `pnpm self-update` from v10 continues to resolve. v10's
self-updater looks up the platform child by its legacy name; the
scope-nested `@pnpm/exe.<platform>-<arch>[-musl]` rename broke that lookup.
Workspace directory layout (`pnpm/artifacts/<platform>-<arch>[-musl]/`)
and the GitHub release asset filenames (`pnpm-linux-x64-musl.tar.gz`,
`pnpm-darwin-*.tar.gz`, `pnpm-win32-*.zip`) stay on the new scheme.
`linkExePlatformBinary` now checks both legacy and future naming
schemes, so a later rename can ship without a v10-compatibility hazard.
* style: fix indentation in setup.test.ts
* refactor: extract legacyOsSegment into a switch helper
* refactor: defer platform detection until after exe dir check
* test: use familySync() in fixtures so musl hosts match implementation
Test fixtures were passing null for libcFamily while linkExePlatformBinary
and setup.js both use detect-libc at runtime. On musl Linux the fixtures
built linux-*/exe.linux-* while the implementation looked up
linuxstatic-*/exe.linux-*-musl. Also bump @pnpm/exe in the changeset.
|
||
|
Zoltan Kochan
|
421317c31a |
feat!: skip npm, npx, and corepack when installing node runtime (#11325)
## Summary - pnpm installing a Node.js runtime (`node@runtime:<ver>`, `pnpm env use`, `pnpm runtime set node`) no longer extracts the bundled `npm`, `npx`, and `corepack`. These make up ~2,800 of ~5,800 files in a typical Node.js archive, so skipping them materially reduces hashing, CAS writes, SQLite index inserts, and import/link work. - Users who still need `npm` can install it as a separate package. ## How A new optional `ignoreFilePattern` (regex source string, serializable across the worker boundary) threads through `FetchOptions` → `tarball-fetcher` → `@pnpm/worker` → `cafs.addFilesFromTarball`. `cafs.addFilesFromTarball` now accepts a per-call ignore on top of the existing cafs-level `ignoreFile`; the two are combined. `@pnpm/fetching.binary-fetcher` defines the Node-specific regex and applies it when `opts.pkg.name === 'node'`: - Tarball path: sets `ignoreFilePattern`. - Windows zip path: new `ignoreEntry?: RegExp` on `AssetInfo`; `extractZipToTarget` strips the `basename/` prefix and skips matching entries before `zip.extractEntryTo`. `@pnpm/engine.runtime.node-resolver`'s `getNodeBinsForCurrentOS` drops `npm`/`npx` so pnpm no longer creates shims for bins that no longer exist. ## Breaking change Shipping in v11. After this lands, `pnpm runtime set node` / `node@runtime:<version>` no longer puts `npm`, `npx`, or `corepack` on `$PATH`. Scripts that call them directly will need to install npm separately. |
||
|
Zoltan Kochan
|
db81c32e0f |
refactor: use pnpm pack-app for SEA artifacts; rename --node-version → --runtime (#11321)
## Summary - **@pnpm/exe build tooling**: Replace the 253-line hand-rolled SEA builder in `pnpm/artifacts/exe/scripts/build-artifacts.ts` with a thin ~60-line driver that delegates to the new `pnpm pack-app` command (shipped in #11312). Per-platform defaults (entry, outputDir, outputName, targets) move to `pnpm.app` in `pnpm/artifacts/exe/package.json`. - **pack-app CLI (breaking, unreleased)**: Rename `--node-version <v>` → `--runtime <name>@<v>`, and `pnpm.app.nodeVersion` → `pnpm.app.runtime`. Only `node@<v>` is accepted today; the `<name>@` prefix reserves room for `bun` / `deno` without a future breaking change. - **Embedded Node**: Pin `node@25.9.0` in the build-artifacts driver. ## Why the rename pack-app's old `--node-version` flag silently inherited from pnpm's global `node-version` rc setting (which controls which Node pnpm uses to run user scripts). That value leaked into `Config['nodeVersion']` and overrode `pnpm.app.nodeVersion`, causing the wrong Node build to be embedded in SEAs for users who had the rc key set — reproduced locally, where `linux-arm64-musl` failed entirely because Node 22.13.0 has no arm64-musl variant. `runtime` doesn't collide with any Config field, so CLI / `pnpm.app` / running-Node precedence now behaves as intended. ## Behavior parity with the old script - Same output paths: `pnpm/artifacts/<target>/pnpm` (or `pnpm.exe` for `win32-*`). - Same signing (pack-app uses `codesign` natively on macOS, `ldid` when cross-signing from Linux). - Same host-based target narrowing: Linux/M1 Mac build the full 8-target matrix; Intel Mac builds only `win32-x64`, `linux-x64`, `darwin-x64`. - Same `dist/` copy + `@reflink/*` prune into `pnpm/artifacts/exe/dist/` for npm publishing. - Same bundle-first step (`pn --filter=pnpm prepublishOnly &&` prefix is unchanged in the `build-artifacts` npm script). |
||
|
Zoltan Kochan
|
8fcf55cefe |
revert: "ci: update pnpm to v11 rc 3"
This reverts commit
|
||
|
Zoltan Kochan
|
bcf49c9cd9 | ci: don't continue release on error | ||
|
Zoltan Kochan
|
48c2d9c71c | ci: update pnpm to v11 rc 3 | ||
|
Zoltan Kochan
|
aa93759d9b |
chore(release): drop eslint from lib prepublishOnly (#11320)
Library packages had `prepublishOnly: pn compile`, which expands to `tsgo --build && pn lint --fix`. During `pn release` that runs eslint against ~150 packages for no benefit — the code has already been linted in CI and the release flow's upfront compile has already built dist/. Switch lib prepublishOnly to a bare `tsgo --build` so the safety-net compile stays but the per-package eslint cost is gone. |
||
|
Zoltan Kochan
|
21fa098e36 | chore(release): always build artifacts v11.0.0-rc.3 | ||
|
Zoltan Kochan
|
5cc5592c65 |
fix(exe): append .exe to win32-* artifacts after target rename
The rename in
|
||
|
Zoltan Kochan
|
fcdd50aaa7 | chore(release): 11.0.0-rc.3 | ||
|
Zoltan Kochan
|
54eec40233 | chore: use 0.0.x versions for @pnpm/agent.* packages | ||
|
Zoltan Kochan
|
e03e8f4d8d |
fix(directory-fetcher): respect absolute paths in resolution.directory (#11318)
`path.join(lockfileDir, resolution.directory)` mangles absolute cross-drive
Windows paths by literally concatenating them (`path.join('D:\\foo',
'C:\\bar')` → `'D:\\foo\\C:\\bar'`). Switch to `path.resolve` so stored
absolute paths are used as-is.
This surfaced as an ENOENT during `pnpm setup` in CI when `PNPM_HOME` and
the OS temp dir (containing the extracted v11 tarball that setup installs
via `pnpm add -g file:<dir>`) were on different drives.
|
||
|
Zoltan Kochan
|
5a293d250c |
refactor: rename @pnpm/exe platform packages to @pnpm/exe.<platform>-<arch>[-musl] (#11316)
* refactor: rename @pnpm/exe platform packages to @pnpm/exe.<platform>-<arch>[-musl] Aligns pnpm's own published platform artifacts with the one naming convention the rest of the codebase already uses (`process.platform` values plus an explicit `-musl` libc suffix), matching what `pnpm pack-app`, `pnpm add --os/--cpu/--libc`, `supportedArchitectures.os`, and Node.js tarball names all already settled on. Package renames: - @pnpm/linux-x64 -> @pnpm/exe.linux-x64 - @pnpm/linux-arm64 -> @pnpm/exe.linux-arm64 - @pnpm/linuxstatic-x64 -> @pnpm/exe.linux-x64-musl (new dir) - @pnpm/linuxstatic-arm64 -> @pnpm/exe.linux-arm64-musl - @pnpm/macos-x64 -> @pnpm/exe.darwin-x64 - @pnpm/macos-arm64 -> @pnpm/exe.darwin-arm64 - @pnpm/win-x64 -> @pnpm/exe.win32-x64 - @pnpm/win-arm64 -> @pnpm/exe.win32-arm64 GitHub release asset names follow suit (`pnpm-linuxstatic-x64.tar.gz` -> `pnpm-linux-x64-musl.tar.gz`, `pnpm-macos-*` -> `pnpm-darwin-*`, `pnpm-win-*` -> `pnpm-win32-*`). Internal artifact directories under `pnpm/artifacts/` renamed to match, which drops the awkward mixed naming between target and directory. The umbrella package `@pnpm/exe` keeps its name so that `pnpm self-update` from v10 and any `npm i -g @pnpm/exe` scripts continue to resolve. Platform children can be renamed freely because npm/pnpm filter optional deps by each child's `os`/`cpu`/`libc` manifest fields, not by package names. Also updates: - `@pnpm/exe`'s `setup.js` (preinstall) and the self-updater's `linkExePlatformBinary` to look up the platform package by the new scheme, using `detect-libc` to append `-musl` on musl Linux hosts. - `.meta-updater` optional-dependency list for @pnpm/exe. - `copy-artifacts.ts` target list and Windows detection prefix. - cspell wordlist (drops `linuxstatic`; it's no longer used anywhere). Final transition publishes of the old package names (pointing at the new ones so direct pins keep resolving) are a release-engineering step handled separately. Refs #11314. * chore: keep "linuxstatic" in cspell wordlist for changeset references * test(pack-app rename): cover the musl branch of platform-package-name lookup Copilot flagged that the musl -> -musl suffix logic in setup.js's preinstall and self-updater's linkExePlatformBinary had no regression coverage. Extract the name-computation from both into small pure helpers and unit-test all four matrix cases (linux+musl, linux+glibc, darwin, win32) plus the win32 ia32->x86 arch normalization: - pnpm/artifacts/exe/platform-pkg-name.js exposes `exePlatformPkgName` (returns `@pnpm/exe.<platform>-<arch>[-musl]`). setup.js imports it instead of inlining the logic; the new setup.test.ts block covers the four-case matrix without having to mock detect-libc or patch process.platform. - engine/pm/commands/src/self-updater/installPnpm.ts exports a new `exePlatformPkgDirName` returning `exe.<platform>-<arch>[-musl]` (the scope-local dir). linkExePlatformBinary calls it; the new selfUpdate.test.ts block covers the same matrix. Both helpers are deliberately pure so the non-musl CI host can still exercise the musl code path. |
||
|
Zoltan Kochan
|
72c1e050e9 |
feat: add pnpm pack-app command for packing CJS entries into standalone executables (#11312)
* fix: give each runtime variant its own global virtual store entry
When a runtime package (e.g. node@runtime:X.Y.Z) uses a variations
resolution, createFullPkgId() in @pnpm/deps.graph-hasher was hashing
the whole VariationsResolution — the same hash on every host — so the
global virtual store path collided between variants. Whichever variant
installed first won, and a later `pnpm add --libc=musl node@runtime:<v>`
silently reused the cached glibc (or macOS/Windows) binary.
The fix threads supportedArchitectures down to createFullPkgId so the
selected variant's integrity is used as the package fingerprint. Two
related cleanups land with it:
- Extract the platform-variant selection logic to @pnpm/resolving.resolver-base
as selectPlatformVariant/resolvePlatformSelector. The helper's libc
match also required a fix: a variant with no libc is the "default"
build, and a request for a non-default libc (e.g. musl) must require
an exact match so the default variant doesn't silently win.
- @pnpm/installing.package-requester's findResolution now delegates to
the shared helper, and the new supportedArchitectures param is plumbed
through calcDepState / calcGraphNodeHash / iterateHashedGraphNodes /
lockfileToDepGraph and their callers in deps-resolver, deps-restorer,
deps-installer, graph-builder, and building.after-install.
* feat: add pnpm build-sea command for building Node.js SEA executables
Adds `pnpm build-sea` under @pnpm/releasing.commands. Takes a CommonJS
entry file and a set of target triplets (linux-x64, linux-x64-musl,
linux-arm64, linux-arm64-musl, macos-x64, macos-arm64, win-x64,
win-arm64) and produces a standalone executable per target under
dist-sea/<target>/.
Each target's Node.js runtime is fetched via `pnpm add node@runtime:<v>
--os=<os> --cpu=<arch> --libc=<libc>` into $PNPM_HOME/build-sea/<target>-<v>/
so binaries are hardlinked from the global content-addressable store and
`pnpm store prune` can reclaim them.
Requires Node.js v25.5+ to perform the --build-sea injection. If the
running Node is older, a v25 binary is downloaded and used as the builder
automatically. macOS outputs are ad-hoc signed with codesign (on macOS)
or ldid (when cross-compiling from Linux), which is required because SEA
injection invalidates the binary's existing signature.
* fix(build-sea): reject malformed --target, --output-name and use mkdtemp for config
Addresses Copilot review feedback on the build-sea command:
- parseTarget() previously destructured the target string, silently
accepting extra `-` segments. Inputs like `linux-x64-musl-../../outside`
would pass validation and flow into path.join. Validation is now done
with a strict anchored regex.
- --output-name was passed into path.join() without sanitization, so a
caller could escape the output directory with path separators or `..`.
validateOutputName() now rejects anything that isn't a plain basename.
- The per-target SEA config file was written to a predictable path under
os.tmpdir() (derived from the target name and Date.now()), which is
unsafe on multi-user systems. It now lives inside a fresh mkdtemp()
directory and is opened with the exclusive "wx" flag.
- New test cases cover extra-segment targets, uppercase/whitespace
variants, and the full matrix of invalid --output-name inputs.
* rename: build-sea → pack-app
`build-sea` required knowing what a SEA is. `pack-app` is self-describing,
doesn't collide with pnpm's existing `bin` concept, and parallels the
existing `pack` command.
- Command name: build-sea → pack-app
- Default output dir: dist-sea → dist-app
- Error codes: PACK_APP_* (was BUILD_SEA_*)
- Export/type: packApp / PackAppOptions (was buildSea / BuildSeaOptions)
- Install cache dir: $PNPM_HOME/pack-app (was $PNPM_HOME/build-sea)
The Node.js `--build-sea` flag name itself is unchanged — that's a
Node.js feature and outside this project's naming.
* fix(pack-app): reject directory entries, pin builder to >=25.5, refuse macOS target on Windows
Addresses Copilot review feedback on the pack-app command:
- entry validation now rejects non-file paths (directories, symlinks to
non-files) with a dedicated PACK_APP_ENTRY_NOT_FILE instead of
surfacing a less actionable error later in the SEA build.
- DEFAULT_BUILDER_SPEC was the bare major ("25"), which would satisfy
with 25.0.x if that version is still present — those point releases
predate --build-sea support. Tightened to ">=25.5.0 <26.0.0" so the
download is guaranteed to support the flag without ever crossing a
major.
- adHocSignMacBinary() silently skipped re-signing on Windows hosts.
Now throws PACK_APP_MACOS_SIGN_UNSUPPORTED_HOST with a hint to build
the target on macOS/Linux or re-sign manually.
- resolvePlatformSelector() JSDoc now matches what the code actually
does (picks the first entry when it is not "current"; later entries
are ignored).
- New test case covers the directory-as-entry rejection.
* refactor(pack-app): switch target OS names to process.platform constants
Previously `pack-app` accepted `macos-*` / `win-*` as the OS portion of a
target triplet and translated them to `darwin` / `win32` internally. The
translation layer made the CLI surface inconsistent with the values that
`pnpm add --os=…` and `supportedArchitectures.os` already use, and added
a small footgun (e.g. users setting `supportedArchitectures: { os: [darwin] }`
but typing `macos-arm64` for pack-app).
The supported target OS set is now `linux | darwin | win32`, matching
`process.platform`. Old inputs like `macos-arm64` or `win-x64` now fail
validation with a clear error pointing to the new naming. The internal
parseTarget helper drops its TARGET_OS_MAP lookup entirely.
This is a change to an unreleased command so there is no back-compat
concern. pnpm's own artifact directory names (`pnpm/artifacts/macos-*/`,
`pnpm/artifacts/win-*/`) are an internal implementation detail and are
not affected by this change.
* feat(pack-app): read defaults from pnpm.app in package.json
Every pack-app flag (--entry, --target, --node-version, --output-dir,
--output-name) can now be preconfigured in the project's package.json
under a new "pnpm.app" object:
{
"name": "my-cli",
"pnpm": {
"app": {
"entry": "dist/index.cjs",
"targets": ["linux-x64", "darwin-arm64", "win32-x64"],
"nodeVersion": "25",
"outputDir": "release",
"outputName": "my-cli"
}
}
}
CLI flags always win. --target replaces the configured list rather than
appending, so a user can narrow the default set at the command line.
The config loader is strict: unknown keys under pnpm.app and any
type-mismatched values throw PACK_APP_INVALID_CONFIG so mistakes surface
at invocation time instead of silently being ignored.
Chose pnpm.app over pnpm.packApp because it's the shorter, cleaner
namespace for anything related to the app bundle (future sibling
commands like run-app / deploy-app could share the same object without
a naming clash). Chose package.json over pnpm-workspace.yaml because
the config is inherently per-project, whereas pnpm-workspace.yaml is
workspace-root-only.
* fix(pack-app): deterministic libc selection and stricter output-name validation
Addresses Copilot review feedback:
- ensureNodeRuntime() now always passes an explicit --libc for linux
targets. Without a suffix, linux-x64 and linux-arm64 default to
--libc=glibc instead of letting the user's supportedArchitectures.libc
config or the host's detected libc decide the variant. The install
cache directory mirrors this, so glibc and musl variants are always
distinct (linux-x64-glibc vs linux-x64-musl).
- resolveBuilderBinary() now pins the host libc when downloading a
builder Node on Linux. A user whose config sets supportedArchitectures.libc
to musl no longer ends up with a musl Node that the glibc host cannot
execute.
- validateOutputName() rejects Windows-invalid filename characters
(<>:"|?* and NUL), Windows reserved device names (CON, NUL, COM1, etc.),
and names ending in a dot or space — problems surface at invocation
time rather than during writeFile(outputFile, ...) on Windows.
- lockfileToDepGraph variants tests no longer derive the "host"
variant from process.platform/process.arch; they always pass an
explicit supportedArchitectures selector so the expectations hold on
any CI host (including Alpine/musl).
* chore: add "toctou" to cspell wordlist
`TOCTOU` (time-of-check-to-time-of-use) is the standard term for the
race-condition class the pack-app SEA-config comment describes. Adding
it to the wordlist unblocks the Lint CI step.
* fix: lint
|
||
|
Allan Kimmer Jensen
|
bcc88a1239 |
fix(sbom): resolve licenses for git-sourced dependencies (#11310)
* fix(sbom): resolve licenses for git-sourced dependencies `readPackageFileMap` did not handle `type: 'git'` resolutions, causing `pnpm sbom` to emit NOASSERTION and `pnpm licenses` to throw for any dependency installed from a git URL. Closes #11260 * fix: add missing store.cafs devDep, test tsconfigs, and size field - Add @pnpm/store.cafs devDependency and tsconfig reference to license-scanner so CI typecheck resolves the PackageFilesIndex import - Add test/tsconfig.json to pkg-finder so CI typechecks the new tests - Add required `size` field to PackageFileInfo test fixtures * fix: replace spellcheck-failing test strings * fix: use spellcheck-safe integrity string in test * style: fix import sort in pkg-finder test * fix(sbom): use packageIdFromSnapshot to match store index keys The SBOM used `snapshot.id ?? depPath` as the package ID, which includes the package name prefix (e.g. `left-pad@git+https://...`). The store index stores git packages under just the git URL without the name prefix. Use `packageIdFromSnapshot` which strips the prefix, matching how the licenses command already does it. Also fixes test store keys to match the real installer layout so the mismatch would have been caught by tests. * refactor: move git resolution check after tarball check Tarball resolutions are more common than type: 'git', so check them first. Per review feedback from @zkochan. |
||
|
Zoltan Kochan
|
ff22872cf0 |
test: replace deleted sveltejs/action-deploy-docs fixture with pnpm-e2e/drupal-js-build (#11315)
The sveltejs/action-deploy-docs repository was deleted from GitHub, causing CI failures in tests that fetched it. Replaces the reference with pnpm-e2e/drupal-js-build, which is owned by the pnpm org and is large enough to cover the regression from #4064. |
||
|
Zoltan Kochan
|
ccc606ed15 |
feat: pnpm agent — server-side resolution for faster installs (#11251)
## Summary
Adds an opt-in **pnpm agent** server that resolves dependencies server-side and streams only the files missing from the client's content-addressable store.
- **`@pnpm/agent.server`** — multi-process HTTP server (Node.js `cluster`) with SQLite-backed metadata and file caches
- **`@pnpm/agent.client`** — streams an NDJSON response, dispatches worker threads to fetch files while the server is still resolving
- **New config**: `agent` in `pnpm-workspace.yaml` (opt-in)
## How it works
1. Client reads integrity hashes from its local store index
2. Sends `POST /v1/install` with dependencies + store integrities
3. Server resolves the dependency tree using pnpm's `install({ lockfileOnly: true })`, with a SQLite-backed `PackageMetaCache` for fast repeat resolution
4. As each package resolves, a wrapped `storeController.requestPackage` looks up its files and immediately streams digests the client is missing (NDJSON `D` lines)
5. Client reads the stream line by line; digest batches fill up and dispatch worker threads to `POST /v1/files` — file downloads overlap with server-side resolution
6. After resolution, server sends index entries (`I` lines) and lockfile (`L` line)
7. Client writes index entries to store, then runs headless install with a wrapped `fetchPackage` that calls `readPkgFromCafs` with `verifyStoreIntegrity: false` (files are trusted from the agent)
8. `/v1/files` response is gzip-streamed (274MB → ~80MB) — server pipes through `createGzip`, worker pipes through `createGunzip`, parsing and writing files to CAFS as data arrives
## Performance
1351-package project, cold local store, warm server (localhost):
| Scenario | Time |
|----------|------|
| Vanilla pnpm install (cold OS cache) | ~48s |
| Vanilla pnpm install (warm OS cache) | ~34s |
| With pnpm agent (consistent) | **~33s** |
### Key optimizations
1. **SQLite metadata cache** — server-side resolution drops from ~3.4s to ~0.9s
2. **SQLite file store** — consistent read performance regardless of OS file cache state
3. **Streaming `/v1/install`** — file digests stream during resolution, downloads start before resolution finishes
4. **Gzip-streamed `/v1/files`** — whole-stream gzip (274MB → ~80MB), significant savings on remote servers
5. **Worker-thread streaming HTTP** — workers pipe gzip → parse → write to CAFS as data arrives, no buffering
6. **No rehashing** — server-provided digests used directly, skipping 33K SHA-512 computations
7. **No re-verification** — wrapped `fetchPackage` calls `readPkgFromCafs` with `verifyStoreIntegrity: false`
8. **Direct `writeFileSync` with `wx`** — no stat + temp + rename
9. **Pre-packed msgpack** — server sends raw store index buffers, client writes directly to SQLite
10. **WAL checkpoint** — ensures store index entries written by agent are visible to headless install's worker threads
## Usage
Start the server:
```bash
node agent/server/lib/bin.js
```
Configure in `pnpm-workspace.yaml`:
```yaml
agent: http://localhost:4873
```
|
||
|
Allan Kimmer Jensen
|
61952c24ec |
fix(sbom): detect legacy licenses array and LICENSE files (#11255)
- `pnpm sbom` now recognizes the deprecated `licenses` array and falls back to scanning on-disk `LICENSE` files, matching the resolution already used by `pnpm licenses`. Fixes packages like `busboy`, `streamsearch`, `limiter` being reported as `NOASSERTION`. - Root component scans the project directory for LICENSE files too, matching transitive-dep behavior. - Shared license resolution lives in a new `@pnpm/deps.compliance.license-resolver` package, with `parseLicenseFromManifest`, `resolveLicense` (store-file-map), and `resolveLicenseFromDir` (on-disk scan). - LICENSE-file-detected names are filtered against `SPDX_LICENSE_IDS` before being emitted to SBOM — long-form names like "Eclipse Public License 1.0" fall through to `NOASSERTION` rather than producing non-compliant SPDX. Manifest-declared licenses pass through untouched. - When both `license` and `licenses` are set, the modern `license` wins (previously `pnpm licenses` preferred `licenses`). - Close #11248. --------- Co-authored-by: Zoltan Kochan <z@kochan.io> |
||
|
btea
|
390ee62577 |
feat(audit): add interactive mode to pnpm audit --fix and respect auditLevel (#11233)
close #11163 --------- Co-authored-by: Zoltan Kochan <z@kochan.io> |
||
|
Zoltan Kochan
|
7d25bc1136 |
fix: suppress packageManager/devEngines.packageManager conflict warning when values match exactly (#11307)
- Suppress the `Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored` warning only when both fields specify the exact same package manager name and the exact same version string. Any other divergence (different name, range vs. exact version, prefixed versions like `v1.2.3`, etc.) still warns. - Lets projects keep both fields during migration (e.g. so v10 installs still auto-switch via `packageManager`, while v11 uses `devEngines.packageManager` and `npm install` still errors) without a noisy warning — as long as the two values are kept in sync. Closes #11301 |
||
|
Zoltan Kochan
|
53668a42b8 |
fix: support explicit versions and --no-commit-hooks / --no-git-tag-version in pnpm version (#11299)
* fix: support explicit versions and --no-commit-hooks / --no-git-tag-version in `pnpm version` Registers options under their canonical names so nopt correctly parses the `--no-*` variants, accepts an explicit semver argument alongside bump types, and creates a git commit + annotated tag for the bump (honoring `--no-git-tag-version`, `--no-commit-hooks`, `--sign-git-tag`, `--message`, and `--tag-version-prefix`). Also fixes `--no-git-checks` which was parsed incorrectly. Closes #11271. * chore: add gpgsign and newversion to cspell; set gpg sign config in version test * fix: address Copilot review feedback on pnpm version * fix: skip git commit and tag in recursive version runs * fix: address Copilot review feedback on pnpm version |
||
|
Zoltan Kochan
|
9ae1ca7253 |
feat: publish base docker image to GHCR (#11302)
* feat: publish base docker image to GHCR Adds a Dockerfile (debian:stable-slim + pnpm standalone binary) and a release-triggered workflow that builds multi-arch images and pushes to ghcr.io/pnpm/pnpm. Users who need Node.js can install it inside the container via `pnpm runtime set node <version>`. Refs #11300 * docs: add docker/README.md * chore(cspell): add buildx to dictionary * docs: mention devEngines.runtime as alternative to pnpm runtime set * fix(docker): pin base image, verify tarball sha256, harden download - Pin `debian:stable-slim` to a digest for reproducibility. - Compute pnpm tarball SHA256 in the workflow and verify it inside the build, detecting tampered artifacts regardless of what `pnpm --version` reports. - Download the tarball to disk with `--retry` instead of `curl | tar` for resilience under multi-arch QEMU builds. - README: use `--load` so the local test image is available to `docker run`. * chore(cspell): sort dictionary additions * fix(docker): address Copilot review feedback - Include $PNPM_HOME/bin on PATH so pnpm-installed globals (node, etc.) are discoverable, and make $PNPM_HOME writable for non-root users. - Document that `pnpm runtime set node` needs `-g` to install globally. - Pass workflow inputs via env: instead of inlining GitHub expressions into shell, and validate the version string before use. * fix(docker): install libatomic1 for pnpm standalone binary The pnpm linux standalone binary dynamically links against libatomic.so.1, which is not present in debian:stable-slim by default. Without it, `pnpm --version` fails during the build with: pnpm: error while loading shared libraries: libatomic.so.1: cannot open shared object file: No such file or directory Caught by local build testing. |
||
|
Zoltan Kochan
|
c86c423bdc |
fix: preserve pnpm 10 peer suffix encoding for linked paths (#11297)
* fix: preserve pnpm 10 peer suffix encoding for linked paths The filenamify upgrade from v4 to v7 changed the peer-suffix "version" token for linked dependency paths: `../packages/b` became `+packages+b` instead of `packages+b`, causing lockfile churn for workspaces with packages linked from outside the workspace root. Replace the filenamify call with a small inline encoder that reproduces v4's output for link paths, and drop the now-unused dependency. Closes #11272. * chore: avoid cspell-flagged word in peer suffix comment * test: cover linkPathToPeerVersion and clarify its lossy encoding Address Copilot review feedback on #11297: - Correct the comment — any leading run of `.` is dropped, not just `./` and `../` segments (so `.hidden/pkg` becomes `hidden+pkg`). - Export the helper and add a focused test that pins the exact token output for link paths, so future lockfile-breaking regressions get caught by the test suite. * refactor: extract linkPathToPeerVersion into its own file |
||
|
Zoltan Kochan
|
7d9aae9662 |
fix: prevent frozen-lockfile error when approving builds in global install (#11294)
In --global mode, globalAdd passes workspaceDir to approve-builds so it can update the global pnpm-workspace.yaml. approve-builds then forwarded that workspaceDir into install.handler, which (with workspacePackagePatterns undefined) recursively discovered sibling install dirs as workspace projects and failed the frozen-lockfile check on stale @pnpm/exe install dirs. |
||
|
Zoltan Kochan
|
9e0833c3cc |
feat: add minimumReleaseAgeIgnoreMissingTime setting (#11293)
Skips the minimumReleaseAge maturity check when the registry metadata lacks the "time" field, instead of throwing ERR_PNPM_MISSING_TIME. Defaults to true, and prints a warning once per affected package. |