Skip to content

Commit 6d35338

Browse files
truffle-devzkochan
truffle-dev
and
zkochan
authored
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 #11158. Fixes #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](#11795 (comment))): 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 <z@kochan.io>
1 parent 30c7590 commit 6d35338

11 files changed

Lines changed: 1417 additions & 7 deletions

File tree

‎.changeset/repeat-install-file-deps.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@pnpm/deps.status": patch
3+
"@pnpm/installing.commands": patch
4+
"pnpm": patch
5+
---
6+
7+
`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).

‎cspell.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@
200200
"msgpackr",
201201
"msvc",
202202
"msys",
203+
"mtimes",
203204
"musleabihf",
204205
"mycomp",
205206
"mycompany",
@@ -298,6 +299,7 @@
298299
"redownload",
299300
"refclone",
300301
"refetched",
302+
"refetches",
301303
"reflattened",
302304
"reflink",
303305
"reflinked",

‎deps/status/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
".test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules --disable-warning=ExperimentalWarning --disable-warning=DEP0169\" jest"
3434
},
3535
"dependencies": {
36+
"@pnpm/catalogs.resolver": "workspace:*",
37+
"@pnpm/catalogs.types": "workspace:*",
3638
"@pnpm/config.parse-overrides": "workspace:*",
3739
"@pnpm/config.reader": "workspace:*",
3840
"@pnpm/constants": "workspace:*",

‎deps/status/src/checkDepsStatus.ts‎

Lines changed: 180 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import fs from 'node:fs'
22
import path from 'node:path'
33
import util from 'node:util'
44

5+
import { resolveFromCatalog } from '@pnpm/catalogs.resolver'
6+
import type { Catalogs } from '@pnpm/catalogs.types'
57
import { parseOverrides } from '@pnpm/config.parse-overrides'
68
import type { Config, ConfigContext } from '@pnpm/config.reader'
79
import { MANIFEST_BASE_NAMES, WANTED_LOCKFILE } from '@pnpm/constants'
@@ -27,11 +29,13 @@ import {
2729
} from '@pnpm/lockfile.verification'
2830
import { globalWarn, logger } from '@pnpm/logger'
2931
import type { WorkspacePackages } from '@pnpm/resolving.resolver-base'
30-
import type {
31-
DependencyManifest,
32-
Project,
33-
ProjectId,
34-
ProjectManifest,
32+
import {
33+
DEPENDENCIES_FIELDS,
34+
type DependencyManifest,
35+
type IncludedDependencies,
36+
type Project,
37+
type ProjectId,
38+
type ProjectManifest,
3539
} from '@pnpm/types'
3640
import { findWorkspaceProjectsNoCheck } from '@pnpm/workspace.projects-reader'
3741
import { loadWorkspaceState, updateWorkspaceState, WORKSPACE_STATE_SETTING_KEYS, type WorkspaceState, type WorkspaceStateSettings } from '@pnpm/workspace.state'
@@ -68,6 +72,26 @@ export type CheckDepsStatusOptions = Pick<Config,
6872
ignoreFilteredInstallCache?: boolean
6973
ignoredWorkspaceStateSettings?: Array<keyof WorkspaceStateSettings>
7074
pnpmfile: string[]
75+
/**
76+
* The checks below only track manifest and lockfile mtimes, so edits inside
77+
* a local file dependency's directory (or a repacked local tarball) go
78+
* unnoticed. Callers that skip the install entirely when this check reports
79+
* up-to-date must set this so that projects with local file dependencies
80+
* (`file:` and bare local path/tarball specifiers) always run a real
81+
* install, which refetches those dependencies
82+
* (https://github.com/pnpm/pnpm/issues/11795).
83+
*/
84+
treatLocalFileDepsAsOutdated?: boolean
85+
/**
86+
* Which dependency groups the current install materializes. Local file
87+
* dependencies in an excluded group (for example `devDependencies` under
88+
* `--prod`) are not installed, so they don't force the
89+
* `treatLocalFileDepsAsOutdated` bail-out. A change to these flags between
90+
* installs is caught separately by the workspace state settings comparison
91+
* (`dev`/`optional`/`production` are part of
92+
* `WORKSPACE_STATE_SETTING_KEYS`).
93+
*/
94+
include?: IncludedDependencies
7195
/**
7296
* When git-branch lockfiles are enabled, the wanted lockfile lives at
7397
* `pnpm-lock.<branch>.yaml`, so a missing `pnpm-lock.yaml` is the steady
@@ -141,6 +165,46 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
141165
workspaceDir,
142166
} = opts
143167

168+
// This check must run before the node-linker=pnp early return below:
169+
// that return reports up-to-date because verify-deps-before-run cannot
170+
// inspect a PnP install, but for the optimistic repeat-install caller
171+
// (the only one setting this flag) "up-to-date" would skip the install
172+
// and break the local-file-deps guarantee.
173+
if (opts.treatLocalFileDepsAsOutdated) {
174+
const manifests = allProjects?.map(({ manifest }) => manifest) ?? []
175+
// `rootProjectManifest` is tracked separately from `allProjects` and the
176+
// recursive project list can omit the workspace root (for example when
177+
// `includeWorkspaceRoot` is false), so scan it too unless `allProjects`
178+
// already covers it.
179+
if (rootProjectManifest != null && !allProjects?.some(({ rootDir }) => rootDir === rootProjectManifestDir)) {
180+
manifests.push(rootProjectManifest)
181+
}
182+
const localFileDep = findLocalFileDep(manifests, opts.include, catalogs)
183+
if (localFileDep != null) {
184+
return {
185+
upToDate: false,
186+
issue: `The dependency "${localFileDep}" is a local file dependency and its contents may have changed`,
187+
workspaceState,
188+
}
189+
}
190+
const localFileOverride = findLocalFileOverride(opts.overrides, catalogs)
191+
if (localFileOverride != null) {
192+
return {
193+
upToDate: false,
194+
issue: `The override "${localFileOverride}" maps to a local file dependency and its contents may have changed`,
195+
workspaceState,
196+
}
197+
}
198+
const localFileExtension = findLocalFilePackageExtension(opts.packageExtensions, opts.include, catalogs)
199+
if (localFileExtension != null) {
200+
return {
201+
upToDate: false,
202+
issue: `The package extension "${localFileExtension}" injects a local file dependency and its contents may have changed`,
203+
workspaceState,
204+
}
205+
}
206+
}
207+
144208
if (nodeLinker === 'pnp') {
145209
globalWarn('verify-deps-before-run does not work with node-linker=pnp')
146210
return { upToDate: true, workspaceState: undefined }
@@ -622,6 +686,117 @@ async function assertWantedLockfileUpToDate (
622686
}
623687
}
624688

689+
/**
690+
* Returns the name of the first dependency declared with a local file
691+
* specifier in any of the given manifests, or `undefined` when there is none.
692+
* `link:` dependencies are excluded: they are symlinked, so changes inside
693+
* them flow through without a reinstall. Dependency groups excluded from the
694+
* current install (per `include`) are skipped: their local file dependencies
695+
* are not installed, so their contents cannot be stale. `catalog:` specs are
696+
* dereferenced through the catalogs config: the catalog resolver only bans
697+
* the `workspace:`, `link:`, and `file:` protocols, so a catalog entry can
698+
* still hold a bare local path (`../lib`, `vendor/pkg.tgz`) that resolves to
699+
* a local file dependency.
700+
*/
701+
function findLocalFileDep (manifests: ProjectManifest[], include?: IncludedDependencies, catalogs?: Catalogs): string | undefined {
702+
for (const manifest of manifests) {
703+
for (const depField of DEPENDENCIES_FIELDS) {
704+
if (include?.[depField] === false) continue
705+
const depName = findLocalFileDepInRecord(manifest[depField], catalogs)
706+
if (depName != null) return depName
707+
}
708+
}
709+
return undefined
710+
}
711+
712+
/**
713+
* Returns the name of the first dependency in `deps` declared with (or
714+
* resolving through a catalog to) a local file specifier, or `undefined`.
715+
*/
716+
function findLocalFileDepInRecord (deps: Record<string, string> | undefined, catalogs?: Catalogs): string | undefined {
717+
if (deps == null) return undefined
718+
for (const [depName, spec] of Object.entries(deps)) {
719+
// A malformed manifest may carry a non-string spec; skip it rather
720+
// than throw — checkDepsStatus() must never crash.
721+
if (typeof spec !== 'string') continue
722+
if (isLocalFileSpec(spec)) return depName
723+
// Only catalog: specs consult the catalogs, so skip the lookup for
724+
// everything else to keep the optimistic fast path cheap.
725+
if (!spec.startsWith('catalog:')) continue
726+
const catalogResult = resolveFromCatalog(catalogs ?? {}, { alias: depName, bareSpecifier: spec })
727+
if (catalogResult.type === 'found' && isLocalFileSpec(catalogResult.resolution.specifier)) return depName
728+
}
729+
return undefined
730+
}
731+
732+
/**
733+
* Returns the selector of the first `packageExtensions` entry that injects a
734+
* local file dependency, or `undefined` when there is none. Package
735+
* extensions are merged into matching packages' manifests by a read-package
736+
* hook during install, so a `file:`/local-path/tarball spec added there has
737+
* the same content-change blind spot as a direct local file dependency
738+
* without appearing in any project manifest. Only `dependencies` and
739+
* `optionalDependencies` are scanned: peer dependencies are resolved from the
740+
* graph rather than fetched, so a local spec there is never installed.
741+
*/
742+
function findLocalFilePackageExtension (packageExtensions: CheckDepsStatusOptions['packageExtensions'], include?: IncludedDependencies, catalogs?: Catalogs): string | undefined {
743+
if (packageExtensions == null) return undefined
744+
for (const [selector, extension] of Object.entries(packageExtensions)) {
745+
if (findLocalFileDepInRecord(extension.dependencies, catalogs) != null) return selector
746+
if (include?.optionalDependencies === false) continue
747+
if (findLocalFileDepInRecord(extension.optionalDependencies, catalogs) != null) return selector
748+
}
749+
return undefined
750+
}
751+
752+
/**
753+
* Returns the selector of the first override that maps to a local file
754+
* specifier, or `undefined` when there is none. An override redirects every
755+
* matching dependency in the graph to its specifier, so a local file override
756+
* makes the installed contents depend on that directory or tarball the same
757+
* way a direct local file dependency does. Overrides are run through
758+
* `parseOverrides` so `catalog:` specs are dereferenced before the check.
759+
* `parseOverrides` throws on a misconfigured catalog or invalid selector;
760+
* that propagates to the outer catch in `checkDepsStatus`, which reports
761+
* not-up-to-date, and the resulting full install surfaces the same error.
762+
*/
763+
function findLocalFileOverride (overrides: Record<string, string> | undefined, catalogs?: Catalogs): string | undefined {
764+
if (overrides == null || isEmpty(overrides)) return undefined
765+
return parseOverrides(overrides, catalogs)
766+
.find(({ newBareSpecifier }) => isLocalFileSpec(newBareSpecifier))?.selector
767+
}
768+
769+
const LOCAL_PATH_PREFIX = /^(?:[./\\]|~[/\\]|[a-z]:)/i
770+
const LOCAL_TARBALL_EXTENSION = /\.(?:tgz|tar\.gz|tar)$/i
771+
772+
/**
773+
* Whether the specifier resolves to a local directory or tarball whose
774+
* contents can change without any manifest or lockfile mtime moving: the
775+
* `file:` protocol, path-prefixed specs (`./`, `../`, `~/`, absolute POSIX
776+
* paths, and Windows drive paths — including drive-relative ones like
777+
* `C:dir`, matching the local resolver's `isFilespec`), and bare tarball
778+
* file names.
779+
*
780+
* Deliberately narrower than the local resolver's bare-path matching: a bare
781+
* `dir/file.tgz`-less path like `user/repo` is statically indistinguishable
782+
* from a git shorthand at this layer, and matching it would disable the
783+
* repeat-install fast path for every project with git dependencies. Such
784+
* specs (and anything else carrying a protocol or URL) stay on the fast
785+
* path. `catalog:` specs also return false here — callers dereference them
786+
* through the catalogs config first, because a catalog entry may hold a
787+
* bare local path (the catalog resolver only bans the `workspace:`,
788+
* `link:`, and `file:` protocols).
789+
*/
790+
function isLocalFileSpec (spec: string): boolean {
791+
if (spec.startsWith('file:')) return true
792+
if (LOCAL_PATH_PREFIX.test(spec)) return true
793+
if (spec.includes(':')) return false
794+
// A `#` here means a hosted-git shorthand committish (`user/repo#release.tgz`),
795+
// not a local tarball — the `file:` and path-prefixed cases already returned above.
796+
if (spec.includes('#')) return false
797+
return LOCAL_TARBALL_EXTENSION.test(spec)
798+
}
799+
625800
function throwLockfileNotFound (wantedLockfileDir: string): never {
626801
throw new PnpmError('RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND', `Cannot find a lockfile in ${wantedLockfileDir}`, {
627802
hint: 'Run `pnpm install` to create the lockfile',

0 commit comments

Comments
 (0)