mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-25 23:17:08 -04:00
fcdfcff331e3823d7cc1a95ad91bdccbbcc0d32f
344 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Zoltan Kochan
|
fcdfcff331 | chore: add build script to package.json (#275) | ||
|
Zoltan Kochan
|
a9018643a0 |
refactor(tarball): extract per-entry loop; propagate tar errors (#272)
* refactor(tarball): extract per-entry loop; propagate tar errors Closes #270. The per-entry loop inside `DownloadTarballToStore::run_without_mem_cache` was a pile of `.unwrap()` / `.expect()` calls that turned any tar-side failure — corrupt header, short body read, path decode, entries-iterator error — into a panic on a blocking-pool thread. A malformed or truncated tarball from the registry should surface as a `TarballError`, not unwind the install. Move the loop into a free-standing `extract_tarball_entries` function (also addresses the pre-existing `TODO: move tarball extraction to its own function`) and rewrite every `.unwrap()` / `.expect()` inside it to propagate via `TarballError::ReadTarballEntries`: * `.filter(|entry| !entry.as_ref().unwrap()...)` → match Ok/Err; Err entries fall through so the `?` inside the loop propagates them. * `entry.unwrap()` → `entry.map_err(TarballError::ReadTarballEntries)?`. * `entry.header().mode().expect("get mode")` → same propagation pattern. * `entry.read_to_end(...).unwrap()` → same. * `entry.path().unwrap()` → same. * `.into_string().expect("entry path must be valid UTF-8")` → `.to_string_lossy().into_owned()`. Lossy coercion matches pnpm's string-based path handling (Node.js's string layer does the same), so a mixed install against the shared `index.db` stays consistent. Add a unit test that feeds 1 KiB of 0xFF (not a valid tar archive — header checksum at bytes 148..156 can't possibly validate) and asserts `extract_tarball_entries` returns `TarballError::ReadTarballEntries` rather than panicking. That's the acceptance case called out on #270. * fix(tarball): cap entry pre-alloc; use fully-qualified doc link Two Copilot review nits on #272: * `Vec::with_capacity(entry.size() as usize)` trusted the tar-header size — a corrupt or malicious tarball claiming gigabytes could turn that into an OOM abort before `read_to_end` had a chance to surface the real error. Clamp the pre-allocation hint to 64 MiB (generous for any legitimate single-file entry in an npm tarball) and use `try_reserve` so an allocation failure comes back as an I/O error instead of aborting. Content beyond the clamp is still read via `Vec`'s geometric growth. * `[Path::to_string_lossy]` in the doc comment didn't have `Path` in scope (only `PathBuf` is imported), so `cargo doc -D warnings` could reject it. Use the fully qualified `[std::path::Path::to_string_lossy]`. * fix(tarball): reject directory-traversal paths; tighten filter to is_file Two Copilot review nits on #272: * `cleaned_entry_path` derives directly from tar entry path components and gets joined onto the CAFS extraction root by `create_cas_files` later. Without validation a hostile tarball could carry `..`, absolute roots, or Windows prefix components that land files outside the store. Validate every component (after dropping the top-level package directory) is `Component::Normal` — reject loudly as `TarballError::ReadTarballEntries(InvalidData)` instead of normalizing silently. Also reject the empty-cleaned-path case. Added a unit test that bypasses `tar::Header::set_path`'s own `..` validation via `as_mut_bytes()` + checksum recompute, the same shape a non-Rust tool could trivially produce. * The per-entry filter was `!entry_type().is_dir()` which kept symlinks, hardlinks, PAX / GNU extension headers, and device files — the doc comment claimed "regular-file entry", which didn't match. Tighten the filter to `entry_type().is_file()`. Silently reading a symlink's 0-byte body into the CAFS as if it were a file would just corrupt the store; npm-publish tarballs only carry regular files in practice. Updated the docstring to call out the filter explicitly. |
||
|
Zoltan Kochan
|
348795fc7a |
fix(tarball): run post-download pipeline on blocking pool (#269)
* fix(tarball): run post-download pipeline on blocking pool
The `frozen-lockfile` benchmark job has been flaky on CI with the 10-minute
step timeout firing during `Benchmark 1: pacquet@HEAD`. The logs show all
downloads completing from the network's point of view but ~1115 tarballs
stuck between "Download completed" and "Checksum verified", then ~3m46s
of total tracing silence before the runner kills the process.
Cause: the post-download work (SHA-512 of the full tarball, gzip inflate,
per-file SHA-512, `write_cas_file`, SQLite index INSERT) ran inside
`tokio::task::spawn(async move { ... })`. The body contains no `.await`
until the final nested `spawn_blocking` for the SQLite write, so each
tarball pins a tokio reactor worker for its whole duration. On a 2-core
GHA runner that caps throughput at ~2 tarballs in flight, and the tail
of large packages (100-200 KB compressed, MB+ inflated, dozens of files
each) overruns the CI step budget.
Swap the outer `spawn` to `spawn_blocking` so the work runs on tokio's
blocking pool (default 512 threads) instead. The nested `spawn_blocking`
around the SQLite write is no longer useful once we're already on the
blocking pool, so inline it.
* style(tarball): apply rustfmt
* fix(tarball): cap post-download concurrency; propagate JoinError
Two changes addressing Copilot review comments on #269:
* Gate the post-download `spawn_blocking` body with a global
`Semaphore(num_cpus * 2, floor 4)`. The body is CPU-bound (SHA-512
over the whole tarball + per-file SHA-512 + gzip inflate) with some
blocking FS I/O, and the default 512-thread blocking pool let
`try_join_all` fan out hundreds of these at once — good for I/O
wait, disastrous for CPU work on a 2-core runner. The permit is
held across the `spawn_blocking.await` so no task enters the body
until one is free.
* Replace `.expect(\"tarball-processing blocking task panicked\")` on
the spawn_blocking `JoinHandle` with
`.map_err(TarballError::TaskJoin)?`. Runtime cancellation or a
blocking-task panic now surfaces as an error rather than unwinding
the whole install. `TarballError::TaskJoin` already exists for this.
* fix(tarball): acquire post-download permit before buffering body
Addresses Copilot review on #269: `post_download_semaphore().acquire()`
used to run *after* `.bytes().await`, so under `try_join_all` fan-out a
fast registry could leave hundreds of fully-buffered tarball bodies
sitting in RAM waiting for a permit to process — a significant peak-RSS
risk on large installs.
Split the HTTP call into `send()` (headers) and `bytes()` (body) and
acquire the permit between them. The permit now covers both buffering
and processing, so the number of concurrently-buffered bodies is
bounded by the post-download cap (`num_cpus * 2`, floor 4). HTTP
headers-only concurrency is still gated separately by `ThrottledClient`
so the cheap head phase can still fan out.
|
||
|
Zoltan Kochan
|
e19163c07d |
refactor(store-dir): batch store-index writes via a single writer task (#265)
* refactor(store-dir): batch store-index writes via a single writer task Port pnpm v11's `queueWrites` / `flush` / `setRawMany` pattern from `store/index/src/index.ts` so the per-tarball store-index INSERT stops opening a fresh SQLite connection and pulling its own blocking-pool thread. * Add `StoreIndex::set_many`, which wraps a batch in `BEGIN IMMEDIATE; INSERT OR REPLACE x N; COMMIT;`. One WAL fsync amortizes across the batch, with a rollback on per-batch error so a partial apply never leaves the index in a half-written state. Encoding happens on the caller's thread before the transaction begins, so the writer only pays SQLite work. * Introduce `StoreIndexWriter`, an `Arc`-cloneable handle around a tokio unbounded channel. Producers call `queue(key, value)` — no SQLite, no `spawn_blocking`, no per-row lock. A single blocking task drains the channel, collects each non-blocking burst into a batch (cap 256), and flushes via `set_many`. * `CreateVirtualStore` and `InstallWithoutLockfile` spawn the writer at the top of the run, thread the `Arc<StoreIndexWriter>` through `InstallPackageBySnapshot` / `InstallPackageFromRegistry` / `DownloadTarballToStore`, drop the orchestration's clone after `try_join_all`, then `await` the writer task so the final batch flushes before the install returns. * `DownloadTarballToStore::run_without_mem_cache` drops its per-tarball `spawn_blocking` for the index write entirely. Batch errors, `JoinError`s, and writer-open failures are all downgraded to `warn!` and "skip the row", matching the read side's graceful-degradation stance from #261. Tests pass against the existing `pacquet-registry-mock` harness. Context: opened while investigating #263. On macOS I haven't been able to demonstrate a wall-time win from this change yet — the measurable bottleneck in pacquet's cold-install sys time sits elsewhere (most likely tarball extraction / `write_cas_file` and sha512 per file), see the comment thread on #263 for the investigation notes and sample profile. The change still stands as structural cleanup and the pnpm-model alignment, and it will compound with whatever fix the real bottleneck gets. * refactor(store-dir): skip per-row encode failures in `set_many` `collect::<Result<_, _>>()?` turned a single `encode_package_files_index` failure into "drop the whole batch" — up to 256 otherwise-valid rows lost because of one malformed `PackageFilesIndex`. That's stricter than the "best-effort index" contract the writer task and the read-side already follow. Encode in a loop, log the failing key at `warn!`, and keep the rest of the batch alive for commit. SQLite errors inside the transaction still roll back the whole batch (that's how atomicity is supposed to work); only the per-row encoding step is skipped. Addresses Copilot review on #265. * refactor(store-dir,tarball): reword #263 ref; quiet `queue` warn on first failure; fix private-doc-link Three follow-up nits from the Copilot review on #265: * The comment in `tarball/src/lib.rs` described #263 as "fixed" — it isn't, this PR is one step toward fixing it. Reworded to "the per-write blocking thread churn described in #263" so the citation survives the landing of both. * `StoreIndexWriter::queue` logged a `warn!` on every `send` failure. When the writer task exits early (open failure, panic) every subsequent `queue` call fails — on a 1352-snapshot install that's a thousand identical warnings drowning everything else out. Add a `warn_on_send_failure: AtomicBool` one-shot guard: first failure logs (with "further failures silenced" hint), subsequent failures go silent. The observable signal the user actually cares about — "this install's rows aren't in the index, the next install will re-download" — surfaces on the next run regardless. * `StoreIndexWriter`'s doc block linked to the private constant `MAX_BATCH_SIZE`. `RUSTDOCFLAGS=-D warnings` in CI rejects the private-intra-doc-link lint, so replace the link with a literal "256 entries" plus a `see MAX_BATCH_SIZE` pointer in prose. Addresses Copilot review on #265. |
||
|
Zoltan Kochan
|
f33481ec83 |
docs: add AGENTS.md with upstream-parity guidance (#268)
* docs: add AGENTS.md with upstream-parity guidance Documents the cardinal rule (mirror pnpm/pnpm main), build/test/lint commands, test targeting, code-reuse expectations, and Conventional Commits conventions for AI coding agents working in this repo. * docs: symlink CLAUDE.md to AGENTS.md So Claude Code picks up the same guidance automatically without duplicating the content. * docs(agents): fix fixture path and relax "just only" guidance - Shared test fixtures live under crates/testing-utils/src/fixtures/, not under __fixtures__ directories (that's a pnpm-ism). - Reword the commands section to allow dropping to cargo/taplo directly when the just recipe doesn't expose the flag you need, matching the later narrow-test guidance that uses `cargo nextest -p <crate>`. |
||
|
Zoltan Kochan
|
4fa08d5495 |
bench(integrated-benchmark): level cold-install comparison on macOS and across platforms (#264)
* bench(integrated-benchmark): pin storeDir in pnpm-workspace.yaml The fixture's `.npmrc` set `store-dir=<bench-dir>/store-dir`, but pacquet reads `.npmrc` for auth/registry only — project-structural settings like `storeDir` must come from `pnpm-workspace.yaml` (see `crates/npmrc/src/lib.rs:173-178`, matching pnpm 11). With no `storeDir` in the yaml the benchmark silently fell through to pacquet's default (`~/Library/pnpm/store` on macOS, `~/.local/share/pnpm/store` on Linux), so `hyperfine --prepare`'s `rm -rf <bench-dir>/store-dir` never actually wiped the store the install read from. On a developer machine whose default store already holds the fixture's 1352 snapshots — any prior `pnpm install` of a similar project — every bench iteration hit the warm-store cache-lookup path instead of the cold-install path. The measured wall time was dominated by the per-file `symlink_metadata` pass of #260, not by the fetch+extract+write pipeline the benchmark was designed to stress. Adding `storeDir: ./store-dir` makes the bench honour the prepared per-revision store. On CI this is a no-op (the default `~/.local/share/pnpm/store` is empty anyway on a fresh runner) — the fix prevents developer-local runs from producing meaningless numbers. * bench(integrated-benchmark): silence pnpm fsevents build warning on macOS `fsevents` is a macOS-only optional dependency of `chokidar` (via several fixture entries). On Linux CI it isn't installed and pnpm never complains; on a developer macOS machine it installs and pnpm emits `ERR_PNPM_IGNORED_BUILDS: fsevents@1.2.13` because the fixture's `.npmrc` sets `ignore-scripts=true`. That error exits pnpm with status 1, which hyperfine flags via the benchmark harness's exit-code check and takes the whole `just integrated-benchmark` run down on the first warmup. Adding `fsevents: false` to `allowBuilds` silences the warning specifically for this package without re-enabling script execution elsewhere. Matches what pnpm emits when you run `pnpm approve-builds` and decline fsevents. * bench(integrated-benchmark): force pnpm to install cross-platform optionals Pacquet doesn't filter optional dependencies by `os` / `cpu` / `libc` at the moment — `create_virtual_store` iterates every snapshot in the lockfile unconditionally, so `fsevents` (and every other platform-specific optional) ends up in the install payload regardless of runner. pnpm does filter, so on Linux CI it skips `fsevents` and friends entirely. That's a real payload difference: pnpm's install does strictly less tarball fetching / extraction / CAS writing than pacquet's on the same lockfile. Small in absolute terms for this fixture, but a hidden handicap in a benchmark whose whole point is to measure the fetch+extract+write pipeline. Set `supportedArchitectures` to cover every OS / CPU / libc combo we care about so pnpm pulls cross-platform optionals on every runner, matching pacquet's payload. The fix belongs on the pnpm side of the fixture because pacquet's filter-by-platform is a future code change with larger blast radius; bringing the two tools to the same install set in one-line config here is the lower-risk intermediate. |
||
|
Zoltan Kochan
|
968bfc7270 |
fix(network): set explicit timeouts on default reqwest client; enable CI tracing (#267)
* fix(network): set explicit timeouts on the default reqwest client `ThrottledClient::new_from_cpu_count` handed back `Client::new()`, which has no connect / request / pool-idle timeouts at all. The existing docstring on the sibling `from_client` already called this out as a footgun — that note stays relevant for tests, but the production path can't afford "wait forever". On CI this is how `integrated-benchmark` hangs at "Benchmark 1: pacquet@HEAD" until the 10-min GHA step cap kicks in: the bench's single-process verdaccio occasionally stalls (GC pause, uplink hiccup, TCP loss without RST) and pacquet sits on the half-open socket indefinitely, emitting nothing even under `--show-output` (see #263 investigation notes). Set explicit defaults: * `connect_timeout(10s)` — generous for localhost, snaps on resource-exhausted runners. * `timeout(60s)` — per-request deadline. Typical npm tarball is under 5 MB, so 60 s on localhost is comfortable; a real hang turns into `TarballError::FetchTarball` and hyperfine reports the non-zero exit instead of sitting silent. * `pool_idle_timeout(30s)` — evict conns verdaccio closed but we didn't notice. Retries on transient failures are tracked separately in #259. Also enable pacquet tracing on the CI bench step via `TRACE=pacquet=info`, so the next time one of these stalls makes it through despite the new timeouts the step log shows which phase pacquet was in (download, extraction, import, index write) before it went silent. `--show-output` was already in place; without a tracing filter it only ever captured an empty stream. Refs #263. * fix(network): use `Self::new_from_cpu_count` in intra-doc link `[`new_from_cpu_count`]` doesn't resolve from inside the `impl` block because rustdoc needs a path; `Self::new_from_cpu_count` does. The Doc CI job runs with `RUSTDOCFLAGS=-D warnings` so the broken link was a hard error. No behavioural change. * fix(network): bump default request timeout from 60s to 5 min 60 seconds is too aggressive for a default that governs real registry traffic, not just the bench's localhost verdaccio. Large npm tarballs can legitimately take more than a minute over a slow network, and pacquet has no retry-with-backoff yet (#259) — a 60-second timeout would turn slow-but-progressing downloads into outright install failures. 5 minutes keeps the \"catch truly stuck sockets\" property (still well inside the bench's step budget and the CLI's reasonable-user patience) while comfortably accommodating slow real-world connections. Also reword the doc to call out that the value is the default for registry traffic too, not just the bench, and that making it user-configurable is the natural next step once retries exist. Addresses Copilot review on #267. |
||
|
Zoltan Kochan
|
0ba3ea0e4b |
bench(integrated-benchmark): replace 26-snapshot fixture with 1352-snapshot one (#262)
* bench(integrated-benchmark): replace 26-snapshot fixture with 1352-snapshot one
The previous `alotta-modules` fixture had 7 direct deps and 26
snapshots, small enough that a frozen-lockfile install completed in
~50 ms. At that scale the wall-time is dominated by per-process
overhead (binary startup, lockfile parse), so scaling wins in the
install pipeline — shared SQLite connections, import-method changes,
symlink-pass parallelism — don't show up.
Swap in a realistic 1352-snapshot lockfile of `alotta-modules` (389 KB
vs 13 KB) so the benchmark reflects a workload where the install
pipeline actually matters. First CI run will need to repopulate the
cached verdaccio storage; subsequent runs are served from the existing
cache step.
* bench(integrated-benchmark): add pnpm-workspace.yaml to the fixture
`allowBuilds: {core-js: false, es5-ext: false}` silences pnpm v11's
`ERR_PNPM_IGNORED_BUILDS` warnings for two packages whose postinstalls
would fire in this 1352-snapshot fixture. Pacquet's benchmark-side
`.npmrc` already sets `ignore-scripts=true` so nothing actually runs,
but pnpm still prints to stderr about the ignored builds and that
noise can reach hyperfine.
Stage the new file the same way `package.json` is staged — embedded
via `include_str!` for the default fixture, copied from `--fixture-dir`
when set. The `--fixture-dir` copy is gated on the file's existence
so existing fixture directories that predate this change don't start
erroring out.
|
||
|
Zoltan Kochan
|
6544b3ace5 |
perf(store-dir): share one read-only StoreIndex across cache lookups (#261)
* perf(store-dir): share one read-only StoreIndex across cache lookups `DownloadTarballToStore::load_cached_cas_paths` previously opened a fresh SQLite `Connection::open_with_flags` + `PRAGMA busy_timeout` on every snapshot, so a 1352-package frozen-lockfile install paid 1352 connection-opens just to read cache hints — all serialized on tokio's blocking pool because `spawn_blocking` can't parallelize opens against one file. Open a `StoreIndex` once per install, wrap it in `Arc<Mutex<…>>` (rusqlite `Connection` is `Send` but not `Sync`), and thread the handle through `CreateVirtualStore`, `InstallWithoutLockfile`, and `DownloadTarballToStore`. Each lookup now takes the mutex just for the sub-millisecond `SELECT`, releases it before the per-file validation pass, and frees the Arc when the install finishes. Measured on the #260 repro (1352 packages, release build, macOS APFS, warm CAS store): `rm -rf node_modules && pacquet install --frozen- lockfile` drops from ~17.4 s to ~13.9 s, with CPU utilization rising from ~135 % to ~160 % as the serialized-open bottleneck clears. * address review feedback * `create_virtual_store.rs`: bind `store_index.as_ref()` to a local and use `async move` so the closure's copy of the `Option<&…>` is moved into each future, rather than borrowing from the closure's per-iteration scope. * `load_cached_cas_paths`: treat a poisoned index mutex as a cache miss rather than propagating the panic. The `SELECT` is stateless, so the prior panic can't have left the index in an inconsistent shape, and cache lookups are a best-effort hint — falling through to a fresh download is strictly safer than crashing every subsequent snapshot. * park StoreIndex open on the blocking pool Copilot flagged that `StoreIndex::shared_readonly_in` does synchronous SQLite work (`Connection::open_with_flags` + `PRAGMA busy_timeout`) and calling it directly in an `async fn` stalls the tokio reactor thread for the duration of the open. It's once per install, not a hot spot, but matches the pattern the store-index writer already uses and costs us nothing to keep consistent. Wrap both call sites (frozen-lockfile and no-lockfile install paths) in `tokio::task::spawn_blocking` and await the result before kicking off concurrent package work. Also moves `tokio` from `dev-dependencies` into `dependencies` in package-manager's `Cargo.toml` since it's now a runtime dep for the library code, not just tests. * degrade JoinError on StoreIndex open to a cache miss `spawn_blocking(...).await` returns `Result<_, JoinError>` which fires on blocking-task panic or on cancellation during runtime shutdown. Panicking on that turns a transient runtime issue into a hard crash for an install that could otherwise succeed — cache lookups just miss. Drop the `.expect(...)` and degrade to `None` via `.ok().flatten()`. `shared_readonly_in` already yields `None` for a first-time install against an empty store, and every downstream caller already handles that shape, so this is the same failure mode the code already copes with — just reached from a different source. * use async move in install_dependencies_from_registry map closure Makes the recursive no-lockfile closure's captures explicit: bind `&node_modules_path` to a local `node_modules_path_ref` (Copy) and switch to `async move` so every captured variable is moved (copied, for all the `&T` and `Option<&T>` captures) into the future instead of borrowing the closure's per-iteration stack frame. Matches the pattern already used in `create_virtual_store.rs` and in the outer no-lockfile closure — keeps all three parallel-map sites consistent. * log JoinError before degrading to cache miss on StoreIndex open The silent `.ok().flatten()` hid two failure modes behind a cache miss: a panic inside the blocking task, and task cancellation during runtime shutdown. Neither is a crash-worthy error, but a blocking-task panic during install really should be visible somewhere. Promote the two call sites to a `match` and emit a `tracing::warn!` on the `Err(JoinError)` branch before falling back to `None`. Uses the `pacquet::install` target in common with the rest of this crate's tracing. Degradation path is unchanged. * chore: cargo fmt * log JoinError in tarball cache lookup, refresh stale comment Mirror the tracing change from the index-open call sites: when the blocking cache-lookup task returns `Err(JoinError)`, emit a `tracing::warn!` with the error and the `cache_key` before degrading to `None`. Panic / cancellation stays diagnosable; the caller still falls through to a fresh download. Also drop the stale "msgpackr-records decoding is a follow-up" wording from the cache-lookup rationale — `StoreIndex::get` has decoded pnpm-written msgpackr-records since |
||
|
Zoltan Kochan
|
1325c6eac9 |
feat(lockfile): support npm-alias dependencies in snapshots (#258)
* feat(lockfile): support npm-alias dependencies in snapshots Snapshot dependency values can be either a plain version (e.g. `5.1.2`) or an npm-alias reference (e.g. `string-width-cjs: string-width@4.2.3`, produced by `"string-width-cjs": "npm:string-width@^4.2.0"` in a consumer's package.json). Parsing the alias form as `PkgVerPeer` failed because it is not a bare semver. Introduce `SnapshotDepRef` — `Plain(PkgVerPeer)` or `Alias(PkgNameVerPeer)` — mirroring pnpm's `refToRelative` alias detection, and thread it through the snapshot → virtual-store symlink path: alias entries use the entry key as the link name under `node_modules/` and the aliased target for the virtual-store lookup. * chore: cargo fmt |
||
|
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
|
||
|
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. |
||
|
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.
|
||
|
Khải
|
6d552525ad | perf: use abbreviated json (#241) | ||
|
Khải
|
305ac5ba35 |
refactor: use utf8 strings as mem cache keys (#222)
* fix: clippy * refactor: use utf8 strings as mem cache keys * refactor: use cleaned_entry_path directly |
||
|
Khải
|
8657d3aa65 | refactor: divide pacquet-fs into multiple files (#214) | ||
|
Khải
|
ed7e211701 | refactor: remove unused error variant (#215) | ||
|
khai96_
|
ab81e48ee4 | fix: clippy | ||
|
await-ovo
|
b9b5d8b222 |
fix(install/clean): infinite loops on circular dependencies (#211)
* fix(package-manager): avoid infinite loops when clean install circular dependencies * chore: use dashset instread of memo-map for resolved_packages cache * chore: fmt * docs: make it appears nicer --------- Co-authored-by: Khải <hvksmr1996@gmail.com> |
||
|
Khải
|
f3521503d8 |
chore(cargo): bump derive_more to beta.6 (#212)
Closes https://github.com/pnpm/pacquet/pull/207 |
||
|
Khải
|
1d4eec5196 | refactor: remove unnecessary function (#204) | ||
|
Khải
|
6ae9f08a19 |
test: big lockfile (#213)
* test: big lockfile * fix(test): ignore failed platforms |
||
|
Khải
|
9d80419b31 |
feat(network): limit concurrent network requests (#210)
сlose #116 |
||
|
dependabot[bot]
|
718daf9167 |
chore(cargo): bump futures-util from 0.3.28 to 0.3.29
Bumps [futures-util](https://github.com/rust-lang/futures-rs) from 0.3.28 to 0.3.29. - [Release notes](https://github.com/rust-lang/futures-rs/releases) - [Changelog](https://github.com/rust-lang/futures-rs/blob/master/CHANGELOG.md) - [Commits](https://github.com/rust-lang/futures-rs/compare/0.3.28...0.3.29) --- updated-dependencies: - dependency-name: futures-util dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
dependabot[bot]
|
fefcebd890 |
chore(cargo): bump tracing-subscriber from 0.3.17 to 0.3.18
Bumps [tracing-subscriber](https://github.com/tokio-rs/tracing) from 0.3.17 to 0.3.18. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](https://github.com/tokio-rs/tracing/compare/tracing-subscriber-0.3.17...tracing-subscriber-0.3.18) --- updated-dependencies: - dependency-name: tracing-subscriber dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
dependabot[bot]
|
4b2f626fc0 |
chore(cargo): bump tempfile from 3.8.0 to 3.8.1
Bumps [tempfile](https://github.com/Stebalien/tempfile) from 3.8.0 to 3.8.1. - [Changelog](https://github.com/Stebalien/tempfile/blob/master/CHANGELOG.md) - [Commits](https://github.com/Stebalien/tempfile/commits) --- updated-dependencies: - dependency-name: tempfile dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
khai96_
|
4b38c23087 | test(lib): use packages from registry-mock | ||
|
khai96_
|
fafd91628d | test(lib): use registry-mock | ||
|
dependabot[bot]
|
55283c4048 |
chore(cargo): bump insta from 1.32.0 to 1.34.0 (#150)
Bumps [insta](https://github.com/mitsuhiko/insta) from 1.32.0 to 1.34.0. - [Changelog](https://github.com/mitsuhiko/insta/blob/master/CHANGELOG.md) - [Commits](https://github.com/mitsuhiko/insta/compare/1.32.0...1.34.0) --- updated-dependencies: - dependency-name: insta dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
ce6c949c34 |
chore(cargo): bump thiserror from 1.0.48 to 1.0.50 (#161)
Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.48 to 1.0.50. - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](https://github.com/dtolnay/thiserror/compare/1.0.48...1.0.50) --- updated-dependencies: - dependency-name: thiserror dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
d653d10e4a |
chore(cargo): bump tracing from 0.1.37 to 0.1.40 (#160)
Bumps [tracing](https://github.com/tokio-rs/tracing) from 0.1.37 to 0.1.40. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](https://github.com/tokio-rs/tracing/compare/tracing-0.1.37...tracing-0.1.40) --- updated-dependencies: - dependency-name: tracing dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
cac3f398fe |
chore(cargo): bump clap from 4.4.4 to 4.4.8 (#194)
Bumps [clap](https://github.com/clap-rs/clap) from 4.4.4 to 4.4.8. - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](https://github.com/clap-rs/clap/compare/clap_complete-v4.4.4...v4.4.8) --- updated-dependencies: - dependency-name: clap dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Khải
|
362607a51c |
refactor: reuse registry-mock in benchmark (#201)
* refactor: reuse registry-mock in benchmark * refactor: move executor to verify * feat(benchmark): install dependencies * ci(benchmark): fix * ci(benchmark): use just * refactor: reduce lines |
||
|
Khải
|
7ad33aa65b |
test: registry-mock (#198)
* test: fix snapshot * test: filter out index files temporarily * chore: install registry-mock * chore(just): add pnpm to test * feat: create registry-mock crate * feat(registry-mock): expose get_or_init instead * refactor: local static * feat(testing-utils): add_mocked_registry * refactor: rename new to init * fix: enable_all * refactor: use * feat: add ending / * fix: must be "localhost" * feat(registry-mock): use portpicker * feat(registry-mock): share a single instance * feat(testing-utils): stop skipping index files * test(cli/install): use registry-mock * fix: forgot to save * fix: forgot increment * fix: used to wrong syntax * feat(registry-mock): use advisory-lock * fix: save anchor before early exit * fix: drop before delete * tmp: stored * refactor: remove unused import * fix: truncate the file * feat: use try_lock * feat(registry-mock): separate lock from anchor * refactor: use RAII pattern * fix(registry-mock): user_count * refactor: rename user_count to ref_count * fix: kill groups * fix(registry-mock): ref_count * chore(git): revert broken fix This reverts commit 8c0ba7bafd7670dd6580a142ccd7dea5809ed782. * fix(registry-mock): use sysinfo to kill * fix(registry-mock): kill algorithm * feat(registry-mock): recurse regardless * feat(registry-mock): use reverse recursion instead * refactor: reduce complexity * refactor: remove unnecessary mod declarations * refactor: marks some struct as must_use * test(cli/add): use registry-mock * test(pnpm-compatibility): use registry-mock * refactor: remove unused items * docs: correction * chore(license): allow MPL-2.0 * chore(license): allow Unlicense * fix(registry-mock): failure on few threads * chore: move pnpm install to just * ci: cache pnpm * fix(test): kill leftovers * fix(test): allow delete to fail * fix(ci): Install just * fix(test): don't kill leftovers This reverts commit ef78d8c54cee691d93fa8e5c169141a0098047f2. * refactor: move some types to their own mods * refactor: move port_to_url * refactor: make listen lazy * refactor: rename `listen` to `url` * refactor: use qualified path * refactor: store port as u16 * feat(registry-mock): PreparedRegistryInfo * refactor: remove unnecessary async * refactor: move init code * refactor: define lib * feat(registry-mock): bin * refactor: call just test * ci: prepare a server * feat(registry-mock): unique prepared * docs(registry-mock): cli * feat(registry-mock/cli): locations of log files * docs(mocked-registry): important items * docs(registry-mock): ignore bin * style: taplo fmt * docs(readme): instruction to run test * refactor: move registry-mock to tasks * ci(codecov): fix * feat(registry-mock): bump max_retries |
||
|
Khải
|
b0d5549210 |
feat(benchmark): allow overriding lockfile (#196)
* feat(benchmark): allow overriding lockfile * test: fix snapshot * test: fix snapshot * test: filter out index files temporarily * test: fix snapshot |
||
|
Khải
|
6963c7036c | refactor: correct the name of an error (#184) | ||
|
Khải
|
672e4003e6 |
refactor: only use cache when required (#182)
* refactor: only use cache when required * refactor: remove unnecessary Arc * refactor: clarify "in-memory" part of the cache |
||
|
Khải
|
65d1a9ab95 |
refactor: parse integrity eagerly (#180)
* refactor: parse integrity eagerly The parsing of integrity has been moved from tarball to serde * docs: clarify * docs: remove the 3rd option * refactor: remove unnecessary `pipe_ref` |
||
|
Khải
|
8493e907f9 |
ci(benchmark): embed the JSON report (#178)
* ci(benchmark): embed the JSON report * fix: quoting |
||
|
Khải
|
2848e5a65a |
refactor: rewrite the bin functions into a struct (#176)
* refactor: convert 2 functions into a struct * refactor: symmetry * refactor: rename a constructor * docs: consistent grammar |
||
|
Khải
|
8f6e09ed36 |
refactor: functional style for testing_utils::fs (#173)
|
||
|
Khải
|
e463cccafd | docs: note about hardlink nuance for future impl (#175) |