Skip to content

Commit 61969fb

Browse files
aqeelatzkochan
aqeelat
and
zkochan
authored
fix(deps-status): detect lockfile-only changes (#12106)
## Summary Fixes `pnpm install` with `optimisticRepeatInstall` incorrectly returning `Already up to date` when `pnpm-lock.yaml` changed but project manifests did not. Fixes #12100. ## Root Cause `checkDepsStatus` used modified manifest mtimes as the only signal for whether it needed to validate dependency status. If no manifest was newer than `workspaceState.lastValidatedTimestamp`, it returned `upToDate: true` before checking whether the wanted lockfile had changed. That skipped lockfile validation for workflows like: - `git checkout HEAD~1 -- pnpm-lock.yaml` - restoring only `pnpm-lock.yaml` from a stash - external tools rewriting the lockfile without touching manifests ## Changes - Check wanted lockfile mtimes before taking the optimistic fast path. - If any wanted lockfile is missing or newer than the workspace state timestamp, validate all projects instead of only modified manifests. - Add a regression test proving a lockfile-only change does not skip wanted-lockfile validation. - Add a patch changeset for `@pnpm/deps.status` and `pnpm`. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
1 parent c9db42d commit 61969fb

17 files changed

Lines changed: 665 additions & 71 deletions

File tree

‎.changeset/fix-optimistic-repeat-install-lockfile.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@pnpm/deps.status": patch
3+
"@pnpm/lockfile.fs": patch
4+
"@pnpm/network.git-utils": patch
5+
"pnpm": patch
6+
---
7+
8+
Fix `pnpm install` with `optimisticRepeatInstall` incorrectly reporting `Already up to date` when `pnpm-lock.yaml` changed but project manifests did not. This affected workflows such as checking out or restoring only the lockfile [#12100](https://github.com/pnpm/pnpm/issues/12100).
9+
10+
Also fixes `checkDepsStatus` to use the correct lockfile path when `useGitBranchLockfile` is enabled, so the optimistic fast-path and lockfile modification detection work with `pnpm-lock.<branch>.yaml` files instead of always stat'ing `pnpm-lock.yaml`. Merge-conflict detection now reads the resolved lockfile name as well, and with `mergeGitBranchLockfiles` enabled every `pnpm-lock.*.yaml` is scanned for modifications and conflicts. The git branch is now resolved by reading `.git/HEAD` directly (no process spawn) and uses the workspace directory rather than `process.cwd()`.

‎cspell.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
"ignorePaths": [
33
"**/nodeReleaseKeys.ts",
44
"**/nodeReleaseKeys.d.ts",
5-
"**/node_release_keys.rs"
5+
"**/node_release_keys.rs",
6+
"bench-work-env/**"
67
],
78
"words": [
89
"adduser",
@@ -123,6 +124,7 @@
123124
"ghes",
124125
"ghsa",
125126
"ghsas",
127+
"gitdir",
126128
"gitea",
127129
"globalconfig",
128130
"globstar",

‎deps/status/src/checkDepsStatus.ts‎

Lines changed: 99 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import { resolveFromCatalog } from '@pnpm/catalogs.resolver'
66
import type { Catalogs } from '@pnpm/catalogs.types'
77
import { parseOverrides } from '@pnpm/config.parse-overrides'
88
import type { Config, ConfigContext } from '@pnpm/config.reader'
9-
import { MANIFEST_BASE_NAMES, WANTED_LOCKFILE } from '@pnpm/constants'
9+
import { MANIFEST_BASE_NAMES } from '@pnpm/constants'
1010
import { hashObjectNullableWithPrefix } from '@pnpm/crypto.object-hasher'
1111
import { PnpmError } from '@pnpm/error'
1212
import { arrayOfWorkspacePackagesToMap } from '@pnpm/installing.context'
1313
import {
14+
getGitBranchLockfileNamesSync,
1415
getLockfileImporterId,
16+
getWantedLockfileName,
1517
type LockfileObject,
1618
readCurrentLockfile,
1719
readWantedLockfile,
@@ -53,6 +55,7 @@ export type CheckDepsStatusOptions = Pick<Config,
5355
| 'injectWorkspacePackages'
5456
| 'linkWorkspacePackages'
5557
| 'lockfileDir'
58+
| 'mergeGitBranchLockfiles'
5659
| 'nodeLinker'
5760
| 'patchedDependencies'
5861
| 'peersSuffixMaxLength'
@@ -242,13 +245,22 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
242245
}
243246
}
244247

245-
const conflictedLockfileDir = findConflictedLockfileDir(getWantedLockfileDirs({
248+
const lockfileDirs = getWantedLockfileDirs({
246249
allProjects,
247250
lockfileDir,
248251
rootProjectManifestDir,
249252
sharedWorkspaceLockfile,
250253
workspaceDir,
251-
}), workspaceState.lastValidatedTimestamp)
254+
})
255+
const wantedLockfileName = await getWantedLockfileName({
256+
useGitBranchLockfile: opts.useGitBranchLockfile,
257+
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
258+
cwd: workspaceDir ?? lockfileDir ?? rootProjectManifestDir,
259+
})
260+
const { conflictedDir: conflictedLockfileDir, anyModified: lockfilesModified, anyMissing: lockfilesMissing } = scanWantedLockfiles(lockfileDirs, workspaceState.lastValidatedTimestamp, {
261+
wantedLockfileName,
262+
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
263+
})
252264
if (conflictedLockfileDir != null) {
253265
return {
254266
upToDate: false,
@@ -338,15 +350,21 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
338350
manifestStats.mtime.valueOf() > workspaceState.lastValidatedTimestamp
339351
)
340352

341-
if (modifiedProjects.length === 0) {
342-
logger.debug({ msg: 'No manifest files were modified since the last validation. Exiting check.' })
343-
const wantedLockfileToRestore = sharedWorkspaceLockfile && !opts.useGitBranchLockfile
344-
? await missingWantedLockfileStandIn(workspaceDir)
353+
if ((modifiedProjects.length === 0) && !lockfilesModified) {
354+
const wantedLockfileToRestore = lockfilesMissing && sharedWorkspaceLockfile && !opts.useGitBranchLockfile
355+
? await missingWantedLockfileStandIn(workspaceDir, wantedLockfileName)
345356
: undefined
346-
return { upToDate: true, workspaceState, wantedLockfileToRestore }
357+
// A missing wanted lockfile only skips the full check when the current
358+
// lockfile can stand in for it. Otherwise fall through so the checks
359+
// below throw RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND instead of silently
360+
// reporting "up to date".
361+
if (!lockfilesMissing || wantedLockfileToRestore != null) {
362+
logger.debug({ msg: 'No manifest files or lockfiles were modified since the last validation. Exiting check.' })
363+
return { upToDate: true, workspaceState, wantedLockfileToRestore }
364+
}
347365
}
348366

349-
logger.debug({ msg: 'Some manifest files were modified since the last validation. Continuing check.' })
367+
logger.debug({ msg: 'Some manifest files or lockfiles were modified since the last validation. Continuing check.' })
350368

351369
let wantedLockfileToRestore: CheckDepsStatusResult['wantedLockfileToRestore']
352370
let readWantedLockfileAndDir: (projectDir: string) => Promise<{
@@ -356,7 +374,7 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
356374
if (sharedWorkspaceLockfile) {
357375
let wantedLockfileStats: fs.Stats | undefined
358376
try {
359-
wantedLockfileStats = fs.statSync(path.join(workspaceDir, WANTED_LOCKFILE))
377+
wantedLockfileStats = fs.statSync(path.join(workspaceDir, wantedLockfileName))
360378
} catch (error) {
361379
if (util.types.isNativeError(error) && 'code' in error && error.code === 'ENOENT') {
362380
wantedLockfileStats = undefined
@@ -382,7 +400,11 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
382400
wantedLockfileDir: workspaceDir,
383401
})
384402
} else {
385-
const wantedLockfilePromise = readWantedLockfile(workspaceDir, { ignoreIncompatible: false })
403+
const wantedLockfilePromise = readWantedLockfile(workspaceDir, {
404+
ignoreIncompatible: false,
405+
useGitBranchLockfile: opts.useGitBranchLockfile,
406+
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
407+
})
386408
if (wantedLockfileStats.mtime.valueOf() > workspaceState.lastValidatedTimestamp) {
387409
const currentLockfile = await readCurrentLockfile(path.join(workspaceDir, 'node_modules/.pnpm'), { ignoreIncompatible: false })
388410
const wantedLockfile = (await wantedLockfilePromise) ?? throwLockfileNotFound(workspaceDir)
@@ -395,8 +417,12 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
395417
}
396418
} else {
397419
readWantedLockfileAndDir = async wantedLockfileDir => {
398-
const wantedLockfilePromise = readWantedLockfile(wantedLockfileDir, { ignoreIncompatible: false })
399-
const wantedLockfileStats = await safeStat(path.join(wantedLockfileDir, WANTED_LOCKFILE))
420+
const wantedLockfilePromise = readWantedLockfile(wantedLockfileDir, {
421+
ignoreIncompatible: false,
422+
useGitBranchLockfile: opts.useGitBranchLockfile,
423+
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
424+
})
425+
const wantedLockfileStats = await safeStat(path.join(wantedLockfileDir, wantedLockfileName))
400426

401427
if (!wantedLockfileStats) return throwLockfileNotFound(wantedLockfileDir)
402428
if (wantedLockfileStats.mtime.valueOf() > workspaceState.lastValidatedTimestamp) {
@@ -432,7 +458,8 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
432458
}
433459

434460
try {
435-
await Promise.all(modifiedProjects.map(async ({ project }) => {
461+
const projectsToCheck = lockfilesModified ? allManifestStats : modifiedProjects
462+
await Promise.all(projectsToCheck.map(async ({ project }) => {
436463
const { wantedLockfile, wantedLockfileDir } = await readWantedLockfileAndDir(project.rootDir)
437464
await assertWantedLockfileUpToDate(assertCtx, {
438465
projectDir: project.rootDir,
@@ -482,14 +509,18 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
482509
if (rootProjectManifest && rootProjectManifestDir) {
483510
const internalPnpmDir = path.join(rootProjectManifestDir, 'node_modules', '.pnpm')
484511
const currentLockfilePromise = readCurrentLockfile(internalPnpmDir, { ignoreIncompatible: false })
485-
const wantedLockfilePromise = readWantedLockfile(rootProjectManifestDir, { ignoreIncompatible: false })
512+
const wantedLockfilePromise = readWantedLockfile(rootProjectManifestDir, {
513+
ignoreIncompatible: false,
514+
useGitBranchLockfile: opts.useGitBranchLockfile,
515+
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
516+
})
486517
const [
487518
currentLockfileStats,
488519
wantedLockfileStats,
489520
manifestStats,
490521
] = await Promise.all([
491522
safeStat(path.join(internalPnpmDir, 'lock.yaml')),
492-
safeStat(path.join(rootProjectManifestDir, WANTED_LOCKFILE)),
523+
safeStat(path.join(rootProjectManifestDir, wantedLockfileName)),
493524
statManifestFile(rootProjectManifestDir),
494525
])
495526

@@ -809,8 +840,8 @@ function throwLockfileNotFound (wantedLockfileDir: string): never {
809840
* `pnpm-lock.yaml` from it. `undefined` when the wanted lockfile is present
810841
* (nothing to restore) or when there is no current lockfile to restore from.
811842
*/
812-
async function missingWantedLockfileStandIn (lockfileDir: string): Promise<CheckDepsStatusResult['wantedLockfileToRestore']> {
813-
if (safeStatSync(path.join(lockfileDir, WANTED_LOCKFILE)) != null) return undefined
843+
async function missingWantedLockfileStandIn (lockfileDir: string, wantedLockfileName: string): Promise<CheckDepsStatusResult['wantedLockfileToRestore']> {
844+
if (safeStatSync(path.join(lockfileDir, wantedLockfileName)) != null) return undefined
814845
const currentLockfile = await readCurrentLockfile(path.join(lockfileDir, 'node_modules/.pnpm'), { ignoreIncompatible: false })
815846
if (currentLockfile == null) return undefined
816847
return { lockfile: currentLockfile, lockfileDir }
@@ -829,21 +860,60 @@ function getWantedLockfileDirs (opts: {
829860
return [opts.lockfileDir ?? opts.workspaceDir ?? opts.rootProjectManifestDir]
830861
}
831862

832-
function findConflictedLockfileDir (lockfileDirs: string[], lastValidatedTimestamp: number): string | undefined {
863+
function scanWantedLockfiles (lockfileDirs: string[], lastValidatedTimestamp: number, opts: {
864+
wantedLockfileName: string
865+
mergeGitBranchLockfiles?: boolean
866+
}): {
867+
conflictedDir: string | undefined
868+
anyModified: boolean
869+
anyMissing: boolean
870+
} {
871+
let conflictedDir: string | undefined
872+
let anyModified = false
873+
let anyMissing = false
833874
for (const lockfileDir of lockfileDirs) {
834-
let mtime: number
835-
try {
836-
mtime = fs.statSync(path.join(lockfileDir, WANTED_LOCKFILE)).mtime.valueOf()
837-
} catch (err: unknown) {
838-
if (util.types.isNativeError(err) && 'code' in err && err.code === 'ENOENT') continue
875+
// With `mergeGitBranchLockfiles`, `readWantedLockfile` merges every
876+
// `pnpm-lock.*.yaml`, so a change in any of them changes the wanted
877+
// lockfile and must be detected here.
878+
const lockfileNames = opts.mergeGitBranchLockfiles
879+
? gitBranchLockfileNames(lockfileDir, opts.wantedLockfileName)
880+
: [opts.wantedLockfileName]
881+
let foundInDir = false
882+
for (const lockfileName of lockfileNames) {
883+
let mtime: number
884+
try {
885+
mtime = fs.statSync(path.join(lockfileDir, lockfileName)).mtime.valueOf()
886+
} catch (err: unknown) {
887+
if (util.types.isNativeError(err) && 'code' in err && err.code === 'ENOENT') continue
888+
throw err
889+
}
890+
foundInDir = true
891+
if (mtime <= lastValidatedTimestamp) continue
892+
anyModified = true
893+
if (wantedLockfileHasMergeConflictsSync(lockfileDir, lockfileName)) {
894+
conflictedDir = lockfileDir
895+
return { conflictedDir, anyModified, anyMissing }
896+
}
897+
}
898+
if (!foundInDir) anyMissing = true
899+
}
900+
return { conflictedDir, anyModified, anyMissing }
901+
}
902+
903+
function gitBranchLockfileNames (lockfileDir: string, wantedLockfileName: string): string[] {
904+
let branchLockfileNames: string[]
905+
try {
906+
branchLockfileNames = getGitBranchLockfileNamesSync(lockfileDir)
907+
} catch (err: unknown) {
908+
if (util.types.isNativeError(err) && 'code' in err && err.code === 'ENOENT') {
909+
branchLockfileNames = []
910+
} else {
839911
throw err
840912
}
841-
// If the lockfile hasn't been modified since the last successful install, it can't have
842-
// grown conflict markers — skip the read to preserve the optimistic fast-path.
843-
if (mtime <= lastValidatedTimestamp) continue
844-
if (wantedLockfileHasMergeConflictsSync(lockfileDir)) return lockfileDir
845913
}
846-
return undefined
914+
return branchLockfileNames.includes(wantedLockfileName)
915+
? branchLockfileNames
916+
: [wantedLockfileName, ...branchLockfileNames]
847917
}
848918

849919
async function patchesOrHooksAreModified (opts: {

0 commit comments

Comments
 (0)