From 3ddde2b975350a0cc134b210be8d981abb101e62 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 16 May 2026 23:58:53 +0200 Subject: [PATCH] fix(pacquet): align GVS slot layout with pnpm (#11689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Adds three end-to-end **GVS parity tests** under `pacquet/crates/cli/tests/pnpm_compatibility.rs` that run `pnpm install` and `pacquet install --frozen-lockfile` against the same workspace + lockfile with `enableGlobalVirtualStore: true`, then diff the resulting `/v11/links/` slot trees. The tests surfaced three independent divergences, each fixed in its own commit set: 1. **`/v11/links` prefix.** `getStorePath` appends `STORE_VERSION` (`v11`) to the configured `storeDir` before `extendInstallOptions.ts:352` joins `'links'` onto it, so pnpm's GVS lives at `/v11/links/` — pacquet's `StoreDir::links()` was one level shallower, joining onto `self.root`. Same gap on `projects()`. Anchored both under `self.v11()` so the on-disk paths agree. 2. **GVS engine-name resolution.** `ENGINE_NAME` was computed from `process.version`, which is wrong in two cases: - **`@pnpm/exe` SEA bundle.** The bundle has its own embedded Node, not the `node` on PATH that runs lifecycle scripts. Two pnpm installs on the same machine (one SEA, one npm-package) therefore disagreed on the cache key, partitioning the side-effects cache and the global virtual store. - **`engines.runtime` / `devEngines.runtime` pin.** When a project pins a Node version, pnpm downloads that Node into `node_modules/node/` and uses it to run lifecycle scripts. But the hash still anchored to whichever Node ran pnpm itself, not to the pinned Node. `@pnpm/engine.runtime.system-node-version` now exports `engineName(nodeVersion?)` and `findRuntimeNodeVersion(snapshotKeys)`. The override has priority; otherwise the helper falls through to `getSystemNodeVersion()` — which already prefers shell `node --version` over `process.version` in SEA contexts — and finally to `process.version` as a last resort. `@pnpm/deps.graph-hasher`'s `calcDepState`, `calcGraphNodeHash`, and `iterateHashedGraphNodes` accept an optional `nodeVersion`. Every install-side caller (`deps.graph-builder`, `installing.deps-resolver`, `installing.deps-restorer`, `installing.deps-installer/install/link`, `building.during-install`, `building.after-install`) derives the project's pinned runtime via `findRuntimeNodeVersion` once per invocation and forwards it. The legacy `ENGINE_NAME` constant in `@pnpm/constants` is unchanged so external consumers and existing tests keep working. Pacquet mirrors this with `find_runtime_node_major` in `install_frozen_lockfile.rs` — it scans the lockfile's `snapshots:` map for a `node@runtime:` entry and uses that major outright, only falling back to the host probe when no pin is present. 3. **Slot bin-shim layout.** Pacquet was emitting `.cmd` / `.ps1` shims on every host platform, even though pnpm only writes them on Windows ([`@zkochan/cmd-shim` `createCmdFile: isWindows`](https://github.com/pnpm/cmd-shim/blob/0d79ca9534/src/index.ts#L32) + `bins/linker`'s [`POWER_SHELL_IS_SUPPORTED = IS_WINDOWS`](https://github.com/pnpm/pnpm/blob/29a42efc3b/bins/linker/src/index.ts#L28) gate). Pacquet also excluded the slot's own package from the slot-local `node_modules/.bin/` based on a stale assumption ("which pnpm doesn't"), but pnpm's [`linkBinsOfDependencies`](https://github.com/pnpm/pnpm/blob/29a42efc3b/building/during-install/src/index.ts#L272-L298) appends `depNode` to the bin-source list unconditionally, so a leaf package like `hello-world-js-bin` writes a self-shim at `/node_modules//node_modules/.bin/`. Both behaviors now match pnpm. ## Test plan - [x] `cargo nextest run -p pacquet-cli --test pnpm_compatibility` — 5 active tests pass, 1 ignored (see below) - [x] `cargo nextest run -p pacquet-store-dir -p pacquet-config -p pacquet-cmd-shim -p pacquet-package-manager` — 600+ tests pass after the prefix / bin-shim updates - [x] `same_global_virtual_store_layout_pure_js` — pacquet & pnpm produce byte-identical `/v11/links/` trees for `@pnpm.e2e/hello-world-js-bin-parent` - [x] `same_global_virtual_store_layout_diamond` — same for `pkg-with-1-dep` + `parent-of-pkg-with-1-dep`, verifying `calc_dep_graph_hash` memoization parity - [x] Three new TS unit tests in `engine/runtime/system-node-version/test/` cover the `engineName(version)` override branch and `findRuntimeNodeVersion`'s extraction rule (with and without peer suffix) - [ ] `same_global_virtual_store_layout_with_approved_postinstall` is currently `#[ignore]`d. It requires pnpm and pacquet to agree on the `;;node` triple in the engine-included hash branch. The `pnpm/setup` action on CI installs an `@pnpm/exe` SEA bundle whose embedded Node (node26) differs from the runner's PATH `node` (node24), so the digests don't line up. The pnpm-side fix in this PR resolves `engineName()` via `getSystemNodeVersion()` which prefers the shell `node`, so once a published pnpm version with the fix reaches `pnpm/setup` the test will pass without modification — re-enable it then. The other two GVS parity tests are unaffected since they exercise the engine-agnostic branch. ## Notes - Two pacquet integration tests in `package-manager/src/install/tests.rs` had hard-coded `/projects/` assertions; updated to `/v11/projects/` to follow the prefix fix. - The `link_bins_rewrites_when_only_sh_flavor_exists` cmd-shim test is now `#[cfg(windows)]` — the upgrade-recovery scenario it exercises is meaningless on Unix where `.cmd`/`.ps1` are no longer written in the first place. - Review feedback addressed: (a) test YAML helper now guarantees a trailing newline before appending GVS keys; (b) `findRuntimeNodeVersion` calls in `installing/deps-restorer/` switched from `Object.keys(graph)` (install-dir-keyed in that module) to extracting `depPath` per node, with the computation lifted out of the recursion; (c) `dlx.e2e.ts`'s `jest.unstable_mockModule` against `@pnpm/engine.runtime.system-node-version` now forwards every exported symbol so transitive importers of `engineName` don't break. - Known caveat: pacquet's non-lockfile install path (`run_with_readdir`) still excludes the slot's own bin via `link_bins_excluding`. That path runs only for the legacy flat layout where GVS parity isn't a constraint, so it's deliberately out of scope here. - Known caveat tracked in #11690: when a dependency's own manifest declares `engines.runtime`, the resolver desugars it into a regular `dependencies.node: 'runtime:'` entry on that package, so the **deps** portion of the hash captures it on both sides. The **engine** portion is still install-wide rather than per-snapshot, so cached side-effects for dep-pinned runtimes can be reused under the wrong host Node. pnpm has this same gap today; closing it on both sides requires per-snapshot engine resolution and is outside this PR's scope. --- .changeset/gvs-engine-name-shell-node.md | 31 +++ building/after-install/package.json | 1 + building/after-install/src/index.ts | 8 + building/after-install/tsconfig.json | 3 + building/during-install/package.json | 1 + building/during-install/src/index.ts | 10 + building/during-install/tsconfig.json | 3 + deps/graph-builder/package.json | 1 + .../src/iteratePkgsForVirtualStore.ts | 30 ++- deps/graph-builder/tsconfig.json | 3 + deps/graph-hasher/package.json | 2 +- deps/graph-hasher/src/index.ts | 34 ++- .../test/calcGraphNodeHash.test.ts | 7 +- deps/graph-hasher/test/index.ts | 8 +- deps/graph-hasher/tsconfig.json | 6 +- .../runtime/system-node-version/src/index.ts | 69 ++++++ .../test/getSystemNodeVersion.test.ts | 42 +++- exec/commands/test/dlx.e2e.ts | 13 +- installing/deps-installer/package.json | 1 + installing/deps-installer/src/install/link.ts | 7 + installing/deps-installer/tsconfig.json | 3 + installing/deps-resolver/package.json | 1 + installing/deps-resolver/src/index.ts | 11 +- installing/deps-resolver/tsconfig.json | 3 + installing/deps-restorer/package.json | 1 + installing/deps-restorer/src/index.ts | 10 + .../deps-restorer/src/linkHoistedModules.ts | 20 ++ installing/deps-restorer/tsconfig.json | 3 + .../crates/cli/tests/pnpm_compatibility.rs | 227 ++++++++++++++++++ pacquet/crates/cmd-shim/src/link_bins.rs | 106 ++++---- .../crates/cmd-shim/src/link_bins/tests.rs | 76 ++++-- pacquet/crates/config/src/lib.rs | 20 +- .../package-manager/src/install/tests.rs | 15 +- .../src/install_frozen_lockfile.rs | 111 +++++++-- .../crates/package-manager/src/link_bins.rs | 64 ++++- pacquet/crates/store-dir/src/store_dir.rs | 33 ++- pnpm-lock.yaml | 24 +- 37 files changed, 858 insertions(+), 150 deletions(-) create mode 100644 .changeset/gvs-engine-name-shell-node.md diff --git a/.changeset/gvs-engine-name-shell-node.md b/.changeset/gvs-engine-name-shell-node.md new file mode 100644 index 0000000000..da97ba3b04 --- /dev/null +++ b/.changeset/gvs-engine-name-shell-node.md @@ -0,0 +1,31 @@ +--- +"@pnpm/building.after-install": patch +"@pnpm/building.during-install": patch +"@pnpm/deps.graph-builder": patch +"@pnpm/deps.graph-hasher": patch +"@pnpm/engine.runtime.system-node-version": minor +"@pnpm/installing.deps-installer": patch +"@pnpm/installing.deps-resolver": patch +"@pnpm/installing.deps-restorer": patch +"pnpm": patch +--- + +**fix**: anchor the side-effects-cache key and global-virtual-store hash to the project's script-runner Node — `engines.runtime` pin when present, shell `node` otherwise — instead of pnpm's own runtime. + +`ENGINE_NAME` (the `;;node` prefix used as the side-effects-cache key and the engine portion of the GVS hash) was computed from `process.version` — the Node that runs pnpm itself. That was wrong in two situations: + +1. **`@pnpm/exe` SEA bundle.** The bundle has its own embedded Node, not the `node` on the user's `PATH` that actually spawns lifecycle scripts. Two pnpm installations on the same machine (one SEA, one npm-package) therefore disagreed on the cache key, partitioning the side-effects cache and the global virtual store across two Node majors even though both installs would run scripts on the same shell `node`. +2. **`engines.runtime` / `devEngines.runtime` pin.** When a project pins a Node version via `devEngines.runtime` (pnpm v11+), pnpm downloads that Node into `node_modules/node/` and uses it to run lifecycle scripts. But the hash still anchored to whichever Node ran pnpm itself, not to the pinned Node — so two installs of the same project with two different runner Nodes would still disagree on the GVS slot path even though scripts run on the same pinned Node. + +Three changes: + +- `@pnpm/engine.runtime.system-node-version` now exports `engineName(nodeVersion?)` and `findRuntimeNodeVersion(snapshotKeys)`. `engineName()` resolves the version in this order: explicit override → `getSystemNodeVersion()` (which already prefers `node --version` over `process.version` in SEA contexts) → `process.version`. `findRuntimeNodeVersion` scans an iterable of lockfile snapshot keys for a `node@runtime:` entry and returns its bare version string. +- `@pnpm/deps.graph-hasher`'s `calcDepState` and `calcGraphNodeHash`/`iterateHashedGraphNodes` now accept a `nodeVersion?` (in the options bag for the first, as a trailing parameter / ctx field for the others), forwarded to `engineName()`. The default (no override) preserves the pre-change behaviour. The legacy `ENGINE_NAME` constant in `@pnpm/constants` is unchanged so external consumers and existing tests keep working; in non-SEA, non-pinned contexts every value lines up. +- Every install-side caller of the graph-hasher (`@pnpm/installing.deps-resolver`, `@pnpm/installing.deps-restorer`, `@pnpm/installing.deps-installer`, `@pnpm/building.during-install`, `@pnpm/building.after-install`, `@pnpm/deps.graph-builder`) now derives the project's pinned runtime via `findRuntimeNodeVersion(Object.keys(graph))` once per invocation and threads it through. + +On upgrade, two one-time GVS slot churns are possible: + +- **SEA-pnpm users** without a runtime pin: slots that previously hashed under the embedded-Node major (e.g. `node26`) now hash under the shell-Node major (e.g. `node24`), matching what pacquet, the npm-published `pnpm` package, and any other pnpm-compatible tool already produce. +- **Projects with a `devEngines.runtime` pin**: slots that previously hashed under the runner's Node major now hash under the pinned Node major, matching what the lifecycle scripts will actually run on. + +In both cases the old slots become prune-eligible. diff --git a/building/after-install/package.json b/building/after-install/package.json index 63886a136a..6a69ad2172 100644 --- a/building/after-install/package.json +++ b/building/after-install/package.json @@ -41,6 +41,7 @@ "@pnpm/deps.graph-hasher": "workspace:*", "@pnpm/deps.graph-sequencer": "workspace:*", "@pnpm/deps.path": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/exec.lifecycle": "workspace:*", "@pnpm/installing.context": "workspace:*", diff --git a/building/after-install/src/index.ts b/building/after-install/src/index.ts index 0f9ffc00e9..d201c3b74b 100644 --- a/building/after-install/src/index.ts +++ b/building/after-install/src/index.ts @@ -13,6 +13,7 @@ import { skippedOptionalDependencyLogger } from '@pnpm/core-loggers' import { calcDepState, type DepsStateCache, lockfileToDepGraph } from '@pnpm/deps.graph-hasher' import { graphSequencer } from '@pnpm/deps.graph-sequencer' import * as dp from '@pnpm/deps.path' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import { PnpmError } from '@pnpm/error' import { runLifecycleHooksConcurrently, @@ -281,6 +282,11 @@ async function _rebuild ( ): Promise<{ pkgsThatWereRebuilt: Set, ignoredPkgs: IgnoredBuilds }> { const depGraph = lockfileToDepGraph(ctx.currentLockfile, opts.supportedArchitectures) const depsStateCache: DepsStateCache = {} + // Resolved `engines.runtime` Node version (when one is pinned) — + // every side-effects-cache key computed below is anchored to it so + // the prefix tracks the script-runner Node rather than pnpm's own + // `process.version`. + const nodeVersion = findRuntimeNodeVersion(Object.keys(depGraph)) const pkgsThatWereRebuilt = new Set() const graph = new Map() const pkgSnapshots: PackageSnapshots = ctx.currentLockfile.packages ?? {} @@ -369,6 +375,7 @@ async function _rebuild ( sideEffectsCacheKey = calcDepState(depGraph, depsStateCache, depPath, { includeDepGraphHash: true, supportedArchitectures: opts.supportedArchitectures, + nodeVersion, }) if (pkgFilesIndex.sideEffects?.has(sideEffectsCacheKey)) { pkgsThatWereRebuilt.add(depPath) @@ -403,6 +410,7 @@ async function _rebuild ( if (!sideEffectsCacheKey) { sideEffectsCacheKey = calcDepState(depGraph, depsStateCache, depPath, { includeDepGraphHash: true, + nodeVersion, }) } await opts.storeController.upload(pkgRoot, { diff --git a/building/after-install/tsconfig.json b/building/after-install/tsconfig.json index 3badc52cb6..0216bd1f6d 100644 --- a/building/after-install/tsconfig.json +++ b/building/after-install/tsconfig.json @@ -39,6 +39,9 @@ { "path": "../../deps/path" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../exec/lifecycle" }, diff --git a/building/during-install/package.json b/building/during-install/package.json index f04f482ddd..7612b22521 100644 --- a/building/during-install/package.json +++ b/building/during-install/package.json @@ -39,6 +39,7 @@ "@pnpm/deps.graph-hasher": "workspace:*", "@pnpm/deps.graph-sequencer": "workspace:*", "@pnpm/deps.path": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/exec.lifecycle": "workspace:*", "@pnpm/fs.hard-link-dir": "workspace:*", diff --git a/building/during-install/src/index.ts b/building/during-install/src/index.ts index 8cbd4b1cc9..4715786d09 100644 --- a/building/during-install/src/index.ts +++ b/building/during-install/src/index.ts @@ -7,6 +7,7 @@ import { linkBins, linkBinsOfPackages } from '@pnpm/bins.linker' import { getWorkspaceConcurrency } from '@pnpm/config.reader' import { skippedOptionalDependencyLogger } from '@pnpm/core-loggers' import { calcDepState, type DepsStateCache } from '@pnpm/deps.graph-hasher' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import { PnpmError } from '@pnpm/error' import { runPostinstallHooks } from '@pnpm/exec.lifecycle' import { logger } from '@pnpm/logger' @@ -61,9 +62,15 @@ export async function buildModules ( } // postinstall hooks + // Resolved `engines.runtime` Node version (when the project pins + // one) so each per-snapshot side-effects-cache key reflects the + // script-runner Node. Computed once over the install-wide graph + // and threaded into [`buildDependency`] via [`buildDepOpts`]. + const nodeVersion = findRuntimeNodeVersion(Object.keys(depGraph)) const buildDepOpts = { ...opts, builtHoistedDeps: opts.hoistedLocations ? {} : undefined, + nodeVersion, warn, } const chunks = buildSequence(depGraph, rootDepPaths) @@ -151,6 +158,8 @@ async function buildDependency ( hoistedLocations?: Record builtHoistedDeps?: Record> enableGlobalVirtualStore?: boolean + /** Resolved `engines.runtime` Node version — see [`buildModules`]. */ + nodeVersion?: string warn: (message: string) => void } ): Promise { @@ -200,6 +209,7 @@ async function buildDependency ( const sideEffectsCacheKey = calcDepState(depGraph, opts.depsStateCache, depPath, { patchFileHash: depNode.patch?.hash, includeDepGraphHash: hasSideEffects, + nodeVersion: opts.nodeVersion, }) await opts.storeController.upload(depNode.dir, { sideEffectsCacheKey, diff --git a/building/during-install/tsconfig.json b/building/during-install/tsconfig.json index d1a31c7d32..4d758c2486 100644 --- a/building/during-install/tsconfig.json +++ b/building/during-install/tsconfig.json @@ -36,6 +36,9 @@ { "path": "../../deps/path" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../exec/lifecycle" }, diff --git a/deps/graph-builder/package.json b/deps/graph-builder/package.json index bb059fafc9..f175da38ef 100644 --- a/deps/graph-builder/package.json +++ b/deps/graph-builder/package.json @@ -35,6 +35,7 @@ "@pnpm/core-loggers": "workspace:*", "@pnpm/deps.graph-hasher": "workspace:*", "@pnpm/deps.path": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/hooks.types": "workspace:*", "@pnpm/installing.modules-yaml": "workspace:*", "@pnpm/lockfile.fs": "workspace:*", diff --git a/deps/graph-builder/src/iteratePkgsForVirtualStore.ts b/deps/graph-builder/src/iteratePkgsForVirtualStore.ts index 8496f7a980..5233fe5e87 100644 --- a/deps/graph-builder/src/iteratePkgsForVirtualStore.ts +++ b/deps/graph-builder/src/iteratePkgsForVirtualStore.ts @@ -11,6 +11,7 @@ import { type PkgMetaAndSnapshot, } from '@pnpm/deps.graph-hasher' import * as dp from '@pnpm/deps.path' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import type { LockfileObject } from '@pnpm/lockfile.fs' import { nameVerFromPkgSnapshot, @@ -30,15 +31,26 @@ export function * iteratePkgsForVirtualStore (lockfile: LockfileObject, opts: { globalVirtualStoreDir: string supportedArchitectures?: SupportedArchitectures }): IterableIterator { + // Resolve the project's pinned runtime Node version once per + // invocation — the result drives every snapshot's GVS hash (or + // the side-effects-cache key prefix in the non-GVS runtime + // branch). `undefined` when no `engines.runtime` / `devEngines.runtime` + // pin reached the lockfile, in which case the hasher falls through + // to the host-detected Node. + const nodeVersion = findRuntimeNodeVersion(Object.keys(lockfile.packages ?? {})) if (opts.enableGlobalVirtualStore) { - for (const { hash, pkgMeta } of hashDependencyPaths(lockfile, opts.allowBuild, opts.supportedArchitectures)) { + for (const { hash, pkgMeta } of hashDependencyPaths(lockfile, { + allowBuild: opts.allowBuild, + supportedArchitectures: opts.supportedArchitectures, + nodeVersion, + })) { yield { dirInVirtualStore: path.join(opts.globalVirtualStoreDir, hash), pkgMeta, } } } else if (lockfile.packages) { - let graphNodeHashOpts: { graph: DepsGraph, cache: DepsStateCache, supportedArchitectures?: SupportedArchitectures } | undefined + let graphNodeHashOpts: { graph: DepsGraph, cache: DepsStateCache, supportedArchitectures?: SupportedArchitectures, nodeVersion?: string } | undefined for (const depPath in lockfile.packages) { if (!Object.hasOwn(lockfile.packages, depPath)) { continue @@ -58,6 +70,7 @@ export function * iteratePkgsForVirtualStore (lockfile: LockfileObject, opts: { cache: {}, graph: lockfileToDepGraph(lockfile, opts.supportedArchitectures), supportedArchitectures: opts.supportedArchitectures, + nodeVersion, } const hash = calcGraphNodeHash(graphNodeHashOpts, pkgMeta) dirInVirtualStore = path.join(opts.globalVirtualStoreDir, hash) @@ -74,9 +87,16 @@ export function * iteratePkgsForVirtualStore (lockfile: LockfileObject, opts: { function hashDependencyPaths ( lockfile: LockfileObject, - allowBuild?: AllowBuild, - supportedArchitectures?: SupportedArchitectures + { + allowBuild, + supportedArchitectures, + nodeVersion, + }: { + allowBuild?: AllowBuild + supportedArchitectures?: SupportedArchitectures + nodeVersion?: string + } ): IterableIterator> { const graph = lockfileToDepGraph(lockfile, supportedArchitectures) - return iterateHashedGraphNodes(graph, iteratePkgMeta(lockfile, graph), allowBuild, supportedArchitectures) + return iterateHashedGraphNodes(graph, iteratePkgMeta(lockfile, graph), allowBuild, supportedArchitectures, nodeVersion) } diff --git a/deps/graph-builder/tsconfig.json b/deps/graph-builder/tsconfig.json index f4d32f77da..b1089aa3dc 100644 --- a/deps/graph-builder/tsconfig.json +++ b/deps/graph-builder/tsconfig.json @@ -24,6 +24,9 @@ { "path": "../../core/types" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../hooks/types" }, diff --git a/deps/graph-hasher/package.json b/deps/graph-hasher/package.json index 3ea64ccb18..608d44cdfe 100644 --- a/deps/graph-hasher/package.json +++ b/deps/graph-hasher/package.json @@ -31,9 +31,9 @@ ".test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules --disable-warning=ExperimentalWarning --disable-warning=DEP0169\" jest" }, "dependencies": { - "@pnpm/constants": "workspace:*", "@pnpm/crypto.object-hasher": "workspace:*", "@pnpm/deps.path": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/lockfile.types": "workspace:*", "@pnpm/lockfile.utils": "workspace:*", "@pnpm/resolving.resolver-base": "workspace:*", diff --git a/deps/graph-hasher/src/index.ts b/deps/graph-hasher/src/index.ts index fb02c0062b..9e9d12dd29 100644 --- a/deps/graph-hasher/src/index.ts +++ b/deps/graph-hasher/src/index.ts @@ -1,6 +1,6 @@ -import { ENGINE_NAME } from '@pnpm/constants' import { hashObject, hashObjectWithoutSorting } from '@pnpm/crypto.object-hasher' import { getPkgIdWithPatchHash, refToRelative } from '@pnpm/deps.path' +import { engineName } from '@pnpm/engine.runtime.system-node-version' import type { LockfileObject, LockfileResolution, PackageSnapshot } from '@pnpm/lockfile.types' import { nameVerFromPkgSnapshot } from '@pnpm/lockfile.utils' import { resolvePlatformSelector, selectPlatformVariant } from '@pnpm/resolving.resolver-base' @@ -30,9 +30,19 @@ export function calcDepState ( patchFileHash?: string includeDepGraphHash: boolean supportedArchitectures?: SupportedArchitectures + /** + * Resolved `engines.runtime` / `devEngines.runtime` Node version + * for the project being installed (e.g. `"22.11.0"`). When set, + * the side-effects-cache key reflects this script-runner Node + * rather than the Node that pnpm itself is running on — see + * {@link engineName} for the full resolution order. Typically + * computed once per install via {@link findRuntimeNodeVersion} + * over the lockfile's snapshot keys. + */ + nodeVersion?: string } ): string { - let result = ENGINE_NAME + let result = engineName(opts.nodeVersion) if (opts.includeDepGraphHash) { const depGraphHash = calcDepGraphHash(depsGraph, cache, new Set(), depPath, opts.supportedArchitectures) result += `;deps=${depGraphHash}` @@ -96,7 +106,18 @@ export function * iterateHashedGraphNodes ( graph: DepsGraph, pkgMetaIterator: PkgMetaIterator, allowBuild?: AllowBuild, - supportedArchitectures?: SupportedArchitectures + supportedArchitectures?: SupportedArchitectures, + /** + * Resolved `engines.runtime` / `devEngines.runtime` Node version + * for the project being installed. Forwarded as-is into each + * snapshot's [`calcGraphNodeHash`] call so the engine portion of + * the GVS hash reflects the Node that will actually run lifecycle + * scripts — typically obtained via [`findRuntimeNodeVersion`] + * over the lockfile's snapshot keys. `undefined` falls back to + * [`engineName`]'s default (system `node --version`, with + * `process.version` as a last resort). + */ + nodeVersion?: string ): IterableIterator> { let builtDepPaths: Set | undefined let entries: Iterable @@ -113,6 +134,7 @@ export function * iterateHashedGraphNodes ( builtDepPaths, buildRequiredCache: builtDepPaths !== undefined ? {} : undefined, supportedArchitectures, + nodeVersion, } for (const pkgMeta of entries) { yield { @@ -123,12 +145,14 @@ export function * iterateHashedGraphNodes ( } export function calcGraphNodeHash ( - { graph, cache, builtDepPaths, buildRequiredCache, supportedArchitectures }: { + { graph, cache, builtDepPaths, buildRequiredCache, supportedArchitectures, nodeVersion }: { graph: DepsGraph cache: DepsStateCache builtDepPaths?: Set buildRequiredCache?: Record supportedArchitectures?: SupportedArchitectures + /** See [`iterateHashedGraphNodes`]'s `nodeVersion` parameter. */ + nodeVersion?: string }, pkgMeta: T ): string { @@ -140,7 +164,7 @@ export function calcGraphNodeHash ( // so they survive Node.js upgrades and architecture changes. const includeEngine = builtDepPaths === undefined || transitivelyRequiresBuild(graph, builtDepPaths, buildRequiredCache ??= {}, depPath, new Set()) - const engine = includeEngine ? ENGINE_NAME : null + const engine = includeEngine ? engineName(nodeVersion) : null const deps = calcDepGraphHash(graph, cache, new Set(), depPath, supportedArchitectures) const hexDigest = hashObjectWithoutSorting({ engine, deps }, { encoding: 'hex' }) return formatGlobalVirtualStorePath(name, version, hexDigest) diff --git a/deps/graph-hasher/test/calcGraphNodeHash.test.ts b/deps/graph-hasher/test/calcGraphNodeHash.test.ts index 2246bd9738..2553e1c4d9 100644 --- a/deps/graph-hasher/test/calcGraphNodeHash.test.ts +++ b/deps/graph-hasher/test/calcGraphNodeHash.test.ts @@ -1,9 +1,14 @@ import { describe, expect, it } from '@jest/globals' -import { ENGINE_NAME } from '@pnpm/constants' import { hashObject, hashObjectWithoutSorting } from '@pnpm/crypto.object-hasher' import { calcGraphNodeHash, type DepsGraph, type DepsStateCache, type PkgMeta } from '@pnpm/deps.graph-hasher' +import { engineName } from '@pnpm/engine.runtime.system-node-version' import type { DepPath, PkgIdWithPatchHash } from '@pnpm/types' +// Track the same script-runner-Node value the production code uses +// instead of importing the legacy `ENGINE_NAME` const from +// `@pnpm/constants`. Identical in non-SEA test runs; correct in SEA. +const ENGINE_NAME = engineName() + describe('calcGraphNodeHash', () => { it('should return correct hash format for unscoped package', () => { const graph: DepsGraph = { diff --git a/deps/graph-hasher/test/index.ts b/deps/graph-hasher/test/index.ts index 3651ee4d94..f1f85d4f21 100644 --- a/deps/graph-hasher/test/index.ts +++ b/deps/graph-hasher/test/index.ts @@ -1,9 +1,15 @@ import { describe, expect, test } from '@jest/globals' -import { ENGINE_NAME } from '@pnpm/constants' import { hashObject, hashObjectWithoutSorting } from '@pnpm/crypto.object-hasher' import { calcDepState, calcGraphNodeHash } from '@pnpm/deps.graph-hasher' +import { engineName } from '@pnpm/engine.runtime.system-node-version' import type { DepPath, PkgIdWithPatchHash } from '@pnpm/types' +// Match the function the production code uses (see +// `deps/graph-hasher/src/index.ts`). In non-SEA test contexts this +// equals `process.version`-derived ENGINE_NAME, so existing assertions +// keep working; in SEA contexts it tracks the script-runner Node. +const ENGINE_NAME = engineName() + const depsGraph = { 'foo@1.0.0': { pkgIdWithPatchHash: 'foo@1.0.0' as PkgIdWithPatchHash, diff --git a/deps/graph-hasher/tsconfig.json b/deps/graph-hasher/tsconfig.json index 027b22c4c2..2f3f2385d8 100644 --- a/deps/graph-hasher/tsconfig.json +++ b/deps/graph-hasher/tsconfig.json @@ -9,15 +9,15 @@ "../../__typings__/**/*.d.ts" ], "references": [ - { - "path": "../../core/constants" - }, { "path": "../../core/types" }, { "path": "../../crypto/object-hasher" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../lockfile/types" }, diff --git a/engine/runtime/system-node-version/src/index.ts b/engine/runtime/system-node-version/src/index.ts index 138cb861f4..d779812747 100644 --- a/engine/runtime/system-node-version/src/index.ts +++ b/engine/runtime/system-node-version/src/index.ts @@ -15,3 +15,72 @@ export function getSystemNodeVersionNonCached (): string | undefined { } export const getSystemNodeVersion = mem(getSystemNodeVersionNonCached) + +/** + * The `;;node` string used as the side-effects + * cache-key prefix and the engine portion of the global-virtual-store + * hash. Identifies the runtime environment that built (or will build) + * a package's lifecycle scripts — so two installs that materialize the + * same package on the same host produce the same key. + * + * The Node version is resolved in this order: + * + * 1. `nodeVersion` argument when provided. Callers use this to thread + * a project-pinned runtime (`engines.runtime` / `devEngines.runtime`) + * through to the hash — see {@link findRuntimeNodeVersion} for the + * helper that extracts the value from a lockfile. + * 2. {@link getSystemNodeVersion} — the `node` on the user's `PATH`, + * or `process.version` when not SEA-bundled. + * 3. `process.version` as a last-resort fallback when the host has + * no `node` on `PATH` (rare: SEA pnpm with no separately-installed + * Node). Scripts cannot run in that scenario regardless, so the + * cache key is effectively unused — the fallback exists only to + * keep the value deterministic. + * + * Anchoring to a project-pinned or script-runner Node — not to pnpm's + * own `process.version` — matters most when pnpm ships via the + * `@pnpm/exe` SEA bundle, which has an embedded Node distinct from + * the one that actually runs lifecycle scripts. Without the override, + * a project with `devEngines.runtime: node@22` would still hash under + * the SEA-runner's Node major, splitting the cache across two pnpm + * installations on the same machine even though both run scripts on + * the same pinned Node. + */ +export function engineName (nodeVersion?: string): string { + const version = nodeVersion ?? getSystemNodeVersion() ?? process.version + const stripped = version.startsWith('v') ? version.slice(1) : version + const major = stripped.split('.')[0] + return `${process.platform};${process.arch};node${major}` +} + +/** + * Scan an iterable of lockfile snapshot keys for the resolved + * `engines.runtime` / `devEngines.runtime` Node version and return + * its bare version string (e.g. `"22.11.0"`), or `undefined` when + * the project doesn't pin a runtime. + * + * Pnpm's runtime resolver writes the pinned Node into the lockfile as + * a snapshot with key `node@runtime:[()]` + * (see [`engine/runtime/node-resolver/src/index.ts`](https://github.com/pnpm/pnpm/blob/29a42efc3b/engine/runtime/node-resolver/src/index.ts)). + * The first such key found is treated as authoritative — workspaces + * with conflicting pins across importers are pathological and the + * resolver rejects them before they reach the lockfile. + * + * Callers typically pass `Object.keys(lockfile.packages ?? {})` — the + * in-memory `LockfileObject` merges the on-disk `packages:` and + * `snapshots:` sections under a single `packages` field, so its keys + * include every snapshot key the install will hash. + */ +export function findRuntimeNodeVersion (snapshotKeys: Iterable): string | undefined { + const prefix = 'node@runtime:' + for (const key of snapshotKeys) { + if (!key.startsWith(prefix)) continue + // Strip peer-context suffix `(...)` — `node@runtime:22.11.0(node@22.11.0)` + // resolves to the same Node version as `node@runtime:22.11.0`, + // so peer-stripped and peer-bearing keys yield the same answer. + const versionWithPeers = key.slice(prefix.length) + const parenAt = versionWithPeers.indexOf('(') + return parenAt === -1 ? versionWithPeers : versionWithPeers.slice(0, parenAt) + } + return undefined +} diff --git a/engine/runtime/system-node-version/test/getSystemNodeVersion.test.ts b/engine/runtime/system-node-version/test/getSystemNodeVersion.test.ts index b9d6df333e..f6bf8e72ac 100644 --- a/engine/runtime/system-node-version/test/getSystemNodeVersion.test.ts +++ b/engine/runtime/system-node-version/test/getSystemNodeVersion.test.ts @@ -11,7 +11,7 @@ jest.unstable_mockModule('execa', () => ({ })), })) -const { getSystemNodeVersionNonCached } = await import('../lib/index.js') +const { getSystemNodeVersionNonCached, engineName, findRuntimeNodeVersion } = await import('../lib/index.js') const execa = await import('execa') test('getSystemNodeVersion() executed from an executable pnpm CLI', () => { @@ -35,3 +35,43 @@ test('getSystemNodeVersion() returns undefined if execa.sync throws an error', ( expect(getSystemNodeVersionNonCached()).toBeUndefined() expect(execa.sync).toHaveBeenCalledWith('node', ['--version']) }) + +test('engineName() honours an explicit nodeVersion over the host probe', () => { + // The pinned-runtime override path: when a project's + // `engines.runtime` / `devEngines.runtime` resolves to a specific + // Node version, the caller forwards it to `engineName(version)` + // and the result reflects that pinned Node — not whatever pnpm + // itself is running on. Format-stable across `v`-prefixed and + // bare versions. + const major22 = `${process.platform};${process.arch};node22` + expect(engineName('22.11.0')).toBe(major22) + expect(engineName('v22.11.0')).toBe(major22) +}) + +test('engineName() falls back to the host Node when no override is provided', () => { + // No-arg call mirrors the pre-runtime-pin behaviour: anchor to + // `getSystemNodeVersion()` (which itself prefers shell `node` over + // `process.version` only when running as a SEA bundle — covered + // by the tests above). Non-SEA test environment, so the system + // version equals `process.version`. + isSea = false + const major = process.version.replace(/^v/, '').split('.')[0] + expect(engineName()).toBe(`${process.platform};${process.arch};node${major}`) +}) + +test('findRuntimeNodeVersion() pulls the pinned major from a node@runtime: snapshot key', () => { + // Mirrors pacquet's `find_runtime_node_major` helper; both must + // agree on the version-extraction rule or the two tools would + // hash GVS slots under different engine majors for the same + // project. The peer-suffixed form must reduce to the same bare + // version as the form without a peer suffix. + expect( + findRuntimeNodeVersion(['leftpad@1.3.0', 'node@runtime:22.11.0']) + ).toBe('22.11.0') + expect( + findRuntimeNodeVersion(['node@runtime:22.11.0(node@22.11.0)']) + ).toBe('22.11.0') + expect( + findRuntimeNodeVersion(['leftpad@1.3.0', 'is-positive@3.1.0']) + ).toBeUndefined() +}) diff --git a/exec/commands/test/dlx.e2e.ts b/exec/commands/test/dlx.e2e.ts index cfb98c063c..39289a3109 100644 --- a/exec/commands/test/dlx.e2e.ts +++ b/exec/commands/test/dlx.e2e.ts @@ -6,9 +6,20 @@ import { prepareEmpty } from '@pnpm/prepare' import { DLX_DEFAULT_OPTS as DEFAULT_OPTS } from './utils/index.js' -const { getSystemNodeVersion: originalGetSystemNodeVersion } = await import('@pnpm/engine.runtime.system-node-version') +const { + getSystemNodeVersion: originalGetSystemNodeVersion, + engineName: originalEngineName, + findRuntimeNodeVersion: originalFindRuntimeNodeVersion, +} = await import('@pnpm/engine.runtime.system-node-version') +// Re-export every public symbol the package surfaces so downstream +// dynamic imports (e.g. `@pnpm/deps.graph-hasher`'s use of +// `engineName` for the GVS hash) keep working under the mock. Only +// `getSystemNodeVersion` is wrapped with `jest.fn` for spy-ability; +// the other two delegate straight back to the originals. jest.unstable_mockModule('@pnpm/engine.runtime.system-node-version', () => ({ getSystemNodeVersion: jest.fn(originalGetSystemNodeVersion), + engineName: originalEngineName, + findRuntimeNodeVersion: originalFindRuntimeNodeVersion, })) const installingCommands = await import('@pnpm/installing.commands') const { add: originalAdd } = installingCommands diff --git a/installing/deps-installer/package.json b/installing/deps-installer/package.json index c5d5b20e90..ae26ae5ace 100644 --- a/installing/deps-installer/package.json +++ b/installing/deps-installer/package.json @@ -75,6 +75,7 @@ "@pnpm/deps.graph-hasher": "workspace:*", "@pnpm/deps.graph-sequencer": "workspace:*", "@pnpm/deps.path": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/exec.lifecycle": "workspace:*", "@pnpm/fs.read-modules-dir": "workspace:*", diff --git a/installing/deps-installer/src/install/link.ts b/installing/deps-installer/src/install/link.ts index 5856b66a65..6d66f8dc0e 100644 --- a/installing/deps-installer/src/install/link.ts +++ b/installing/deps-installer/src/install/link.ts @@ -7,6 +7,7 @@ import { statsLogger, } from '@pnpm/core-loggers' import { calcDepState, type DepsStateCache } from '@pnpm/deps.graph-hasher' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import { symlinkDependency } from '@pnpm/fs.symlink-dependency' import type { DependenciesGraph, @@ -474,6 +475,11 @@ async function linkAllPkgs ( supportedArchitectures?: SupportedArchitectures } ): Promise { + // Resolved `engines.runtime` Node version (when present) so the + // side-effects-cache key prefix tracks the script-runner Node + // rather than pnpm's own `process.version`. Computed once outside + // the per-node loop. + const nodeVersion = findRuntimeNodeVersion(Object.keys(opts.depGraph)) await Promise.all( depNodes.map(async (depNode): Promise => { const { files } = await depNode.fetching() @@ -486,6 +492,7 @@ async function linkAllPkgs ( includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built patchFileHash: depNode.patch?.hash, supportedArchitectures: opts.supportedArchitectures, + nodeVersion, }) } } diff --git a/installing/deps-installer/tsconfig.json b/installing/deps-installer/tsconfig.json index c58ab38dad..a838b53526 100644 --- a/installing/deps-installer/tsconfig.json +++ b/installing/deps-installer/tsconfig.json @@ -90,6 +90,9 @@ { "path": "../../deps/path" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../exec/lifecycle" }, diff --git a/installing/deps-resolver/package.json b/installing/deps-resolver/package.json index e4ae5774b4..ba64f1fd4f 100644 --- a/installing/deps-resolver/package.json +++ b/installing/deps-resolver/package.json @@ -40,6 +40,7 @@ "@pnpm/deps.graph-hasher": "workspace:*", "@pnpm/deps.path": "workspace:*", "@pnpm/deps.peer-range": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/fetching.pick-fetcher": "workspace:*", "@pnpm/hooks.types": "workspace:*", diff --git a/installing/deps-resolver/src/index.ts b/installing/deps-resolver/src/index.ts index bdf3b8d7c6..3558b322ee 100644 --- a/installing/deps-resolver/src/index.ts +++ b/installing/deps-resolver/src/index.ts @@ -6,6 +6,7 @@ import { } from '@pnpm/core-loggers' import { iterateHashedGraphNodes } from '@pnpm/deps.graph-hasher' import { isRuntimeDepPath } from '@pnpm/deps.path' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import type { LockfileObject, ProjectSnapshot, @@ -494,7 +495,15 @@ function extendGraph ( const pkgMetaIter = iterateGraphPkgMetaEntries(graph, !opts.enableGlobalVirtualStore) // Only use allowBuild for engine-agnostic hash optimization when GVS is on const allowBuild = opts.enableGlobalVirtualStore ? opts.allowBuild : undefined - for (const { pkgMeta: { depPath }, hash } of iterateHashedGraphNodes(graph, pkgMetaIter, allowBuild, opts.supportedArchitectures)) { + // Anchor every snapshot's engine hash to the project-pinned Node + // version (from `engines.runtime` / `devEngines.runtime`) when the + // resolver produced one — the graph carries it as a + // `node@runtime:` key. Without this, GVS slots for + // approved-build packages would hash under the runner's + // `process.version` instead of the script-runner Node, splitting + // the cache between pinned and non-pinned installs on the same host. + const nodeVersion = findRuntimeNodeVersion(Object.keys(graph)) + for (const { pkgMeta: { depPath }, hash } of iterateHashedGraphNodes(graph, pkgMetaIter, allowBuild, opts.supportedArchitectures, nodeVersion)) { const modules = path.join(opts.globalVirtualStoreDir, hash, 'node_modules') const node = graph[depPath] Object.assign(node, { diff --git a/installing/deps-resolver/tsconfig.json b/installing/deps-resolver/tsconfig.json index 0d5b77c4b1..6330cfcb2e 100644 --- a/installing/deps-resolver/tsconfig.json +++ b/installing/deps-resolver/tsconfig.json @@ -42,6 +42,9 @@ { "path": "../../deps/peer-range" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../fetching/pick-fetcher" }, diff --git a/installing/deps-restorer/package.json b/installing/deps-restorer/package.json index 82f12cb173..be1d47db77 100644 --- a/installing/deps-restorer/package.json +++ b/installing/deps-restorer/package.json @@ -48,6 +48,7 @@ "@pnpm/deps.graph-builder": "workspace:*", "@pnpm/deps.graph-hasher": "workspace:*", "@pnpm/deps.path": "workspace:*", + "@pnpm/engine.runtime.system-node-version": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/exec.lifecycle": "workspace:*", "@pnpm/fs.symlink-dependency": "workspace:*", diff --git a/installing/deps-restorer/src/index.ts b/installing/deps-restorer/src/index.ts index 8c62adfd28..29a6e4b4dc 100644 --- a/installing/deps-restorer/src/index.ts +++ b/installing/deps-restorer/src/index.ts @@ -24,6 +24,7 @@ import { } from '@pnpm/deps.graph-builder' import { calcDepState, type DepsStateCache } from '@pnpm/deps.graph-hasher' import * as dp from '@pnpm/deps.path' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import { PnpmError } from '@pnpm/error' import { makeNodeRequireOption, @@ -909,6 +910,14 @@ async function linkAllPkgs ( needsBuildMarkerSrc = path.join(opts.storeDir, '.pnpm-needs-build-marker') await fs.writeFile(needsBuildMarkerSrc, '') } + // Resolved `engines.runtime` Node version (when present) anchors + // the side-effects-cache key prefix to the script-runner Node, not + // pnpm's own `process.version`. The restorer's `depGraph` is keyed + // by install directory, so scanning `Object.keys(opts.depGraph)` + // would never see a `node@runtime:` entry — pull the + // depPath off each node instead. Computed once outside the + // per-node loop. + const nodeVersion = findRuntimeNodeVersion(depNodes.map((node) => node.depPath)) await Promise.all( depNodes.map(async (depNode) => { if (!depNode.fetching) return @@ -928,6 +937,7 @@ async function linkAllPkgs ( includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built patchFileHash: depNode.patch?.hash, supportedArchitectures: opts.supportedArchitectures, + nodeVersion, }) } } diff --git a/installing/deps-restorer/src/linkHoistedModules.ts b/installing/deps-restorer/src/linkHoistedModules.ts index ee2fe4ff16..7cd9913003 100644 --- a/installing/deps-restorer/src/linkHoistedModules.ts +++ b/installing/deps-restorer/src/linkHoistedModules.ts @@ -11,6 +11,7 @@ import type { DepHierarchy, } from '@pnpm/deps.graph-builder' import { calcDepState, type DepsStateCache } from '@pnpm/deps.graph-hasher' +import { findRuntimeNodeVersion } from '@pnpm/engine.runtime.system-node-version' import { logger } from '@pnpm/logger' import type { PackageFilesResponse, @@ -52,6 +53,15 @@ export async function linkHoistedModules ( // We should avoid removing unnecessary directories while simultaneously adding new ones. // Doing so can sometimes lead to a race condition when linking commands to `node_modules/.bin`. await Promise.all(dirsToRemove.map((dir) => tryRemoveDir(dir))) + // Resolve the project's pinned runtime Node version once, before + // the recursive walk. The graph is keyed by install directory in + // this module, so scanning `Object.keys(graph)` would miss every + // `node@runtime:` entry — pull the depPath off each + // node instead. Threading it down via `opts` also avoids a + // re-scan at every recursion level. + const nodeVersion = findRuntimeNodeVersion( + Object.values(graph).map((node) => node.depPath) + ) await Promise.all( Object.entries(hierarchy) .map(([parentDir, depsHierarchy]) => { @@ -63,6 +73,7 @@ export async function linkHoistedModules ( } return linkAllPkgsInOrder(storeController, graph, depsHierarchy, parentDir, { ...opts, + nodeVersion, warn, }) }) @@ -99,6 +110,14 @@ async function linkAllPkgsInOrder ( preferSymlinkedExecutables?: boolean sideEffectsCacheRead: boolean supportedArchitectures?: SupportedArchitectures + /** + * Resolved `engines.runtime` Node version, computed once by + * [`linkHoistedModules`] before the recursion. Threaded into + * each [`calcDepState`] call so the side-effects-cache key + * prefix tracks the script-runner Node rather than pnpm's own + * `process.version`. + */ + nodeVersion?: string warn: (message: string) => void } ): Promise { @@ -122,6 +141,7 @@ async function linkAllPkgsInOrder ( includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built patchFileHash: depNode.patch?.hash, supportedArchitectures: opts.supportedArchitectures, + nodeVersion: opts.nodeVersion, }) } } diff --git a/installing/deps-restorer/tsconfig.json b/installing/deps-restorer/tsconfig.json index b9d2042f22..196d57dbcb 100644 --- a/installing/deps-restorer/tsconfig.json +++ b/installing/deps-restorer/tsconfig.json @@ -60,6 +60,9 @@ { "path": "../../deps/path" }, + { + "path": "../../engine/runtime/system-node-version" + }, { "path": "../../exec/lifecycle" }, diff --git a/pacquet/crates/cli/tests/pnpm_compatibility.rs b/pacquet/crates/cli/tests/pnpm_compatibility.rs index 393e7fbc77..67a0c068ec 100644 --- a/pacquet/crates/cli/tests/pnpm_compatibility.rs +++ b/pacquet/crates/cli/tests/pnpm_compatibility.rs @@ -205,3 +205,230 @@ fn pnpm_reads_pacquet_written_rows() { drop((root, mock_instance)); // cleanup } + +/// Filter a full store-dir listing down to the GVS slot subtree. +/// +/// pnpm writes GVS slots under `v11/links/////...` +/// because [`getStorePath`](https://github.com/pnpm/pnpm/blob/29a42efc3b/store/path/src/index.ts#L39-L42) +/// appends `STORE_VERSION` (`"v11"`) to the user-configured `storeDir`. +/// Pacquet's [`StoreDir::links`](../../../../store-dir/src/store_dir.rs) +/// puts them at `links/////...` — one level +/// shallower. Both prefixes pass through unmodified, so when the two +/// path sets are diffed in `assert_eq!` the prefix divergence shows up +/// alongside any inner-shape disagreement instead of being silently +/// normalized away. +fn gvs_paths_only(files: Vec) -> Vec { + files.into_iter().filter(|p| p.starts_with("links/") || p.starts_with("v11/links/")).collect() +} + +/// Append GVS opt-in (and any extra fields) to the `pnpm-workspace.yaml` +/// that [`CommandTempCwd::add_mocked_registry`] already populated with +/// `storeDir` / `cacheDir`. `enableGlobalVirtualStore: true` is the +/// switch that flips both pnpm and pacquet to the shared-store layout. +fn enable_gvs_in_workspace_yaml(workspace: &std::path::Path, extra_yaml: &str) { + let yaml_path = workspace.join("pnpm-workspace.yaml"); + let mut yaml = fs::read_to_string(&yaml_path).expect("read pnpm-workspace.yaml"); + // Guarantee a newline before the appended keys. If the helper + // that wrote the file ever drops the trailing newline, naive + // concatenation would merge its last key with + // `enableGlobalVirtualStore` and produce invalid YAML — flagged + // by CodeRabbit on PR #11689. + if !yaml.ends_with('\n') { + yaml.push('\n'); + } + yaml.push_str("enableGlobalVirtualStore: true\n"); + yaml.push_str(extra_yaml); + fs::write(&yaml_path, yaml).expect("write pnpm-workspace.yaml"); +} + +/// Run pnpm-then-pacquet against a shared workspace and compare the +/// GVS slot trees they each materialize. Pnpm runs first so the +/// lockfile exists before pacquet starts — pacquet's GVS write path +/// is gated on `frozen_lockfile && enable_global_virtual_store` (see +/// `package-manager/src/install.rs:299` and the +/// [`VirtualStoreLayout::legacy`](../../../../package-manager/src/virtual_store_layout.rs) +/// docstring), so a fresh install with no lockfile would silently fall +/// through to the project-local layout and the test would pass for the +/// wrong reason. +/// +/// Caller passes `pnpm_extra_args` so individual tests can add things +/// like `--ignore-scripts` without hard-coding it here. The store and +/// `node_modules` are wiped between the two installs so pacquet writes +/// the slot tree from scratch rather than reading pnpm's leftovers. +fn install_then_compare_gvs( + pnpm: std::process::Command, + pacquet: std::process::Command, + store_dir: &std::path::Path, + modules_dir: &std::path::Path, + pnpm_extra_args: &[&str], +) { + let mut pnpm_args = vec!["install"]; + pnpm_args.extend_from_slice(pnpm_extra_args); + eprintln!("Installing with pnpm (writes lockfile + pnpm-side GVS slots)..."); + pnpm.with_args(pnpm_args).assert().success(); + let pnpm_gvs_paths = gvs_paths_only(get_all_files(store_dir)); + assert!( + !pnpm_gvs_paths.is_empty(), + "pnpm must have written GVS slots; got nothing matching v11/links/ or links/", + ); + + eprintln!("Wiping store + node_modules (keeping lockfile so pacquet runs in frozen mode)..."); + fs::remove_dir_all(store_dir).expect("delete store dir"); + fs::remove_dir_all(modules_dir).expect("delete node_modules"); + + eprintln!("Installing with pacquet --frozen-lockfile (writes pacquet-side GVS slots)..."); + pacquet.with_args(["install", "--frozen-lockfile"]).assert().success(); + let pacquet_gvs_paths = gvs_paths_only(get_all_files(store_dir)); + + eprintln!("Comparing GVS layouts (pnpm on the right, pacquet on the left)..."); + assert_eq!(&pacquet_gvs_paths, &pnpm_gvs_paths); +} + +/// Pure-JS GVS parity: a package with one transitive dep, no install +/// scripts. With `allowBuilds` left at the GVS default of `{}` — +/// upstream's +/// [`extendInstallOptions.ts:354`](https://github.com/pnpm/pnpm/blob/29a42efc3b/installing/deps-installer/src/install/extendInstallOptions.ts#L354) +/// applies `??= {}` whenever `enableGlobalVirtualStore` is on — every +/// snapshot hashes with `engine = null`, so the GVS slot tree is +/// engine-agnostic and the comparison is independent of the host +/// Node.js / OS / arch the test runs on. +#[test] +fn same_global_virtual_store_layout_pure_js() { + let CommandTempCwd { pacquet, pnpm, root, workspace, npmrc_info } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { store_dir, mock_instance, .. } = npmrc_info; + + enable_gvs_in_workspace_yaml(&workspace, ""); + + eprintln!("Creating package.json..."); + fs::write( + workspace.join("package.json"), + serde_json::json!({ + "dependencies": { + "@pnpm.e2e/hello-world-js-bin-parent": "1.0.0", + }, + }) + .to_string(), + ) + .expect("write package.json"); + + install_then_compare_gvs( + pnpm, + pacquet, + &store_dir, + &workspace.join("node_modules"), + &["--ignore-scripts"], + ); + + drop((root, mock_instance)); // cleanup +} + +/// Engine-included GVS parity: `pre-and-postinstall-scripts-example` +/// has install scripts and is explicitly approved via `allowBuilds`, +/// so it lands in upstream's `builtDepPaths` set and its GVS hash +/// includes the `ENGINE_NAME` string (see +/// [`calcGraphNodeHash`](https://github.com/pnpm/pnpm/blob/29a42efc3b/deps/graph-hasher/src/index.ts#L140-L146)). +/// Pacquet's +/// [`calc_graph_node_hash`](../../../../graph-hasher/src/global_virtual_store_path.rs) +/// must produce the same engine-included digest, or pnpm and pacquet +/// would split the same approved-build package across two slot +/// directories. +/// +/// Scripts run on both sides (neither install uses `--ignore-scripts`) +/// because pacquet doesn't expose `--ignore-scripts` yet +/// (pacquet/crates/cli/README.md lists it as a TODO) — if pnpm +/// skipped scripts while pacquet ran them the slot trees would +/// diverge on the script-generated `generated-by-*.js` files even +/// though the hash itself agreed. +/// +/// **Ignored until a pnpm release ships the engine-name fix from +/// commit 8f05529c11.** This test requires pnpm and pacquet to agree +/// on the `;;node` triple used in the +/// engine-included hash branch. Pre-fix pnpm anchored the value to +/// `process.version` — the Node embedded in the `@pnpm/exe` SEA +/// bundle on Linux/macOS CI runners, currently Node 26 — while +/// pacquet (and any non-SEA caller) detects the `node` on `PATH`, +/// which on GHA's standard runners is Node 24. The hash digests +/// therefore land at different majors and the slot paths diverge. +/// The pnpm-side fix in this PR resolves `engineName()` via +/// `getSystemNodeVersion()` which prefers the shell `node`, so once +/// a published pnpm version with that fix reaches +/// [`pnpm/setup`](https://github.com/pnpm/setup) the test will pass +/// without modification — re-enable it then. +#[test] +#[ignore = "depends on a published pnpm version that includes commit 8f05529c11; see test doc comment"] +fn same_global_virtual_store_layout_with_approved_postinstall() { + let CommandTempCwd { pacquet, pnpm, root, workspace, npmrc_info } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { store_dir, mock_instance, .. } = npmrc_info; + + enable_gvs_in_workspace_yaml( + &workspace, + "allowBuilds:\n '@pnpm.e2e/pre-and-postinstall-scripts-example': true\n", + ); + + eprintln!("Creating package.json..."); + fs::write( + workspace.join("package.json"), + serde_json::json!({ + "dependencies": { + "@pnpm.e2e/pre-and-postinstall-scripts-example": "1.0.0", + }, + }) + .to_string(), + ) + .expect("write package.json"); + + install_then_compare_gvs( + pnpm, + pacquet, + &store_dir, + &workspace.join("node_modules"), + &[], // scripts must run on both sides; see fn doc above + ); + + drop((root, mock_instance)); // cleanup +} + +/// Diamond GVS parity: the root depends on both `pkg-with-1-dep` and +/// `parent-of-pkg-with-1-dep`, and `parent-of-pkg-with-1-dep` itself +/// depends on `pkg-with-1-dep`. So `pkg-with-1-dep` is reachable +/// through two paths from the root, and `calc_dep_graph_hash` must +/// hit its memoization cache on the second visit — if the cache key +/// or the hash payload disagreed between pnpm and pacquet, the +/// `pkg-with-1-dep` slot would land at one path on pnpm and another +/// on pacquet. Mirrors the cache-correctness guarantee that the unit +/// test [`diamond_graph_resolves_consistently`](../../../../graph-hasher/src/dep_state.rs) +/// already covers in isolation, here exercised through the full +/// install pipeline. +#[test] +fn same_global_virtual_store_layout_diamond() { + let CommandTempCwd { pacquet, pnpm, root, workspace, npmrc_info } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { store_dir, mock_instance, .. } = npmrc_info; + + enable_gvs_in_workspace_yaml(&workspace, ""); + + eprintln!("Creating package.json..."); + fs::write( + workspace.join("package.json"), + serde_json::json!({ + "dependencies": { + "@pnpm.e2e/pkg-with-1-dep": "100.0.0", + "@pnpm.e2e/parent-of-pkg-with-1-dep": "1.0.0", + }, + }) + .to_string(), + ) + .expect("write package.json"); + + install_then_compare_gvs( + pnpm, + pacquet, + &store_dir, + &workspace.join("node_modules"), + &["--ignore-scripts"], + ); + + drop((root, mock_instance)); // cleanup +} diff --git a/pacquet/crates/cmd-shim/src/link_bins.rs b/pacquet/crates/cmd-shim/src/link_bins.rs index 9c6f8f710f..d8fa919d8e 100644 --- a/pacquet/crates/cmd-shim/src/link_bins.rs +++ b/pacquet/crates/cmd-shim/src/link_bins.rs @@ -366,17 +366,29 @@ fn pick_winner( } } -/// Write all three shim flavors for `target_path` (the canonical `.sh` -/// at `shim_path`, plus the `.cmd` and `.ps1` siblings) and chmod them -/// executable. Idempotent on warm reinstalls via [`is_shim_pointing_at`]. +/// Write the canonical bin shim for `target_path` at `shim_path`, +/// plus the `.cmd` and `.ps1` Windows-style siblings *when the host +/// is Windows*. Idempotent on warm reinstalls via +/// [`is_shim_pointing_at`]. /// -/// Pnpm always emits all three flavors per bin (independent of host -/// platform), so a project installed on Linux stays usable when the -/// same `node_modules` is reused from Windows via a network share or -/// a `git clone` of a checked-in install. Pacquet matches that -/// contract here: `generate_sh_shim`, `generate_cmd_shim`, and -/// `generate_pwsh_shim` are unconditional, and the writer emits all -/// three. +/// Platform gating mirrors pnpm: +/// +/// - `@zkochan/cmd-shim` defaults `createCmdFile: isWindows` +/// ([index.js#L32](https://github.com/pnpm/cmd-shim/blob/0d79ca9534/src/index.ts#L32)), +/// so `.cmd` only lands on Windows. +/// - pnpm's `bins.linker` overrides `createPwshFile` per call as +/// `POWER_SHELL_IS_SUPPORTED && manifest.name !== 'pnpm'`, where +/// [`POWER_SHELL_IS_SUPPORTED = IS_WINDOWS`](https://github.com/pnpm/pnpm/blob/29a42efc3b/bins/linker/src/index.ts#L28). +/// So `.ps1` also only lands on Windows. +/// +/// Earlier versions of pacquet emitted all three flavors +/// unconditionally on the theory that a Linux-installed +/// `node_modules` should stay usable when carried to Windows via +/// network share or git clone. That doesn't match pnpm — pnpm's +/// Windows install rebuilds the shims on extraction — and produced +/// extra `.cmd`/`.ps1` files in every slot on Unix, splitting the +/// GVS file lists between the two tools (see the +/// `same_global_virtual_store_layout_*` parity tests). /// /// The chmod step (`set_executable` for the canonical shim and /// `ensure_executable_bits` for the target binary, matching pnpm's @@ -395,26 +407,28 @@ where })?; let sh_body = generate_sh_shim(target_path, shim_path, runtime.as_ref()); - let cmd_path = with_extension_appended(shim_path, "cmd"); - let ps1_path = with_extension_appended(shim_path, "ps1"); - let cmd_body = generate_cmd_shim(target_path, &cmd_path, runtime.as_ref()); - let ps1_body = generate_pwsh_shim(target_path, &ps1_path, runtime.as_ref()); + // Windows siblings are off on Unix to match pnpm. The bodies + // themselves still get computed inside the `cfg!(windows)` branch + // below — moving the `generate_*` calls there keeps Unix builds + // off the `relative_target_windows` allocation path entirely. + let windows_shims = if cfg!(windows) { + let cmd_path = with_extension_appended(shim_path, "cmd"); + let ps1_path = with_extension_appended(shim_path, "ps1"); + let cmd_body = generate_cmd_shim(target_path, &cmd_path, runtime.as_ref()); + let ps1_body = generate_pwsh_shim(target_path, &ps1_path, runtime.as_ref()); + Some((cmd_path, cmd_body, ps1_path, ps1_body)) + } else { + None + }; - // Idempotent skip only fires when all three flavors are already - // present *and pointing at the right target*. Gating on the `.sh` - // flavor alone (an earlier version of this code) left the upgrade - // path broken: a previous install (e.g. older pacquet, - // partial-write crash) might have written `.sh` correctly but - // never written `.cmd`/`.ps1`, in which case the marker check - // would short-circuit and the missing siblings would never be - // repaired. - // - // The `.sh` flavor carries a `# cmd-shim-target=` trailer - // that [`is_shim_pointing_at`] reads; the `.cmd` and `.ps1` - // flavors don't, so we compare them byte-for-byte against the - // freshly generated body. That catches stale/corrupted siblings - // that an existence-only check would let slip through (Copilot - // flagged this on + // Idempotent skip fires only when every flavor that *should* be + // present is present and pointing at the right target. The `.sh` + // flavor carries a `# cmd-shim-target=` trailer that + // [`is_shim_pointing_at`] reads; the `.cmd` and `.ps1` flavors + // don't, so we compare them byte-for-byte against the freshly + // generated body. That catches stale/corrupted siblings that an + // existence-only check would let slip through (Copilot flagged + // this on // ): // a manually-edited `.cmd` pointing at a stale target, or an // earlier pacquet write with a different relative path, would @@ -425,23 +439,31 @@ where Api::read_to_string(shim_path), Ok(existing) if is_shim_pointing_at(&existing, target_path), ); - let cmd_ok = matches!( - Api::read_to_string(&cmd_path), - Ok(existing) if existing == cmd_body, - ); - let ps1_ok = matches!( - Api::read_to_string(&ps1_path), - Ok(existing) if existing == ps1_body, - ); - let already_correct = sh_marker_ok && cmd_ok && ps1_ok; + let windows_ok = match &windows_shims { + None => true, + Some((cmd_path, cmd_body, ps1_path, ps1_body)) => { + let cmd_ok = matches!( + Api::read_to_string(cmd_path), + Ok(existing) if &existing == cmd_body, + ); + let ps1_ok = matches!( + Api::read_to_string(ps1_path), + Ok(existing) if &existing == ps1_body, + ); + cmd_ok && ps1_ok + } + }; + let already_correct = sh_marker_ok && windows_ok; if !already_correct { Api::write(shim_path, sh_body.as_bytes()) .map_err(|error| LinkBinsError::WriteShim { path: shim_path.to_path_buf(), error })?; - Api::write(&cmd_path, cmd_body.as_bytes()) - .map_err(|error| LinkBinsError::WriteShim { path: cmd_path.clone(), error })?; - Api::write(&ps1_path, ps1_body.as_bytes()) - .map_err(|error| LinkBinsError::WriteShim { path: ps1_path.clone(), error })?; + if let Some((cmd_path, cmd_body, ps1_path, ps1_body)) = &windows_shims { + Api::write(cmd_path, cmd_body.as_bytes()) + .map_err(|error| LinkBinsError::WriteShim { path: cmd_path.clone(), error })?; + Api::write(ps1_path, ps1_body.as_bytes()) + .map_err(|error| LinkBinsError::WriteShim { path: ps1_path.clone(), error })?; + } } Api::set_executable(shim_path) diff --git a/pacquet/crates/cmd-shim/src/link_bins/tests.rs b/pacquet/crates/cmd-shim/src/link_bins/tests.rs index 65d15b7e66..3f392e225a 100644 --- a/pacquet/crates/cmd-shim/src/link_bins/tests.rs +++ b/pacquet/crates/cmd-shim/src/link_bins/tests.rs @@ -8,22 +8,30 @@ use crate::{ }; use serde_json::{Value, json}; use std::{ - fs::{ - create_dir_all, metadata, read as read_file, read_to_string, remove_file, - write as write_file, - }, + fs::{create_dir_all, metadata, read as read_file, read_to_string, write as write_file}, iter::{Empty, empty}, path::{Path, PathBuf}, sync::Arc, }; +// `remove_file` is only used by the Windows-only upgrade-recovery +// test below; importing it unconditionally trips the +// `unused-imports` dylint on Unix builds. +#[cfg(windows)] +use std::fs::remove_file; use tempfile::tempdir; -/// All three shim flavors (`.sh` / no-extension, `.cmd`, `.ps1`) must -/// be written for every linked bin so a project installed on Linux -/// remains usable on Windows after a `git clone`. Mirrors pnpm's -/// always-write-all-flavors behavior. +/// On Windows pacquet writes all three shim flavors (the canonical +/// no-extension shim, `.cmd`, `.ps1`) per linked bin. On Unix only +/// the canonical shim lands — mirrors pnpm's +/// [`@zkochan/cmd-shim` `createCmdFile: isWindows`](https://github.com/pnpm/cmd-shim/blob/0d79ca9534/src/index.ts#L32) +/// default and `bins.linker`'s +/// [`POWER_SHELL_IS_SUPPORTED = IS_WINDOWS`](https://github.com/pnpm/pnpm/blob/29a42efc3b/bins/linker/src/index.ts#L28) +/// gate on the `createPwshFile` opt. The previous "always write all +/// three" behavior produced extra `.cmd` / `.ps1` files in every GVS +/// slot on Unix, splitting the file list between the two tools (see +/// the `same_global_virtual_store_layout_*` parity tests). #[test] -fn writes_all_three_shim_flavors_per_bin() { +fn writes_shim_flavors_matching_host_platform() { let tmp = tempdir().unwrap(); let pkg_dir = tmp.path().join("node_modules/foo"); create_dir_all(&pkg_dir).unwrap(); @@ -46,17 +54,26 @@ fn writes_all_three_shim_flavors_per_bin() { let sh = bins_dir.join("foo"); let cmd = bins_dir.join("foo.cmd"); let ps1 = bins_dir.join("foo.ps1"); - assert!(sh.exists(), "missing .sh shim"); - assert!(cmd.exists(), "missing .cmd shim"); - assert!(ps1.exists(), "missing .ps1 shim"); + assert!(sh.exists(), "missing canonical shim"); - let cmd_body = read_to_string(&cmd).unwrap(); - assert!(cmd_body.starts_with("@SETLOCAL\r\n"), "cmd shim must use CRLF SETLOCAL"); - assert!(cmd_body.contains("\"%~dp0\\..\\foo\\cli.js\""), "cmd target should be windows-style"); + if cfg!(windows) { + assert!(cmd.exists(), "missing .cmd shim on Windows"); + assert!(ps1.exists(), "missing .ps1 shim on Windows"); - let ps1_body = read_to_string(&ps1).unwrap(); - assert!(ps1_body.starts_with("#!/usr/bin/env pwsh\n")); - assert!(ps1_body.contains("\"$basedir/../foo/cli.js\"")); + let cmd_body = read_to_string(&cmd).unwrap(); + assert!(cmd_body.starts_with("@SETLOCAL\r\n"), "cmd shim must use CRLF SETLOCAL"); + assert!( + cmd_body.contains("\"%~dp0\\..\\foo\\cli.js\""), + "cmd target should be windows-style", + ); + + let ps1_body = read_to_string(&ps1).unwrap(); + assert!(ps1_body.starts_with("#!/usr/bin/env pwsh\n")); + assert!(ps1_body.contains("\"$basedir/../foo/cli.js\"")); + } else { + assert!(!cmd.exists(), ".cmd shim must not be written on Unix (pnpm parity)"); + assert!(!ps1.exists(), ".ps1 shim must not be written on Unix (pnpm parity)"); + } } /// End-to-end exercise: a package with a `bin` field has a shim written @@ -254,14 +271,21 @@ fn link_bins_skips_existing_shim_with_matching_marker() { assert_eq!(read_to_string(bins.join("foo")).unwrap(), sentinel); } -/// [`link_bins`] must NOT skip when only the canonical `.sh` shim exists. +/// [`link_bins`] must NOT skip when only the canonical shim exists. /// The `.cmd` and `.ps1` siblings could be missing because an older -/// pacquet wrote `.sh`-only or because a partial-write crash interrupted -/// the writer mid-batch. Gating on the `.sh` marker alone (an earlier -/// version of [`super::write_shim`]) caused those upgrade paths to leave -/// the missing siblings permanently absent. +/// pacquet wrote the canonical shim only or because a partial-write +/// crash interrupted the writer mid-batch. Gating on the canonical +/// shim's marker alone (an earlier version of [`super::write_shim`]) +/// caused those upgrade paths to leave the missing siblings +/// permanently absent. +/// +/// Windows-only: on Unix `.cmd` and `.ps1` are not written in the +/// first place (matches pnpm — see +/// [`writes_shim_flavors_matching_host_platform`]), so there's +/// nothing to recover. +#[cfg(windows)] #[test] -fn link_bins_rewrites_when_only_sh_flavor_exists() { +fn link_bins_rewrites_when_only_canonical_flavor_exists() { let tmp = tempdir().unwrap(); let modules = tmp.path().join("node_modules"); create_dir_all(modules.join("foo")).unwrap(); @@ -273,14 +297,14 @@ fn link_bins_rewrites_when_only_sh_flavor_exists() { link_bins::(&modules, &bins).unwrap(); // Simulate the partial-write / older-pacquet state: delete the - // .cmd and .ps1 siblings, leaving only the `.sh` shim with its + // .cmd and .ps1 siblings, leaving only the canonical shim with its // (still correct) target marker. remove_file(bins.join("foo.cmd")).unwrap(); remove_file(bins.join("foo.ps1")).unwrap(); link_bins::(&modules, &bins).unwrap(); - assert!(bins.join("foo").exists(), ".sh shim must remain"); + assert!(bins.join("foo").exists(), "canonical shim must remain"); assert!(bins.join("foo.cmd").exists(), ".cmd sibling must be re-created on second pass"); assert!(bins.join("foo.ps1").exists(), ".ps1 sibling must be re-created on second pass"); } diff --git a/pacquet/crates/config/src/lib.rs b/pacquet/crates/config/src/lib.rs index daef6ce02a..e161151257 100644 --- a/pacquet/crates/config/src/lib.rs +++ b/pacquet/crates/config/src/lib.rs @@ -200,17 +200,23 @@ pub struct Config { /// /// When [`enable_global_virtual_store`] is `true` and the user has not /// explicitly set this field, [`Config::current`] re-points it at - /// `/links` to mirror upstream's - /// [`extendInstallOptions.ts:343-355`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/extendInstallOptions.ts#L343-L355). + /// `/v11/links` to mirror upstream's + /// [`extendInstallOptions.ts:350-358`](https://github.com/pnpm/pnpm/blob/29a42efc3b/installing/deps-installer/src/install/extendInstallOptions.ts#L350-L358). + /// The `v11/` segment comes from pnpm's [`getStorePath`](https://github.com/pnpm/pnpm/blob/29a42efc3b/store/path/src/index.ts#L39-L42), + /// which appends `STORE_VERSION` to the configured `storeDir` + /// before `extendInstallOptions` runs its `path.join(storeDir, + /// 'links')` — so the join lands one level deeper than the + /// configured root. /// /// [`enable_global_virtual_store`]: Self::enable_global_virtual_store #[default(_code = "default_virtual_store_dir()")] pub virtual_store_dir: PathBuf, /// When `true`, the virtual store is shared across every project on - /// the machine: packages live under `/links/...` and each - /// project registers itself at `/projects/`. - /// When `false`, each project keeps its own virtual store at + /// the machine: packages live under `/v11/links/...` and + /// each project registers itself at + /// `/v11/projects/`. When `false`, each + /// project keeps its own virtual store at /// `/node_modules/.pnpm`. /// /// Default `false` — matches pnpm v11's effective default for @@ -227,8 +233,8 @@ pub struct Config { /// The shared global-virtual-store directory. When /// [`enable_global_virtual_store`] is `true` this is the same path as /// [`virtual_store_dir`]; when `false`, it is still computed as - /// `/links` (matching upstream's unconditional assignment - /// at [`extendInstallOptions.ts:354-355`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/extendInstallOptions.ts#L354-L355)) + /// `/v11/links` (matching upstream's unconditional + /// assignment at [`extendInstallOptions.ts:356-358`](https://github.com/pnpm/pnpm/blob/29a42efc3b/installing/deps-installer/src/install/extendInstallOptions.ts#L356-L358)) /// even though no install path consults it in that mode today. /// /// Populated by [`Config::current`] after yaml has been applied; the diff --git a/pacquet/crates/package-manager/src/install/tests.rs b/pacquet/crates/package-manager/src/install/tests.rs index 956b3b8746..5fc2acd008 100644 --- a/pacquet/crates/package-manager/src/install/tests.rs +++ b/pacquet/crates/package-manager/src/install/tests.rs @@ -1513,14 +1513,14 @@ async fn frozen_lockfile_under_gvs_registers_project_and_runs_clean() { .await .expect("frozen-lockfile install under GVS should succeed"); - // `register_project` wrote `/projects/` + // `register_project` wrote `/v11/projects/` // pointing back at the project dir. Canonicalize the *entry // path* (not `read_link`'s output) so the kernel follows the // symlink — pacquet, like upstream pnpm, writes the target as // a path relative to the link's parent, so canonicalizing the // raw `read_link` string from the CWD would never resolve. - let projects_dir = store_dir.join("projects"); - assert!(projects_dir.is_dir(), "GVS-on install must create /projects/"); + let projects_dir = store_dir.join("v11/projects"); + assert!(projects_dir.is_dir(), "GVS-on install must create /v11/projects/"); let entries: Vec<_> = std::fs::read_dir(&projects_dir).unwrap().collect::>().unwrap(); assert_eq!(entries.len(), 1, "exactly one project entry per `Install::run` invocation"); @@ -1586,7 +1586,7 @@ async fn frozen_lockfile_with_gvs_off_skips_project_registry() { .expect("frozen-lockfile install with GVS off should succeed"); assert!( - !store_dir.join("projects").exists(), + !store_dir.join("v11/projects").exists(), "GVS-off install must NOT create the project-registry directory", ); @@ -1665,8 +1665,11 @@ async fn frozen_lockfile_under_gvs_registers_each_workspace_importer() { // Exactly two registry entries — one per importer. Resolve the // symlink targets and confirm both project roots are present. - let projects_dir = store_dir.join("projects"); - assert!(projects_dir.is_dir(), "GVS-on workspace install must create /projects/"); + let projects_dir = store_dir.join("v11/projects"); + assert!( + projects_dir.is_dir(), + "GVS-on workspace install must create /v11/projects/", + ); let mut targets: Vec = std::fs::read_dir(&projects_dir) .unwrap() .map(|entry| { diff --git a/pacquet/crates/package-manager/src/install_frozen_lockfile.rs b/pacquet/crates/package-manager/src/install_frozen_lockfile.rs index 2f74e8dedf..dbee982477 100644 --- a/pacquet/crates/package-manager/src/install_frozen_lockfile.rs +++ b/pacquet/crates/package-manager/src/install_frozen_lockfile.rs @@ -15,7 +15,9 @@ use miette::Diagnostic; use pacquet_cmd_shim::LinkBinsError; use pacquet_config::{Config, NodeLinker, matcher::create_matcher}; use pacquet_executor::ScriptsPrependNodePath as ExecScriptsPrependNodePath; -use pacquet_lockfile::{Lockfile, PackageKey, PackageMetadata, ProjectSnapshot, SnapshotEntry}; +use pacquet_lockfile::{ + Lockfile, PackageKey, PackageMetadata, Prefix, ProjectSnapshot, SnapshotEntry, +}; use pacquet_modules_yaml::{RealApi, read_modules_manifest}; use pacquet_network::ThrottledClient; use pacquet_package_manifest::DependencyGroup; @@ -508,33 +510,51 @@ where // `VirtualStoreLayout` is built with `None` here, which // is fine because GVS is off and the layout ignores the // field in that path. + // Honour `engines.runtime` / `devEngines.runtime` pin (if + // one reached the lockfile): pnpm's runtime resolver writes + // the chosen Node as a `node@runtime:` snapshot + // (see + // [`engine/runtime/node-resolver`](https://github.com/pnpm/pnpm/blob/29a42efc3b/engine/runtime/node-resolver/src/index.ts)), + // and pnpm's `engineName` helper anchors the GVS hash and the + // side-effects-cache key prefix to that pinned Node. Mirror + // it here — otherwise pacquet hashes under whatever + // `node --version` returns from the shell, splitting the + // shared store between pinned and non-pinned installs on the + // same host. + let runtime_pinned_major = find_runtime_node_major(snapshots); let (initial_engine_name, deferred_engine_handle): ( Option, Option>>, - ) = match &host_node { - Some((true, ver)) => ( - parse_major_from_version(ver) - .map(|major| pacquet_graph_hasher::engine_name(major, None, None)), - None, - ), - Some((false, _)) => (None, None), - None if config.enable_global_virtual_store => ( - tokio::task::spawn_blocking(|| { - pacquet_graph_hasher::detect_node_major() - .map(|major| pacquet_graph_hasher::engine_name(major, None, None)) - }) - .await - .ok() - .flatten(), - None, - ), - None => ( - None, - Some(tokio::task::spawn_blocking(|| { - pacquet_graph_hasher::detect_node_major() - .map(|major| pacquet_graph_hasher::engine_name(major, None, None)) - })), - ), + ) = if let Some(major) = runtime_pinned_major { + // Lockfile-driven major wins outright; skip the host + // probe / `node --version` spawn entirely. + (Some(pacquet_graph_hasher::engine_name(major, None, None)), None) + } else { + match &host_node { + Some((true, ver)) => ( + parse_major_from_version(ver) + .map(|major| pacquet_graph_hasher::engine_name(major, None, None)), + None, + ), + Some((false, _)) => (None, None), + None if config.enable_global_virtual_store => ( + tokio::task::spawn_blocking(|| { + pacquet_graph_hasher::detect_node_major() + .map(|major| pacquet_graph_hasher::engine_name(major, None, None)) + }) + .await + .ok() + .flatten(), + None, + ), + None => ( + None, + Some(tokio::task::spawn_blocking(|| { + pacquet_graph_hasher::detect_node_major() + .map(|major| pacquet_graph_hasher::engine_name(major, None, None)) + })), + ), + } }; let engine_name = initial_engine_name; @@ -1244,3 +1264,44 @@ fn parse_major_from_version(version: &str) -> Option { let after_v = version.strip_prefix('v').unwrap_or(version); after_v.split('.').next()?.parse().ok() } + +/// Pull the `node@runtime:` major out of a lockfile's +/// `snapshots:` map, if the project pinned a runtime Node. +/// +/// Pnpm v11's runtime resolver writes the pinned Node into the +/// lockfile as a snapshot with key `node@runtime:` (see +/// [`engine/runtime/node-resolver`](https://github.com/pnpm/pnpm/blob/29a42efc3b/engine/runtime/node-resolver/src/index.ts#L67)). +/// Pnpm's +/// [`engineName(nodeVersion)`](https://github.com/pnpm/pnpm/blob/HEAD/engine/runtime/system-node-version/src/index.ts) +/// anchors the GVS hash and the side-effects-cache key prefix to +/// that pinned major instead of pnpm's own `process.version`. The +/// helper here is pacquet's mirror — same snapshot-scan, same +/// "first hit wins" semantics (the resolver rejects workspaces with +/// conflicting pins before they reach the lockfile). +/// +/// Returns `None` when no importer pinned a runtime — callers should +/// then fall through to the host probe (`node --version` or the +/// cached `host_node`). +fn find_runtime_node_major(snapshots: Option<&HashMap>) -> Option { + let snapshots = snapshots?; + for key in snapshots.keys() { + if key.suffix.prefix() != Prefix::Runtime { + continue; + } + // Pnpm currently emits `node@runtime:` only — `bun@runtime:` + // and `deno@runtime:` exist as separate runtime kinds but + // don't feed the Node-shaped engine string. Match the + // upstream helper which scans for `node@runtime:` exclusively. + if key.name.scope.is_some() || key.name.bare != "node" { + continue; + } + // `Version::major` is `u64`; pnpm's major is small (<=99 in + // practice), so the cast is lossless. The downstream + // `engine_name` argument is `u32`, matching upstream's + // `process.version.split('.')[0].substring(1)`-derived + // integer. + let major = key.suffix.version().major; + return Some(major as u32); + } + None +} diff --git a/pacquet/crates/package-manager/src/link_bins.rs b/pacquet/crates/package-manager/src/link_bins.rs index 1b1d7d53f3..4d591049b8 100644 --- a/pacquet/crates/package-manager/src/link_bins.rs +++ b/pacquet/crates/package-manager/src/link_bins.rs @@ -393,14 +393,30 @@ where .flatten() .chain(snapshot.optional_dependencies.iter().flatten()); - // First pass: figure out which children (if any) have a bin - // declared. Cheap — just hash-set lookups against the - // pre-built `has_bin_set` and a `without_peer` materialisation - // per child. If no child has a bin, skip the slot entirely — - // we don't even build the slot's path. Slots in this category - // are the bulk of a real lockfile (~95% in the integrated - // benchmark fixture); skipping them removes the dominant - // chunk of the per-install bin-link work. + // First pass: figure out which packages contribute a bin to + // this slot's `node_modules/.bin`. Two kinds: + // + // 1. Every child whose manifest declares `bin`. Cheap to + // detect via `has_bin_set` (pre-built from the lockfile's + // `packages:` rows). Without a child or a self-bin the + // slot needs no `.bin` directory at all, so the early + // return below skips ~95% of slots on a real-world + // lockfile (measured on the integrated-benchmark + // fixture). + // + // 2. The slot's own package, when it carries a bin. Pnpm's + // [`linkBinsOfDependencies`](https://github.com/pnpm/pnpm/blob/29a42efc3b/building/during-install/src/index.ts#L272-L298) + // appends `depNode` to the bin-source list unconditionally + // (line 287) and lets the inner reader's manifest check + // drop self when there's nothing to write — so for a + // package like `hello-world-js-bin` (no deps, one bin) + // pnpm writes `/node_modules//node_modules/.bin/` + // as a self-shim. An earlier version of this function + // skipped the self-bin on the assumption that pnpm did the + // same. The + // `same_global_virtual_store_layout_*` parity tests + // surfaced that assumption as a divergence: pnpm did write + // the self-shim. Mirror it here. let with_bin: Vec<(&PkgName, PackageKey)> = children .filter_map(|(alias, dep_ref)| { let child_key = dep_ref.resolve(alias); @@ -412,7 +428,17 @@ where keep.then_some((alias, metadata_key)) }) .collect(); - if with_bin.is_empty() { + let self_metadata_key = slot_key.without_peer(); + let self_has_bin = match has_bin_set { + Some(set) => set.contains(&self_metadata_key), + // No `has_bin_set` — fall back to the conservative + // include-self path. The downstream manifest read in + // `link_bins_of_packages` filters out a self with no + // actual `bin` field, so an over-inclusion at this gate + // costs at most one `package.json` read. + None => true, + }; + if with_bin.is_empty() && !self_has_bin { return Ok(()); } @@ -421,7 +447,8 @@ where let self_pkg_dir = slot_own_pkg_dir(&modules_dir, slot_key); let bins_dir = self_pkg_dir.join("node_modules/.bin"); - let mut bin_sources: Vec = Vec::with_capacity(with_bin.len()); + let mut bin_sources: Vec = + Vec::with_capacity(with_bin.len() + usize::from(self_has_bin)); for (alias, metadata_key) in with_bin { let child_location = pkg_dir_under(&modules_dir, alias); if let Some(manifest) = package_manifests.get(&metadata_key) { @@ -448,6 +475,23 @@ where } } + // Self-bin source (slot's own package), when its lockfile row + // declared a bin. Same warm-vs-cold dispatch as the children + // above. `self_pkg_dir` is an invariant of + // [`crate::create_virtual_dir_by_snapshot`], so the cold + // fallback is the same `read_package` used elsewhere. + if self_has_bin { + if let Some(manifest) = package_manifests.get(&self_metadata_key) { + bin_sources.push(PackageBinSource::new(self_pkg_dir.clone(), Arc::clone(manifest))); + } else { + match read_package::(&self_pkg_dir) { + Ok(Some(pkg)) => bin_sources.push(pkg), + Ok(None) => {} + Err(error) => return Err(LinkVirtualStoreBinsError::LinkBins(error)), + } + } + } + if bin_sources.is_empty() { return Ok(()); } diff --git a/pacquet/crates/store-dir/src/store_dir.rs b/pacquet/crates/store-dir/src/store_dir.rs index ddb99ac867..59e122a5ac 100644 --- a/pacquet/crates/store-dir/src/store_dir.rs +++ b/pacquet/crates/store-dir/src/store_dir.rs @@ -112,23 +112,32 @@ impl StoreDir { } /// Path to the shared global-virtual-store directory inside the - /// store. Matches pnpm's [`extendInstallOptions.ts:343-355`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/extendInstallOptions.ts#L343-L355): - /// when `enableGlobalVirtualStore` is on and the user hasn't - /// pinned `virtualStoreDir`, packages live under `/links`. - /// Note: this directory sits next to (not inside) `/v11/`, - /// matching upstream's layout — sharing the `/links` path - /// across pnpm and pacquet is the whole point. + /// store. Matches pnpm's + /// [`extendInstallOptions.ts:350-358`](https://github.com/pnpm/pnpm/blob/29a42efc3b/installing/deps-installer/src/install/extendInstallOptions.ts#L350-L358): + /// `globalVirtualStoreDir = path.join(extendedOpts.storeDir, 'links')`. + /// `extendedOpts.storeDir` has already been routed through + /// [`getStorePath`](https://github.com/pnpm/pnpm/blob/29a42efc3b/store/path/src/index.ts#L39-L42) + /// by the time that join runs, and `getStorePath` appends + /// `STORE_VERSION` (`"v11"`) to whatever the user configured. So + /// the resulting on-disk location is `/v11/links`, not + /// `/links` — the latter would put pacquet one level + /// shallower than pnpm and split slot caches across the two + /// tools. Sharing the path across pnpm and pacquet is the whole + /// point, so anchor under [`Self::v11`]. pub fn links(&self) -> PathBuf { - self.root.join("links") + self.v11().join("links") } /// Path to the per-store projects registry — a flat directory of - /// symlinks (`/projects/` → project dir) the - /// global-virtual-store prune sweep walks when deciding which - /// `/links/...` slots are still referenced. Mirrors - /// pnpm's [`getProjectsRegistryDir`](https://github.com/pnpm/pnpm/blob/94240bc046/store/controller/src/storeController/projectRegistry.ts). + /// symlinks (`/v11/projects/` → project dir) + /// the global-virtual-store prune sweep walks when deciding which + /// `/v11/links/...` slots are still referenced. Mirrors + /// pnpm 11's + /// [`{storeDir}/v11/projects/` layout](https://github.com/pnpm/pnpm/blob/29a42efc3b/store/controller/CHANGELOG.md#L136) + /// — same `getStorePath`-driven `v11` reasoning as + /// [`Self::links`]. pub fn projects(&self) -> PathBuf { - self.root.join("projects") + self.v11().join("projects") } /// Borrow the raw store-root path. Most code should prefer the diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 48c05e14ec..37c0205078 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1748,6 +1748,9 @@ importers: '@pnpm/deps.path': specifier: workspace:* version: link:../../deps/path + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/error': specifier: workspace:* version: link:../../core/error @@ -1960,6 +1963,9 @@ importers: '@pnpm/deps.path': specifier: workspace:* version: link:../../deps/path + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/error': specifier: workspace:* version: link:../../core/error @@ -3256,6 +3262,9 @@ importers: '@pnpm/deps.path': specifier: workspace:* version: link:../path + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/hooks.types': specifier: workspace:* version: link:../../hooks/types @@ -3299,15 +3308,15 @@ importers: deps/graph-hasher: dependencies: - '@pnpm/constants': - specifier: workspace:* - version: link:../../core/constants '@pnpm/crypto.object-hasher': specifier: workspace:* version: link:../../crypto/object-hasher '@pnpm/deps.path': specifier: workspace:* version: link:../path + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/lockfile.types': specifier: workspace:* version: link:../../lockfile/types @@ -5608,6 +5617,9 @@ importers: '@pnpm/deps.path': specifier: workspace:* version: link:../../deps/path + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/error': specifier: workspace:* version: link:../../core/error @@ -5882,6 +5894,9 @@ importers: '@pnpm/deps.peer-range': specifier: workspace:* version: link:../../deps/peer-range + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/error': specifier: workspace:* version: link:../../core/error @@ -6024,6 +6039,9 @@ importers: '@pnpm/deps.path': specifier: workspace:* version: link:../../deps/path + '@pnpm/engine.runtime.system-node-version': + specifier: workspace:* + version: link:../../engine/runtime/system-node-version '@pnpm/error': specifier: workspace:* version: link:../../core/error