From 6d353386913a8e685a738f27e99335a98fc890a2 Mon Sep 17 00:00:00 2001 From: Truffle Date: Tue, 16 Jun 2026 13:31:45 -0700 Subject: [PATCH] fix: detect changes inside file: dependencies on repeat install (pacquet + pnpm) (#12317) ## Summary - `pnpm install` reports "Already up to date" after edits inside a `file:` dependency's directory or after repacking a `file:` tarball. This is a v11 regression from the `optimisticRepeatInstall` default flip in pnpm/pnpm#11158. Fixes pnpm/pnpm#11795. - `checkDepsStatus` gains a `treatLocalFileDepsAsOutdated` option: when set, any project manifest declaring a local file dependency makes the check report not up to date. `installDeps` sets it on the optimistic fast path, so projects with local file dependencies always run a real install, which refetches those dependencies (the v10 behavior). - The predicate covers `file:` specs, path-prefixed specs (`./`, `../`, `~/`, absolute POSIX paths, and Windows drive paths including drive-relative ones like `C:dir`, matching the local resolver's `isFilespec`), and bare tarball file names (`vendor/pkg.tgz`). It is deliberately narrower than the local resolver's bare-path matching: a bare `user/repo` is statically indistinguishable from a git shorthand at this layer, and matching it would kill the fast path for every project with git dependencies, so protocol-carrying and URL specs stay on the fast path. - `pnpm.overrides` entries are scanned with the same predicate: an override mapping to a local file spec redirects every matching dependency in the graph to that directory, so it has the same blind spot as a direct local file dependency. Registry and `link:` overrides keep the fast path. - The option is caller-scoped on purpose. `verifyDepsBeforeRun` also consumes `checkDepsStatus`, and treating `file:` deps as always stale there would force a reinstall before every `pnpm run`. Its behavior is unchanged, and a regression test pins that. - pacquet port in the same commit: `check_optimistic_repeat_install` bails unconditionally on `file:` specifiers, because its only caller is the install command, the one consumer that sets the flag upstream. `link:` specifiers are excluded on both sides: they are symlinked, so changes inside them flow through without a reinstall. ## Why Both branches of `checkDepsStatus` are blind to content changes inside a `file:` dependency. The workspace branch exits early with `upToDate: true` when no project manifest's mtime moved, without ever reaching `linkedPackagesAreUpToDate`. The non-workspace branch exits at the manifest-vs-lockfile mtime gate the same way. Editing a source file inside a `file:` dependency bumps neither, so the fast path can never see it; the fix has to bail before those gates rather than refine them. This is the fix shape (a) I proposed in my diagnosis on the issue thread ([comment](https://github.com/pnpm/pnpm/issues/11795#issuecomment-4504177744)): the cost is a full resolution on repeat installs only for projects that declare `file:` dependencies, which is exactly what v10 did. The manifest-only comparison in `@pnpm/lockfile.verification` (`allProjectsAreUpToDate`) is intentional for the install-proper path and asserted by its tests, so this PR leaves it untouched. ## Checks - `pnpm --filter @pnpm/deps.status test test/checkDepsStatus.test.ts` (31 passed, 13 new) - `pnpm --filter @pnpm/deps.status run compile` and `pnpm --filter @pnpm/installing.commands run compile` (tsgo + eslint clean) - `cargo test -p pacquet-package-manager optimistic_repeat_install` (51 passed, 7 new; run in a rust:1.95.0 container) - `cargo fmt --check -p pacquet-package-manager` - `RUSTDOCFLAGS="-D warnings" cargo doc -p pacquet-package-manager --no-deps` --- Written by an agent (Claude Code, claude-fable-5). --------- Co-authored-by: Zoltan Kochan --- .changeset/repeat-install-file-deps.md | 7 + cspell.json | 2 + deps/status/package.json | 2 + deps/status/src/checkDepsStatus.ts | 185 ++++++- deps/status/test/checkDepsStatus.test.ts | 464 ++++++++++++++++ deps/status/tsconfig.json | 6 + installing/commands/src/installDeps.ts | 1 + installing/commands/test/install.ts | 35 ++ .../src/optimistic_repeat_install.rs | 211 +++++++- .../src/optimistic_repeat_install/tests.rs | 505 ++++++++++++++++++ pnpm-lock.yaml | 6 + 11 files changed, 1417 insertions(+), 7 deletions(-) create mode 100644 .changeset/repeat-install-file-deps.md diff --git a/.changeset/repeat-install-file-deps.md b/.changeset/repeat-install-file-deps.md new file mode 100644 index 0000000000..868fd6aa48 --- /dev/null +++ b/.changeset/repeat-install-file-deps.md @@ -0,0 +1,7 @@ +--- +"@pnpm/deps.status": patch +"@pnpm/installing.commands": patch +"pnpm": patch +--- + +`pnpm install` detects changes inside local file dependencies again. The optimistic repeat-install fast path only tracks manifest and lockfile modification times, so edits inside a local dependency's directory (or a repacked local tarball) were reported as "Already up to date". Projects with local file dependencies (`file:` and bare local path or tarball specifiers, declared directly or through `pnpm.overrides`) now always run a full install, which refetches those dependencies, matching pnpm v10 behavior [#11795](https://github.com/pnpm/pnpm/issues/11795). diff --git a/cspell.json b/cspell.json index 3156bc6fbd..f7f276437b 100644 --- a/cspell.json +++ b/cspell.json @@ -200,6 +200,7 @@ "msgpackr", "msvc", "msys", + "mtimes", "musleabihf", "mycomp", "mycompany", @@ -298,6 +299,7 @@ "redownload", "refclone", "refetched", + "refetches", "reflattened", "reflink", "reflinked", diff --git a/deps/status/package.json b/deps/status/package.json index bd01a36e2b..c9fbd6ec6e 100644 --- a/deps/status/package.json +++ b/deps/status/package.json @@ -33,6 +33,8 @@ ".test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules --disable-warning=ExperimentalWarning --disable-warning=DEP0169\" jest" }, "dependencies": { + "@pnpm/catalogs.resolver": "workspace:*", + "@pnpm/catalogs.types": "workspace:*", "@pnpm/config.parse-overrides": "workspace:*", "@pnpm/config.reader": "workspace:*", "@pnpm/constants": "workspace:*", diff --git a/deps/status/src/checkDepsStatus.ts b/deps/status/src/checkDepsStatus.ts index 01704043e5..37b6a59d3f 100644 --- a/deps/status/src/checkDepsStatus.ts +++ b/deps/status/src/checkDepsStatus.ts @@ -2,6 +2,8 @@ import fs from 'node:fs' import path from 'node:path' import util from 'node:util' +import { resolveFromCatalog } from '@pnpm/catalogs.resolver' +import type { Catalogs } from '@pnpm/catalogs.types' import { parseOverrides } from '@pnpm/config.parse-overrides' import type { Config, ConfigContext } from '@pnpm/config.reader' import { MANIFEST_BASE_NAMES, WANTED_LOCKFILE } from '@pnpm/constants' @@ -27,11 +29,13 @@ import { } from '@pnpm/lockfile.verification' import { globalWarn, logger } from '@pnpm/logger' import type { WorkspacePackages } from '@pnpm/resolving.resolver-base' -import type { - DependencyManifest, - Project, - ProjectId, - ProjectManifest, +import { + DEPENDENCIES_FIELDS, + type DependencyManifest, + type IncludedDependencies, + type Project, + type ProjectId, + type ProjectManifest, } from '@pnpm/types' import { findWorkspaceProjectsNoCheck } from '@pnpm/workspace.projects-reader' import { loadWorkspaceState, updateWorkspaceState, WORKSPACE_STATE_SETTING_KEYS, type WorkspaceState, type WorkspaceStateSettings } from '@pnpm/workspace.state' @@ -68,6 +72,26 @@ export type CheckDepsStatusOptions = Pick pnpmfile: string[] + /** + * The checks below only track manifest and lockfile mtimes, so edits inside + * a local file dependency's directory (or a repacked local tarball) go + * unnoticed. Callers that skip the install entirely when this check reports + * up-to-date must set this so that projects with local file dependencies + * (`file:` and bare local path/tarball specifiers) always run a real + * install, which refetches those dependencies + * (https://github.com/pnpm/pnpm/issues/11795). + */ + treatLocalFileDepsAsOutdated?: boolean + /** + * Which dependency groups the current install materializes. Local file + * dependencies in an excluded group (for example `devDependencies` under + * `--prod`) are not installed, so they don't force the + * `treatLocalFileDepsAsOutdated` bail-out. A change to these flags between + * installs is caught separately by the workspace state settings comparison + * (`dev`/`optional`/`production` are part of + * `WORKSPACE_STATE_SETTING_KEYS`). + */ + include?: IncludedDependencies /** * When git-branch lockfiles are enabled, the wanted lockfile lives at * `pnpm-lock..yaml`, so a missing `pnpm-lock.yaml` is the steady @@ -141,6 +165,46 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W workspaceDir, } = opts + // This check must run before the node-linker=pnp early return below: + // that return reports up-to-date because verify-deps-before-run cannot + // inspect a PnP install, but for the optimistic repeat-install caller + // (the only one setting this flag) "up-to-date" would skip the install + // and break the local-file-deps guarantee. + if (opts.treatLocalFileDepsAsOutdated) { + const manifests = allProjects?.map(({ manifest }) => manifest) ?? [] + // `rootProjectManifest` is tracked separately from `allProjects` and the + // recursive project list can omit the workspace root (for example when + // `includeWorkspaceRoot` is false), so scan it too unless `allProjects` + // already covers it. + if (rootProjectManifest != null && !allProjects?.some(({ rootDir }) => rootDir === rootProjectManifestDir)) { + manifests.push(rootProjectManifest) + } + const localFileDep = findLocalFileDep(manifests, opts.include, catalogs) + if (localFileDep != null) { + return { + upToDate: false, + issue: `The dependency "${localFileDep}" is a local file dependency and its contents may have changed`, + workspaceState, + } + } + const localFileOverride = findLocalFileOverride(opts.overrides, catalogs) + if (localFileOverride != null) { + return { + upToDate: false, + issue: `The override "${localFileOverride}" maps to a local file dependency and its contents may have changed`, + workspaceState, + } + } + const localFileExtension = findLocalFilePackageExtension(opts.packageExtensions, opts.include, catalogs) + if (localFileExtension != null) { + return { + upToDate: false, + issue: `The package extension "${localFileExtension}" injects a local file dependency and its contents may have changed`, + workspaceState, + } + } + } + if (nodeLinker === 'pnp') { globalWarn('verify-deps-before-run does not work with node-linker=pnp') return { upToDate: true, workspaceState: undefined } @@ -622,6 +686,117 @@ async function assertWantedLockfileUpToDate ( } } +/** + * Returns the name of the first dependency declared with a local file + * specifier in any of the given manifests, or `undefined` when there is none. + * `link:` dependencies are excluded: they are symlinked, so changes inside + * them flow through without a reinstall. Dependency groups excluded from the + * current install (per `include`) are skipped: their local file dependencies + * are not installed, so their contents cannot be stale. `catalog:` specs are + * dereferenced through the catalogs config: the catalog resolver only bans + * the `workspace:`, `link:`, and `file:` protocols, so a catalog entry can + * still hold a bare local path (`../lib`, `vendor/pkg.tgz`) that resolves to + * a local file dependency. + */ +function findLocalFileDep (manifests: ProjectManifest[], include?: IncludedDependencies, catalogs?: Catalogs): string | undefined { + for (const manifest of manifests) { + for (const depField of DEPENDENCIES_FIELDS) { + if (include?.[depField] === false) continue + const depName = findLocalFileDepInRecord(manifest[depField], catalogs) + if (depName != null) return depName + } + } + return undefined +} + +/** + * Returns the name of the first dependency in `deps` declared with (or + * resolving through a catalog to) a local file specifier, or `undefined`. + */ +function findLocalFileDepInRecord (deps: Record | undefined, catalogs?: Catalogs): string | undefined { + if (deps == null) return undefined + for (const [depName, spec] of Object.entries(deps)) { + // A malformed manifest may carry a non-string spec; skip it rather + // than throw — checkDepsStatus() must never crash. + if (typeof spec !== 'string') continue + if (isLocalFileSpec(spec)) return depName + // Only catalog: specs consult the catalogs, so skip the lookup for + // everything else to keep the optimistic fast path cheap. + if (!spec.startsWith('catalog:')) continue + const catalogResult = resolveFromCatalog(catalogs ?? {}, { alias: depName, bareSpecifier: spec }) + if (catalogResult.type === 'found' && isLocalFileSpec(catalogResult.resolution.specifier)) return depName + } + return undefined +} + +/** + * Returns the selector of the first `packageExtensions` entry that injects a + * local file dependency, or `undefined` when there is none. Package + * extensions are merged into matching packages' manifests by a read-package + * hook during install, so a `file:`/local-path/tarball spec added there has + * the same content-change blind spot as a direct local file dependency + * without appearing in any project manifest. Only `dependencies` and + * `optionalDependencies` are scanned: peer dependencies are resolved from the + * graph rather than fetched, so a local spec there is never installed. + */ +function findLocalFilePackageExtension (packageExtensions: CheckDepsStatusOptions['packageExtensions'], include?: IncludedDependencies, catalogs?: Catalogs): string | undefined { + if (packageExtensions == null) return undefined + for (const [selector, extension] of Object.entries(packageExtensions)) { + if (findLocalFileDepInRecord(extension.dependencies, catalogs) != null) return selector + if (include?.optionalDependencies === false) continue + if (findLocalFileDepInRecord(extension.optionalDependencies, catalogs) != null) return selector + } + return undefined +} + +/** + * Returns the selector of the first override that maps to a local file + * specifier, or `undefined` when there is none. An override redirects every + * matching dependency in the graph to its specifier, so a local file override + * makes the installed contents depend on that directory or tarball the same + * way a direct local file dependency does. Overrides are run through + * `parseOverrides` so `catalog:` specs are dereferenced before the check. + * `parseOverrides` throws on a misconfigured catalog or invalid selector; + * that propagates to the outer catch in `checkDepsStatus`, which reports + * not-up-to-date, and the resulting full install surfaces the same error. + */ +function findLocalFileOverride (overrides: Record | undefined, catalogs?: Catalogs): string | undefined { + if (overrides == null || isEmpty(overrides)) return undefined + return parseOverrides(overrides, catalogs) + .find(({ newBareSpecifier }) => isLocalFileSpec(newBareSpecifier))?.selector +} + +const LOCAL_PATH_PREFIX = /^(?:[./\\]|~[/\\]|[a-z]:)/i +const LOCAL_TARBALL_EXTENSION = /\.(?:tgz|tar\.gz|tar)$/i + +/** + * Whether the specifier resolves to a local directory or tarball whose + * contents can change without any manifest or lockfile mtime moving: the + * `file:` protocol, path-prefixed specs (`./`, `../`, `~/`, absolute POSIX + * paths, and Windows drive paths — including drive-relative ones like + * `C:dir`, matching the local resolver's `isFilespec`), and bare tarball + * file names. + * + * Deliberately narrower than the local resolver's bare-path matching: a bare + * `dir/file.tgz`-less path like `user/repo` is statically indistinguishable + * from a git shorthand at this layer, and matching it would disable the + * repeat-install fast path for every project with git dependencies. Such + * specs (and anything else carrying a protocol or URL) stay on the fast + * path. `catalog:` specs also return false here — callers dereference them + * through the catalogs config first, because a catalog entry may hold a + * bare local path (the catalog resolver only bans the `workspace:`, + * `link:`, and `file:` protocols). + */ +function isLocalFileSpec (spec: string): boolean { + if (spec.startsWith('file:')) return true + if (LOCAL_PATH_PREFIX.test(spec)) return true + if (spec.includes(':')) return false + // A `#` here means a hosted-git shorthand committish (`user/repo#release.tgz`), + // not a local tarball — the `file:` and path-prefixed cases already returned above. + if (spec.includes('#')) return false + return LOCAL_TARBALL_EXTENSION.test(spec) +} + function throwLockfileNotFound (wantedLockfileDir: string): never { throw new PnpmError('RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND', `Cannot find a lockfile in ${wantedLockfileDir}`, { hint: 'Run `pnpm install` to create the lockfile', diff --git a/deps/status/test/checkDepsStatus.test.ts b/deps/status/test/checkDepsStatus.test.ts index 0fc6d0fd88..d9dec4995b 100644 --- a/deps/status/test/checkDepsStatus.test.ts +++ b/deps/status/test/checkDepsStatus.test.ts @@ -790,3 +790,467 @@ describe('checkDepsStatus - missing wanted lockfile fallback', () => { }) }) }) + +describe('checkDepsStatus - treatLocalFileDepsAsOutdated', () => { + beforeEach(() => { + jest.resetModules() + jest.clearAllMocks() + }) + + const currentLockfile = { + lockfileVersion: '9.0', + importers: { '.': { specifiers: {} } }, + } as unknown as LockfileObject + + const mockWorkspaceState = (lastValidatedTimestamp: number): WorkspaceState => ({ + lastValidatedTimestamp, + pnpmfiles: [], + settings: { + excludeLinksFromLockfile: false, + linkWorkspacePackages: true, + preferWorkspacePackages: true, + }, + projects: {}, + filteredInstall: false, + }) + + function mockUpToDateSingleProjectStats (lastValidatedTimestamp: number): void { + jest.mocked(fsUtils.safeStat).mockImplementation(async () => ({ + mtime: new Date(lastValidatedTimestamp - 10_000), + mtimeMs: lastValidatedTimestamp - 10_000, + } as Stats)) + 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) + } + + it('returns upToDate: false when the root manifest has a file: dependency', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: 'file:../foo' }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('returns upToDate: false when a workspace project has a file: dependency', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + allProjects: [ + { + rootDir: '/workspace' as ProjectRootDir, + rootDirRealPath: '/workspace' as ProjectRootDirRealPath, + manifest: { name: 'root', version: '1.0.0' }, + writeProjectManifest: async () => {}, + }, + { + rootDir: '/workspace/packages/bar' as ProjectRootDir, + rootDirRealPath: '/workspace/packages/bar' as ProjectRootDirRealPath, + manifest: { + name: 'bar', + version: '1.0.0', + devDependencies: { tar: 'file:./vendor/tar.tgz' }, + }, + writeProjectManifest: async () => {}, + }, + ], + workspaceDir: '/workspace', + sharedWorkspaceLockfile: true, + rootProjectManifest: { name: 'root', version: '1.0.0' }, + rootProjectManifestDir: '/workspace', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "tar" is a local file dependency and its contents may have changed') + }) + + it('returns upToDate: false when the root manifest has a file: dependency but allProjects omits the root', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + allProjects: [ + { + rootDir: '/workspace/packages/bar' as ProjectRootDir, + rootDirRealPath: '/workspace/packages/bar' as ProjectRootDirRealPath, + manifest: { name: 'bar', version: '1.0.0' }, + writeProjectManifest: async () => {}, + }, + ], + workspaceDir: '/workspace', + sharedWorkspaceLockfile: true, + rootProjectManifest: { + name: 'root', + version: '1.0.0', + dependencies: { foo: 'file:../foo' }, + }, + rootProjectManifestDir: '/workspace', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('reports up-to-date when there is a file: dependency but the option is not set', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: 'file:../foo' }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) + + it('does not report link: and registry dependencies as outdated', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { + foo: 'link:../foo', + bar: '^1.0.0', + }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) + + it.each([ + ['a bare local tarball path', 'vendor/pkg.tgz'], + ['a relative directory path', '../sibling-dir'], + ['a home-relative path', '~/pkgs/foo'], + ['an absolute path', '/abs/path/foo'], + ['an absolute Windows drive path', 'C:\\pkgs\\foo'], + ['a drive-relative Windows path', 'C:pkgs'], + ])('returns upToDate: false when the root manifest has %s dependency', async (_desc, spec) => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: spec }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('returns upToDate: false when an override maps to a local file dependency', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: '^1.0.0' }, + }, + rootProjectManifestDir: '/project', + overrides: { bar: 'file:../bar' }, + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The override "bar" maps to a local file dependency and its contents may have changed') + }) + + it('does not report registry and link: overrides as outdated', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const overrides = { bar: '^2.0.0', baz: 'link:../baz' } + const workspaceState = mockWorkspaceState(lastValidatedTimestamp) + workspaceState.settings = { ...workspaceState.settings, overrides } + jest.mocked(loadWorkspaceState).mockReturnValue(workspaceState) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: '^1.0.0' }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...workspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) + + it('does not report git, remote tarball, and tilde range dependencies as outdated', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { + foo: 'user/repo', + bar: 'github:user/repo', + baz: 'https://example.com/pkg.tgz', + qux: '~1.2.3', + // A git shorthand whose committish ends in .tgz must not be + // mistaken for a local tarball. + quux: 'user/repo#release.tgz', + }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) + + it('returns upToDate: false for a local file dependency even when nodeLinker is pnp', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: 'file:../foo' }, + }, + rootProjectManifestDir: '/project', + nodeLinker: 'pnp', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('reports up-to-date when the only file: dependency is in a group excluded from the install', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + optionalDependencies: { foo: 'file:../foo' }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + include: { + dependencies: true, + devDependencies: true, + optionalDependencies: false, + }, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) + + it('returns upToDate: false for a file: dependency in a group included in the install', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: 'file:../foo' }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + include: { + dependencies: true, + devDependencies: false, + optionalDependencies: false, + }, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('skips non-string dependency specs in malformed manifests without throwing', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { + broken: 42 as unknown as string, + foo: 'file:../foo', + }, + }, + rootProjectManifestDir: '/project', + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('returns upToDate: false when a catalog: dependency resolves to a bare local path', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: 'catalog:' }, + }, + rootProjectManifestDir: '/project', + catalogs: { default: { foo: '../foo' } }, + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed') + }) + + it('does not report a catalog: dependency resolving to a registry range as outdated', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const catalogs = { default: { foo: '^1.0.0' } } + const workspaceState = mockWorkspaceState(lastValidatedTimestamp) + workspaceState.settings = { ...workspaceState.settings, catalogs } + jest.mocked(loadWorkspaceState).mockReturnValue(workspaceState) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: 'catalog:' }, + }, + rootProjectManifestDir: '/project', + catalogs, + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...workspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) + + it('returns upToDate: false when an override maps through a catalog to a local path', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: '^1.0.0' }, + }, + rootProjectManifestDir: '/project', + overrides: { bar: 'catalog:' }, + catalogs: { default: { bar: './vendor/bar' } }, + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The override "bar" maps to a local file dependency and its contents may have changed') + }) + + it('returns upToDate: false when a packageExtension injects a local file dependency', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp)) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: '^1.0.0' }, + }, + rootProjectManifestDir: '/project', + packageExtensions: { + 'foo@1': { dependencies: { bar: 'file:../bar' } }, + }, + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + ...mockWorkspaceState(lastValidatedTimestamp).settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(false) + expect(result.issue).toBe('The package extension "foo@1" injects a local file dependency and its contents may have changed') + }) + + it('does not report a packageExtension optionalDependency as outdated when optionals are excluded', async () => { + const lastValidatedTimestamp = Date.now() - 10_000 + const packageExtensions = { 'foo@1': { optionalDependencies: { bar: 'file:../bar' } } } + const workspaceState = mockWorkspaceState(lastValidatedTimestamp) + workspaceState.settings = { ...workspaceState.settings, packageExtensions } + jest.mocked(loadWorkspaceState).mockReturnValue(workspaceState) + mockUpToDateSingleProjectStats(lastValidatedTimestamp) + + const opts: CheckDepsStatusOptions = { + rootProjectManifest: { + dependencies: { foo: '^1.0.0' }, + }, + rootProjectManifestDir: '/project', + packageExtensions, + pnpmfile: [], + treatLocalFileDepsAsOutdated: true, + include: { + dependencies: true, + devDependencies: true, + optionalDependencies: false, + }, + ...workspaceState.settings, + } + const result = await checkDepsStatus(opts) + + expect(result.upToDate).toBe(true) + }) +}) diff --git a/deps/status/tsconfig.json b/deps/status/tsconfig.json index 899aa8d7a1..be7480e936 100644 --- a/deps/status/tsconfig.json +++ b/deps/status/tsconfig.json @@ -12,6 +12,12 @@ { "path": "../../__utils__/prepare" }, + { + "path": "../../catalogs/resolver" + }, + { + "path": "../../catalogs/types" + }, { "path": "../../config/parse-overrides" }, diff --git a/installing/commands/src/installDeps.ts b/installing/commands/src/installDeps.ts index 674d6b8c10..0ab5433d3d 100644 --- a/installing/commands/src/installDeps.ts +++ b/installing/commands/src/installDeps.ts @@ -186,6 +186,7 @@ export async function installDeps ( const { upToDate, wantedLockfileToRestore } = await checkDepsStatus({ ...opts, ignoreFilteredInstallCache: true, + treatLocalFileDepsAsOutdated: true, }) if (upToDate && await restoreWantedLockfileIfMissing(wantedLockfileToRestore, opts)) { if (opts.hooks?.customResolvers?.some(r => r.shouldRefreshResolution)) { diff --git a/installing/commands/test/install.ts b/installing/commands/test/install.ts index d2df3eb2ef..216c1722b6 100644 --- a/installing/commands/test/install.ts +++ b/installing/commands/test/install.ts @@ -382,3 +382,38 @@ test('a config-level dryRun does not turn add into a no-op', async () => { const pkg = loadJsonFileSync<{ dependencies?: Record }>(path.resolve('package.json')) expect(pkg.dependencies).toStrictEqual({ 'is-positive': '1.0.0' }) }) + +// Covers https://github.com/pnpm/pnpm/issues/11795 +test('repeat install refetches a file: dependency after its contents change', async () => { + prepareEmpty() + + const localDepDir = path.resolve('..', 'local-dep') + fs.mkdirSync(localDepDir, { recursive: true }) + fs.writeFileSync(path.join(localDepDir, 'package.json'), JSON.stringify({ name: 'local-dep', version: '1.0.0' }), 'utf8') + fs.writeFileSync(path.join(localDepDir, 'index.js'), 'v1', 'utf8') + fs.writeFileSync('package.json', JSON.stringify({ + name: 'project', + version: '1.0.0', + dependencies: { 'local-dep': 'file:../local-dep' }, + }), 'utf8') + + await install.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + optimisticRepeatInstall: true, + }) + expect(fs.readFileSync('node_modules/local-dep/index.js', 'utf8')).toBe('v1') + + // A short delay so the edited file's mtime is unambiguously newer. + await delay(200) + fs.writeFileSync(path.join(localDepDir, 'index.js'), 'v2', 'utf8') + + // Without the local-file-deps guard the optimistic fast path would + // report "Already up to date" here and leave node_modules at v1. + await install.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + optimisticRepeatInstall: true, + }) + expect(fs.readFileSync('node_modules/local-dep/index.js', 'utf8')).toBe('v2') +}) diff --git a/pacquet/crates/package-manager/src/optimistic_repeat_install.rs b/pacquet/crates/package-manager/src/optimistic_repeat_install.rs index 6805d3c0f0..0c5dd7f3f1 100644 --- a/pacquet/crates/package-manager/src/optimistic_repeat_install.rs +++ b/pacquet/crates/package-manager/src/optimistic_repeat_install.rs @@ -31,8 +31,15 @@ //! the pnpmfile branch of `patchesOrHooksAreModified` (an added, removed, //! or edited workspace pnpmfile invalidates the fast path; plugin //! pnpmfiles from config dependencies are covered by the -//! `config_dependencies` comparison instead of the mtime check). The -//! `isLocalFileDepUpdated` branch of `linkedPackagesAreUpToDate` is NOT +//! `config_dependencies` comparison instead of the mtime check), and the +//! local-file-dependency bail (upstream's `treatLocalFileDepsAsOutdated` +//! option, set by `installDeps`): no tracked mtime covers the *contents* +//! of a local file dependency (a `file:` specifier or a bare local +//! path/tarball spec, declared directly or through a `pnpm.overrides` +//! entry), so projects declaring one always take the +//! full install path, which refetches those dependencies +//! (pnpm/pnpm#11795). The `isLocalFileDepUpdated` branch of +//! `linkedPackagesAreUpToDate` is 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 @@ -54,6 +61,7 @@ use std::{ time::SystemTime, }; +use pacquet_catalogs_resolver::{CatalogResolutionResult, WantedDependency, resolve_from_catalog}; use pacquet_catalogs_types::Catalogs; use pacquet_config::{Config, LinkWorkspacePackages, NodeLinker}; use pacquet_lockfile::{ImporterDepVersion, Lockfile, MaybeLazyLockfile, ProjectSnapshot}; @@ -152,6 +160,29 @@ pub fn check_optimistic_repeat_install(check: &OptimisticRepeatInstallCheck<'_>) return Decision::Skipped { reason: "no workspace state on disk" }; }; + // Unconditional where upstream gates it behind + // `treatLocalFileDepsAsOutdated`: the only caller here is the + // install command — the one consumer that sets the flag upstream. + if has_local_file_dep(project_manifests, included, catalogs) { + return Decision::Skipped { + reason: "a dependency is a local file dependency and its contents may have changed", + }; + } + match has_local_file_override(config, catalogs) { + Ok(true) => { + return Decision::Skipped { + reason: "an override maps to a local file dependency and its contents may have changed", + }; + } + Err(reason) => return Decision::Skipped { reason }, + Ok(false) => {} + } + if has_local_file_package_extension(config, included, catalogs) { + return Decision::Skipped { + reason: "a package extension injects a local file dependency and its contents may have changed", + }; + } + if !settings_match(&state, config, node_linker, included) { return Decision::Skipped { reason: "settings drift" }; } @@ -282,6 +313,182 @@ pub fn check_optimistic_repeat_install(check: &OptimisticRepeatInstallCheck<'_>) } } +/// Whether any project declares a dependency with a local file +/// specifier in `dependencies`, `devDependencies`, or +/// `optionalDependencies`. Port of upstream's `findLocalFileDep` in +/// `deps/status/src/checkDepsStatus.ts` +/// (). `link:` specifiers +/// don't count: they are symlinked, so changes inside them flow +/// through without a reinstall. Groups excluded from the current +/// install (per `included`) are skipped: their local file dependencies +/// are not installed, so their contents cannot be stale. A change to +/// the include flags between installs is caught separately by +/// `settings_match`. `catalog:` specs are dereferenced through the +/// workspace catalogs: the catalog resolver only bans the `workspace:`, +/// `link:`, and `file:` protocols, so a catalog entry can still hold a +/// bare local path (`../lib`, `vendor/pkg.tgz`) that resolves to a +/// local file dependency. +fn has_local_file_dep( + project_manifests: &[(PathBuf, &PackageManifest)], + included: IncludedDependencies, + catalogs: &Catalogs, +) -> bool { + let fields: [(&str, bool); 3] = [ + ("dependencies", included.dependencies), + ("devDependencies", included.dev_dependencies), + ("optionalDependencies", included.optional_dependencies), + ]; + project_manifests.iter().any(|(_, manifest)| { + fields.iter().any(|(field, group_included)| { + *group_included + && manifest.value().get(*field).and_then(|value| value.as_object()).is_some_and( + |deps| { + deps.iter().any(|(alias, spec)| { + spec.as_str().is_some_and(|spec| { + is_local_file_spec(spec) + || catalog_resolves_to_local_file(catalogs, alias, spec) + }) + }) + }, + ) + }) + }) +} + +/// Whether a `catalog:` spec dereferences (through the workspace +/// catalogs) to a local file specifier. Non-catalog specs and +/// misconfigured catalog entries return `false`: the former never +/// consult the catalogs, and the latter fail the full install with the +/// proper error anyway — the fast path only needs to not report +/// up-to-date for a *valid* catalog entry holding a local path. +fn catalog_resolves_to_local_file(catalogs: &Catalogs, alias: &str, spec: &str) -> bool { + // `resolve_from_catalog` returns `Unused` for any non-`catalog:` spec, so + // short-circuit before allocating the owned `WantedDependency` it needs. + if !spec.starts_with("catalog:") { + return false; + } + match resolve_from_catalog( + catalogs, + &WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() }, + ) { + CatalogResolutionResult::Found(found) => is_local_file_spec(&found.resolution.specifier), + _ => false, + } +} + +/// Whether any `pnpm.overrides` entry maps to a local file specifier. +/// Port of upstream's `findLocalFileOverride` in +/// `deps/status/src/checkDepsStatus.ts`: an override redirects every +/// matching dependency in the graph to its specifier, so a local file +/// override makes the installed contents depend on that directory or +/// tarball the same way a direct local file dependency does. Overrides +/// are run through `parse_config_overrides` so `catalog:` specs are +/// dereferenced before the check. A parse failure (misconfigured +/// catalog, invalid selector) bails to the full install path with its +/// own distinct reason — not the local-file reason, which would +/// misattribute the cause — mirroring upstream, where `parseOverrides` +/// throws to `checkDepsStatus`'s outer catch and the caller falls back +/// to a full install. +fn has_local_file_override(config: &Config, catalogs: &Catalogs) -> Result { + match crate::install::parse_config_overrides(config, catalogs) { + Ok(Some(overrides)) => { + Ok(overrides.iter().any(|entry| is_local_file_spec(&entry.new_bare_specifier))) + } + Ok(None) => Ok(false), + Err(_) => Err("pnpm.overrides cannot be parsed"), + } +} + +/// Whether any `packageExtensions` entry injects a dependency with a +/// local file specifier. Package extensions are merged into matching +/// packages' manifests by the read-package hook during the full +/// install, so a `file:`/local-path/tarball spec added there has the +/// same content-change blind spot as a direct local file dependency +/// without appearing in any project manifest. Only `dependencies` and +/// `optionalDependencies` are scanned: peer dependencies are resolved +/// from the graph rather than fetched, so a local spec there is never +/// installed. `optionalDependencies` are skipped when the install +/// excludes them, mirroring `has_local_file_dep`. +fn has_local_file_package_extension( + config: &Config, + included: IncludedDependencies, + catalogs: &Catalogs, +) -> bool { + let Some(extensions) = config.package_extensions.as_ref() else { + return false; + }; + extensions.values().any(|extension| { + let optional = included + .optional_dependencies + .then_some(extension.optional_dependencies.as_ref()) + .flatten(); + [extension.dependencies.as_ref(), optional].into_iter().flatten().any(|deps| { + deps.iter().any(|(alias, spec)| { + is_local_file_spec(spec) || catalog_resolves_to_local_file(catalogs, alias, spec) + }) + }) + }) +} + +/// Whether the specifier resolves to a local directory or tarball whose +/// contents can change without any manifest or lockfile mtime moving: +/// the `file:` protocol, path-prefixed specs (`./`, `../`, `~/`, +/// absolute POSIX paths, and Windows drive paths including +/// drive-relative ones like `c:dir`), and bare tarball file names. +/// Port of upstream's `isLocalFileSpec` in +/// `deps/status/src/checkDepsStatus.ts`. +/// +/// Deliberately narrower than the local resolver's bare-path matching: +/// a bare path like `user/repo` is statically indistinguishable from a +/// git shorthand at this layer, and matching it would disable the +/// repeat-install fast path for every project with git dependencies. +/// Such specs (and anything else carrying a protocol or URL) stay on +/// the fast path. `catalog:` specs also return `false` here — callers +/// dereference them through the workspace catalogs first, because a +/// catalog entry may hold a bare local path (the catalog resolver only +/// bans the `workspace:`, `link:`, and `file:` protocols). +fn is_local_file_spec(spec: &str) -> bool { + if spec.starts_with("file:") { + return true; + } + if spec.starts_with(['.', '/', '\\']) + || spec.starts_with("~/") + || spec.starts_with(r"~\") + || is_windows_drive_path(spec) + { + return true; + } + if spec.contains(':') { + return false; + } + // A `#` here means a hosted-git shorthand committish + // (`user/repo#release.tgz`), not a local tarball — the `file:` and + // path-prefixed cases already returned above. + if spec.contains('#') { + return false; + } + ends_with_ignore_ascii_case(spec, ".tgz") + || ends_with_ignore_ascii_case(spec, ".tar.gz") + || ends_with_ignore_ascii_case(spec, ".tar") +} + +/// Case-insensitive (ASCII) suffix check that, unlike +/// `spec.to_ascii_lowercase().ends_with(suffix)`, does not allocate. +fn ends_with_ignore_ascii_case(spec: &str, suffix: &str) -> bool { + let spec = spec.as_bytes(); + let suffix = suffix.as_bytes(); + spec.len() >= suffix.len() && spec[spec.len() - suffix.len()..].eq_ignore_ascii_case(suffix) +} + +/// `c:/...`, `c:\...`, or drive-relative `c:foo` — a Windows drive +/// path. No separator is required after the colon, matching the local +/// resolver's `isFilespec` (`resolving/local-resolver/src/parseBareSpecifier.ts`); +/// no registry protocol is a single letter, so `[a-z]:` is unambiguous. +fn is_windows_drive_path(spec: &str) -> bool { + let bytes = spec.as_bytes(); + bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' +} + /// 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 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 e5bcc5cecb..b1e9544735 100644 --- a/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs +++ b/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs @@ -2,6 +2,7 @@ use super::{ Decision, OptimisticRepeatInstallCheck, check_optimistic_repeat_install, current_settings, current_settings_with_catalogs, }; +use indexmap::IndexMap; use pacquet_catalogs_types::Catalogs; use pacquet_config::Config; use pacquet_lockfile::{Lockfile, MaybeLazyLockfile}; @@ -100,6 +101,25 @@ fn setup_fresh_install( project_name: &str, project_version: &str, manifest_extra_json: &str, +) -> (tempfile::TempDir, &'static Config, PackageManifest) { + setup_fresh_install_with_config( + config_kind, + project_name, + project_version, + manifest_extra_json, + |_| {}, + ) +} + +/// Same as [`setup_fresh_install`] but applies `configure` to the +/// `Config` before the workspace-state snapshot is taken, so the +/// settings comparison sees the configured values as unchanged. +fn setup_fresh_install_with_config( + config_kind: pacquet_config::NodeLinker, + project_name: &str, + project_version: &str, + manifest_extra_json: &str, + configure: impl FnOnce(&mut Config), ) -> (tempfile::TempDir, &'static Config, PackageManifest) { let dir = tempdir().unwrap(); let workspace_root = dir.path(); @@ -130,6 +150,7 @@ fn setup_fresh_install( let mut config = Config::new(); config.modules_dir = workspace_root.join("node_modules"); + configure(&mut config); let config = Box::leak(Box::new(config)); // Pre-create the modules dir so the "missing node_modules" guard // doesn't fire on the happy-path tests. @@ -163,6 +184,490 @@ fn returns_up_to_date_when_state_and_manifests_agree() { assert_eq!(decision, Decision::UpToDate); } +/// A `file:` dependency must never short-circuit: nothing the fast +/// path stats covers the dependency's *contents*, so the full install +/// path has to run and refetch it +/// (). +#[test] +fn returns_skipped_when_a_project_has_a_file_dependency() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"file:../foo"}"#, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("local file dependency")), + ); +} + +/// Same bail for a `file:` *tarball* in any dependency group — a +/// repacked `.tgz` bumps no manifest mtime either. +#[test] +fn returns_skipped_when_a_project_has_a_file_tarball_dev_dependency() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""devDependencies":{"tar":"file:./vendor/tar.tgz"}"#, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("local file dependency")), + ); +} + +/// Bare local path and tarball specs resolve to local file dependencies +/// too — same bail as `file:`, for the same reason. +#[test] +fn returns_skipped_when_a_project_has_a_bare_local_path_dependency() { + for spec in + ["vendor/pkg.tgz", "../sibling-dir", "~/pkgs/foo", "/abs/path/foo", "c:/pkgs/foo", "c:pkgs"] + { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + &format!(r#""dependencies":{{"foo":"{spec}"}}"#), + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("local file dependency")), + "spec {spec:?} must bail", + ); + } +} + +/// A local file dependency in a group excluded from the install must +/// not bail: the group isn't materialized, so its contents can't be +/// stale. A change to the include flags themselves is caught by the +/// settings comparison instead. +#[test] +fn returns_up_to_date_when_the_local_file_dependency_is_in_an_excluded_group() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""optionalDependencies":{"foo":"file:../foo"}"#, + ); + + let included = IncludedDependencies { + dependencies: true, + dev_dependencies: true, + optional_dependencies: false, + }; + // Re-stamp the state with the same include flags the check runs + // under, so the settings comparison passes and the include gate is + // what gets exercised. + let settings = current_settings(config, pacquet_config::NodeLinker::Isolated, included); + let mut projects = BTreeMap::new(); + projects.insert( + dir.path().to_string_lossy().into_owned(), + ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) }, + ); + write_state(dir.path(), now_millis(), settings, projects); + + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included, + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: false, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &BTreeMap::default(), + }); + assert_eq!(decision, Decision::UpToDate); +} + +/// The include gate is per-group: a local file dependency in a group +/// that *is* installed still bails even when other groups are excluded. +#[test] +fn returns_skipped_when_the_local_file_dependency_is_in_an_included_group() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"file:../foo"}"#, + ); + + let included = IncludedDependencies { + dependencies: true, + dev_dependencies: false, + optional_dependencies: false, + }; + let settings = current_settings(config, pacquet_config::NodeLinker::Isolated, included); + let mut projects = BTreeMap::new(); + projects.insert( + dir.path().to_string_lossy().into_owned(), + ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) }, + ); + write_state(dir.path(), now_millis(), settings, projects); + + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included, + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: false, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &BTreeMap::default(), + }); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("local file dependency")), + ); +} + +/// A `pnpm.overrides` entry mapping to a local file spec must bail the +/// same way a direct local file dependency does: the override redirects +/// every matching dependency in the graph to that directory, and +/// nothing the fast path stats covers its contents. +#[test] +fn returns_skipped_when_an_override_maps_to_a_local_file_dependency() { + let (dir, config, manifest) = setup_fresh_install_with_config( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"^1.0.0"}"#, + |config| { + config.overrides = + Some(IndexMap::from([("bar".to_string(), "file:../bar".to_string())])); + }, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("override")), + "decision was {decision:?}", + ); +} + +/// Registry and `link:` overrides keep the fast path: neither redirects +/// a dependency to contents only a refetch would pick up. +#[test] +fn returns_up_to_date_when_overrides_are_not_local_paths() { + let (dir, config, manifest) = setup_fresh_install_with_config( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"^1.0.0"}"#, + |config| { + config.overrides = Some(IndexMap::from([ + ("bar".to_string(), "^2.0.0".to_string()), + ("baz".to_string(), "link:../baz".to_string()), + ])); + }, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert_eq!(decision, Decision::UpToDate); +} + +/// A `packageExtensions` entry injecting a local file dependency must +/// bail: extensions are merged into matching packages' manifests during +/// the full install, so the spec never appears in a project manifest. +#[test] +fn returns_skipped_when_a_package_extension_injects_a_local_file_dependency() { + let (dir, config, manifest) = setup_fresh_install_with_config( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"^1.0.0"}"#, + |config| { + config.package_extensions = Some(IndexMap::from([( + "foo@1".to_string(), + pacquet_config::PackageExtension { + dependencies: Some(BTreeMap::from([( + "bar".to_string(), + "file:../bar".to_string(), + )])), + ..Default::default() + }, + )])); + }, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("package extension")), + "decision was {decision:?}", + ); +} + +/// A local file dependency injected via a packageExtension's +/// optionalDependencies does not bail when optionals are excluded from +/// the install — they aren't installed, so their contents can't be stale. +#[test] +fn returns_up_to_date_when_a_package_extension_optional_dependency_is_excluded() { + let (dir, config, manifest) = setup_fresh_install_with_config( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"^1.0.0"}"#, + |config| { + config.package_extensions = Some(IndexMap::from([( + "foo@1".to_string(), + pacquet_config::PackageExtension { + optional_dependencies: Some(BTreeMap::from([( + "bar".to_string(), + "file:../bar".to_string(), + )])), + ..Default::default() + }, + )])); + }, + ); + + let included = IncludedDependencies { + dependencies: true, + dev_dependencies: true, + optional_dependencies: false, + }; + let settings = current_settings(config, pacquet_config::NodeLinker::Isolated, included); + let mut projects = BTreeMap::new(); + projects.insert( + dir.path().to_string_lossy().into_owned(), + ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) }, + ); + write_state(dir.path(), now_millis(), settings, projects); + + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included, + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: false, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &BTreeMap::default(), + }); + assert_eq!(decision, Decision::UpToDate); +} + +/// An unparsable `pnpm.overrides` (here a `catalog:` reference with no +/// matching catalog entry) bails to the full install with the +/// parse-error reason, not the local-file reason: the cause is a +/// misconfiguration, and attributing it to a local file dependency +/// would mislead troubleshooting. +#[test] +fn returns_skipped_with_parse_error_reason_when_overrides_cannot_be_parsed() { + let (dir, config, manifest) = setup_fresh_install_with_config( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"^1.0.0"}"#, + |config| { + config.overrides = Some(IndexMap::from([("bar".to_string(), "catalog:".to_string())])); + }, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("cannot be parsed")), + "decision was {decision:?}", + ); +} + +/// A `catalog:` dependency whose catalog entry holds a bare local path +/// is a local file dependency after dereferencing — the catalog +/// resolver only bans the `workspace:`, `link:`, and `file:` protocols, +/// so the bare-path spelling reaches the local resolver. Same bail as +/// a direct local path. +#[test] +fn returns_skipped_when_a_catalog_dependency_resolves_to_a_local_path() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"catalog:"}"#, + ); + + let catalogs: Catalogs = BTreeMap::from([( + "default".to_string(), + BTreeMap::from([("foo".to_string(), "../foo".to_string())]), + )]); + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: false, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &catalogs, + }); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("local file dependency")), + "decision was {decision:?}", + ); +} + +/// A `catalog:` dependency resolving to a registry range keeps the +/// fast path: the dereferenced specifier is not a local path. +#[test] +fn returns_up_to_date_when_a_catalog_dependency_resolves_to_a_registry_range() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"catalog:"}"#, + ); + + let catalogs: Catalogs = BTreeMap::from([( + "default".to_string(), + BTreeMap::from([("foo".to_string(), "^1.0.0".to_string())]), + )]); + // Record the catalogs in the state so the catalog-cache comparison + // passes and the spec-deref path is what gets exercised, not the + // "catalogs cache outdated" bail. + let settings = current_settings_with_catalogs( + config, + pacquet_config::NodeLinker::Isolated, + isolated_included(), + &catalogs, + ); + let mut projects = BTreeMap::new(); + projects.insert( + dir.path().to_string_lossy().into_owned(), + ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) }, + ); + write_state(dir.path(), now_millis(), settings, projects); + + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: false, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &catalogs, + }); + assert_eq!(decision, Decision::UpToDate); +} + +/// A `pnpm.overrides` entry spelled `catalog:` whose catalog entry +/// holds a local path bails like a direct local file override — +/// overrides are dereferenced through `parse_config_overrides` before +/// the check. +#[test] +fn returns_skipped_when_an_override_maps_through_a_catalog_to_a_local_path() { + let (dir, config, manifest) = setup_fresh_install_with_config( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"^1.0.0"}"#, + |config| { + config.overrides = Some(IndexMap::from([("bar".to_string(), "catalog:".to_string())])); + }, + ); + + let catalogs: Catalogs = BTreeMap::from([( + "default".to_string(), + BTreeMap::from([("bar".to_string(), "./vendor/bar".to_string())]), + )]); + let decision = check_optimistic_repeat_install(&OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + node_linker: pacquet_config::NodeLinker::Isolated, + included: isolated_included(), + project_manifests: &[(dir.path().to_path_buf(), &manifest)], + is_workspace_install: false, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &catalogs, + }); + assert!( + matches!(decision, Decision::Skipped { reason } if reason.contains("override")), + "decision was {decision:?}", + ); +} + +/// Specs the git, remote-tarball, and registry resolvers claim must not +/// bail — matching them would disable the fast path for every project +/// with git dependencies. +#[test] +fn returns_up_to_date_when_specs_are_not_local_paths() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + concat!( + r#""dependencies":{"foo":"user/repo","bar":"github:user/repo","#, + // `quux` is a git shorthand whose committish ends in .tgz — it + // must not be mistaken for a local tarball. + r#""baz":"https://example.com/pkg.tgz","qux":"~1.2.3","quux":"user/repo#release.tgz"}"#, + ), + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert_eq!(decision, Decision::UpToDate); +} + +/// `link:` dependencies are symlinked — changes inside them flow +/// through without a reinstall, so they don't invalidate the fast path. +#[test] +fn returns_up_to_date_when_a_project_has_only_link_dependencies() { + let (dir, config, manifest) = setup_fresh_install( + pacquet_config::NodeLinker::Isolated, + "root", + "1.0.0", + r#""dependencies":{"foo":"link:../foo"}"#, + ); + + let decision = check( + dir.path(), + config, + pacquet_config::NodeLinker::Isolated, + &[(dir.path().to_path_buf(), &manifest)], + ); + assert_eq!(decision, Decision::UpToDate); +} + /// `optimistic_repeat_install: false` opts the user out entirely. #[test] fn returns_skipped_when_config_disabled() { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d16ae79b66..6be9bd0e1c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3833,6 +3833,12 @@ importers: deps/status: dependencies: + '@pnpm/catalogs.resolver': + specifier: workspace:* + version: link:../../catalogs/resolver + '@pnpm/catalogs.types': + specifier: workspace:* + version: link:../../catalogs/types '@pnpm/config.parse-overrides': specifier: workspace:* version: link:../../config/parse-overrides