mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-22 21:53:02 -04:00
node-runtime-fix
126 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Zoltan Kochan
|
62900806fe |
feat(config,cli,tarball): support offline / preferOffline settings (#513)
* feat(config,cli,tarball): support `offline` / `preferOffline` settings Adds the two settings to `Config`, `pnpm-workspace.yaml` parsing, and `pacquet install` as `--offline` / `--prefer-offline` flags. CLI flags merge into Config (boolean OR) before leak, so any source (yaml, CLI) flips `true` and never the other way. Upstream pnpm gates the metadata-fetch path in [`pickPackage`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/npm-resolver/src/pickPackage.ts): when `offline: true` and no cached metadata exists, it fails with `ERR_PNPM_NO_OFFLINE_META`; `preferOffline: true` biases the resolver toward the cached copy. Pacquet's frozen-install path has **no metadata fetch** (the lockfile pins every resolution), so the upstream flag is semantically a no-op on pacquet's current flow. To make the flag actually useful for frozen installs, pacquet adds a **tarball-side gate** (no upstream equivalent — pnpm doesn't gate tarball downloads on `offline`): when both the warm prefetch and the SQLite `index.db` lookup miss, the fetcher refuses the network and errors with `ERR_PACQUET_NO_OFFLINE_TARBALL`. Same shape as `ERR_PNPM_NO_OFFLINE_META`, scoped to tarballs because that's what pacquet's frozen install needs network for. Documented as a pacquet- specific divergence in the field's rustdoc. `prefer_offline` is a no-op today — the warm prefetch + `load_cached_cas_paths` already prefer the local store. The field exists so `.npmrc` / yaml / CLI all parse cleanly; Stage 2's resolver will honour it on the metadata path. - `Config.offline` / `Config.prefer_offline`: bool, default `false`. - `WorkspaceYaml.offline` / `WorkspaceYaml.prefer_offline`: `Option<bool>`; applied via the existing `apply!` macro. - `InstallArgs.offline` / `InstallArgs.prefer_offline`: bool CLI flags; merged into Config in `cli_args.rs` between `config()` and `State::init` (the brief window when `Config::leak` returns `&'static mut Config`). - `DownloadTarballToStore.offline` / `DownloadZipArchiveToStore.offline`: bool fields. Wired through the three construction sites in `package-manager`. - `TarballError::NoOfflineTarball { package_id, url }`: new variant. `tarball_error_to_request_retry` updated for exhaustiveness; `code: ERR_PACQUET_NO_OFFLINE_TARBALL` in the retry-logger projection so a future surface that runs this error through the retry path renders the right code. - `offline_mode_skips_network_on_cache_miss` — `mockito` server with `.expect(0)`; the offline gate must fire before the network is touched. Asserts variant shape AND the diagnostic code (the code is part of the user-facing surface). - `offline_mode_still_uses_prefetched_cache` — same `.expect(0)` guard but with a prefetched cache hit; pins that the prefetch branch short-circuits *before* the offline check so warm installs under `--offline` succeed. Regression-catch verified: gating the gate behind `false &&` flipped `offline_mode_skips_network_on_cache_miss` red with the wrong-variant panic message. Reverted before commit. All bulk-edit fixups (15 `DownloadTarballToStore` / `DownloadZipArchiveToStore` literals in tests + micro-benchmark, 1 `Config` literal in package- manager tests) get `offline: false` to match the prior implicit default. * docs(cli): pin pickPackage link to commit SHA (#513 review) Per project guideline: upstream pnpm citations link to a specific commit SHA (first 10 hex chars), not a branch name. Same SHA the PR's rustdoc and commit body already use for the same file. Resolves CodeRabbit review comment on `crates/cli/README.md`: <https://github.com/pnpm/pacquet/pull/513#discussion_r3238182177>. |
||
|
Zoltan Kochan
|
888184226d |
feat(cli): --node-linker flag (#438 slice 8) (#514)
Add the `--node-linker [isolated|hoisted|pnp]` CLI flag to `pacquet install`, mirroring pnpm's flag. Overrides `Config.node_linker` for the invocation; absent flag → config's yaml/npmrc value wins. The CLI parses into a `NodeLinkerArg` mirror enum (kept in the CLI crate so `pacquet-config` stays free of `clap`), then `into_config()` converts to the canonical `pacquet_config::NodeLinker`. Threaded into the `Install` runner as an explicit `node_linker: NodeLinker` field rather than reading `config.node_linker` directly. Matches the existing pattern `supported_architectures` uses (`state.config` is `&'static`, so the override-merge happens at the CLI layer and lands here as a fully-resolved value). `build_modules_manifest` consumes it on the `.modules.yaml.nodeLinker` write so the persisted setting reflects the invocation, not just the config. `pacquet add` uses Config's value by default (`add` doesn't expose `--node-linker` per the umbrella scope; Slice 6's pipeline integration will revisit if needed). 5 new CLI tests: - `node_linker_default_is_none` — flag absent → field is None. - `node_linker_hoisted` / `node_linker_isolated` / `node_linker_pnp` — each value parses and round-trips through `into_config()`. - `node_linker_invalid_value_rejected` — clap rejects unknown values with the bad value in the error message. - `node_linker_arg_into_config_matches_every_variant` — every ValueEnum variant has a canonical mapping (compile-fails on future variants that forget the mapping). `NodeLinker` gains `Clone + Copy + Eq` so it can be threaded by value into `Install` and matched in tests. |
||
|
Zoltan Kochan
|
13a6970ae6 |
feat(package-manager): runtime bin-link + --no-runtime (#437 slice D3 + E) (#506)
* feat(package-manager): runtime bin-link + --no-runtime (#437 slice D3 + E) Closes the install-pipeline side of #437: **Slice D3 — runtime bin-link integration.** Runtime archives (`node@runtime:`, etc.) don't ship a `package.json`, so the existing bin-link step had nothing to consume off the slot. `fetch_binary_resolution_to_cas` now synthesizes one in the same shape upstream's `appendManifest` does: { "name": "<pkg.name>", "version": "<pkg.version>", "bin": <BinarySpec> } The bytes go through `StoreDir::write_cas_file` (same content- addressing as every other CAS-imported file), the resulting cas-path is inserted into the snapshot's `cas_paths` under `package.json`, and the existing `link_bins_of_packages` flow picks it up. `link_bins::build_has_bin_set` now includes `Binary` / `Variations` resolutions unconditionally — pnpm v11 doesn't emit `hasBin: true` on runtime metadata, but the synthesized manifest always carries bins, so the lookup must not be gated on the metadata flag. **Slice E — `--no-runtime` flag.** New `Config::skip_runtimes` (default false) and `InstallArgs::no_runtime` CLI flag. The CLI computes `config.skip_runtimes || --no-runtime` and threads it through `Install` → `InstallFrozenLockfile`. When set, every snapshot whose metadata resolution is `Binary` or `Variations` is added to the install-time skip set (re-using `add_optional_excluded` since the bucket count and `.modules.yaml.skipped` semantics line up with `--no-optional`). **Unit tests.** - `synthesize_runtime_manifest_emits_name_version_and_bin_single` — pins `BinarySpec::Single` → JSON string. - `synthesize_runtime_manifest_emits_name_version_and_bin_map` — pins `BinarySpec::Map` → JSON object with all bin entries. - `synthesize_runtime_manifest_preserves_scoped_name` — `@scope/name` round-trips through the `name` field. - `build_has_bin_set_includes_runtime_resolutions_even_when_has_bin_is_absent` — pins the runtime arm. Verified by removing the arm — test fails as expected. End-to-end install fixtures (slice F's remaining items — recorded-archive frozen-lockfile install, variant-mismatch error, `--no-runtime` integration, integrity-mismatch path) need a small recorded Node archive in the repo (or a mockable `pnpm-resolution-mirror`). Tracking them as a separate follow-up so this PR stays reviewable. * fix(package-manager): narrow --no-runtime to importer-direct deps Address CodeRabbit review on #506: my initial implementation iterated `packages` (every metadata entry) and added all `Binary` / `Variations` snapshots to the skip set, which widened the behavior beyond pnpm's `skipRuntimes`. Per the cardinal rule (CLAUDE.md: "Port behavior faithfully. ... Do not invent behavior that pnpm does not have."), match upstream exactly. Upstream's filter at [`installing/deps-installer/src/install/index.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/index.ts#L1374-L1387) iterates `dependenciesByProjectId` (per-importer direct deps) and adds `depPath` to `ctx.skipped` when `depPath.includes('@runtime:')`. Transitive runtime entries — unusual but possible — stay in the install. Pacquet now mirrors that exactly: walk each importer's `dependencies` / `dev_dependencies` / `optional_dependencies`, build the candidate snapshot key from `(alias, version)`, check for the `@runtime:` substring, and only add the metadata-confirmed `Binary` / `Variations` keys to `skipped`. Aliased runtime deps (unusual) fall through the same way upstream does — pnpm's lookup is by depPath, not by alias. |
||
|
Zoltan Kochan
|
52364d4b5a |
feat(config,network,tarball,registry): per-registry TLS overrides (#502)
Closes #497.
## Summary
Adds per-registry TLS overrides keyed by nerf-darted `.npmrc` URI, the natural follow-up to #490's top-level TLS keys. Corporate environments running a private Verdaccio (or any registry with its own self-signed cert) can now pin scoped `:cafile=…` / `:cert=…` / `:key=…` per host without disabling strict-ssl globally.
Three commits, layered:
- **`feat(network)`** (eff1248e): adds `RegistryTls` + `PerRegistryTls` types in `pacquet-network` plus the lookup machinery — `pick_for_url` ports pnpm's [5-step `pickSettingByUrl`](https://github.com/pnpm/pnpm/blob/94240bc046/network/fetch/src/dispatcher.ts#L338-L375) exactly (exact > nerf-dart > no-port > shorter prefix > recursive no-port retry). `ThrottledClient::for_installs` gains a third `&PerRegistryTls` parameter and pre-builds one reqwest `Client` per non-empty override. New `acquire_for_url(url: &str)` routes per-request; `acquire()` keeps the default-client behavior for callers without a URL.
- **`feat(config)`** (4e69868a): `NpmrcAuth` parses the six per-registry TLS suffixes (`:ca`, `:cafile`, `:cert`, `:certfile`, `:key`, `:keyfile`) matching pnpm's `SSL_SUFFIX_RE` and applies onto `Config.tls_by_uri`. `*file` variants read from disk at parse time (silent on error); inline values get `\\n` → `\n` expansion. `:cert` and `:certfile` share the same `tls.cert` slot — last-write-wins inside one `.npmrc`.
- **`refactor(tarball,registry)`** (5f9cae93): three production call sites (registry metadata + version-tag fetches, plus two tarball download paths) move from `acquire()` to `acquire_for_url(url)` so the per-registry routing actually fires.
## Parity policy
Bug-for-bug with pnpm v11 ([SHA
|
||
|
Zoltan Kochan
|
6d9d0ad08c |
feat(config,network,cli): TLS keys + local-address from .npmrc (#490)
Closes #482. ## Summary Ports pnpm v11's TLS + `local-address` `.npmrc` keys onto pacquet (SHA [`94240bc046`](https://github.com/pnpm/pnpm/blob/94240bc046/network/fetch/src/dispatcher.ts)), the natural pair to the proxy support that landed in #476. Three layers: - **`feat(network)`** (e9ed56c9): adds `TlsConfig` next to `ProxyConfig` in `pacquet-network`, with `ca: Vec<String>`, `cert`/`key`, `strict_ssl: Option<bool>`, and `local_address: Option<IpAddr>`. `ThrottledClient::for_installs` gains a `&TlsConfig` parameter; the unified error surface is the new `ForInstallsError` enum carrying either `ProxyError` or `TlsError`. CAs route through `Certificate::from_pem`, client identities through `Identity::from_pkcs8_pem` (the only PKCS path reqwest exposes on the native-tls backend pacquet builds with). `strict_ssl` defaults to `true` at build site, matching pnpm's per-emit-site `strictSsl ?? true` rather than a config-layer default. - **`feat(config)`** (e8dcd87e): extends `NpmrcAuth` with the six new keys (`ca`, `cafile`, `cert`, `key`, `strict-ssl`, `local-address`) and adds `apply_tls_and_local_address` to populate `Config.tls`. `cafile` reads from disk and splits on `-----END CERTIFICATE-----` to mirror pnpm's [`loadCAFile`](https://github.com/pnpm/pnpm/blob/94240bc046/config/reader/src/loadNpmrcFiles.ts#L249-L265). Unreadable `cafile` is silently treated as unset. Invalid `strict-ssl` and `local-address` values drop silently. - **`feat(cli)`** (1759f9d2): one-line swap in `State::init` to pass `&config.tls` instead of `TlsConfig::default()`. Folds in two CI-parity fixups: the self-signed test cert moves to a shared `.pem` fixture under `crates/network/tests/fixtures/` (typos linter false positives on base64 DER), and four reqwest intra-doc links become plain backticks. ## Parity policy Faithful to pnpm — see the [research brief](https://github.com/pnpm/pacquet/issues/482) on the issue. Highlights: - **No new error codes.** pnpm doesn't define `ERR_PNPM_INVALID_CA` etc.; invalid PEMs surface as raw `tls.connect` errors at request time upstream. Pacquet validates eagerly via `Certificate::from_pem` / `Identity::from_pkcs8_pem` (pushing the failure to per-request time would silently degrade every install behind a broken `ca`) but deliberately omits a `code(...)` attribute on `TlsError` so reviewers can see at a glance it's a pacquet-only diagnostic, not a pnpm error code. - **Silent `cafile`-not-found.** Matches pnpm's `catch {}` swallow in `loadCAFile`. - **No env-var fallback.** pnpm reads only `.npmrc`; Node's implicit `NODE_EXTRA_CA_CERTS` / `NODE_TLS_REJECT_UNAUTHORIZED` honoring doesn't apply to pacquet's reqwest stack. - **`strict-ssl: false` disables both chain-of-trust and hostname verification**, matching Node's `rejectUnauthorized=false` short-circuit (pacquet uses reqwest's `danger_accept_invalid_certs(true)` which has the same combined semantics). ## Reviewer flags - **PKCS#8-only client keys.** Reqwest's native-tls backend exposes only `Identity::from_pkcs8_pem`; legacy PKCS#1 keys (`-----BEGIN RSA PRIVATE KEY-----`) and PKCS#12 bundles are not supported by this constructor. Documented at the `apply_tls` callsite with the `openssl pkcs8 -topk8 -nocrypt` conversion command. Switching to rustls-tls would broaden the supported formats but is out of scope here. - **Per-registry TLS overrides** (`//host:cafile=`, `//host:ca=`, `//host:cert=`, `//host:key=`) are **not** included. Same shape as the existing scoped-auth handling but a sizeable feature on its own; flagged in #482 as a follow-up. |
||
|
Zoltan Kochan
|
b33a481b17 |
feat: proxy support (.npmrc + env-var cascade, HTTP/HTTPS/SOCKS) (#476)
* feat(network): add ThrottledClient::for_installs with proxy support Ports pnpm v11's `getDispatcher` (`network/fetch/src/dispatcher.ts` at SHA |
||
|
Khải
|
9a0199c0a1 |
feat: auth (#337)
Resolves <https://github.com/pnpm/pacquet/issues/336>. ## Summary Ports pnpm v11's auth flow so `pacquet install` can talk to private registries on the same footing as `pnpm install`. Pacquet now parses credentials out of `.npmrc`, expands `${VAR}` references against the process environment, and attaches the right `Authorization` header on every metadata fetch and tarball download — including tarballs hosted on a CDN host that differs from the registry host. Upstream reference: [`pnpm/pnpm@601317e7a3`](https://github.com/pnpm/pnpm/tree/601317e7a3). ## What's wired up * `_auth`, `_authToken`, `username`, and `_password` keys, both default-registry (bare) and per-registry (`//host[:port]/path/:_authToken=…` family), parsed in `crates/config` (formerly `crates/npmrc`, renamed by #420). * `${VAR}` and `${VAR:-default}` substitution with backslash-escape semantics, ported from [`@pnpm/config.env-replace`](https://github.com/pnpm/components/blob/9c2bd17/config/env-replace/env-replace.ts). Unresolvable placeholders surface a `tracing::warn!` and leave the value verbatim — same best-effort behaviour as pnpm's [`substituteEnv`](https://github.com/pnpm/pnpm/blob/601317e7a3/config/reader/src/loadNpmrcFiles.ts#L156-L162). * URL-keyed lookup (`AuthHeaders`) in `crates/network`, mirroring [`createGetAuthHeaderByURI`](https://github.com/pnpm/pnpm/blob/601317e7a3/network/auth-header/src/index.ts): nerf-darts the request URL, walks parent path prefixes from longest to host-only, falls back to a port-stripped lookup matching upstream's [`removePort`](https://github.com/pnpm/pnpm/blob/601317e7a3/network/auth-header/src/helpers/removePort.ts) (any port, not just protocol defaults), and prefers inline `user:password@` basic auth when present. * Default-registry creds key against the *resolved* registry. The two-phase `apply_registry_and_warn` / `build_auth_headers` split inside `Config::current` ensures `pnpm-workspace.yaml`'s `registry` override propagates to the bearer-token nerf-dart key. * The `Authorization` header is attached on `Package::fetch_from_registry`, `PackageVersion::fetch_from_registry`, and inside `DownloadTarballToStore`'s retry loop. ## Test porting checklist Boxes ticked in `plans/TEST_PORTING.md` for the upstream auth tests this PR ports: * `network/auth-header/test/getAuthHeadersFromConfig.test.ts` — `should convert auth token to Bearer header`, `should convert basicAuth to Basic header`, `should handle default registry auth (empty key)`. * `network/auth-header/test/getAuthHeaderByURI.ts` — all 7 entries (`getAuthHeaderByURI()`, basic-auth-without-settings, basic-auth-with-settings, https-port-443, default-ports, registry-with-pathnames, default-registry-auth). * `config/reader/test/parseCreds.test.ts` — `authToken`, `authPairBase64`, `authUsername and authPassword`. The remaining auth checkboxes (`tokenHelper`, `pnpm auth file` precedence, redirect header-stripping, the `installing/deps-installer/test/install/auth.ts` integration tests) need features pacquet doesn't yet have — token helpers, the `auth.ini` layer, scoped registries, redirect handling — and stay open for follow-ups. ## Notes / scope * `always-auth` is intentionally not honoured. pnpm v11 doesn't either — the auth header is selected by URL match alone, and `always-auth` was deprecated upstream. * TLS / proxy / scoped `@scope:registry` keys remain unparsed; the parser silently ignores them, matching the pre-#336 behaviour. The `creds_by_uri` shape is ready to grow as those land. * The 401/403/404 fail-fast retry policy in `crates/tarball` was already in place from #259; this PR's auth header attaches before the retry loop, so the policy still applies unchanged. * Env-var injection follows the trait-per-capability DI pattern from [pnpm/pacquet#339](https://github.com/pnpm/pacquet/issues/339): `pub trait EnvVar` + `pub struct RealApi` in `crates/config/src/api.rs`, threaded through `env_replace`, `NpmrcAuth::from_ini`, and `Config::current` under one `Api: EnvVar` bound. Production callers turbofish `RealApi` explicitly: `Config::current::<RealApi, _, _, _, _>(...)`. |
||
|
Zoltan Kochan
|
0a89e132fc |
feat: add hoisting support (hoistPattern + publicHoistPattern) (#435) (#445)
Closes #435. ## Summary - Ports `@pnpm/config.matcher` (`*` glob, `!` negation, first-match-wins) into a new `pacquet_config::matcher` module — no `regex` dependency. - Ports `getHoistedDependencies` and `symlinkHoistedDependencies` from upstream `installing/linking/hoist/src/index.ts` into a new `pacquet_package_manager::hoist` module. BFS by depth + lex on nodeId, public wins ties, first-seen wins per alias, root importer's direct-dep names seed the "already hoisted" set. - Wires the hoist pass into `InstallFrozenLockfile::run` between the per-slot bin link and the `importing_done` emit. Result is threaded through `Install::run` into `.modules.yaml`'s `hoistedDependencies`. - Promotes `Config.hoist_pattern` and `Config.public_hoist_pattern` to `Option<Vec<String>>` so the install-time guard mirrors upstream's `!= null` check (issue §D). Upstream pinned at pnpm v11 [`94240bc046`](https://github.com/pnpm/pnpm/tree/94240bc046). ## Tests - **Matcher (`crates/config/src/matcher.rs`)** — 6 tests, including direct ports of upstream's `matcher()` and `createMatcherWithIndex()` test cases plus regex-special-char-literal coverage. - **Hoist algorithm (`crates/package-manager/src/hoist/tests.rs`)** — 12 unit tests covering empty graph, default `*` pattern, public hoist, public-wins-ties, negation, first-seen-wins, root direct-dep blocking, skipped-snapshot exclusion, bin-set collection, and graph-builder + direct-deps-builder helpers. - **End-to-end (`crates/cli/tests/hoist.rs`)** — 9 integration tests using pnpm to generate the lockfile and pacquet to install with `--frozen-lockfile`. Cover default private hoist, `publicHoistPattern: '*'`, both-empty, `shamefullyHoist` legacy, scoped-pattern filter, `!`-negation, `.modules.yaml` round-trip, and bin linking on both private and public sides. - **Known failures (17 stubs)** — Ports the remainder of `installing/deps-installer/test/install/hoist.ts` into `known_failures` modules via `pacquet_testing_utils::allow_known_failure!`. Each stub names the upstream URL line + the feature blocking it (workspace install #431, partial install #433, skipped optional deps, direct-dep bin precedence, `extendNodePath`). `plans/TEST_PORTING.md` updated to mark every ported `hoist.ts` entry with the corresponding pacquet test name. |
||
|
Zoltan Kochan
|
970100131e |
feat: supportedArchitectures config + --cpu/--os/--libc CLI + real libc detection (#434 slice 2) (#456)
Slice 2 of the [#434 umbrella](https://github.com/pnpm/pacquet/issues/434) (`Proper optionalDependencies support`). Slice 1 (#439) wired `pacquet-package-is-installable` into the install pipeline but the `SupportedArchitectures` input was always `None`, so every install behaved as if the user had opted into \"host triple only\". This PR closes that gap and replaces the slice 1 libc stub with a real Linux probe. Mirrors the three upstream input sources pinned at [pnpm/pnpm@94240bc046](https://github.com/pnpm/pnpm/tree/94240bc046): - **`Config.supported_architectures` from `pnpm-workspace.yaml`** — same shape as upstream's `getOptionsFromRootManifest.ts` ([`config/reader/src/getOptionsFromRootManifest.ts#L23`](https://github.com/pnpm/pnpm/blob/94240bc046/config/reader/src/getOptionsFromRootManifest.ts#L23)). - **`--cpu` / `--os` / `--libc` flags on `install` / `add`** — multi-valued, comma-separated. Per-axis CLI override replaces the yaml axis wholesale, matching upstream's [`overrideSupportedArchitecturesWithCLI.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/config/reader/src/overrideSupportedArchitecturesWithCLI.ts). - **Real Linux libc detection** — replaces the slice 1 `\"unknown\"` stub with a `/lib*/ld-musl-*` probe cached via `OnceLock`. Mirrors `detect-libc.familySync()` semantics: `\"musl\"` / `\"glibc\"` on Linux, `\"unknown\"` everywhere else. No new dep — open-coded `read_dir` is cheaper than spawning `ldd --version` and works in slim containers without `ldd` on PATH. Threaded through `Install` / `InstallFrozenLockfile` / `Add` as an explicit field instead of mutating `State.config`, because `State.config` is `&'static Config` — the CLI override merge happens at the CLI layer and the fully-resolved value lands on the install pipeline. ## Test plan Unit tests added; full suite ran via `just ready` + `cargo doc --no-deps` + `taplo format --check` + `just dylint`. - [x] `cargo test -p pacquet-config workspace_yaml::tests::*supported_architectures*` — yaml round-trip across `os` / `cpu` / `libc`, omission, partial-axis (3 tests). - [x] `cargo test -p pacquet-package-manager --lib installability::tests::supported_architectures*` — `supportedArchitectures` widens the accept set so a darwin-only package stays on a linux host when the user lists `darwin`; conversely, listing `linux` keeps the darwin-only package skipped (no implicit host include). - [x] `just ready` — typos + fmt + check + nextest (792 tests pass) + clippy. - [x] `taplo format --check`. - [x] `just dylint` (perfectionist). - [x] `RUSTDOCFLAGS=\"-D warnings\" cargo doc --no-deps --workspace`. ## Out of scope - `.modules.yaml.skipped` persistence (umbrella slice 3). - Current-lockfile diffing for `removeOptionalDependenciesThatAreNotUsed` (umbrella slice 6). - `--no-optional` plumbing (umbrella slice 5). Closes #453. |
||
|
Zoltan Kochan
|
582ef43b6c |
refactor(config): rename pacquet-npmrc → pacquet-config and drop ini wiring (#420)
## Summary - pnpm 11 reads project-structural settings (storeDir, hoist, nodeLinker, fetchRetries, …) from `pnpm-workspace.yaml` and limits `.npmrc` to auth/registry/network keys. The crate that holds pacquet's resolved runtime config was still named `pacquet-npmrc` with a `Npmrc` type and carried `serde_ini` deserializer wiring on every field, which no longer reflected how the config is actually loaded on this branch. - Two-commit refactor: rename the crate / type / imports, then drop the dead serde_ini machinery and the seven duplicate ini-based tests. ## Commits 1. **`refactor(config): rename pacquet-npmrc crate to pacquet-config`** — mechanical rename. `crates/npmrc/` → `crates/config/`, `Npmrc` → `Config`, `pacquet-npmrc`/`pacquet_npmrc` references updated workspace-wide. `NpmrcAuth` (the `.npmrc` auth-subset parser) keeps its name. 2. **`refactor(config): drop ini deserializer wiring from Config`** — removes `Deserialize` + `#[serde(rename_all = "kebab-case")]` from `Config`, every per-field `deserialize_with`/`default = "..."` attribute, the helper fns that backed them (`bool_true`, `deserialize_bool`/`u32`/`u64`/`pathbuf`/`store_dir`/`registry`), and the `serde_ini` workspace dep. Drops seven ini-based `lib.rs` tests that already had equivalent yaml/npmrc-auth coverage. Renames `custom_deserializer` → `defaults` (only default factories left), and the `npmrc: &mut Config` local in `apply_to` / `Config::current` becomes `config`. After the cleanup `Config` is purely a merged runtime struct: yaml deserializes into `WorkspaceSettings` and applies onto a `Config`, `.npmrc` is hand-parsed by `NpmrcAuth::from_ini`, and `Config` itself is no longer deserialized from any file. |
||
|
Zoltan Kochan
|
2f64c727ea |
feat(executor): swallow optional-dep build failures and report via pnpm:skipped-optional-dependency (#397) (#419)
## Summary Closes item #6 of #397. Optional dependencies whose `postinstall` hooks fail no longer abort the install — pacquet now reports the failure through the `pnpm:skipped-optional-dependency` channel (reason `build_failure`) and continues, matching [pnpm v11's behavior at `building/during-install/src/index.ts:218-240`](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/building/during-install/src/index.ts#L218-L240). Three commits: - **`feat(lockfile): surface snapshot-level optional flag`** — pacquet's lockfile reader previously dropped the `snapshots[<key>].optional` field. Adding it as `pub optional: bool` on `SnapshotEntry` with `#[serde(default, skip_serializing_if = "is_false")]` so absent ↔ false and `false` never serializes. The flag is pre-computed by pnpm's resolver at install time (ALL-paths- optional fold), so pacquet trusts the precomputed value rather than re-deriving it — matching [`lockfileToDepGraph` at deps/graph-builder/src/lockfileToDepGraph.ts:315](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/deps/graph-builder/src/lockfileToDepGraph.ts#L315). - **`feat(reporter): add pnpm:skipped-optional-dependency event`** — new `LogEvent::SkippedOptionalDependency(SkippedOptionalDependencyLog)` variant + `SkippedOptionalPackage` + `SkippedOptionalReason` enum. Wire shape mirrors [pnpm's `SkippedOptionalDependencyMessage`](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/core/core-loggers/src/skippedOptionalDependencyLogger.ts). All four upstream reasons (`BuildFailure`, `UnsupportedEngine`, `UnsupportedPlatform`, `ResolutionFailure`) are declared even though only `BuildFailure` is emitted today — keeps the enum closed so the other emit sites can land without widening it. - **`feat(package-manager): swallow optional-dep build failures`** — `BuildModules.run` reads `snapshot.optional`. When `run_postinstall_hooks` returns an `Err`, the dep's optional flag decides: optional ⇒ emit a `pnpm:skipped-optional-dependency` event (`reason: build_failure`, `package.id` = the dep's pkg dir to match upstream's `depNode.dir`) and continue; non-optional ⇒ propagate as `BuildModulesError::LifecycleScript` so the install aborts. Two `cfg(unix)`-gated tests cover both branches. |
||
|
Khải
|
bbfa18f12b |
feat: bin (#333)
Resolves #330.
Mirrors pnpm v11's `@pnpm/bins.resolver`, `@pnpm/bins.linker`, and `@zkochan/cmd-shim` (resolved at pnpm `4750fd370c` and cmd-shim `0d79ca9534`):
- New `pacquet-cmd-shim` crate parses the `bin` field of a package manifest into commands. Both `bin: string | object` and the `directories.bin` glob fallback are supported, with the same path-traversal and URL-safe-name guards as upstream. Conflict resolution follows `pkgOwnsBin` + lexical compare.
- Shim generator writes all three flavors per bin: Unix `/bin/sh` (byte-compatible with pnpm's `generateShShim`), Windows `.cmd` (CRLF, `%~dp0` paths), and PowerShell `.ps1` (`$basedir`/`$exe` header with Windows/POSIX-pwsh detection). Mirrors `generateCmdShim` and `generatePwshShim` minus the unused `nodePath`/`prependToPath`/`progArgs` branches.
- All filesystem IO behind per-capability DI traits — `FsReadHead`, `FsReadFile`, `FsReadString`, `FsReadDir`, `FsWalkFiles`, `FsCreateDirAll`, `FsWrite`, `FsSetExecutable`, `FsEnsureExecutableBits` — following the pattern documented in [#332](https://github.com/pnpm/pacquet/pull/332#issuecomment-4345054524). `FsWrite` is a thin shim over `std::fs::write` and is **not atomic**; a future atomic-write capability can build on it without renaming. Production callers turbofish the unit-struct provider `::<RealApi>`; tests inject unit-struct fakes to cover IO error paths real fs can't trigger portably. Generic param is named `Api` (not `Fs`) so the same provider can grow non-fs capabilities without a rename.
- New `LinkVirtualStoreBins` step walks each virtual-store slot and links its child packages' bins into the slot's own `node_modules/.bin`, matching `linkBinsOfDependencies` in pnpm's `building/during-install`. Slot iteration, per-slot child reads, and shim writes parallelised on rayon.
- `SymlinkDirectDependencies` and `InstallWithoutLockfile` call the bin linker after the symlink layout is in place. Direct-dep linking and the symlink loop are exposed as free `link_direct_dep_bins` / `symlink_direct_deps_into_node_modules` functions so they're unit-testable without the mock-registry scaffold.
Tests port the relevant `bins/resolver` and `bins/linker` cases from pnpm (`directories.bin` discovery, scoped bin names, dangerous locations, idempotent shim skip, conflict resolution) and add fakes-injected coverage for the IO error variants.
**Not in this PR**: hoisted-bin precedence and lifecycle-script-created bins. Both depend on subsystems pacquet hasn't built yet — hoisting (`hoistPattern` / `publicHoistPattern` resolution + the lift step) and lifecycle scripts (`exec/lifecycle/` runner + `allowBuilds` plumbing). The bin-linking side already does the right thing for both (shims are paths, so they pick up files generated post-extract; the precedence rule is a small addition once hoisting tags candidates as hoisted). Tracked separately in with full porting coordinates.
## Performance
When the bin-linking step landed it added ~7% to the warm-cache Frozen Lockfile install ([benchmark](https://github.com/pnpm/pacquet/pull/333#issuecomment-4344568317)). Closing that gap took a sequence of changes that mirror how pnpm v11 avoids the same costs in its own `linkBinsOfDependencies` ([
|
||
|
Khải
|
a8e8dd3091 |
ci: setup dylint with perfectionist (#416)
Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Allan Kimmer Jensen
|
d685005de9 |
feat: lifecycle script execution and allowBuilds policy (#391)
## Summary - Add lifecycle script runner to `pacquet-executor`: preinstall, install, postinstall in correct order, `binding.gyp` auto-detection, `INIT_CWD`/`PNPM_SCRIPT_SRC_DIR`/PATH environment setup - Add `allowBuilds` build policy matching pnpm 11's default-deny behavior, with `dangerouslyAllowAllBuilds` escape hatch - Wire build step into the frozen-lockfile install path - Port `buildSequence` + `graphSequencer` so packages with `requires_build` run children-first - Add `pnpm:lifecycle` and `pnpm:ignored-scripts` reporter channels — script start/stdio/exit events flow through the same NDJSON pipeline `@pnpm/cli.default-reporter` consumes from upstream pnpm - Port 11 upstream lifecycle script tests as `known_failures` Upstream refs: - [`runPostinstallHooks`](https://github.com/pnpm/pnpm/blob/7e91e4b35f/exec/lifecycle/src/index.ts) - [`runLifecycleHook`](https://github.com/pnpm/pnpm/blob/80037699fb/exec/lifecycle/src/runLifecycleHook.ts) - [`createAllowBuildFunction`](https://github.com/pnpm/pnpm/blob/7e91e4b35f/building/policy/src/index.ts) - [`buildSequence`](https://github.com/pnpm/pnpm/blob/80037699fb/building/during-install/src/buildSequence.ts) - [`graphSequencer`](https://github.com/pnpm/pnpm/blob/80037699fb/deps/graph-sequencer/src/index.ts) - [`lifecycleLogger`](https://github.com/pnpm/pnpm/blob/80037699fb/core/core-loggers/src/lifecycleLogger.ts) - [`ignoredScriptsLogger`](https://github.com/pnpm/pnpm/blob/80037699fb/core/core-loggers/src/ignoredScriptsLogger.ts) ## What's implemented - `run_postinstall_hooks` in `pacquet-executor` faithfully ports pnpm's hook runner. The hook is now generic over `R: Reporter` and emits `pnpm:lifecycle` events through the standard pacquet reporter pipeline: `Script` before each spawn, one `Stdio` event per stdout/stderr line read through `Stdio::piped()` byline buffering, and `Exit` after the script returns - `AllowBuildPolicy` reads `pnpm.allowBuilds` and `pnpm.dangerouslyAllowAllBuilds` from `package.json` with correct tri-state semantics (allow/deny/ignored) - Default-deny matches pnpm 11: packages must be explicitly listed in `allowBuilds` to run their scripts - `BuildModules::run::<R>` runs lifecycle scripts for `requires_build: true` packages in the frozen-lockfile path and returns the sorted (peer-stripped) list of packages whose scripts were skipped because they weren't in `allowBuilds`. `InstallFrozenLockfile::run` emits one `pnpm:ignored-scripts` event with that list (always, even when empty), mirroring upstream - `build_sequence` walks the dep graph from each importer, keeps only nodes whose subtree contains a build node, and feeds that subgraph into `graph_sequencer` to produce topologically ordered chunks. `BuildModules` iterates the chunks in order so children build before parents - `AllowBuildPolicy` and `BuildModules.lockfile_dir` use the existing `requester` field (derived from `manifest.path().parent()`) instead of `std::env::current_dir()` - New `LogEvent::Lifecycle` and `LogEvent::IgnoredScripts` variants with wire-shape tests - Recording-fake reporter tests for `run_postinstall_hooks` (Script→Stdio→Exit ordering, non-zero exit propagation) and for `BuildModules::run` (sorted ignored-builds, explicit-deny exclusion). Existing `install_emits_pnpm_event_sequence` updated to expect the new `IgnoredScripts` event between `Stats` and `ImportingDone` - Unit tests for the build policy, key parsing, `graph_sequencer`, and `build_sequence` --------- Co-authored-by: Zoltan Kochan <z@kochan.io> |
||
|
Khải
|
ef0fe2c01b |
docs(cli): fix long and wrong help messages (#405)
* docs(cli): trim verbose --reporter doc comments The doc comments on `CliArgs.reporter` and `ReporterType` had grown into multi-paragraph explanations with markdown backticks, internal-attribute notes (`global = true`), and GitHub issue references that all leaked straight into `pacquet --help` output. Match pnpm's terse style: a single short line on the field, no enum-level prose. Per-variant docs already describe what `ndjson` and `silent` do. * docs: re-add docs with improvement * docs: more specific message Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Khải
|
0df762b02c |
feat: propagate pnpm-workspace.yaml load errors (#404)
Resolves <https://github.com/pnpm/pacquet/issues/402>. ## What this PR does Pacquet already reads `pnpm-workspace.yaml` (`crates/npmrc/src/workspace_yaml.rs` defines `WorkspaceSettings` and `Npmrc::current` walks up from cwd to layer it on top of `.npmrc`). The remaining divergence from pnpm 11 was on the error path. Upstream's [`workspace-manifest-reader`](https://github.com/pnpm/pnpm/blob/8eb1be4988/workspace/workspace-manifest-reader/src/index.ts) silently treats `ENOENT` as "no manifest" and propagates every other failure (read errors, parse errors, `EISDIR`, permission denied, …). Pacquet swallowed all errors via `if let Ok(Some(...))`, and its `find_workspace_manifest` helper used an `is_file()` filter that also masked non-regular entries (e.g. a directory named `pnpm-workspace.yaml`). This PR closes both gaps. ## Changes - `crates/npmrc/src/workspace_yaml.rs` - Replace the bare `Debug`-only `LoadWorkspaceYamlError` with a `Display + Error + Diagnostic` enum that stores `path` + `source` directly on each struct variant, mirroring `pacquet_modules_yaml::ReadModulesError`. Only `serde_saphyr::Error` is boxed (`io::Error` fits the variant inline). The single-place `path` formatting keeps the miette `Caused by:` chain free of duplication. - Rewrite `find_and_load` as a single read-and-catch loop over `start_dir.ancestors()`: at each level read `pnpm-workspace.yaml`, treat only `ErrorKind::NotFound` as "no manifest here, walk up", and propagate every other error through `LoadWorkspaceYamlError`. This matches pnpm's `readManifestRaw` semantics, closes the previous TOCTOU window between the old `is_file()` check and the read, and surfaces non-regular entries (directory, EISDIR, permission denied) as hard errors instead of silently walking up. - `crates/npmrc/src/lib.rs` - `Npmrc::current` is now `Result<Self, LoadWorkspaceYamlError>`. The `.npmrc` parsing stays best-effort; the yaml parsing fails loud. - Two regression tests added in this round: `invalid_workspace_yaml_propagates_error` (parse error) and `find_propagates_when_manifest_path_is_a_directory` (`EISDIR`-class error). Existing tests adopt the fallible signature with `.expect(...)`. - `crates/cli/src/cli_args.rs` - The `npmrc` closure now returns `miette::Result<&'static mut Npmrc>` and is wrapped with `wrap_err("load configuration")`. The `state` closure threads `?`. - `crates/cli/src/cli_args/store.rs` - `StoreCommand::run` accepts `impl FnOnce() -> miette::Result<&Npmrc>` so `Prune` and `Path` get the new error path; `Store` and `Add` still panic before loading config. - `crates/npmrc/Cargo.toml` - Add `derive_more` and `miette` deps for the new error type. |
||
|
Zoltan Kochan
|
bf1937bebf |
fix: render NDJSON output cleanly through @pnpm/default-reporter (#403)
* fix(cli): canonicalize `--dir` so reporter `prefix` matches subprocess `process.cwd()` `pacquet --reporter=ndjson` piped into `@pnpm/cli.default-reporter` emitted every progress / stats line with a `. | ` path prefix because the bunyan-envelope `prefix` field was the literal `"."` (derived from the manifest's parent of `./package.json`). The reporter compares each event's `prefix` to its own `cwd` (an absolute, canonicalized path from `process.cwd()`), and the mismatch routed every log through `zoomOut`. Canonicalize `--dir` once at the top of `CliArgs::run`, mirroring pnpm's `config/reader/src/index.ts:270` where `cwd = fs.realpathSync(betterPathResolve(cliOptions.dir ...))` becomes `pnpmConfig.dir` and is threaded into every event's `prefix` as `lockfileDir`. Ports the same realpath behavior to pacquet so a default-reporter subprocess and pacquet agree on the project root. Side effect: `pacquet init` now derives the package `name` from the working-directory basename (matching pnpm) instead of leaving it as `""` — the `.` parent had no `file_name`. Snapshot updated. * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(tarball): emit `pnpm:fetching-progress started` with one-indexed `attempt` The default reporter's `reportBigTarballsProgress` filters started events with `log.attempt === 1` so a transient retry doesn't reset the "Downloading <pkg>: …/<size>" progress line. Pacquet's loop counter is zero-indexed and was being passed straight through, so every started event was emitted with `attempt: 0` and the reporter silently dropped the entire channel — no big-tarball progress lines ever rendered. Pnpm's underlying `node-retry` `op.attempt(cb)` callback hands `cb` a 1-indexed counter, which `packageRequester` forwards verbatim into the log. Match that wire shape by emitting `attempt + 1` at the fetching-progress site, mirroring the same `+ 1` adjustment already present at the neighbouring `pnpm:request-retry` emit. Updates the recording-fake test to assert `[1, 2]` (one-indexed) and fixes a stray `attempt: 0` in the reporter serialization test. * test(cli): add NDJSON `prefix` regression test, refine review nits Addresses three review comments on #403: * Add an integration test (`install_emits_canonical_prefix_in_ndjson_events`) that runs `pacquet --reporter=ndjson install` and asserts every event's `prefix` is the canonicalized workspace path — never `"."`. Pins the user-visible regression: the original failure mode was the default-reporter subprocess prepending `<prefix> | ` to every progress / stats line because the emitted `"."` never matched its own `process.cwd()`. * Reword the canonicalize-failure context to reference the `--dir` argument explicitly so a stat / permission failure points the user at the flag they passed. * Restore the `assertion_line` field on `init__should_create_package_json.snap` so its metadata matches the surrounding insta snapshots. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Allan Kimmer Jensen
|
c5dd6b5cd3 |
refactor: extract all inline test modules into dedicated files (#369)
* refactor: extract all inline test modules into dedicated files
Move every inline `#[cfg(test)] mod tests { ... }` block across the
workspace into external `tests.rs` files and update CODE_STYLE_GUIDE.md
to require external test files unconditionally (no threshold).
28 files extracted across 8 crates: store-dir, package-manager, fs,
package-manifest, lockfile, npmrc, cli, registry.
Closes #358.
* fix(package-manager): relocate snapshot to match new test file location
When `install::tests::should_install_dependencies` moved from the
inline `mod tests` in `install.rs` to the external
`install/tests.rs`, insta's snapshot lookup directory shifted from
`crates/package-manager/src/snapshots/` to
`crates/package-manager/src/install/snapshots/`. CI failed on every
platform because no snapshot existed at the new location. Move the
existing `.snap` file to its new home.
* test: address review feedback on extracted test files
Apply the actionable review comments on the test-extraction PR:
- save_lockfile: replace hardcoded `/nonexistent-pacquet-dir/...`
with a tempdir-based missing path so the invalid-path test is
deterministic across environments; add diagnostic message to the
`matches!` assertion.
- snapshot_dep_ref: convert `looks_like_alias_rules` to a
table-driven `assert_eq!` form with per-case logging.
- npmrc/workspace_yaml: compare `StoreDir` values directly instead
of `display().to_string()`, which produces backslashes on Windows
and would fail the `apply_overrides_npmrc_defaults` test there.
- package-manager/install: clear the static `EVENTS` mutex at the
start of `install_emits_pnpm_event_sequence` so retries don't
inherit stale events; add per-path logging before the
`is_symlink_or_junction` / `exists` / `is_dir` assertions.
- package-manager/link_file: same `EVENTS.clear()` defense in
`log_method_once_emits_first_call_per_method_only`; add
diagnostics before nlink and symlink-metadata assertions.
- package-manager/build_snapshot: log the relevant fields before
the `matches!` and `is_none` assertions.
- package-manager/install_package_from_registry: log
`virtual_store_path` state before the `is_dir` assertion.
- package-manifest: assert the actual return value of
`manifest.script("test", false)` (was `Some("echo")`) and
`manifest.script("invalid", true)` (was `None`); add `dbg!` /
`eprintln!` diagnostics around the bare `assert!` checks per
CODE_STYLE_GUIDE.md.
- store-dir/check_pkg_files_integrity: add `dbg!(&result)` and
path-existence logging before each non-`assert_eq!` assertion
across all 11 tests.
---------
Co-authored-by: Zoltan Kochan <z@kochan.io>
|
||
|
Khải
|
8679cc8a54 |
refactor: no star imports (#364)
https://claude.ai/code/session_011yRgFgrJbtbneNyrvTJp32 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Zoltan Kochan
|
a8840772f7 |
feat(reporter): add NDJSON reporting engine with pnpm:stage smoke test (#349)
* feat(reporter): add NDJSON reporting engine with pnpm:stage smoke test Introduces `pacquet-reporter` carrying the typed `LogEvent` enum, the no-`self` `Reporter` capability trait per #339, and `NdjsonReporter` / `SilentReporter` implementations. Adds the `--reporter=ndjson|silent` flag and threads a `R: Reporter` generic through `Install::run`, emitting `pnpm:stage` `importing_started` / `importing_done` to bracket the install — one channel wired end-to-end as a smoke test for the schema-mirror approach. The wire format mirrors what `@pnpm/core-loggers` defines and `@pnpm/cli.default-reporter` consumes under `pnpm --reporter=ndjson`. Engine only. The JS-reporter spawn-and-pipe (`--reporter=default`), the backfill of missing log events in already-ported code (#347), and the porting convention in AGENTS.md (#346) land in follow-up PRs. The default reporter is `silent` until the spawn-and-pipe exists, so user- facing output is unchanged. Refs #345. Design: #344. * docs(package-manager): name pnpm's `lockfileDir` and link findWorkspaceDir Upstream emits stage events with `prefix: lockfileDir` — the directory holding `pnpm-lock.yaml`, which collapses to the workspace root when a workspace is present. The previous comment described the *result* in pacquet's no-workspace world but didn't name the upstream concept or point at the helper to mirror once workspaces land. Replacing the comment with a TODO that links `findWorkspaceDir` makes the migration path explicit so the next person editing this site doesn't have to re-derive it. No behavior change. * fix(cli): make --reporter accepted on either side of the subcommand Adding `global = true` to the clap attribute lets users write either `pacquet --reporter=ndjson install` or `pacquet install --reporter=ndjson`, matching pnpm. Without it the flag is parsed by the top-level `CliArgs` only, so a sub-command-side use ("pacquet install --reporter=...") produced "unexpected argument '--reporter' found". * refactor(reporter,package-manager): apply review fixes from #349 Address feedback on the reporter PR. reporter: - Drop the dependency-injection issue link (#339) from `Reporter`, the recording-fake test, and the module-level docs; convention is visible from the code. - Wrap `bunyan` and `bole` references as markdown links. - Demote `Envelope`'s `#[serde(flatten)]` rationale to a regular `//` comment so private impl detail isn't part of `///` rustdoc. - Introduce `pub trait EnvVar` + `pub struct RealEnvVar` (reporter- local capability trait) and split `hostname` into a generic `resolve_hostname<E>` plus a non-generic cached production wrapper. Add four unit tests covering `HOSTNAME`, `COMPUTERNAME` fallback, precedence, and the unset case. - Switch the wire-shape test to `pipe-trait`; add `pipe-trait` to dev-dependencies. - Fix intra-link rule violation in the `SilentReporter` test doc. package-manager: - Tighten the install `prefix` extraction to fail loudly on non-UTF-8 / parentless manifest paths instead of silently emitting an empty string. Workspace-aware resolution tracked at #357. - Replace `concat!` YAML literals in two tests with `text_block_macros::text_block!`; add as dev-dependency. - Convert bare-prose identifiers in `install_emits_stage_events_bracketing_the_run` to intra-doc links; drop the recording-fake DI prose. Long inline `mod tests` extraction tracked at #358. * fix(reporter): drop private-item link from `EnvVar` public doc `EnvVar` is `pub` but `resolve_hostname` is private; rustdoc rejects the intra-link with `-D rustdoc::private-intra-doc-links` (the rule added to `CODE_STYLE_GUIDE.md` in #351). Rephrase the doc to talk about "env-dependent helpers" generally without naming the private function. * refactor(reporter): switch hostname to gethostname syscall + DI capability Address the second round of review feedback on #349. reporter: - Replace env-var-based hostname (HOSTNAME / COMPUTERNAME) with the real gethostname(2) syscall via the `gethostname` crate. The env-var approach was unsound: shells set `HOST` (zsh) or `HOSTNAME` (bash) as private variables that don't propagate to spawned binaries, so a packaged `pacquet` would have observed an empty hostname in most production environments. - Introduce `pub trait GetHostName` + `pub struct RealApi`. Real resolution lives in `RealApi::get_host_name`. Tests can supply their own impl. - Cache the production value in `static HOSTNAME: LazyLock<String> = LazyLock::new(RealApi::get_host_name)`. The wire emit path reads `&HOSTNAME` directly; the previous `hostname()` wrapper is no longer needed. - Drop the now-redundant `EnvVar` trait, `RealEnvVar`, `resolve_hostname`, and the four env-resolution unit tests (those scenarios no longer exist after the syscall switch). reporter test layout: - Move the tests module into its own file at `crates/reporter/src/tests.rs` per the unit-test-layout rule in `CODE_STYLE_GUIDE.md`. Drop `use super::*` in favor of explicit merged imports of just the items the tests touch. - Two new tests: `get_host_name_capability_is_mockable` (proves the trait dispatch works for fakes) and `real_api_returns_a_non_empty_host_name` (smoke-checks the real syscall path). Documentation style: - Restructure the workspace-prefix and stage-bracketing comments to drop em dashes per `CONTRIBUTING.md`'s writing-style guidance and to render `bunyan` as a markdown link. - Fix the `@pnpm/core-loggers` link to point at the package directory rather than its `src` subdirectory. Imports: - Replace seven occurrences of `pacquet_reporter::SilentReporter` qualified at the call site with a single `use pacquet_reporter:: SilentReporter` in the test module. |
||
|
Khải
|
51571b78c5 |
chore(rust): edition 2024 (#322)
* chore(workspace): switch to Rust edition 2024
Bump `edition` to 2024 in the workspace package metadata. The toolchain
is already pinned at 1.95.0 (latest stable as of 2026-04-14), which has
2024 edition support. Migration touches:
- collapse `if let { if let { … } }` chains into `let … && let …` to
satisfy the new `clippy::collapsible_if` shape under edition 2024
(let-chains are stable in 2024).
- wrap `env::set_var` / `env::remove_var` calls in `unsafe { … }`
blocks now that they require an `unsafe` context. The test-only
`EnvGuard` already serializes env mutation against the rest of the
test process via a process-wide `Mutex`, so the safety condition
("no other thread accesses the env concurrently") still holds; the
unsafe markers just make that contract explicit.
- bind a `Client` to a named local in `AutoMockInstance::load_or_init`
so it outlives the borrow inside `MockInstanceOptions`. Edition 2024
shortens temporary lifetimes in `let` initializers, so the previous
`&Client::new()` inside a struct literal would have dangled.
- replace `prefetched.get(&index_key).is_none()` with
`!prefetched.contains_key(&index_key)` per
`clippy::unnecessary_get_then_check`.
The remaining diff is `cargo fmt` reflowing imports under the 2024
style edition (alphabetical ordering inside grouped `use { … }`
blocks).
* fix(npmrc): collapse Windows get_drive_letter `if let` chain
The `clippy::collapsible_if` lint also fires on the Windows-only
`get_drive_letter` helper, which was missed by the Linux/macOS clippy
pass during the edition 2024 migration. Apply the same `if let … &&
let …` collapse the lint suggested.
* style(lockfile): re-sort imports under 2024 style edition
Files added by `refactor: replace serde_yaml with serde_saphyr` (#320)
on main use the 2021 import-name ordering (`{serialize_yaml, Lockfile}`,
`{ser, ..., SerializerOptions}`). The 2024 style edition sorts use names
alphabetically, so the PR's merge ref tripped `cargo fmt --all -- --check`
once main caught up.
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Zoltan Kochan
|
e7ce0b9155 |
perf: batch store-index prefetch + drive warm link phase on rayon (#292)
## Summary
Two structural changes that together cut warm-cache install wall time on a **1352-package lockfile** from ~10.85s → ~6.5s, beating pnpm v11.0.0-rc.5 (~8.5s on the same machine) by ~23%.
| | median wall |
|---|---|
| **this PR** | **6.52s** |
| pnpm v11.0.0-rc.5 | 8.51s |
| `main` baseline | 10.85s |
Measured on macOS (10-core M-class, APFS, store on same volume), warm cache, 8 runs each, `ulimit -n 65536`.
## Changes
### 1. Batched store-index prefetch (`crates/tarball/src/lib.rs::prefetch_cas_paths`)
Walks every `(integrity, pkg_id)` the lockfile mentions in one `spawn_blocking` task at install start, runs all integrity checks, and returns a `cache_key → cas_paths` HashMap. `DownloadTarballToStore` consults this map before falling through to the per-call `load_cached_cas_paths`.
This eliminates 1352 separate `spawn_blocking` round-trips a warm install was doing. Sample-profiling showed those each accumulating 20-60 ms of OS scheduling overhead while their actual work (≈40 µs SELECT + per-file integrity stats) is sub-ms — the queue across the default 512-thread blocking pool was dwarfing the work itself.
### 2. Warm batch via one global `rayon::par_iter` (`crates/package-manager/src/create_virtual_store.rs`)
Partition snapshots into "warm" (prefetched) and "cold" (cache miss). The warm batch bypasses tokio futures entirely:
```rust
warm.par_iter().try_for_each(|(snapshot_key, snapshot, cas_paths)| {
CreateVirtualDirBySnapshot { ... }.run().map_err(...)
})?;
```
The cold batch keeps the existing `try_join_all` + download path (rare on warm install, common on cold).
**Why this matters:** previously every per-snapshot tokio future called sync `rayon::join` from a tokio worker. With up to 10 such futures progressing concurrently and each one's inner `par_iter` saturating the rayon pool, the pool ended up processing **one snapshot at a time** despite tokio's worker count — `sum-of-link ≈ wall`, i.e. effectively 1× parallelism. Going straight to one big `par_iter` lets rayon schedule across all 1352 snapshots as one work-stealing graph, the shape pnpm's piscina pool gives implicitly.
### 3. Rayon pool sized at `availableParallelism * 2` (`crates/cli/src/lib.rs::configure_rayon_pool`)
The link phase is dominated by `clonefile` syscalls that block on the kernel's metadata journal, not CPU work. Oversubscribing CPUs gives more in-flight syscalls without losing wall time. Sweep on alot7:
| RAYON_NUM_THREADS | wall |
|---|---|
| 4 | 9.5s |
| 10 (default) | 9.4s |
| **20** | **9.25s** |
| 30 | 10.0s |
| 50 | 9.4s |
| 100 | 10.2s |
2x was the knee. Honours an explicit `RAYON_NUM_THREADS` env var.
|
||
|
Zoltan Kochan
|
00e1d3da2f |
perf(network,tarball): port pnpm v11 HTTP topology for tarball fetch (#281)
* perf(network,tarball): port pnpm v11 HTTP topology for tarball fetch Three HTTP/fetch-layer ports from pnpm v11, tracked in #280. All three are decisions pnpm landed upstream with explicit benchmark evidence, and all three are cases where a default `reqwest::Client` diverges from pnpm's chosen shape. 1. **HTTP/1.1 only** (`.http1_only()`). A default `reqwest::Client` negotiates HTTP/2 via ALPN whenever the registry advertises it, and registry.npmjs.org does. Pnpm v11's `network/fetch/src/dispatcher.ts:17-22` documents why they disabled H2: > With HTTP/2, undici multiplexes many streams over 1-2 TCP > connections sharing a single congestion window. In benchmarks > this was slower than opening ~50 independent HTTP/1.1 > connections that each get their own congestion window and can > saturate bandwidth in parallel. 2. **50 concurrent sockets**, matching pnpm's `DEFAULT_MAX_SOCKETS`. The previous `num_cpus.max(16)` semaphore under-subscribed on every machine benchmarked — on a 4-core GHA runner pacquet had 1/3 of pnpm's concurrent-fetch budget; even on a 10-core M3 the floor of 16 left bandwidth on the table. 3. **Pre-allocate the tarball buffer from `Content-Length`**. Ports `fetching/tarball-fetcher/src/remoteTarballFetcher.ts:148-164`. The old `response_head.bytes().await` let reqwest/hyper grow an internal `BytesMut` by doubling when CL wasn't used to pre-size — on a 1352-tarball cold install that's a lot of wasted realloc + copy per tarball. Switch to `bytes_stream()`, check `content_length()` on the response head, pre-allocate a `Vec<u8>::with_capacity(cl)` when known, and `extend_from_slice` each chunk in sequentially. Also catches size-mismatch via a new `TarballError::BadTarballSize` variant — pnpm surfaces the equivalent `BadTarballError` for the same case. An upstream bug or middleware truncation that produces fewer (or more) bytes than `Content-Length` now fails fast at the byte-accounting layer instead of silently succeeding until the ssri integrity check fails with a less-specific diagnostic. Requires adding `reqwest`'s `stream` feature (for `bytes_stream`) and pulling `futures-util` into the tarball crate (for `StreamExt::next`). Both are already in the workspace; this just scopes them to the tarball crate. Items 4 (post-download concurrency cap) and 5 (hardware SHA-512) from #280 are deliberately deferred: (4) wants Apple Silicon before/after numbers since the current cap was picked for a Linux CI failure, (5) wants a macOS profile to confirm SHA-512 is actually in the hot path before a crypto-crate swap. * style(cargo): reflow taplo-aligned columns Taplo recomputes column widths across the file when anything else in the block changes length. The 7ddb561 edit added `stream` to reqwest's feature list (longer line than the previous max in Cargo.toml) and `futures-util` to pacquet-tarball (a new longest name in that block), so taplo tightens the comment padding on `sha2` and widens the trailing padding on the surrounding deps. Ran `just fmt`; no logic changes. * fix(network,tarball): OOM-safe buffer alloc; rename constructor Three review fixes for #281: 1. **OOM protection on `Content-Length`.** Replaced the infallible `Vec::with_capacity(size as usize)` with a new helper, `allocate_tarball_buffer`, that guards the header as untrusted input. Two layers: * `usize::try_from(size)` — a `u64` CL may exceed `usize::MAX` on 32-bit targets. * `Vec::try_reserve_exact(capacity)` — refuses the allocation gracefully when memory pressure or an absurd claim would otherwise abort the process via the infallible path. Both failures surface as a new `TarballError::TarballTooLarge` variant so the install can reject the one offending package and continue instead of OOM-killing the whole process. 2. **Rename `ThrottledClient::new_from_cpu_count` → `new_for_installs`.** The previous commit dropped CPU-count sizing in favor of a fixed 50-socket cap matching pnpm's `DEFAULT_MAX_SOCKETS`, so the old name described a behavior the method no longer has. `new_for_installs` reads as "the default client tuned for pacquet install traffic", which is what it actually returns. Callers updated: `package-manager`'s `install_package_from_registry`, `cli::state`, and the `micro-benchmark` task. 3. **Dropped `BadTarballSize` variant and its checks.** Reviewer asked for a test exercising the variant; on closer look the checks can't actually fire behind reqwest/hyper, which enforces `Content-Length` framing on the receive side itself (a body shorter than CL errors the stream before our code sees a chunk sequence ending early; a body longer gets truncated or queued as the next request on a keep-alive connection). Pnpm's equivalent `BadTarballError` exists because undici in Node.js doesn't always enforce this. Keeping the variant as dead defense-in-depth on our side would just mean an unreachable branch and an untestable error path, so remove both. Adds three unit tests for `allocate_tarball_buffer` covering the chunked-transfer fallback, reasonable pre-sizing, and the `u64::MAX`-rejection case. The rejection test exercises both guards depending on target word size: 64-bit targets pass `try_from` and fail on `try_reserve_exact`; 32-bit targets fail on `try_from`. Either way the observable result is `TarballTooLarge`. |
||
|
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
|
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
|
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
|
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.
|
||
|
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
|
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 |
||
|
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
|
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
|
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
|
222bc2ce1b |
feat: store dir structure compatible with pnpm (#166)
Resolves https://github.com/pnpm/pacquet/issues/165 **Other changes:** * A new crate named `pacquet-store-dir` has been created. * The field `store-dir` in `Npmrc` has been changed from `PathBuf` to `StoreDir`. * The function `write_sync` in `pacquet-cafs` has been replaced by 2 functions ( |
||
|
Khải
|
36fee01f64 |
test: define custom .npmrc and snapshot the store (#169)
|
||
|
Khải
|
0db25f85a2 |
refactor: re-organize code (#143)
## Summary * Move code that handles package managing to its own crate named `pacquet_package_manager`. * Convert functions with multiple arguments into structs. * Make `pacquet add` reuses the algorithm from `pacquet install`. * Achieved by having `pacquet add` creates an in-memory `package.json` and pass it to the `Install` subroutine. * Convert the error-prone tests in `crates/cli` into proper integration tests that invoke the `pacquet` binary. * Tests become shorter and more readable. * Solve race condition issues with `cargo test`. * All code with `set_current_dir` have been replaced by either integration tests or dependency injection. * Solve race condition issues with `cargo test`. * `thiserror` has been completely removed. `derive_more` is used in its stead. * `thiserror` cannot handle generic trait bound. * `derive_more` is already used to generate custom string types in the lockfile. * one less macro crate leads to slightly faster compile time. * `pacquet_package_json::PackageJson` has been renamed to `pacquet_package_manifest::PackageManifest`. * Other minor changes. |
||
|
Khải
|
768e4eb39b |
ci: clippy and doc on all platforms (#153)
* ci: clippy and doc on all platforms * ci: fewer jobs * chore(cargo): remove unnecessary aliases * chore(just): add --locked to lint * fix(clippy): windows * fix(clippy): windows * ci(clippy): add --locked * fix(clippy): windows * fix(ci): windows |
||
|
Nacho Aldama
|
2ddd6cf8d3 |
feat: add support for CoW in Win Dev Drives (#141)
* feat: add support for CoW in Win Dev Drives * fix: use junctions when symlinks fail * fix: create junction only for error `1314` * style: format code * perf: use always junctions in Windows * feat: use current drive for default store path * fix: add missing `os` import * fix: fix lint errors * test: add tests for store path in Windows * docs: explain why we use junctions in windows * perf: check drive letter only in Windows * refactor: check for Windows OS in match condition * revert: check for Windows OS in match condition * fix: remove windows condition * fix: remove windows condition * fix: fix lint errors * refactor: change windows condition Co-authored-by: Khải <hvksmr1996@gmail.com> * style: format code * fix: check if target is symlink or junction * perf: optimize `get_drive_letter` fn * style: remove `return` * perf: return drive letter as `char` * fix: fix test error * refactor: panic when drive letter is none Co-authored-by: Khải <hvksmr1996@gmail.com> * style: format code * perf: load drive letter fn only in Windows * fix: return correct types in Windows * style: format code * fix: fix lint errors * bump `reflink-copy` version to 0.1.9 * test: improve windows store dir test * refactor: follow style guides Co-authored-by: Khải <hvksmr1996@gmail.com> * Update crates/npmrc/src/custom_deserializer.rs Co-authored-by: Khải <hvksmr1996@gmail.com> * test: test the entire store path instead of drive * test: fix Windows tests * fix: correct argument types for win store dir * test: check store dir in same drive --------- Co-authored-by: Khải <hvksmr1996@gmail.com> |
||
|
Khải
|
5ce161a552 | feat: initial lockfile implementation (#128) | ||
|
Yagiz Nizipli
|
b963f0151a | chore: release 0.2.1 | ||
|
Yagiz Nizipli
|
eaab9774cf | chore: release v0.2.0 | ||
|
Zoltan Kochan
|
e2fd0c3436 | fix: share the http client between requests (#118) | ||
|
Khải
|
95e50497ea | feat: leak Npmrc, remove cloning (#115) | ||
|
Khải
|
899dd23a13 | fix: install more packages without panics (#111) | ||
|
Ramana Venkata
|
85bce88c32 | feat: add --save-peer option to add subcommand (#103) |