From d976edf4ecf7c41be5d7ec7f5cea87364bebf010 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 10 Jun 2026 21:24:47 +0200 Subject: [PATCH] perf: content-check modified manifests and fall back to the current lockfile on the repeat-install fast path (pacquet + pnpm) (#12315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why On [benchmarks.vlt.sh](https://benchmarks.vlt.sh/) (2026-06-10 run, pacquet 0.11.2), pacquet ranked **8th–9th of 10** in every `lockfile+node_modules` variation — slower than pnpm, npm, yarn and vlt — e.g. astro: pacquet 936 ms vs pnpm 502 ms; babylon: pacquet 9.08 s vs pnpm 0.85 s. It also trailed vlt/npm in the `node_modules` and `cache+node_modules` variations (astro 1.5 s / 0.7 s, babylon 8.9 s / 6.4 s). ### Root cause Tracing the actual runner (a `pacquet` PATH shim logging per-invocation file stats) showed the harness's prepare step rewrites `package.json` with **identical content but a fresh mtime** before every timed run, while `clean_all_cache` wipes `~/.cache/pnpm` (the packument cache and `lockfile-verified.jsonl`), and the `node_modules` variations additionally delete `pnpm-lock.yaml`. - **pnpm**: `checkDepsStatus`'s modified-manifests branch re-checks the *content* against the lockfile (`assertWantedLockfileUpToDate`, `assertLockfilesEqual`, `linkedPackagesAreUpToDate`) and reports "Already up to date" with zero network — ~0.5 s is just Node startup. Verified locally: with all caches wiped and the network blocked, `pnpm install` still prints "Already up to date" in 228 ms. - **pacquet**: the optimistic repeat-install check bailed on *any* newer manifest mtime, fell into the full pipeline, and the awaited `minimumReleaseAge` lockfile-verification gate — its verdict cache wiped — re-fetched **one packument per locked package** per run: 0.94 s on astro, 9.1 s on babylon. - With `pnpm-lock.yaml` deleted, both stacks pay a similar fan-out on the synthesized-from-current lockfile (`tryLockfileVerificationCache` bails before the content-hash index when the lockfile file can't be stat'd), which is why even pnpm needs 2.2–11.6 s there. ## What **Commit 1 — port the modified-manifests branch of `checkDepsStatus`** (at pnpm/pnpm@cc4ff817aa) into `optimistic_repeat_install`: - a manifest whose mtime is newer than `lastValidatedTimestamp` is re-checked against the wanted lockfile instead of invalidating the fast path: lockfile-settings drift (`getOutdatedLockfileSetting`), per-importer `satisfiesPackageManifest`, and a port of `linkedPackagesAreUpToDate` for workspace links (`isLocalFileDepUpdated` for `file:` directory specifiers is not ported — those conservatively fall through to the full install); - `assertLockfilesEqual` runs when the wanted lockfile is newer than the reference (workspace: `lastValidatedTimestamp`; single-project: the current lockfile's mtime, mirroring upstream's branch shapes); - the workspace branch refreshes `lastValidatedTimestamp` after a passing content check, like upstream's `updateWorkspaceState` call; - the frozen-dispatch freshness gate is split into reusable pieces (`parse_config_overrides`, `check_lockfile_settings_drift`, `check_importer_satisfies`) shared with the new check, and the per-importer slice is no longer hard-wired to the root importer. **Commit 2 — treat the current lockfile as the wanted one when `pnpm-lock.yaml` is missing (pacquet)** (requested by @zkochan): when `node_modules` is intact, `/lock.yaml` — the record of what the previous install materialized — stands in as the wanted lockfile for the same content checks, and `pnpm-lock.yaml` is regenerated from it (byte-identical to what the full install's synthesize-from-current path would write) before the fast path reports "Already up to date". Single-project installs with no lockfile on either side still refuse the fast path; `lockfile: false` skips the regeneration; a manifest that no longer matches (e.g. `pacquet add`) still takes the full resolve. ## Validation Re-ran the actual vlt.sh harness (same scripts, ubuntu-24.04-arm runner) with the patched binary swapped into the npm-installed pacquet; all hyperfine runs exited 0: | fixture, variation | pacquet 0.11.2 (official run) | patched | pnpm (same validation run) | |---|---|---|---| | astro, `lockfile+node_modules` | 935.6 ms (rank 9/10) | **38–39 ms** | 599–621 ms | | babylon, `lockfile+node_modules` | 9 084 ms (rank 8/10) | **86.6 ms ± 0.6** | 767.7 ms | | astro, `node_modules` | 1 501 ms (rank 4/10) | **41.2 ms ± 0.8** | 2 226 ms | | astro, `cache+node_modules` | 704 ms (rank 5/10) | **42.9 ms ± 0.9** | 2 017 ms | | babylon, `node_modules` | 8 962 ms (rank 6/10) | **107.8 ms ± 1.0** | 11 566 ms | After this change only aube (~5 ms) and bun (~8 ms) stay ahead in these five variations. `cargo nextest run -p pacquet-package-manager` (438 tests), `-p pacquet-cli` install suites, workspace clippy `-D warnings`, dylint, fmt, taplo and `typos pacquet` are clean. New tests cover the touched-but-identical manifest, a manifest that adds a dependency, a diverged wanted-vs-current lockfile, the state-timestamp refresh, linked siblings inside/outside the manifest range, lockfile regeneration (modified and unmodified manifests, workspace state bump), and `lockfile: false`. Two offline e2e tests additionally pin the "zero network, zero pipeline" property through `Install::run`'s real dispatch: a real install, registry dropped, caches wiped, repeat install pointed at a dead port — both verified discriminating by temporarily disabling the content check. Two existing tests were adjusted: `fresh_install_records_lockfile_verification_for_mtime_bypassed_noop` now disables the optimistic check explicitly so it keeps guarding the verification-cache wiring it was written for, and `optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing` now passes `lockfile: None` (matching the CLI contract for a missing file) and documents that the guard requires *both* lockfiles to be absent. **Commit `1ee88c5107` — the same fallback in the pnpm CLI** (`@pnpm/deps.status` + `@pnpm/installing.commands`): `checkDepsStatus` lets the current lockfile stand in when `pnpm-lock.yaml` is missing (workspace shared-lockfile branch and single-project branch), runs the same content checks against it, and returns it as `wantedLockfileToRestore`; `installDeps` writes `pnpm-lock.yaml` back from it before reporting "Already up to date". Guard rails: no lockfile on either side still refuses the fast path, `useLockfile: false` skips the restore, a failed restore falls through to the full install, and the stand-in is disabled under `useGitBranchLockfile` (there a missing plain `pnpm-lock.yaml` is the steady state and the branch lockfile may legitimately differ from the current one). Verified with the bundled CLI: install → delete `pnpm-lock.yaml` → `pnpm install --registry=http://127.0.0.1:9/` prints "Already up to date" in 29 ms and restores the lockfile byte-identically. Covered by 5 new `checkDepsStatus` unit tests and an `installing/commands` integration test that runs the repeat install against a dead registry. Changeset bumps `@pnpm/deps.status`, `@pnpm/installing.commands`, and `pnpm` (minor). --- .../fast-repeat-install-restores-lockfile.md | 7 + deps/status/src/checkDepsStatus.ts | 111 ++- deps/status/test/checkDepsStatus.test.ts | 226 ++++++ installing/commands/src/install.ts | 1 + installing/commands/src/installDeps.ts | 31 +- installing/commands/test/install.ts | 24 + pacquet/crates/package-manager/src/install.rs | 154 +++-- .../package-manager/src/install/tests.rs | 312 ++++++++- .../src/optimistic_repeat_install.rs | 651 +++++++++++++++--- .../src/optimistic_repeat_install/tests.rs | 538 ++++++++++++--- 10 files changed, 1775 insertions(+), 280 deletions(-) create mode 100644 .changeset/fast-repeat-install-restores-lockfile.md diff --git a/.changeset/fast-repeat-install-restores-lockfile.md b/.changeset/fast-repeat-install-restores-lockfile.md new file mode 100644 index 0000000000..fe92f73068 --- /dev/null +++ b/.changeset/fast-repeat-install-restores-lockfile.md @@ -0,0 +1,7 @@ +--- +"@pnpm/deps.status": minor +"@pnpm/installing.commands": minor +"pnpm": minor +--- + +`pnpm install` completes without re-resolving when `pnpm-lock.yaml` was deleted but `node_modules` is intact: the up-to-date check now treats the current lockfile (`node_modules/.pnpm/lock.yaml`) — the record of what the previous install materialized — as the wanted lockfile, verifies the manifests still match it, restores `pnpm-lock.yaml` from it, and reports "Already up to date". Previously this scenario triggered a full resolution and a re-verification of every locked package against the registry. diff --git a/deps/status/src/checkDepsStatus.ts b/deps/status/src/checkDepsStatus.ts index c29d545fe3..01704043e5 100644 --- a/deps/status/src/checkDepsStatus.ts +++ b/deps/status/src/checkDepsStatus.ts @@ -68,12 +68,30 @@ export type CheckDepsStatusOptions = Pick pnpmfile: string[] + /** + * When git-branch lockfiles are enabled, the wanted lockfile lives at + * `pnpm-lock..yaml`, so a missing `pnpm-lock.yaml` is the steady + * state — the current-lockfile stand-in must not kick in. + */ + useGitBranchLockfile?: boolean } & WorkspaceStateSettings export interface CheckDepsStatusResult { upToDate: boolean | undefined issue?: string workspaceState: WorkspaceState | undefined + /** + * Set when `pnpm-lock.yaml` was missing and the current lockfile + * (`/node_modules/.pnpm/lock.yaml`) stood in as the wanted + * lockfile for the up-to-date checks. The current lockfile records + * exactly what the previous install materialized, so the caller can + * restore `pnpm-lock.yaml` from it without resolving — `installDeps` + * does that before reporting "Already up to date". + */ + wantedLockfileToRestore?: { + lockfile: LockfileObject + lockfileDir: string + } } export async function checkDepsStatus (opts: CheckDepsStatusOptions): Promise { @@ -258,37 +276,59 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W if (modifiedProjects.length === 0) { logger.debug({ msg: 'No manifest files were modified since the last validation. Exiting check.' }) - return { upToDate: true, workspaceState } + const wantedLockfileToRestore = sharedWorkspaceLockfile && !opts.useGitBranchLockfile + ? await missingWantedLockfileStandIn(workspaceDir) + : undefined + return { upToDate: true, workspaceState, wantedLockfileToRestore } } logger.debug({ msg: 'Some manifest files were modified since the last validation. Continuing check.' }) + let wantedLockfileToRestore: CheckDepsStatusResult['wantedLockfileToRestore'] let readWantedLockfileAndDir: (projectDir: string) => Promise<{ wantedLockfile: LockfileObject wantedLockfileDir: string }> if (sharedWorkspaceLockfile) { - let wantedLockfileStats: fs.Stats + let wantedLockfileStats: fs.Stats | undefined try { wantedLockfileStats = fs.statSync(path.join(workspaceDir, WANTED_LOCKFILE)) } catch (error) { if (util.types.isNativeError(error) && 'code' in error && error.code === 'ENOENT') { - return throwLockfileNotFound(workspaceDir) + wantedLockfileStats = undefined } else { throw error } } - const wantedLockfilePromise = readWantedLockfile(workspaceDir, { ignoreIncompatible: false }) - if (wantedLockfileStats.mtime.valueOf() > workspaceState.lastValidatedTimestamp) { + if (wantedLockfileStats == null) { + // `pnpm-lock.yaml` is gone, but the current lockfile records + // exactly what the previous install materialized — let it stand + // in as the wanted lockfile for the checks below, and report it + // back so `installDeps` can restore `pnpm-lock.yaml` from it + // without resolving. There is no second lockfile to compare + // against, so the wanted-vs-current equality assertion doesn't + // apply on this path. + if (opts.useGitBranchLockfile) return throwLockfileNotFound(workspaceDir) const currentLockfile = await readCurrentLockfile(path.join(workspaceDir, 'node_modules/.pnpm'), { ignoreIncompatible: false }) - const wantedLockfile = (await wantedLockfilePromise) ?? throwLockfileNotFound(workspaceDir) - assertLockfilesEqual(currentLockfile, wantedLockfile, workspaceDir) + if (currentLockfile == null) return throwLockfileNotFound(workspaceDir) + wantedLockfileToRestore = { lockfile: currentLockfile, lockfileDir: workspaceDir } + readWantedLockfileAndDir = async () => ({ + wantedLockfile: currentLockfile, + wantedLockfileDir: workspaceDir, + }) + } else { + const wantedLockfilePromise = readWantedLockfile(workspaceDir, { ignoreIncompatible: false }) + if (wantedLockfileStats.mtime.valueOf() > workspaceState.lastValidatedTimestamp) { + const currentLockfile = await readCurrentLockfile(path.join(workspaceDir, 'node_modules/.pnpm'), { ignoreIncompatible: false }) + const wantedLockfile = (await wantedLockfilePromise) ?? throwLockfileNotFound(workspaceDir) + assertLockfilesEqual(currentLockfile, wantedLockfile, workspaceDir) + } + readWantedLockfileAndDir = async () => ({ + wantedLockfile: (await wantedLockfilePromise) ?? throwLockfileNotFound(workspaceDir), + wantedLockfileDir: workspaceDir, + }) } - readWantedLockfileAndDir = async () => ({ - wantedLockfile: (await wantedLockfilePromise) ?? throwLockfileNotFound(workspaceDir), - wantedLockfileDir: workspaceDir, - }) } else { readWantedLockfileAndDir = async wantedLockfileDir => { const wantedLockfilePromise = readWantedLockfile(wantedLockfileDir, { ignoreIncompatible: false }) @@ -355,7 +395,7 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W filteredInstall: workspaceState.filteredInstall, }) - return { upToDate: true, workspaceState } + return { upToDate: true, workspaceState, wantedLockfileToRestore } } if (!allProjects) { @@ -389,12 +429,26 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W statManifestFile(rootProjectManifestDir), ]) - if (!wantedLockfileStats) return throwLockfileNotFound(rootProjectManifestDir) + if (!wantedLockfileStats && (!currentLockfileStats || opts.useGitBranchLockfile)) return throwLockfileNotFound(rootProjectManifestDir) + + // When `pnpm-lock.yaml` is gone but the current lockfile + // (`node_modules/.pnpm/lock.yaml`) survives, the current one stands + // in as the wanted lockfile: it records exactly what the previous + // install materialized, so the checks below run against it and the + // caller can restore `pnpm-lock.yaml` from it without resolving. + // The wanted-vs-current equality assertion doesn't apply on this + // path — the two are the same object. + const wantedLockfileIsMissing = !wantedLockfileStats + const effectiveWantedLockfileStats = (wantedLockfileStats ?? currentLockfileStats)! + const readEffectiveWantedLockfile = async (): Promise => { + const lockfile = wantedLockfileIsMissing ? await currentLockfilePromise : await wantedLockfilePromise + return lockfile ?? throwLockfileNotFound(rootProjectManifestDir) + } const issue = await patchesOrHooksAreModified({ patchedDependencies, rootDir: rootProjectManifestDir, - lastValidatedTimestamp: wantedLockfileStats.mtime.valueOf(), + lastValidatedTimestamp: effectiveWantedLockfileStats.mtime.valueOf(), currentPnpmfiles: opts.pnpmfile, previousPnpmfiles: workspaceState.pnpmfiles, }) @@ -402,7 +456,7 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W return { upToDate: false, issue, workspaceState } } - if (currentLockfileStats && wantedLockfileStats.mtime.valueOf() > currentLockfileStats.mtime.valueOf()) { + if (!wantedLockfileIsMissing && currentLockfileStats && wantedLockfileStats.mtime.valueOf() > currentLockfileStats.mtime.valueOf()) { const currentLockfile = await currentLockfilePromise const wantedLockfile = (await wantedLockfilePromise) ?? throwLockfileNotFound(rootProjectManifestDir) assertLockfilesEqual(currentLockfile, wantedLockfile, rootProjectManifestDir) @@ -413,7 +467,7 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W throw new Error(`Cannot find one of ${MANIFEST_BASE_NAMES.join(', ')} in ${rootProjectManifestDir}`) } - if (manifestStats.mtime.valueOf() > wantedLockfileStats.mtime.valueOf()) { + if (manifestStats.mtime.valueOf() > effectiveWantedLockfileStats.mtime.valueOf()) { logger.debug({ msg: 'The manifest is newer than the lockfile. Continuing check.' }) try { await assertWantedLockfileUpToDate({ @@ -429,7 +483,7 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W projectDir: rootProjectManifestDir, projectId: '.' as ProjectId, projectManifest: rootProjectManifest, - wantedLockfile: (await wantedLockfilePromise) ?? throwLockfileNotFound(rootProjectManifestDir), + wantedLockfile: await readEffectiveWantedLockfile(), wantedLockfileDir: rootProjectManifestDir, }) } catch (err) { @@ -450,6 +504,16 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W } } + if (wantedLockfileIsMissing) { + const currentLockfile = await currentLockfilePromise + if (currentLockfile != null) { + return { + upToDate: true, + workspaceState, + wantedLockfileToRestore: { lockfile: currentLockfile, lockfileDir: rootProjectManifestDir }, + } + } + } return { upToDate: true, workspaceState } } @@ -564,6 +628,19 @@ function throwLockfileNotFound (wantedLockfileDir: string): never { }) } +/** + * When `/pnpm-lock.yaml` is missing but the current lockfile + * exists, returns the current lockfile so the caller can restore + * `pnpm-lock.yaml` from it. `undefined` when the wanted lockfile is present + * (nothing to restore) or when there is no current lockfile to restore from. + */ +async function missingWantedLockfileStandIn (lockfileDir: string): Promise { + if (safeStatSync(path.join(lockfileDir, WANTED_LOCKFILE)) != null) return undefined + const currentLockfile = await readCurrentLockfile(path.join(lockfileDir, 'node_modules/.pnpm'), { ignoreIncompatible: false }) + if (currentLockfile == null) return undefined + return { lockfile: currentLockfile, lockfileDir } +} + function getWantedLockfileDirs (opts: { allProjects: Project[] | undefined lockfileDir: string | undefined diff --git a/deps/status/test/checkDepsStatus.test.ts b/deps/status/test/checkDepsStatus.test.ts index be64853d72..0fc6d0fd88 100644 --- a/deps/status/test/checkDepsStatus.test.ts +++ b/deps/status/test/checkDepsStatus.test.ts @@ -564,3 +564,229 @@ async function writeConflictedLockfile (lockfileDir: string): Promise { '', ].join('\n')) } + +describe('checkDepsStatus - missing wanted lockfile fallback', () => { + beforeEach(() => { + jest.resetModules() + jest.clearAllMocks() + }) + + const currentLockfile = { + lockfileVersion: '9.0', + importers: { '.': { specifiers: {} } }, + } as unknown as LockfileObject + + function mockSingleProjectStats (opts: { + wantedLockfileExists: boolean + currentLockfileMtime: number + manifestMtime: number + }): void { + jest.mocked(fsUtils.safeStat).mockImplementation(async (filePath: string) => { + if (filePath === path.join('/project', 'node_modules', '.pnpm', 'lock.yaml')) { + return { + mtime: new Date(opts.currentLockfileMtime), + mtimeMs: opts.currentLockfileMtime, + } as Stats + } + if (filePath === path.join('/project', 'pnpm-lock.yaml') && opts.wantedLockfileExists) { + return { + mtime: new Date(opts.currentLockfileMtime), + mtimeMs: opts.currentLockfileMtime, + } as Stats + } + return undefined + }) + jest.mocked(fsUtils.safeStatSync).mockReturnValue(undefined) + jest.mocked(statManifestFileUtils.statManifestFile).mockImplementation(async () => ({ + mtime: new Date(opts.manifestMtime), + mtimeMs: opts.manifestMtime, + } as Stats)) + jest.mocked(lockfileFs.readCurrentLockfile).mockImplementation(async () => currentLockfile) + jest.mocked(lockfileFs.readWantedLockfile).mockImplementation(async () => null) + } + + it('returns the current lockfile to restore when pnpm-lock.yaml is missing in a single project', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const mockWorkspaceState: WorkspaceState = { + lastValidatedTimestamp, + pnpmfiles: [], + settings: { + excludeLinksFromLockfile: false, + linkWorkspacePackages: true, + preferWorkspacePackages: true, + }, + projects: {}, + filteredInstall: false, + } + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState) + mockSingleProjectStats({ + wantedLockfileExists: false, + currentLockfileMtime: lastValidatedTimestamp - 10_000, + manifestMtime: lastValidatedTimestamp - 20_000, + }) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: {}, + rootProjectManifestDir: '/project', + pnpmfile: [], + ...mockWorkspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + expect(result.wantedLockfileToRestore).toEqual({ + lockfile: currentLockfile, + lockfileDir: '/project', + }) + }) + + it('does not set a lockfile to restore when pnpm-lock.yaml exists', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const mockWorkspaceState: WorkspaceState = { + lastValidatedTimestamp, + pnpmfiles: [], + settings: { + excludeLinksFromLockfile: false, + linkWorkspacePackages: true, + preferWorkspacePackages: true, + }, + projects: {}, + filteredInstall: false, + } + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState) + mockSingleProjectStats({ + wantedLockfileExists: true, + currentLockfileMtime: lastValidatedTimestamp - 10_000, + manifestMtime: lastValidatedTimestamp - 20_000, + }) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: {}, + rootProjectManifestDir: '/project', + pnpmfile: [], + ...mockWorkspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + expect(result.wantedLockfileToRestore).toBeUndefined() + }) + + it('still reports the lockfile as not found when the current lockfile is missing too', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const mockWorkspaceState: WorkspaceState = { + lastValidatedTimestamp, + pnpmfiles: [], + settings: { + excludeLinksFromLockfile: false, + linkWorkspacePackages: true, + preferWorkspacePackages: true, + }, + projects: {}, + filteredInstall: false, + } + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState) + jest.mocked(fsUtils.safeStat).mockResolvedValue(undefined) + jest.mocked(fsUtils.safeStatSync).mockReturnValue(undefined) + jest.mocked(statManifestFileUtils.statManifestFile).mockResolvedValue(undefined) + jest.mocked(lockfileFs.readCurrentLockfile).mockResolvedValue(null) + jest.mocked(lockfileFs.readWantedLockfile).mockResolvedValue(null) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: {}, + rootProjectManifestDir: '/project', + pnpmfile: [], + ...mockWorkspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toMatch(/Cannot find a lockfile/) + expect(result.wantedLockfileToRestore).toBeUndefined() + }) + + it('does not stand in for the wanted lockfile when git-branch lockfiles are enabled', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const mockWorkspaceState: WorkspaceState = { + lastValidatedTimestamp, + pnpmfiles: [], + settings: { + excludeLinksFromLockfile: false, + linkWorkspacePackages: true, + preferWorkspacePackages: true, + }, + projects: {}, + filteredInstall: false, + } + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState) + mockSingleProjectStats({ + wantedLockfileExists: false, + currentLockfileMtime: lastValidatedTimestamp - 10_000, + manifestMtime: lastValidatedTimestamp - 20_000, + }) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: {}, + rootProjectManifestDir: '/project', + pnpmfile: [], + useGitBranchLockfile: true, + ...mockWorkspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toMatch(/Cannot find a lockfile/) + expect(result.wantedLockfileToRestore).toBeUndefined() + }) + + it('returns the current lockfile to restore for a workspace with unmodified manifests', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const projectRootDir = '/workspace' as ProjectRootDir + const mockWorkspaceState: WorkspaceState = { + lastValidatedTimestamp, + pnpmfiles: [], + settings: { + excludeLinksFromLockfile: false, + linkWorkspacePackages: true, + preferWorkspacePackages: true, + }, + projects: { + [projectRootDir]: { name: 'root', version: '1.0.0' }, + }, + filteredInstall: false, + } + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState) + jest.mocked(fsUtils.safeStat).mockResolvedValue(undefined) + jest.mocked(fsUtils.safeStatSync).mockReturnValue(undefined) + jest.mocked(statManifestFileUtils.statManifestFile).mockImplementation(async () => ({ + mtime: new Date(lastValidatedTimestamp - 20_000), + mtimeMs: lastValidatedTimestamp - 20_000, + } as Stats)) + jest.mocked(lockfileFs.readCurrentLockfile).mockImplementation(async () => currentLockfile) + jest.mocked(lockfileFs.readWantedLockfile).mockResolvedValue(null) + + const opts: CheckDepsStatusOptions = { + allProjects: [ + { + rootDir: projectRootDir, + rootDirRealPath: '/workspace' as ProjectRootDirRealPath, + manifest: { name: 'root', version: '1.0.0' }, + writeProjectManifest: async () => {}, + }, + ], + workspaceDir: '/workspace', + sharedWorkspaceLockfile: true, + rootProjectManifest: { name: 'root', version: '1.0.0' }, + rootProjectManifestDir: '/workspace', + pnpmfile: [], + ...mockWorkspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + expect(result.wantedLockfileToRestore).toEqual({ + lockfile: currentLockfile, + lockfileDir: '/workspace', + }) + }) +}) diff --git a/installing/commands/src/install.ts b/installing/commands/src/install.ts index c7771eae2b..19ac180141 100644 --- a/installing/commands/src/install.ts +++ b/installing/commands/src/install.ts @@ -314,6 +314,7 @@ export type InstallCommandOptions = Pick> +} & Partial> export async function installDeps ( opts: InstallDepsOptions, params: string[] ): Promise { if (!opts.update && !opts.dedupe && params.length === 0 && opts.optimisticRepeatInstall) { - const { upToDate } = await checkDepsStatus({ + const { upToDate, wantedLockfileToRestore } = await checkDepsStatus({ ...opts, ignoreFilteredInstallCache: true, }) - if (upToDate) { + if (upToDate && await restoreWantedLockfileIfMissing(wantedLockfileToRestore, opts)) { if (opts.hooks?.customResolvers?.some(r => r.shouldRefreshResolution)) { logger.warn({ message: 'shouldRefreshResolution hooks were skipped because optimisticRepeatInstall is enabled.', @@ -592,3 +593,27 @@ function preferNonvulnerablePackageVersions (packageVulnerabilityAudit: PackageV } return preferredVersions } + +/** + * Restore a missing `pnpm-lock.yaml` from the current lockfile before the + * optimistic repeat-install short-circuit reports "Already up to date", so + * the fast path leaves the same on-disk contract a full install would. + * Returns `true` when the short-circuit may proceed: nothing to restore, + * lockfile writing is disabled (`useLockfile: false`), or the restore + * succeeded. A failed write returns `false` so the caller falls through to + * the full install instead of reporting up to date while `pnpm-lock.yaml` + * stays missing. + */ +async function restoreWantedLockfileIfMissing ( + wantedLockfileToRestore: { lockfile: LockfileObject, lockfileDir: string } | undefined, + opts: Pick +): Promise { + if (wantedLockfileToRestore == null || opts.useLockfile === false) return true + try { + await writeWantedLockfile(wantedLockfileToRestore.lockfileDir, wantedLockfileToRestore.lockfile) + return true + } catch (error) { + logger.debug({ msg: 'Failed to restore pnpm-lock.yaml from the current lockfile', error }) + return false + } +} diff --git a/installing/commands/test/install.ts b/installing/commands/test/install.ts index a52d5aaefb..ca185dbbc8 100644 --- a/installing/commands/test/install.ts +++ b/installing/commands/test/install.ts @@ -175,3 +175,27 @@ test('do not install Node.js when devEngines runtime is not set to onFail=downlo const lockfile = project.readLockfile() expect(lockfile.importers['.'].devDependencies).toBeUndefined() }) + +test('install restores a deleted pnpm-lock.yaml from the current lockfile without resolution', async () => { + prepareEmpty() + + await add.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + }, ['is-positive@1.0.0']) + + const originalLockfile = fs.readFileSync('pnpm-lock.yaml', 'utf8') + rimrafSync('pnpm-lock.yaml') + + // The dead registry proves the repeat install neither resolves nor + // verifies: the current lockfile (node_modules/.pnpm/lock.yaml) stands in + // as the wanted lockfile and pnpm-lock.yaml is restored from it. + await install.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + optimisticRepeatInstall: true, + registries: { default: 'http://127.0.0.1:9/' }, + }) + + expect(fs.readFileSync('pnpm-lock.yaml', 'utf8')).toBe(originalLockfile) +}) diff --git a/pacquet/crates/package-manager/src/install.rs b/pacquet/crates/package-manager/src/install.rs index ae2c75d6ce..c3a54d6cdc 100644 --- a/pacquet/crates/package-manager/src/install.rs +++ b/pacquet/crates/package-manager/src/install.rs @@ -1,7 +1,8 @@ use crate::{ BuildVerifiersError, HoistedDependencies, InstallFrozenLockfile, InstallFrozenLockfileError, - InstallWithFreshLockfile, InstallWithFreshLockfileError, ResolvedPackages, UpdateSeedPolicy, - build_resolution_verifiers, check_optimistic_repeat_install, + InstallWithFreshLockfile, InstallWithFreshLockfileError, OptimisticRepeatInstallCheck, + ResolvedPackages, UpdateSeedPolicy, build_resolution_verifiers, + check_optimistic_repeat_install, optimistic_repeat_install::Decision as OptimisticRepeatInstallDecision, }; use derive_more::{Display, Error}; @@ -493,14 +494,17 @@ where // `KeepAll` keeps `install` / `add` on the fast path. if matches!(update_seed_policy, UpdateSeedPolicy::KeepAll) && !frozen_lockfile - && let OptimisticRepeatInstallDecision::UpToDate = check_optimistic_repeat_install( - &workspace_root, - config, - node_linker, - included, - &project_manifests, - workspace_manifest.is_some(), - ) + && let OptimisticRepeatInstallDecision::UpToDate = + check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: &workspace_root, + config, + node_linker, + included, + project_manifests: &project_manifests, + is_workspace_install: workspace_manifest.is_some(), + lockfile, + catalogs: &catalogs, + }) { Reporter::emit(&LogEvent::Pnpm(PnpmLog { level: LogLevel::Info, @@ -1199,39 +1203,60 @@ fn check_lockfile_freshness( catalogs: &Catalogs, ignore_manifest_check: bool, ) -> Result<(), FreshnessCheckError> { + let parsed_overrides_opt = parse_config_overrides(config, catalogs)?; + check_lockfile_settings_drift(lockfile, config, parsed_overrides_opt.as_deref())?; + + if ignore_manifest_check { + return Ok(()); + } + // Pacquet has only one importer today ( tracks workspaces), // so the root project is the only thing to verify; once // workspaces land this becomes a per-project loop over // `lockfile.importers`. - let importer = lockfile.importers.get(Lockfile::ROOT_IMPORTER_KEY).ok_or_else(|| { - FreshnessCheckError::NoImporter { importer_id: Lockfile::ROOT_IMPORTER_KEY.to_string() } - })?; + check_importer_satisfies( + lockfile, + manifest, + Lockfile::ROOT_IMPORTER_KEY, + config, + parsed_overrides_opt.as_deref(), + ) +} - // `pnpm.overrides` values can use the `catalog:` protocol, which - // pnpm resolves against the workspace's catalogs *before* writing - // them to `pnpm-lock.yaml#overrides`. Mirror that here so an - // override declared as `"foo": "catalog:"` compares equal to the - // lockfile's already-resolved `"foo": ""`. Mirrors - // upstream's - // - // → `createOverridesMapFromParsed` pipeline. - let parsed_overrides_opt = match config.overrides.as_ref() { - Some(map) if !map.is_empty() => Some( +/// Parse `pnpm.overrides` from the config. Values can use the +/// `catalog:` protocol, which pnpm resolves against the workspace's +/// catalogs *before* writing them to `pnpm-lock.yaml#overrides` — +/// resolving here keeps an override declared as `"foo": "catalog:"` +/// comparable to the lockfile's already-resolved `"foo": ""`. +/// Mirrors upstream's +/// +/// → `createOverridesMapFromParsed` pipeline. +pub(crate) fn parse_config_overrides( + config: &Config, + catalogs: &Catalogs, +) -> Result>, FreshnessCheckError> { + match config.overrides.as_ref() { + Some(map) if !map.is_empty() => Ok(Some( pacquet_config_parse_overrides::parse_overrides_iter(map.iter(), catalogs) .map_err(FreshnessCheckError::InvalidOverrides)?, - ), - _ => None, - }; - let overrides_map: Option> = parsed_overrides_opt - .as_deref() - .map(pacquet_config_parse_overrides::create_overrides_map_from_parsed); + )), + _ => Ok(None), + } +} - // Outdated-settings gate (umbrella slice 7): check - // `ignoredOptionalDependencies` + `overrides` + - // `packageExtensionsChecksum` drift between the lockfile-recorded - // values and the current config before the per-importer specifier - // check. Mirrors upstream's - // [`getOutdatedLockfileSetting`](https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/settings-checker/src/getOutdatedLockfileSetting.ts). +/// Outdated-settings gate (umbrella slice 7): check +/// `ignoredOptionalDependencies` + `overrides` + +/// `packageExtensionsChecksum` drift between the lockfile-recorded +/// values and the current config before the per-importer specifier +/// check. Mirrors upstream's +/// [`getOutdatedLockfileSetting`](https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/settings-checker/src/getOutdatedLockfileSetting.ts). +pub(crate) fn check_lockfile_settings_drift( + lockfile: &Lockfile, + config: &Config, + parsed_overrides: Option<&[pacquet_config_parse_overrides::VersionOverride]>, +) -> Result<(), FreshnessCheckError> { + let overrides_map: Option> = + parsed_overrides.map(pacquet_config_parse_overrides::create_overrides_map_from_parsed); let package_extensions_checksum = config .package_extensions .as_ref() @@ -1254,11 +1279,26 @@ fn check_lockfile_freshness( config.inject_workspace_packages, config.peers_suffix_max_length, ) - .map_err(FreshnessCheckError::Stale)?; + .map_err(FreshnessCheckError::Stale) +} - if ignore_manifest_check { - return Ok(()); - } +/// Per-importer slice of the freshness gate: the manifest of the +/// project at `importer_id` must still be satisfied by the lockfile's +/// importer snapshot. Mirrors upstream's `satisfiesPackageManifest` +/// call inside +/// [`allProjectsAreUpToDate`](https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/verification/src/allProjectsAreUpToDate.ts) +/// / `assertWantedLockfileUpToDate`. +pub(crate) fn check_importer_satisfies( + lockfile: &Lockfile, + manifest: &PackageManifest, + importer_id: &str, + config: &Config, + parsed_overrides: Option<&[pacquet_config_parse_overrides::VersionOverride]>, +) -> Result<(), FreshnessCheckError> { + let importer = lockfile + .importers + .get(importer_id) + .ok_or_else(|| FreshnessCheckError::NoImporter { importer_id: importer_id.to_string() })?; // Apply `pnpm.overrides` to a *cloned* manifest before the // per-importer specifier check so the lockfile's specifiers — @@ -1268,19 +1308,18 @@ fn check_lockfile_freshness( // from the perspective of every consumer downstream of the // resolver. let overrider_manifest_holder; - let manifest_for_freshness: &PackageManifest = - if let Some(parsed) = parsed_overrides_opt.as_deref() { - let root_dir = manifest.path().parent().unwrap_or_else(|| Path::new(".")); - let overrider = crate::VersionsOverrider::new(parsed, root_dir); - overrider_manifest_holder = { - let mut cloned: PackageManifest = manifest.clone(); - overrider.apply(&mut cloned, Some(root_dir)); - cloned - }; - &overrider_manifest_holder - } else { - manifest + let manifest_for_freshness: &PackageManifest = if let Some(parsed) = parsed_overrides { + let root_dir = manifest.path().parent().unwrap_or_else(|| Path::new(".")); + let overrider = crate::VersionsOverrider::new(parsed, root_dir); + overrider_manifest_holder = { + let mut cloned: PackageManifest = manifest.clone(); + overrider.apply(&mut cloned, Some(root_dir)); + cloned }; + &overrider_manifest_holder + } else { + manifest + }; // Build the `ignoredOptionalDependencies` filter set. Mirrors // upstream's @@ -1307,13 +1346,8 @@ fn check_lockfile_freshness( .unwrap_or_default(); let is_ignored_optional: &dyn Fn(&str) -> bool = &|name: &str| ignored_set.contains(name); - satisfies_package_manifest( - importer, - manifest_for_freshness, - Lockfile::ROOT_IMPORTER_KEY, - is_ignored_optional, - ) - .map_err(FreshnessCheckError::Stale) + satisfies_package_manifest(importer, manifest_for_freshness, importer_id, is_ignored_optional) + .map_err(FreshnessCheckError::Stale) } /// Outcome of [`check_lockfile_freshness`]. Splits "user @@ -1321,7 +1355,7 @@ fn check_lockfile_freshness( /// (fatal for `--frozen-lockfile`, fall-through to the fresh-resolve /// path under `preferFrozenLockfile: true`). #[derive(Debug, Display, Error, Diagnostic)] -enum FreshnessCheckError { +pub(crate) enum FreshnessCheckError { /// The lockfile has no entry for the root importer. #[display( r#"Cannot install with "frozen-lockfile" because pnpm-lock.yaml has no `importers["{importer_id}"]` entry. Regenerate the lockfile with `pnpm install --lockfile-only`."# @@ -1662,7 +1696,7 @@ fn build_projects_map( /// are omitted; pnpm's `checkDepsStatus` /// only iterates fields present in the serialized object, so an /// absent key is silently skipped rather than treated as a drift. -fn build_workspace_state( +pub(crate) fn build_workspace_state( config: &Config, node_linker: NodeLinker, included: IncludedDependencies, diff --git a/pacquet/crates/package-manager/src/install/tests.rs b/pacquet/crates/package-manager/src/install/tests.rs index 8d94270198..ef29776be0 100644 --- a/pacquet/crates/package-manager/src/install/tests.rs +++ b/pacquet/crates/package-manager/src/install/tests.rs @@ -5671,15 +5671,18 @@ async fn frozen_lockfile_disables_optimistic_short_circuit() { // absent so the polarity of the gate is clear. } -/// Regression: a single-project install where `node_modules` is -/// still on disk (so the workspace-state file survives) but -/// `pnpm-lock.yaml` is gone must NOT short-circuit. This is the -/// `cache+node_modules` and `node_modules`-only benchmark scenario -/// pnpm finishes in ~5 s and pacquet was silently completing in -/// ~35 ms before the single-project lockfile gate landed. Mirrors -/// pnpm's [`throwLockfileNotFound`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L396-L401) -/// converting into `upToDate: false`. Companion to the workspace- -/// mode tolerance proved by +/// Regression: a single-project install with NO lockfile anywhere — +/// `pnpm-lock.yaml` is gone and the virtual store has no current +/// `lock.yaml` to stand in for it — must NOT short-circuit, even when +/// `node_modules` and the workspace-state file survive. There is +/// nothing to content-check the manifests against and nothing to +/// regenerate `pnpm-lock.yaml` from, so the full install must run. +/// Mirrors pnpm's [`throwLockfileNotFound`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L396-L401) +/// converting into `upToDate: false`. When the current lockfile IS +/// present, the fast path instead treats it as the wanted lockfile — +/// see `regenerates_missing_wanted_lockfile_from_current_when_manifests_unchanged` +/// in the `optimistic_repeat_install` tests. Companion to the +/// workspace-mode tolerance proved by /// [`returns_up_to_date_in_workspace_mode_without_lockfile`](crate::optimistic_repeat_install::tests::returns_up_to_date_in_workspace_mode_without_lockfile). #[tokio::test] async fn optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing() { @@ -5706,8 +5709,9 @@ async fn optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing( manifest.add_dependency("sibling", "link:../sibling", DependencyGroup::Prod).unwrap(); manifest.save().unwrap(); - // Deliberately do NOT write `pnpm-lock.yaml` — that's the - // scenario under test. + // Deliberately do NOT write `pnpm-lock.yaml` and do NOT seed a + // current `lock.yaml` in the virtual store — that's the scenario + // under test. let mut config = Config::new(); config.lockfile = false; @@ -5716,19 +5720,6 @@ async fn optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing( config.virtual_store_dir = virtual_store_dir; let config = config.leak(); - let lockfile: Lockfile = serde_saphyr::from_str(text_block! { - "lockfileVersion: '9.0'" - "importers:" - " .:" - " dependencies:" - " sibling:" - " specifier: link:../sibling" - " version: link:../sibling" - "packages: {}" - "snapshots: {}" - }) - .expect("parse lockfile"); - let included = pacquet_modules_yaml::IncludedDependencies { dependencies: true, dev_dependencies: false, @@ -5788,7 +5779,7 @@ async fn optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing( http_client_arc: std::sync::Arc::new(Default::default()), config, manifest: &manifest, - lockfile: Some(&lockfile), + lockfile: None, lockfile_path: None, dependency_groups: [DependencyGroup::Prod], frozen_lockfile: false, @@ -5816,7 +5807,7 @@ async fn optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing( LogEvent::Pnpm(log) if log.message == "Already up to date" )), "the optimistic 'Already up to date' log MUST NOT fire when \ - `pnpm-lock.yaml` is missing in a single-project install; got events: {captured:#?}", + no lockfile exists in a single-project install; got events: {captured:#?}", ); } @@ -5977,6 +5968,11 @@ async fn optimistic_repeat_install_round_trips_on_single_project_install() { drop((dir, mock_instance)); } +/// A fresh install records its lockfile-verification verdict, so a +/// repeat install that reaches the full path (the optimistic fast +/// path is disabled here — it would otherwise absorb the touched +/// manifest via the content re-check) hits the cache and never fans +/// out to the registry. #[tokio::test] async fn fresh_install_records_lockfile_verification_for_mtime_bypassed_noop() { let mock_instance = TestRegistry::start(); @@ -6065,6 +6061,7 @@ async fn fresh_install_records_lockfile_verification_for_mtime_bypassed_noop() { second_config.modules_dir = modules_dir; second_config.virtual_store_dir = virtual_store_dir; second_config.registry = "http://127.0.0.1:9/".to_string(); + second_config.optimistic_repeat_install = false; let second_config = second_config.leak(); Install { @@ -6112,6 +6109,269 @@ async fn fresh_install_records_lockfile_verification_for_mtime_bypassed_noop() { drop(dir); } +/// Shared setup for the offline repeat-install regression tests below: +/// a real install against the mock registry, after which the registry +/// is dropped and the packument cache is wiped. Any code path that +/// falls off the optimistic fast path — the resolver, the +/// lockfile-verification fan-out, a tarball fetch — would have to +/// reach the dead `127.0.0.1:9` registry and fail the install, so the +/// `expect` on the second run is the regression tripwire for the +/// repeat-install optimizations (the benchmarks don't run in CI; these +/// tests are what pins the "zero network, zero pipeline" property). +async fn install_then_go_offline() -> (tempfile::TempDir, &'static Config, PackageManifest) { + let mock_instance = TestRegistry::start(); + + let dir = tempdir().unwrap(); + let cache_dir = dir.path().join("cache"); + let store_dir = dir.path().join("pacquet-store"); + let project_root = dir.path().join("project"); + let modules_dir = project_root.join("node_modules"); + let virtual_store_dir = modules_dir.join(".pacquet"); + + std::fs::create_dir_all(&project_root).expect("create project root"); + let manifest_path = project_root.join("package.json"); + let mut manifest = PackageManifest::create_if_needed(manifest_path.clone()).unwrap(); + manifest + .add_dependency("@pnpm.e2e/hello-world-js-bin", "1.0.0", DependencyGroup::Prod) + .unwrap(); + manifest.save().unwrap(); + + let mut config = Config::new(); + config.cache_dir = cache_dir.clone(); + config.store_dir = store_dir.clone().into(); + config.modules_dir = modules_dir.clone(); + config.virtual_store_dir = virtual_store_dir.clone(); + config.registry = mock_instance.url(); + let config = config.leak(); + + Install { + tarball_mem_cache: Default::default(), + http_client: &Default::default(), + http_client_arc: std::sync::Arc::new(Default::default()), + config, + manifest: &manifest, + lockfile: None, + lockfile_path: None, + dependency_groups: [DependencyGroup::Prod], + frozen_lockfile: false, + prefer_frozen_lockfile: None, + ignore_manifest_check: false, + skip_runtimes: false, + trust_lockfile: false, + update_checksums: false, + is_full_install: true, + supported_architectures: None, + node_linker: pacquet_config::NodeLinker::default(), + lockfile_only: false, + resolved_packages: &Default::default(), + update_seed_policy: crate::UpdateSeedPolicy::KeepAll, + auth_override: None, + resolution_observer: None, + } + .run::() + .await + .expect("first install must succeed"); + + drop(mock_instance); + // The benchmark harness wipes `~/.cache/pnpm` (packument cache + + // `lockfile-verified.jsonl`) before every run; do the same so a + // regression can't hide behind a cache hit. + std::fs::remove_dir_all(&cache_dir).expect("wipe the cache dir"); + + let mut offline_config = Config::new(); + offline_config.cache_dir = cache_dir; + offline_config.store_dir = store_dir.into(); + offline_config.modules_dir = modules_dir; + offline_config.virtual_store_dir = virtual_store_dir; + offline_config.registry = "http://127.0.0.1:9/".to_string(); + let offline_config = offline_config.leak(); + + (dir, offline_config, manifest) +} + +/// Rewrite `package.json` with identical content but a strictly newer +/// mtime — the shape the vlt.sh benchmark prepare step (`npm pkg +/// delete`, `touch`) produces before every timed run. +fn touch_manifest(manifest: &PackageManifest) -> PackageManifest { + let manifest_path = manifest.path().to_path_buf(); + let manifest_text = std::fs::read_to_string(&manifest_path).expect("read package.json"); + std::fs::write(&manifest_path, manifest_text).expect("refresh package.json mtime"); + let forced_mtime = std::time::SystemTime::now() + std::time::Duration::from_secs(2); + std::fs::OpenOptions::new() + .write(true) + .open(&manifest_path) + .expect("open package.json") + .set_times(std::fs::FileTimes::new().set_modified(forced_mtime)) + .expect("force package.json mtime"); + PackageManifest::from_path(manifest_path).expect("reload manifest") +} + +/// A repeat install whose manifest was rewritten with identical +/// content (newer mtime) must short-circuit offline: no resolver, no +/// lockfile-verification fan-out, no install pipeline. Guards the +/// modified-manifests content re-check end-to-end through +/// `Install::run`'s dispatch ordering — the fast path has to run +/// *before* the verification gate for this to pass with a dead +/// registry and an empty packument/verdict cache. +#[tokio::test] +async fn optimistic_repeat_install_short_circuits_offline_when_touched_manifest_is_unchanged() { + let (dir, offline_config, manifest) = install_then_go_offline().await; + let project_root = manifest.path().parent().unwrap().to_path_buf(); + let touched_manifest = touch_manifest(&manifest); + let lockfile_path = project_root.join(Lockfile::FILE_NAME); + let wanted_lockfile = + Lockfile::load_wanted_from_dir(&project_root).expect("load wanted lockfile").unwrap(); + + static EVENTS: Mutex> = Mutex::new(Vec::new()); + EVENTS.lock().unwrap().clear(); + + struct RecordingReporter; + impl Reporter for RecordingReporter { + fn emit(event: &LogEvent) { + EVENTS.lock().unwrap().push(event.clone()); + } + } + + Install { + tarball_mem_cache: Default::default(), + http_client: &Default::default(), + http_client_arc: std::sync::Arc::new(Default::default()), + config: offline_config, + manifest: &touched_manifest, + lockfile: Some(&wanted_lockfile), + lockfile_path: Some(&lockfile_path), + dependency_groups: [DependencyGroup::Prod], + frozen_lockfile: false, + prefer_frozen_lockfile: None, + ignore_manifest_check: false, + skip_runtimes: false, + trust_lockfile: false, + update_checksums: false, + is_full_install: true, + supported_architectures: None, + node_linker: pacquet_config::NodeLinker::default(), + lockfile_only: false, + resolved_packages: &Default::default(), + update_seed_policy: crate::UpdateSeedPolicy::KeepAll, + auth_override: None, + resolution_observer: None, + } + .run::() + .await + .expect("repeat install with an unchanged-content manifest must not need the registry"); + + let captured = EVENTS.lock().unwrap(); + assert!( + captured.iter().any(|event| matches!( + event, + LogEvent::Pnpm(log) if log.message == "Already up to date" + )), + "the touched-but-unchanged manifest must take the fast path; got {captured:#?}", + ); + let pipeline_emits = captured + .iter() + .filter(|event| { + matches!( + event, + LogEvent::Context(_) | LogEvent::Stage(_) | LogEvent::LockfileVerification(_), + ) + }) + .count(); + assert_eq!( + pipeline_emits, 0, + "the fast path must not run any install-setup step; got {captured:#?}", + ); + + drop(dir); +} + +/// A repeat install with `pnpm-lock.yaml` deleted but `node_modules` +/// intact must short-circuit offline by treating the current lockfile +/// (`/lock.yaml`) as the wanted one, and must +/// restore `pnpm-lock.yaml` byte-identically. Guards the +/// current-as-wanted fallback end-to-end: a regression into the full +/// pipeline (resolution or the verification fan-out against an empty +/// cache) fails on the dead registry. +#[tokio::test] +async fn optimistic_repeat_install_restores_missing_lockfile_offline() { + let (dir, offline_config, manifest) = install_then_go_offline().await; + let project_root = manifest.path().parent().unwrap().to_path_buf(); + let lockfile_path = project_root.join(Lockfile::FILE_NAME); + let original_lockfile_bytes = + std::fs::read(&lockfile_path).expect("read pnpm-lock.yaml written by the first install"); + std::fs::remove_file(&lockfile_path).expect("delete pnpm-lock.yaml"); + let touched_manifest = touch_manifest(&manifest); + + static EVENTS: Mutex> = Mutex::new(Vec::new()); + EVENTS.lock().unwrap().clear(); + + struct RecordingReporter; + impl Reporter for RecordingReporter { + fn emit(event: &LogEvent) { + EVENTS.lock().unwrap().push(event.clone()); + } + } + + Install { + tarball_mem_cache: Default::default(), + http_client: &Default::default(), + http_client_arc: std::sync::Arc::new(Default::default()), + config: offline_config, + manifest: &touched_manifest, + lockfile: None, + lockfile_path: None, + dependency_groups: [DependencyGroup::Prod], + frozen_lockfile: false, + prefer_frozen_lockfile: None, + ignore_manifest_check: false, + skip_runtimes: false, + trust_lockfile: false, + update_checksums: false, + is_full_install: true, + supported_architectures: None, + node_linker: pacquet_config::NodeLinker::default(), + lockfile_only: false, + resolved_packages: &Default::default(), + update_seed_policy: crate::UpdateSeedPolicy::KeepAll, + auth_override: None, + resolution_observer: None, + } + .run::() + .await + .expect("repeat install with a deleted pnpm-lock.yaml must not need the registry"); + + let captured = EVENTS.lock().unwrap(); + assert!( + captured.iter().any(|event| matches!( + event, + LogEvent::Pnpm(log) if log.message == "Already up to date" + )), + "the deleted-lockfile repeat install must take the fast path; got {captured:#?}", + ); + let pipeline_emits = captured + .iter() + .filter(|event| { + matches!( + event, + LogEvent::Context(_) | LogEvent::Stage(_) | LogEvent::LockfileVerification(_), + ) + }) + .count(); + assert_eq!( + pipeline_emits, 0, + "the fast path must not run any install-setup step; got {captured:#?}", + ); + + let regenerated_bytes = + std::fs::read(&lockfile_path).expect("pnpm-lock.yaml must be regenerated"); + assert_eq!( + regenerated_bytes, original_lockfile_bytes, + "the regenerated pnpm-lock.yaml must be byte-identical to the one the install wrote", + ); + + drop(dir); +} + #[tokio::test] async fn fresh_lockfile_applies_overrides_to_direct_dependencies() { let (_dir, lockfile) = fresh_lockfile_only_with_overrides( diff --git a/pacquet/crates/package-manager/src/optimistic_repeat_install.rs b/pacquet/crates/package-manager/src/optimistic_repeat_install.rs index 079dc9a369..b5b865b858 100644 --- a/pacquet/crates/package-manager/src/optimistic_repeat_install.rs +++ b/pacquet/crates/package-manager/src/optimistic_repeat_install.rs @@ -16,19 +16,24 @@ //! — the underlying check. //! //! Scope: the mtime-vs-`lastValidatedTimestamp` branch (upstream's -//! `modifiedProjects.length === 0` exit at lines 263-271), plus the +//! `modifiedProjects.length === 0` exit at lines 263-271), the //! patch-file branch of upstream's -//! [`patchesOrHooksAreModified`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L597-L612): -//! a configured patch file whose mtime is newer than +//! [`patchesOrHooksAreModified`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L597-L612) +//! (a configured patch file whose mtime is newer than //! `lastValidatedTimestamp` invalidates the fast path even when its -//! `patchedDependencies` config entry is unchanged (a content edit the -//! key→path settings comparison can't see). The pnpmfile branch of -//! `patchesOrHooksAreModified` and the `assertWantedLockfileUpToDate` -//! re-verification are NOT ported here. When this function returns -//! `Decision::Skipped` the caller proceeds with the full install path, -//! which still has its own freshness guards (`check_lockfile_freshness`, -//! the no-op short-circuit). Remaining work tracked at -//! pnpm/pnpm#11940 (this issue). +//! `patchedDependencies` config entry is unchanged — a content edit the +//! key→path settings comparison can't see), and the modified-manifests +//! content re-check: when a manifest's mtime is newer but its +//! dependency-relevant content still matches the lockfile, upstream's +//! [`assertWantedLockfileUpToDate`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L483-L561) +//! still reports up-to-date (a `touch package.json`, a `scripts` edit, or +//! an `npm pkg set/delete` rewrite must not trigger a full install). The +//! pnpmfile branch of `patchesOrHooksAreModified` and the +//! `isLocalFileDepUpdated` branch of `linkedPackagesAreUpToDate` are NOT +//! ported here. When this function returns `Decision::Skipped` the caller +//! proceeds with the full install path, which still has its own freshness +//! guards (`check_lockfile_freshness`, the no-op short-circuit). Remaining +//! work tracked at pnpm/pnpm#11940 (this issue). //! //! [`checkDepsStatus`]: https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts //! @@ -46,13 +51,14 @@ use std::{ time::SystemTime, }; +use pacquet_catalogs_types::Catalogs; use pacquet_config::{Config, LinkWorkspacePackages, NodeLinker}; -use pacquet_lockfile::Lockfile; +use pacquet_lockfile::{ImporterDepVersion, Lockfile, ProjectSnapshot}; use pacquet_modules_yaml::IncludedDependencies; -use pacquet_package_manifest::PackageManifest; +use pacquet_package_manifest::{DependencyGroup, PackageManifest}; use pacquet_workspace_state::{ NodeLinker as WorkspaceStateNodeLinker, WorkspaceState, WorkspaceStateSettings, - load_workspace_state, + load_workspace_state, update_workspace_state, }; /// Outcome of [`check_optimistic_repeat_install`]. @@ -70,40 +76,63 @@ pub enum Decision { Skipped { reason: &'static str }, } +/// Inputs to [`check_optimistic_repeat_install`]. +pub struct OptimisticRepeatInstallCheck<'a> { + /// The directory containing `pnpm-workspace.yaml` (or the project + /// root when no workspace manifest exists — same fallback as + /// [`Install::run`](crate::Install::run)). + pub workspace_root: &'a Path, + pub config: &'a Config, + pub node_linker: NodeLinker, + pub included: IncludedDependencies, + /// Every importer's `(root_dir, manifest)` pair. For a + /// single-project install it's just the root manifest; for a + /// workspace install it's every project the resolver would + /// otherwise walk. The caller passes this in (rather than this + /// function rediscovering it) so the same walk seeds the regular + /// install path on the fall-through. + pub project_manifests: &'a [(PathBuf, &'a PackageManifest)], + /// `true` when a `pnpm-workspace.yaml` drives the install — that + /// selects pnpm's + /// [`allProjects && workspaceDir` branch](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L187) + /// which keys the manifest and lockfile comparisons off + /// `lastValidatedTimestamp`. `false` (no workspace manifest) + /// selects the + /// [single-project branch](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L387-L462) + /// which additionally requires `pnpm-lock.yaml` to exist on disk — + /// pnpm throws `RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND` otherwise, which + /// the outer `try` converts into `upToDate: false` — and keys its + /// comparisons off the lockfile mtimes instead. + pub is_workspace_install: bool, + /// The wanted lockfile as loaded by the CLI (`None` when + /// `pnpm-lock.yaml` is absent or empty). Consulted only by the + /// modified-manifests content re-check; the pure-mtime fast path + /// never reads it. When `None` and `/lock.yaml` + /// exists, the current lockfile stands in as the wanted one — it + /// records exactly what the previous install materialized — and + /// `pnpm-lock.yaml` is regenerated from it before the check + /// reports up-to-date. + pub lockfile: Option<&'a Lockfile>, + /// Workspace catalogs, for resolving `catalog:` values inside + /// `pnpm.overrides` before the lockfile settings comparison. + pub catalogs: &'a Catalogs, +} + /// Run the workspace-state freshness fast path. Returns /// [`Decision::UpToDate`] when the install can short-circuit. /// -/// `workspace_root` is the directory containing `pnpm-workspace.yaml` -/// (or the project root when no workspace manifest exists — same -/// fallback as [`Install::run`](crate::Install::run)). -/// -/// `project_manifests` lists every importer's `(root_dir, manifest)` -/// pair. For a single-project install it's just the root manifest; -/// for a workspace install it's every project the resolver would -/// otherwise walk. The caller passes this in (rather than this -/// function rediscovering it) so the same walk seeds the regular -/// install path on the fall-through. -/// -/// `is_workspace_install` is `true` when a `pnpm-workspace.yaml` -/// drives the install — that selects pnpm's -/// [`allProjects && workspaceDir` branch](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L187) -/// which exits purely on the per-manifest mtime check. `false` (no -/// workspace manifest) selects the -/// [single-project branch](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L387-L462) -/// which additionally requires `pnpm-lock.yaml` to exist on disk — -/// pnpm throws `RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND` otherwise, which -/// the outer `try` converts into `upToDate: false`. -/// /// Always returns `Decision::Skipped` when /// `config.optimistic_repeat_install` is `false`. -pub fn check_optimistic_repeat_install( - workspace_root: &Path, - config: &Config, - node_linker: NodeLinker, - included: IncludedDependencies, - project_manifests: &[(PathBuf, &PackageManifest)], - is_workspace_install: bool, -) -> Decision { +pub fn check_optimistic_repeat_install(check: &OptimisticRepeatInstallCheck<'_>) -> Decision { + let &OptimisticRepeatInstallCheck { + workspace_root, + config, + node_linker, + included, + project_manifests, + is_workspace_install, + .. + } = check; if !config.optimistic_repeat_install { return Decision::Skipped { reason: "optimistic_repeat_install disabled" }; } @@ -140,17 +169,27 @@ pub fn check_optimistic_repeat_install( }; } - // Single-project installs require `pnpm-lock.yaml` on disk to - // even attempt the fast path. Upstream's single-project branch - // at + // Single-project installs require a lockfile to even attempt the + // fast path. Upstream's single-project branch at + // // throws `RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND` when // `wantedLockfileStats` is absent, which the outer `try` - // converts into `upToDate: false`. Workspace installs skip this - // gate — pnpm's workspace branch returns `upToDate: true` purely - // off the manifest-mtime check (its only lockfile probe, - // `findConflictedLockfileDir`, silently `continue`s on ENOENT at + // converts into `upToDate: false`. Pacquet additionally accepts + // the *current* lockfile (`/lock.yaml`) as a + // stand-in when `pnpm-lock.yaml` is missing: it records exactly + // what the previous install materialized, so the content checks + // can run against it and `pnpm-lock.yaml` is regenerated from it + // on success — the same substitution the full install path makes + // when it synthesizes the wanted lockfile from the current one. + // Workspace installs skip this gate — pnpm's workspace branch + // returns `upToDate: true` purely off the manifest-mtime check + // (its only lockfile probe, `findConflictedLockfileDir`, silently + // `continue`s on ENOENT at // ). - if !is_workspace_install && !workspace_root.join(Lockfile::FILE_NAME).exists() { + if !is_workspace_install + && !workspace_root.join(Lockfile::FILE_NAME).exists() + && !config.virtual_store_dir.join(Lockfile::CURRENT_FILE_NAME).exists() + { return Decision::Skipped { reason: "wanted lockfile missing" }; } @@ -168,12 +207,470 @@ pub fn check_optimistic_repeat_install( // returns `upToDate: true` when none have an mtime newer than // `workspaceState.lastValidatedTimestamp`. The walk has to // succeed (read errors mean we can't *prove* freshness, so fall - // through), and any newer mtime invalidates. - if !manifests_unchanged_since(state.last_validated_timestamp, project_manifests) { - return Decision::Skipped { reason: "a manifest is newer than the last validation" }; + // through). + let Some(manifest_stats) = stat_manifests(project_manifests) else { + return Decision::Skipped { reason: "failed to stat a project manifest" }; + }; + let modified: Vec<&ManifestStat<'_>> = manifest_stats + .iter() + .filter(|stat| stat.mtime_ms > state.last_validated_timestamp) + .collect(); + if modified.is_empty() { + return match regenerate_wanted_lockfile_if_missing(check, None) { + Ok(()) => Decision::UpToDate, + Err(reason) => Decision::Skipped { reason }, + }; } - Decision::UpToDate + // A newer mtime alone doesn't invalidate: upstream's + // modified-manifests branch re-checks the *content* against the + // wanted lockfile (`assertWantedLockfileUpToDate`) so a rewrite + // that left the dependency fields intact — `touch`, a `scripts` + // edit, `npm pkg set/delete` — still reports up to date. + match modified_manifests_match_lockfile(check, &state, &modified) { + Ok(loaded_current) => { + if let Err(reason) = regenerate_wanted_lockfile_if_missing(check, loaded_current) { + return Decision::Skipped { reason }; + } + // "update lastValidatedTimestamp to prevent pointless + // repeat" — upstream's workspace branch rewrites the + // state after the content checks pass at + // . + // The single-project branch keys its comparisons off the + // lockfile mtimes instead and leaves the state alone. A + // failed write only costs the next run a repeat of the + // content check, so it degrades rather than fails. + if is_workspace_install { + let new_state = crate::install::build_workspace_state( + config, + node_linker, + included, + project_manifests, + ); + if let Err(error) = update_workspace_state(workspace_root, &new_state) { + tracing::warn!( + target: "pacquet::install", + ?error, + "Failed to refresh the workspace state after the repeat-install content check", + ); + } + } + Decision::UpToDate + } + Err(reason) => Decision::Skipped { reason }, + } +} + +/// Restore a missing `pnpm-lock.yaml` from the current lockfile before +/// the fast path reports "Already up to date", so the short-circuit +/// leaves the same on-disk contract a full install would (the full +/// path synthesizes the wanted lockfile from the current one and +/// rewrites it). No-op when `pnpm-lock.yaml` was loaded, when lockfile +/// writing is disabled (`lockfile: false`), or when there is no +/// current lockfile to restore from (a dependency-less project). +/// A write failure falls through to the full install path rather than +/// reporting up-to-date while leaving the lockfile missing. +fn regenerate_wanted_lockfile_if_missing( + check: &OptimisticRepeatInstallCheck<'_>, + loaded_current: Option, +) -> Result<(), &'static str> { + if check.lockfile.is_some() || !check.config.lockfile { + return Ok(()); + } + let current = match loaded_current { + Some(current) => Some(current), + None => Lockfile::load_current_from_virtual_store_dir(&check.config.virtual_store_dir) + .map_err(|_| "the current lockfile cannot be loaded")?, + }; + let Some(current) = current else { + return Ok(()); + }; + current + .save_to_path(&check.workspace_root.join(Lockfile::FILE_NAME)) + .map_err(|_| "failed to regenerate pnpm-lock.yaml from the current lockfile") +} + +/// One project manifest's stat outcome, paired with the inputs the +/// content re-check needs. +struct ManifestStat<'a> { + root_dir: &'a Path, + manifest: &'a PackageManifest, + mtime_ms: i64, +} + +/// Port of upstream's modified-manifests branch: the lockfile-equality +/// assertion ([`assertLockfilesEqual`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/assertLockfilesEqual.ts)) +/// plus [`assertWantedLockfileUpToDate`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L483-L561) +/// (settings drift, per-importer specifier match, linked-package +/// freshness) for every project whose manifest is newer than the last +/// validation. `Err` carries the `Decision::Skipped` reason; upstream +/// converts the equivalent throws into `upToDate: false`. +/// +/// When `pnpm-lock.yaml` is absent, the current lockfile stands in as +/// the wanted one (see the lockfile gate in +/// [`check_optimistic_repeat_install`]); `Ok(Some(_))` then carries the +/// loaded current lockfile so the caller can regenerate +/// `pnpm-lock.yaml` from it without a second read. +fn modified_manifests_match_lockfile( + check: &OptimisticRepeatInstallCheck<'_>, + state: &WorkspaceState, + modified: &[&ManifestStat<'_>], +) -> Result, &'static str> { + let &OptimisticRepeatInstallCheck { + workspace_root, + config, + project_manifests, + is_workspace_install, + lockfile, + catalogs, + .. + } = check; + let mut loaded_current: Option = None; + let mut wanted_is_current = false; + let (wanted, wanted_mtime_ms): (&Lockfile, i64) = match lockfile { + Some(wanted) => { + let Some(mtime) = mtime_ms(&workspace_root.join(Lockfile::FILE_NAME)) else { + return Err( + "a manifest is newer than the last validation and pnpm-lock.yaml cannot be stat'd", + ); + }; + (wanted, mtime) + } + None => { + let current_path = config.virtual_store_dir.join(Lockfile::CURRENT_FILE_NAME); + let Some(mtime) = mtime_ms(¤t_path) else { + return Err( + "a manifest is newer than the last validation and no lockfile is loaded", + ); + }; + let current = Lockfile::load_current_from_virtual_store_dir(&config.virtual_store_dir) + .map_err(|_| "the current lockfile cannot be loaded")? + .ok_or("a manifest is newer than the last validation and no lockfile is loaded")?; + wanted_is_current = true; + (&*loaded_current.insert(current), mtime) + } + }; + + // Decide which modified projects need the full content check, and + // whether the wanted lockfile must be compared against the current + // one (`/lock.yaml`). + let to_check: &[&ManifestStat<'_>] = if wanted_is_current { + // The wanted lockfile IS the current one — there's no second + // lockfile to assert equality against, and the mtime + // short-circuits below compare the two lockfile files, so they + // don't apply. Every modified project gets the content check. + modified + } else if is_workspace_install { + // Workspace branch: a wanted lockfile newer than the last + // validation must equal what the previous install materialized. + // Mirrors . + if wanted_mtime_ms > state.last_validated_timestamp { + assert_wanted_lockfile_equals_current(wanted, config)?; + } + modified + } else { + // Single-project branch keys off the lockfile mtimes instead of + // `lastValidatedTimestamp`. Mirrors + // . + let current_mtime_ms = + mtime_ms(&config.virtual_store_dir.join(Lockfile::CURRENT_FILE_NAME)); + if let Some(current_mtime_ms) = current_mtime_ms + && wanted_mtime_ms > current_mtime_ms + { + assert_wanted_lockfile_equals_current(wanted, config)?; + } + let root = modified.first().expect("modified-manifests branch requires a modified project"); + if root.mtime_ms > wanted_mtime_ms { + modified + } else if current_mtime_ms.is_some() { + // "The manifest file is not newer than the lockfile. + // Exiting check." + &[] + } else if wanted.packages.as_ref().is_some_and(|packages| !packages.is_empty()) { + // RUN_CHECK_DEPS_NO_DEPS: the lockfile requires + // dependencies but nothing was ever installed. + return Err("the lockfile requires dependencies but none were installed"); + } else { + &[] + } + }; + + if to_check.is_empty() { + return Ok(loaded_current); + } + + let parsed_overrides = crate::install::parse_config_overrides(config, catalogs) + .map_err(|_| "pnpm.overrides cannot be parsed")?; + if let Err(error) = + crate::install::check_lockfile_settings_drift(wanted, config, parsed_overrides.as_deref()) + { + tracing::debug!(target: "pacquet::install", %error, "repeat-install content check: lockfile settings drift"); + return Err("a lockfile setting drifted from the current configuration"); + } + + let linked_ctx = LinkedPackagesContext::new(config, project_manifests); + for project in to_check { + let importer_id = + pacquet_workspace::importer_id_from_root_dir(workspace_root, project.root_dir); + if let Err(error) = crate::install::check_importer_satisfies( + wanted, + project.manifest, + &importer_id, + config, + parsed_overrides.as_deref(), + ) { + tracing::debug!(target: "pacquet::install", %error, importer_id, "repeat-install content check: manifest no longer satisfied"); + return Err("a modified manifest is no longer satisfied by the lockfile"); + } + let Some(importer) = wanted.importers.get(&importer_id) else { + return Err("a modified project has no importer entry in the lockfile"); + }; + if !linked_packages_are_up_to_date( + &linked_ctx, + project.root_dir, + project.manifest, + importer, + ) { + return Err("a linked package is out of date"); + } + } + Ok(loaded_current) +} + +/// Port of upstream's +/// [`assertLockfilesEqual`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/assertLockfilesEqual.ts): +/// with no current lockfile every importer of the wanted one must be +/// dependency-free (`RUN_CHECK_DEPS_NO_DEPS`); otherwise the two parsed +/// lockfiles must be equal (`RUN_CHECK_DEPS_OUTDATED_DEPS`). +fn assert_wanted_lockfile_equals_current( + wanted: &Lockfile, + config: &Config, +) -> Result<(), &'static str> { + let current = Lockfile::load_current_from_virtual_store_dir(&config.virtual_store_dir) + .map_err(|_| "the current lockfile cannot be loaded")?; + match current { + None => { + let any_deps = wanted.importers.values().any(|snapshot| { + snapshot + .dependencies_by_groups([ + DependencyGroup::Prod, + DependencyGroup::Dev, + DependencyGroup::Optional, + ]) + .next() + .is_some() + }); + if any_deps { + Err("the lockfile requires dependencies but none were installed") + } else { + Ok(()) + } + } + Some(current) => { + if ¤t == wanted { + Ok(()) + } else { + Err("the installed dependencies are not up to date with the lockfile") + } + } + } +} + +/// Shared lookups for [`linked_packages_are_up_to_date`], built once +/// per content check. Mirrors the `bind(null, {...})` context upstream +/// creates in +/// [`checkDepsStatus`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L317-L329). +struct LinkedPackagesContext<'a> { + link_workspace_packages: bool, + manifests_by_dir: std::collections::HashMap<&'a Path, &'a PackageManifest>, + /// `name → version → root_dir` over the workspace's projects — + /// the same index upstream's `workspacePackages` map carries. + workspace_packages: + std::collections::HashMap>, +} + +impl<'a> LinkedPackagesContext<'a> { + fn new(config: &Config, project_manifests: &'a [(PathBuf, &'a PackageManifest)]) -> Self { + let mut manifests_by_dir = std::collections::HashMap::new(); + let mut workspace_packages: std::collections::HashMap< + String, + std::collections::HashMap, + > = std::collections::HashMap::new(); + for (root_dir, manifest) in project_manifests { + manifests_by_dir.insert(root_dir.as_path(), *manifest); + if let (Some(name), Some(version)) = ( + manifest_string_field(manifest, "name"), + manifest_string_field(manifest, "version"), + ) { + workspace_packages.entry(name).or_default().insert(version, root_dir.as_path()); + } + } + LinkedPackagesContext { + link_workspace_packages: config.link_workspace_packages != LinkWorkspacePackages::Off, + manifests_by_dir, + workspace_packages, + } + } + + /// The version of the package manifest at `dir`, preferring the + /// already-loaded workspace manifests over a disk read. Mirrors + /// upstream's `manifestsByDir[linkedDir] ?? safeReadPackageJsonFromDir`. + fn linked_version(&self, dir: &Path) -> Option { + if let Some(manifest) = self.manifests_by_dir.get(dir) { + return manifest_string_field(manifest, "version"); + } + pacquet_package_manifest::safe_read_package_json_from_dir(dir) + .ok() + .flatten() + .and_then(|value| value.get("version").and_then(|v| v.as_str()).map(str::to_string)) + } +} + +/// Port of upstream's +/// [`linkedPackagesAreUpToDate`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/lockfile/verification/src/linkedPackagesAreUpToDate.ts): +/// every importer dependency that resolved to a workspace link must +/// still link under today's manifest spec, and every one that resolved +/// to the registry must not have become linkable. The +/// `isLocalFileDepUpdated` branch (a `file:` directory specifier) is +/// not ported — those entries conservatively report "not up to date" so +/// the full install path re-evaluates them. +fn linked_packages_are_up_to_date( + ctx: &LinkedPackagesContext<'_>, + project_dir: &Path, + manifest: &PackageManifest, + snapshot: &ProjectSnapshot, +) -> bool { + const GROUPS: [(DependencyGroup, &str); 3] = [ + (DependencyGroup::Optional, "optionalDependencies"), + (DependencyGroup::Prod, "dependencies"), + (DependencyGroup::Dev, "devDependencies"), + ]; + for (group, manifest_field) in GROUPS { + let Some(lockfile_deps) = snapshot.get_map_by_group(group) else { + continue; + }; + let Some(manifest_deps) = + manifest.value().get(manifest_field).and_then(|value| value.as_object()) + else { + continue; + }; + for (dep_name, dep) in lockfile_deps { + let dep_name = dep_name.to_string(); + let Some(current_spec) = manifest_deps.get(&dep_name).and_then(|v| v.as_str()) else { + continue; + }; + if ref_is_local_directory(&dep.specifier) { + // A `file:` specifier that resolved to `link:` (e.g. an + // injected self-reference) is a local link with no + // `packages:` entry — up to date by construction. + if matches!(dep.version, ImporterDepVersion::Link(_)) { + continue; + } + return false; + } + let link_target = dep.version.as_link_target(); + let is_linked = link_target.is_some(); + if is_linked + && (current_spec.starts_with("link:") + || current_spec.starts_with("file:") + || current_spec.starts_with("workspace:.")) + { + continue; + } + // A linked dependency whose spec is a distribution tag is + // considered up to date to skip full resolution + // (). + if is_linked && spec_is_distribution_tag(current_spec) { + continue; + } + let linked_dir: Option> = match link_target { + Some(target) => Some(std::borrow::Cow::Owned(project_dir.join(target))), + None => dep + .version + .as_regular() + .map(|ver| ver.to_string()) + .and_then(|version| ctx.workspace_packages.get(&dep_name)?.get(&version)) + .map(|dir| std::borrow::Cow::Borrowed(*dir)), + }; + let Some(linked_dir) = linked_dir else { + continue; + }; + if !ctx.link_workspace_packages && !current_spec.starts_with("workspace:") { + // A linkable dir exists, but nothing requests linking it. + continue; + } + let available_range = version_range_of_spec(current_spec); + let local_package_satisfies_range = matches!(available_range, "*" | "^" | "~") + || ctx + .linked_version(&linked_dir) + .is_some_and(|version| semver_satisfies_loosely(&version, available_range)); + if is_linked != local_package_satisfies_range { + return false; + } + } + } + true +} + +/// Port of upstream's +/// [`refIsLocalDirectory`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/lockfile/utils/src/refIsLocalTarball.ts): +/// a `file:` specifier that is not a tarball. +fn ref_is_local_directory(specifier: &str) -> bool { + specifier.starts_with("file:") + && !(specifier.ends_with(".tgz") + || specifier.ends_with(".tar.gz") + || specifier.ends_with(".tar")) +} + +/// Whether a bare specifier is an npm distribution tag (`latest`, +/// `beta`, ...). Approximates upstream's +/// `getVersionSelectorType(spec)?.type === 'tag'` — anything that +/// doesn't parse as a semver range and contains only characters a tag +/// name may carry. Protocol-ish specs (`workspace:^1.0.0`, +/// `npm:foo@1`) contain `:`/`@`/`/` and therefore never match, same as +/// `version-selector-type` rejecting them. +fn spec_is_distribution_tag(spec: &str) -> bool { + !spec.is_empty() + && spec.parse::().is_err() + && spec.chars().all(|char| char.is_ascii_alphanumeric() || matches!(char, '-' | '_' | '.')) +} + +/// Port of upstream's `getVersionRange`: strips the `workspace:` / +/// `npm:` envelope so the remainder can be compared as a semver range. +fn version_range_of_spec(spec: &str) -> &str { + if let Some(rest) = spec.strip_prefix("workspace:") { + return rest; + } + if let Some(rest) = spec.strip_prefix("npm:") { + // `npm:@` — the `@` search starts at index 1 so a + // leading scope `@` isn't mistaken for the separator. + return match rest.get(1..).and_then(|tail| tail.find('@')) { + Some(at) => { + let range = &rest[at + 2..]; + if range.is_empty() { "*" } else { range } + } + None => "*", + }; + } + spec +} + +/// `semver.satisfies(version, range, { loose: true })` — a version or +/// range that doesn't parse fails the match. +fn semver_satisfies_loosely(version: &str, range: &str) -> bool { + let Ok(version) = version.parse::() else { return false }; + let Ok(range) = range.parse::() else { return false }; + range.satisfies(&version) +} + +/// Millisecond mtime of `path`, `None` when it can't be stat'd. +/// Converts wall-clock to ms-since-epoch the same way +/// `pacquet_workspace_state::now_millis` does on the write side, so +/// comparisons against `lastValidatedTimestamp` are apples-to-apples. +fn mtime_ms(path: &Path) -> Option { + let modified = fs::metadata(path).and_then(|metadata| metadata.modified()).ok()?; + let elapsed = modified.duration_since(SystemTime::UNIX_EPOCH).ok()?; + Some(i64::try_from(elapsed.as_millis()).unwrap_or(i64::MAX)) } /// Compare today's settings against what the previous install @@ -449,12 +946,6 @@ fn manifest_string_field(manifest: &PackageManifest, key: &str) -> Option bool { - project_manifests.iter().all(|(_, manifest)| { - let Ok(metadata) = fs::metadata(manifest.path()) else { - return false; - }; - let Ok(modified) = metadata.modified() else { - return false; - }; - // Convert wall-clock to ms-since-epoch the same way - // `pacquet_workspace_state::now_millis` does on the write - // side, so a `> cutoff` comparison is apples-to-apples. - let Ok(elapsed) = modified.duration_since(SystemTime::UNIX_EPOCH) else { - return false; - }; - let modified_ms = i64::try_from(elapsed.as_millis()).unwrap_or(i64::MAX); - modified_ms <= cutoff_ms - }) +/// Stat every project's `package.json`. `None` on any stat failure — +/// "can't prove freshness, fall through" — matching pnpm's +/// [`statManifestFile`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/statManifestFile.ts) +/// behavior on missing files (it throws, which `checkDepsStatus` +/// catches via the outer `try`). +fn stat_manifests<'a>( + project_manifests: &'a [(PathBuf, &'a PackageManifest)], +) -> Option>> { + project_manifests + .iter() + .map(|(root_dir, manifest)| { + mtime_ms(manifest.path()).map(|mtime_ms| ManifestStat { + root_dir: root_dir.as_path(), + manifest, + mtime_ms, + }) + }) + .collect() } #[cfg(test)] diff --git a/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs b/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs index 2a23a1a52c..42af0b564d 100644 --- a/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs +++ b/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs @@ -1,4 +1,6 @@ -use super::{Decision, check_optimistic_repeat_install, current_settings}; +use super::{ + Decision, OptimisticRepeatInstallCheck, check_optimistic_repeat_install, current_settings, +}; use pacquet_config::Config; use pacquet_lockfile::Lockfile; use pacquet_modules_yaml::IncludedDependencies; @@ -13,6 +15,27 @@ fn isolated_included() -> IncludedDependencies { IncludedDependencies { dependencies: true, dev_dependencies: true, optional_dependencies: true } } +/// Run the fast-path check in single-project mode with no loaded +/// lockfile and no catalogs — the shape every pre-content-check test +/// exercises. +fn check( + workspace_root: &std::path::Path, + config: &Config, + node_linker: pacquet_config::NodeLinker, + project_manifests: &[(std::path::PathBuf, &PackageManifest)], +) -> Decision { + check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root, + config, + node_linker, + included: isolated_included(), + project_manifests, + is_workspace_install: false, + lockfile: None, + catalogs: &Default::default(), + }) +} + /// Write an empty `pnpm-lock.yaml` to satisfy the single-project /// branch's lockfile-existence gate. The fast path only checks /// existence, not contents. @@ -101,13 +124,11 @@ fn returns_up_to_date_when_state_and_manifests_agree() { let (dir, config, manifest) = setup_fresh_install(pacquet_config::NodeLinker::Isolated, "root", "1.0.0", ""); - let decision = check_optimistic_repeat_install( + let decision = check( dir.path(), config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(dir.path().to_path_buf(), &manifest)], - false, ); assert_eq!(decision, Decision::UpToDate); } @@ -129,13 +150,11 @@ fn returns_skipped_when_config_disabled() { // Even though the state file is missing (would also skip), the // disabled-config branch is checked first — that's the reason // string we assert on. - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("disabled"))); } @@ -154,13 +173,11 @@ fn returns_skipped_when_no_state_file() { config.modules_dir = workspace_root.join("node_modules"); let config = config.leak(); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!( matches!(decision, Decision::Skipped { reason } if reason.contains("no workspace state")), @@ -180,13 +197,11 @@ fn returns_skipped_when_manifest_is_newer_than_validation() { fs::write(&manifest_path, r#"{"name":"root","version":"1.0.0"}"#).unwrap(); let refreshed_manifest = PackageManifest::from_path(manifest_path).unwrap(); - let decision = check_optimistic_repeat_install( + let decision = check( dir.path(), config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(dir.path().to_path_buf(), &refreshed_manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("newer"))); } @@ -199,13 +214,11 @@ fn returns_skipped_when_node_linker_drifts() { let (dir, config, manifest) = setup_fresh_install(pacquet_config::NodeLinker::Hoisted, "root", "1.0.0", ""); - let decision = check_optimistic_repeat_install( + let decision = check( dir.path(), config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(dir.path().to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -234,13 +247,11 @@ fn returns_skipped_when_workspace_project_set_changes() { // fire — we want to prove the project-list branch fires. write_state(dir.path(), now_millis() + 60_000, settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( dir.path(), config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(dir.path().to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("project list"))); } @@ -284,13 +295,11 @@ fn returns_skipped_when_overrides_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -327,13 +336,11 @@ fn returns_skipped_when_inject_workspace_packages_drifts() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -370,13 +377,11 @@ fn returns_skipped_when_enable_global_virtual_store_drifts() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -402,13 +407,11 @@ fn returns_up_to_date_when_recorded_global_virtual_store_is_explicit_off() { ); write_state(dir.path(), now_millis(), settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( dir.path(), config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(dir.path().to_path_buf(), &manifest)], - false, ); assert_eq!(decision, Decision::UpToDate); } @@ -444,13 +447,11 @@ fn returns_skipped_when_exclude_links_from_lockfile_drifts() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -485,13 +486,11 @@ fn returns_skipped_when_minimum_release_age_drifts() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -525,13 +524,11 @@ fn returns_skipped_when_minimum_release_age_ignore_missing_time_drifts() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -568,13 +565,11 @@ fn returns_skipped_when_ignored_optional_dependencies_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -614,13 +609,11 @@ fn returns_skipped_when_patched_dependencies_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -662,13 +655,11 @@ fn returns_skipped_when_patch_file_modified_after_validation() { sleep(Duration::from_millis(20)); fs::write(&patch_path, "--- a\n+++ b\n+edited\n").unwrap(); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("patch"))); } @@ -708,13 +699,11 @@ fn returns_up_to_date_when_patch_file_unchanged() { sleep(Duration::from_millis(20)); write_state(workspace_root, now_millis(), settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert_eq!(decision, Decision::UpToDate); } @@ -749,13 +738,11 @@ fn returns_skipped_when_dedupe_peers_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -791,13 +778,11 @@ fn returns_skipped_when_prefer_workspace_packages_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -833,13 +818,11 @@ fn returns_skipped_when_peers_suffix_max_length_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -889,13 +872,11 @@ fn returns_skipped_when_package_extensions_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -931,13 +912,11 @@ fn returns_skipped_when_allow_builds_drift() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -972,13 +951,11 @@ fn returns_skipped_when_dedupe_direct_deps_drifts() { ); write_state(workspace_root, now_millis() + 60_000, stale_settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("settings"))); } @@ -1030,13 +1007,11 @@ fn returns_up_to_date_when_state_carries_unported_pnpm_settings() { ); write_state(workspace_root, now_millis() + 60_000, settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert_eq!(decision, Decision::UpToDate); } @@ -1076,13 +1051,11 @@ fn returns_up_to_date_when_state_has_empty_allow_builds_and_current_has_none() { ); write_state(workspace_root, now_millis() + 60_000, settings, projects); - let decision = check_optimistic_repeat_install( + let decision = check( workspace_root, config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(workspace_root.to_path_buf(), &manifest)], - false, ); assert_eq!(decision, Decision::UpToDate); } @@ -1129,14 +1102,19 @@ fn returns_skipped_when_sibling_node_modules_missing_for_project_with_deps() { ); write_state(dir.path(), now_millis() + 60_000, settings, projects); - let decision = check_optimistic_repeat_install( - dir.path(), + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), config, - pacquet_config::NodeLinker::Isolated, - isolated_included(), - &[(dir.path().to_path_buf(), &root_manifest), (sibling_dir, &sibling_manifest)], - true, - ); + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests: &[ + (dir.path().to_path_buf(), &root_manifest), + (sibling_dir, &sibling_manifest), + ], + is_workspace_install: true, + lockfile: None, + catalogs: &Default::default(), + }); assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("node_modules"))); } @@ -1159,13 +1137,11 @@ fn returns_skipped_when_lockfile_missing_in_single_project_mode() { // lockfile branch. fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).expect("remove seeded lockfile"); - let decision = check_optimistic_repeat_install( + let decision = check( dir.path(), config, pacquet_config::NodeLinker::Isolated, - isolated_included(), &[(dir.path().to_path_buf(), &manifest)], - false, ); assert!( matches!(decision, Decision::Skipped { reason } if reason.contains("wanted lockfile")), @@ -1191,13 +1167,389 @@ fn returns_up_to_date_in_workspace_mode_without_lockfile() { // wiped first — the workspace branch shouldn't care. fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).expect("remove seeded lockfile"); - let decision = check_optimistic_repeat_install( - dir.path(), + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), config, - pacquet_config::NodeLinker::Isolated, - isolated_included(), - &[(dir.path().to_path_buf(), &manifest)], - true, - ); + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: true, + lockfile: None, + catalogs: &Default::default(), + }); assert_eq!(decision, Decision::UpToDate); } + +/// Minimal valid lockfile matching a manifest with +/// `"dependencies": {"foo": "^1.0.0"}`. +const FOO_LOCKFILE: &str = "lockfileVersion: '9.0' + +importers: + + .: + dependencies: + foo: + specifier: ^1.0.0 + version: 1.0.0 + +packages: + + foo@1.0.0: + resolution: {integrity: sha512-aaa} + +snapshots: + + foo@1.0.0: {} +"; + +const FOO_MANIFEST: &str = r#"{"name":"root","version":"1.0.0","dependencies":{"foo":"^1.0.0"}}"#; + +/// Build a single project whose manifest, wanted lockfile, and current +/// lockfile all agree, with the workspace state stamped after every +/// file write. Content-check tests then touch or rewrite individual +/// files and assert the decision. +fn setup_content_check_project() -> (tempfile::TempDir, &'static Config) { + let dir = tempdir().unwrap(); + let workspace_root = dir.path(); + fs::write(workspace_root.join("package.json"), FOO_MANIFEST).unwrap(); + fs::write(workspace_root.join(Lockfile::FILE_NAME), FOO_LOCKFILE).unwrap(); + + let mut config = Config::new(); + config.modules_dir = workspace_root.join("node_modules"); + config.virtual_store_dir = workspace_root.join("node_modules/.pnpm"); + fs::create_dir_all(&config.virtual_store_dir).unwrap(); + fs::write(config.virtual_store_dir.join(Lockfile::CURRENT_FILE_NAME), FOO_LOCKFILE).unwrap(); + let config = config.leak(); + + sleep(Duration::from_millis(20)); + let settings = + current_settings(config, pacquet_config::NodeLinker::Isolated, isolated_included()); + let mut projects = BTreeMap::new(); + projects.insert( + workspace_root.to_string_lossy().into_owned(), + ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) }, + ); + write_state(workspace_root, now_millis(), settings, projects); + sleep(Duration::from_millis(20)); + + (dir, config) +} + +fn content_check_decision( + dir: &tempfile::TempDir, + config: &'static Config, + is_workspace_install: bool, + project_manifests: &[(std::path::PathBuf, &PackageManifest)], +) -> Decision { + let lockfile = Lockfile::load_wanted_from_dir(dir.path()).expect("parse pnpm-lock.yaml"); + check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests, + is_workspace_install, + lockfile: lockfile.as_ref(), + catalogs: &Default::default(), + }) +} + +/// A manifest rewrite that leaves the dependency fields intact — the +/// shape `touch package.json` / `npm pkg set/delete` produce — must +/// still short-circuit. Ports the contract behind upstream's +/// `assertWantedLockfileUpToDate` pass at +/// . +#[test] +fn returns_up_to_date_when_touched_manifest_still_satisfies_lockfile() { + let (dir, config) = setup_content_check_project(); + + fs::write(dir.path().join("package.json"), FOO_MANIFEST).unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, false, &[(dir.path().to_path_buf(), &manifest)]); + assert_eq!(decision, Decision::UpToDate); +} + +/// A manifest rewrite that *changes* the dependency fields falls +/// through to the full install. +#[test] +fn returns_skipped_when_touched_manifest_adds_a_dependency() { + let (dir, config) = setup_content_check_project(); + + fs::write( + dir.path().join("package.json"), + r#"{"name":"root","version":"1.0.0","dependencies":{"foo":"^1.0.0","bar":"^2.0.0"}}"#, + ) + .unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, false, &[(dir.path().to_path_buf(), &manifest)]); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("satisfied")), + "expected Skipped(no longer satisfied), got {decision:?}", + ); +} + +/// A wanted lockfile rewritten after the last install (newer than the +/// current lockfile, different content) cannot short-circuit: the +/// modules directory no longer reflects it. Ports upstream's +/// [`assertLockfilesEqual`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/assertLockfilesEqual.ts) +/// `RUN_CHECK_DEPS_OUTDATED_DEPS` outcome. +#[test] +fn returns_skipped_when_wanted_lockfile_diverged_from_current() { + let (dir, config) = setup_content_check_project(); + + fs::write(dir.path().join("package.json"), FOO_MANIFEST).unwrap(); + fs::write(dir.path().join(Lockfile::FILE_NAME), FOO_LOCKFILE.replace("1.0.0", "1.0.1")) + .unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, false, &[(dir.path().to_path_buf(), &manifest)]); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("not up to date")), + "expected Skipped(outdated deps), got {decision:?}", + ); +} + +/// Workspace branch: a passing content check refreshes +/// `lastValidatedTimestamp` so the next run exits on the pure-mtime +/// path. Mirrors upstream's `updateWorkspaceState` call at +/// . +#[test] +fn workspace_content_check_refreshes_last_validated_timestamp() { + let (dir, config) = setup_content_check_project(); + let before = pacquet_workspace_state::load_workspace_state(dir.path()) + .unwrap() + .unwrap() + .last_validated_timestamp; + + fs::write(dir.path().join("package.json"), FOO_MANIFEST).unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, true, &[(dir.path().to_path_buf(), &manifest)]); + assert_eq!(decision, Decision::UpToDate); + + let after = pacquet_workspace_state::load_workspace_state(dir.path()) + .unwrap() + .unwrap() + .last_validated_timestamp; + assert!(after > before, "expected the state timestamp to advance ({before} -> {after})"); +} + +/// Workspace lockfile whose root importer links a sibling: the link +/// stays valid while the sibling's version satisfies the manifest +/// range, and a bump outside the range falls through to the full +/// install. Ports upstream's +/// [`linkedPackagesAreUpToDate`](https://github.com/pnpm/pnpm/blob/cc4ff817aa/lockfile/verification/src/linkedPackagesAreUpToDate.ts). +fn linked_sibling_decision(sibling_version: &str) -> Decision { + let dir = tempdir().unwrap(); + let workspace_root = dir.path(); + fs::write( + workspace_root.join("package.json"), + r#"{"name":"root","version":"1.0.0","devDependencies":{"pkg-a":"^1.0.0"}}"#, + ) + .unwrap(); + let sibling_dir = workspace_root.join("pkg-a"); + fs::create_dir_all(&sibling_dir).unwrap(); + fs::write( + sibling_dir.join("package.json"), + format!(r#"{{"name":"pkg-a","version":"{sibling_version}"}}"#), + ) + .unwrap(); + fs::write( + workspace_root.join(Lockfile::FILE_NAME), + "lockfileVersion: '9.0' + +importers: + + .: + devDependencies: + pkg-a: + specifier: ^1.0.0 + version: link:pkg-a + + pkg-a: {} +", + ) + .unwrap(); + + let mut config = Config::new(); + config.modules_dir = workspace_root.join("node_modules"); + config.virtual_store_dir = workspace_root.join("node_modules/.pnpm"); + config.link_workspace_packages = pacquet_config::LinkWorkspacePackages::DirectOnly; + fs::create_dir_all(&config.modules_dir).unwrap(); + let config = config.leak(); + + let root_manifest = PackageManifest::from_path(workspace_root.join("package.json")).unwrap(); + let sibling_manifest = PackageManifest::from_path(sibling_dir.join("package.json")).unwrap(); + + sleep(Duration::from_millis(20)); + let settings = + current_settings(config, pacquet_config::NodeLinker::Isolated, isolated_included()); + let mut projects = BTreeMap::new(); + projects.insert( + workspace_root.to_string_lossy().into_owned(), + ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) }, + ); + projects.insert( + sibling_dir.to_string_lossy().into_owned(), + ProjectEntry { name: Some("pkg-a".into()), version: Some(sibling_version.into()) }, + ); + write_state(workspace_root, now_millis(), settings, projects); + sleep(Duration::from_millis(20)); + + // Touch the root manifest so the content re-check runs. + fs::write( + workspace_root.join("package.json"), + r#"{"name":"root","version":"1.0.0","devDependencies":{"pkg-a":"^1.0.0"}}"#, + ) + .unwrap(); + let root_manifest_touched = + PackageManifest::from_path(workspace_root.join("package.json")).unwrap(); + let _ = root_manifest; + + let lockfile = Lockfile::load_wanted_from_dir(workspace_root).expect("parse pnpm-lock.yaml"); + check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root, + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests: &[ + (workspace_root.to_path_buf(), &root_manifest_touched), + (sibling_dir, &sibling_manifest), + ], + is_workspace_install: true, + lockfile: lockfile.as_ref(), + catalogs: &Default::default(), + }) +} + +#[test] +fn returns_up_to_date_when_linked_sibling_still_satisfies_range() { + assert_eq!(linked_sibling_decision("1.5.0"), Decision::UpToDate); +} + +#[test] +fn returns_skipped_when_linked_sibling_no_longer_satisfies_range() { + let decision = linked_sibling_decision("2.0.0"); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("linked")), + "expected Skipped(linked package out of date), got {decision:?}", + ); +} + +/// `pnpm-lock.yaml` deleted while `node_modules` (and its current +/// lockfile) is intact: the current lockfile stands in as the wanted +/// one, and the fast path regenerates `pnpm-lock.yaml` from it instead +/// of falling into the full install pipeline. +#[test] +fn regenerates_missing_wanted_lockfile_from_current_when_manifests_unchanged() { + let (dir, config) = setup_content_check_project(); + fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, false, &[(dir.path().to_path_buf(), &manifest)]); + assert_eq!(decision, Decision::UpToDate); + + let regenerated = Lockfile::load_wanted_from_dir(dir.path()) + .expect("parse regenerated pnpm-lock.yaml") + .expect("pnpm-lock.yaml must be regenerated from the current lockfile"); + let current = + Lockfile::load_current_from_virtual_store_dir(&config.virtual_store_dir).unwrap().unwrap(); + assert_eq!(regenerated, current); +} + +/// Same as above with a touched (content-identical) manifest — the +/// content re-check runs against the current lockfile. +#[test] +fn regenerates_missing_wanted_lockfile_when_touched_manifest_satisfies_current() { + let (dir, config) = setup_content_check_project(); + fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).unwrap(); + fs::write(dir.path().join("package.json"), FOO_MANIFEST).unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, false, &[(dir.path().to_path_buf(), &manifest)]); + assert_eq!(decision, Decision::UpToDate); + assert!(dir.path().join(Lockfile::FILE_NAME).exists(), "pnpm-lock.yaml must be regenerated"); +} + +/// A manifest that no longer matches the current lockfile cannot ride +/// the current-as-wanted fallback — the full install must resolve. +#[test] +fn returns_skipped_when_missing_wanted_lockfile_and_manifest_adds_a_dependency() { + let (dir, config) = setup_content_check_project(); + fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).unwrap(); + fs::write( + dir.path().join("package.json"), + r#"{"name":"root","version":"1.0.0","dependencies":{"foo":"^1.0.0","bar":"^2.0.0"}}"#, + ) + .unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, false, &[(dir.path().to_path_buf(), &manifest)]); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("satisfied")), + "expected Skipped(no longer satisfied), got {decision:?}", + ); + assert!( + !dir.path().join(Lockfile::FILE_NAME).exists(), + "must not regenerate on a failed check", + ); +} + +/// Workspace mode: deleted `pnpm-lock.yaml` + touched manifest takes +/// the current-as-wanted fallback, regenerates the lockfile, and +/// refreshes the state timestamp. +#[test] +fn workspace_regenerates_missing_wanted_lockfile_and_bumps_state() { + let (dir, config) = setup_content_check_project(); + let before = pacquet_workspace_state::load_workspace_state(dir.path()) + .unwrap() + .unwrap() + .last_validated_timestamp; + fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).unwrap(); + fs::write(dir.path().join("package.json"), FOO_MANIFEST).unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = + content_check_decision(&dir, config, true, &[(dir.path().to_path_buf(), &manifest)]); + assert_eq!(decision, Decision::UpToDate); + assert!(dir.path().join(Lockfile::FILE_NAME).exists(), "pnpm-lock.yaml must be regenerated"); + let after = pacquet_workspace_state::load_workspace_state(dir.path()) + .unwrap() + .unwrap() + .last_validated_timestamp; + assert!(after > before, "expected the state timestamp to advance ({before} -> {after})"); +} + +/// `lockfile: false` (pnpm's `useLockfile: false`) disables the +/// regeneration but keeps the fast path. +#[test] +fn does_not_regenerate_wanted_lockfile_when_lockfile_writing_disabled() { + let (dir, config) = setup_content_check_project(); + // `Config` is leaked per test; build a second one with `lockfile` + // off instead of mutating the shared reference. + let mut no_lockfile_config = Config::new(); + no_lockfile_config.modules_dir = config.modules_dir.clone(); + no_lockfile_config.virtual_store_dir = config.virtual_store_dir.clone(); + no_lockfile_config.lockfile = false; + let no_lockfile_config = no_lockfile_config.leak(); + fs::remove_file(dir.path().join(Lockfile::FILE_NAME)).unwrap(); + let manifest = PackageManifest::from_path(dir.path().join("package.json")).unwrap(); + + let decision = content_check_decision( + &dir, + no_lockfile_config, + false, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert_eq!(decision, Decision::UpToDate); + assert!(!dir.path().join(Lockfile::FILE_NAME).exists(), "lockfile: false must skip the write"); +}