mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-31 12:10:49 -04:00
The lockfile verification cache currently only records the lockfile that exists at the **start** of an install. So a flow like:
```
pnpm install <pkg>
rm -rf node_modules
pnpm install
```
re-runs the per-package registry round-trip against the newly written lockfile, even though the local resolver already enforced the policy when picking those versions. The fresh lockfile is now recorded immediately after each install-time write, so the second install takes the cache fast path.
## Implementation
### Recording the post-resolution lockfile
- New helper `recordLockfileVerified` (in `installing/deps-installer/src/install/`). Gated on `cacheDir` + non-empty `resolutionVerifiers` — same gate the pre-resolution verifier uses.
- Two thin combiners over the lockfile writers: `writeWantedLockfileAndRecordVerified` and `writeLockfilesAndRecordVerified`. The install paths use these so the record always runs alongside the write.
### Hash stability: writer returns the canonical lockfile
The cache stores `hashObject(LockfileObject)` and the next install computes the same hash off the file it loads from disk. For the hashes to match, both ends must compute over structurally identical objects. They don't, naïvely: the in-memory write object can carry `undefined` optional fields (e.g. `settings.dedupePeers = undefined` from `opts.dedupePeers || undefined` in install code) that YAML drops on serialize — `object-hash` treats undefined vs missing as distinct values.
- `writeWantedLockfile` / `writeLockfiles` (in `@pnpm/lockfile.fs`) now return the canonical post-write `LockfileObject`: `convertToLockfileObject(stripUndefinedDeep(lockfileFile))`. The strip walks the existing object graph in memory rather than going through a `yaml.load` round-trip, so non-cache callers (deploy, deps-restorer, make-dedicated-lockfile, agent server) pay near-zero cost.
- Install hooks hash the writer's returned value, not the raw in-memory input. Guaranteed by construction to match what the next reader produces.
### `useGitBranchLockfile` correctness
The pre-resolution verification gate and the new post-write recorder were both keying cache records on a hard-coded `pnpm-lock.yaml`. Under `useGitBranchLockfile` the actual file is `pnpm-lock.<branch>.yaml`, so the stat shortcut hit `ENOENT` and the cache effectively never engaged for git-branch users. Both sites now resolve the real filename via `getWantedLockfileName`. The wrappers compute it once and pass it to the writer via a new optional `lockfileName` opt so `useGitBranchLockfile` installs don't fork `getCurrentBranch` twice per write.
### Bug fix unrelated to the cache, found during review
`writeLockfiles`' differs branch was deciding whether to remove or keep `node_modules/.pnpm/lock.yaml` based on `isEmptyLockfile(wantedLockfile)`. Filtered-current callers (deps-restorer) pass an empty current against a non-empty wanted, so this could leave a stale current lockfile on disk. Fixed to key off the current.
### Comments policy
`AGENTS.md` (and `pacquet/AGENTS.md`) now spell out the comment defaults: write self-documenting code, do not restate at call sites what the callee's JSDoc / doc comment already says, comments are reserved for the non-obvious *why*. The pruning pass in this PR brings the changed code in line.
## API surface
- `@pnpm/lockfile.fs` (minor):
- `writeWantedLockfile`: return widened from `Promise<void>` to `Promise<LockfileObject>`. New optional `lockfileName` opt.
- `writeCurrentLockfile`: return widened to `Promise<LockfileObject | undefined>` (undefined when the empty-lockfile branch unlinks).
- `writeLockfiles`: return widened from `Promise<void>` to `Promise<{ wantedLockfile, currentLockfile }>`. New optional `wantedLockfileName` opt. New exported `WriteLockfilesResult` type.
- New export: `getWantedLockfileName`.
- `@pnpm/installing.deps-installer` (patch): internal-only wrappers; no external API change.
425 lines
13 KiB
TypeScript
425 lines
13 KiB
TypeScript
import fs from 'node:fs'
|
|
import path from 'node:path'
|
|
|
|
import { expect, jest, test } from '@jest/globals'
|
|
import { LOCKFILE_VERSION, WANTED_LOCKFILE } from '@pnpm/constants'
|
|
import type { ProjectId } from '@pnpm/types'
|
|
import { temporaryDirectory } from 'tempy'
|
|
import yaml from 'yaml-tag'
|
|
|
|
jest.unstable_mockModule('@pnpm/network.git-utils', () => ({ getCurrentBranch: jest.fn() }))
|
|
|
|
const { getCurrentBranch } = await import('@pnpm/network.git-utils')
|
|
const {
|
|
readCurrentLockfile,
|
|
readWantedLockfile,
|
|
writeLockfiles,
|
|
writeWantedLockfile,
|
|
} = await import('@pnpm/lockfile.fs')
|
|
|
|
test('writeLockfiles()', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
'is-negative': '1.0.0',
|
|
'is-positive': '1.0.0',
|
|
},
|
|
specifiers: {
|
|
'is-negative': '^1.0.0',
|
|
'is-positive': '^1.0.0',
|
|
},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {
|
|
'/is-negative@1.0.0': {
|
|
os: ['darwin'],
|
|
dependencies: {
|
|
'is-positive': '2.0.0',
|
|
},
|
|
cpu: ['x86'],
|
|
libc: ['glibc'],
|
|
engines: {
|
|
node: '>=10',
|
|
npm: '\nfoo\n',
|
|
},
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
'/is-positive@1.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
'/is-positive@2.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
},
|
|
}
|
|
await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
})
|
|
expect(await readCurrentLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
|
|
expect(await readWantedLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
|
|
|
|
// Verifying the formatting of the lockfile
|
|
expect(fs.readFileSync(path.join(projectPath, WANTED_LOCKFILE), 'utf8')).toMatchSnapshot()
|
|
})
|
|
|
|
test('writeLockfiles() when no specifiers but dependencies present', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
'is-positive': 'link:../is-positive',
|
|
},
|
|
specifiers: {},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {},
|
|
}
|
|
await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
})
|
|
expect(await readCurrentLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
|
|
expect(await readWantedLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
|
|
})
|
|
|
|
test('writeWantedLockfile() returns the canonical lockfile — matches what readWantedLockfile produces, even when the input carries undefined optional fields', async () => {
|
|
// Cache-key contract: callers (today, the verification cache) need a
|
|
// hash of the *as-saved* lockfile, not the in-memory write object.
|
|
// Those two diverge specifically because YAML drops `undefined` on
|
|
// serialize. To exercise that drop, the fixture has to actually
|
|
// carry an explicit `undefined` — `settings.dedupePeers` here, the
|
|
// same field install-time code produces (see
|
|
// installing/deps-installer/src/install/index.ts where it's set to
|
|
// `opts.dedupePeers || undefined`). Without this, the test would
|
|
// happily pass against a writer that returned a near-canonical-but-
|
|
// still-divergent object.
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
specifiers: { 'is-positive': '^1.0.0' },
|
|
dependencies: { 'is-positive': '1.0.0' },
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
settings: {
|
|
autoInstallPeers: true,
|
|
excludeLinksFromLockfile: false,
|
|
dedupePeers: undefined,
|
|
},
|
|
packages: {
|
|
'/is-positive@1.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
},
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
} as any
|
|
const written = await writeWantedLockfile(projectPath, wantedLockfile)
|
|
const loaded = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
|
|
expect(written).toEqual(loaded)
|
|
// Verify the canonicalization actually dropped the undefined field —
|
|
// toEqual is lenient about undefined-vs-missing, so check explicitly.
|
|
expect('dedupePeers' in (written.settings ?? {})).toBe(false)
|
|
})
|
|
|
|
test('writeLockfiles() return matches readWantedLockfile/readCurrentLockfile output', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
specifiers: { 'is-positive': '^1.0.0' },
|
|
dependencies: { 'is-positive': '1.0.0' },
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {
|
|
'/is-positive@1.0.0': {
|
|
resolution: { integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=' },
|
|
},
|
|
},
|
|
}
|
|
const written = await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
})
|
|
const loadedWanted = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
|
|
const loadedCurrent = await readCurrentLockfile(projectPath, { ignoreIncompatible: false })
|
|
expect(written.wantedLockfile).toEqual(loadedWanted)
|
|
expect(written.currentLockfile).toEqual(loadedCurrent)
|
|
})
|
|
|
|
test('write does not use yaml anchors/aliases', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
'is-negative': '1.0.0',
|
|
'is-positive': '1.0.0',
|
|
},
|
|
specifiers: {
|
|
'is-negative': '1.0.0',
|
|
'is-positive': '1.0.0',
|
|
},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: yaml`
|
|
/react-dnd@2.5.4(react@15.6.1):
|
|
dependencies:
|
|
disposables: 1.0.2
|
|
dnd-core: 2.5.4
|
|
hoist-non-react-statics: 2.5.0
|
|
invariant: 2.2.3
|
|
lodash: 4.15.0
|
|
prop-types: 15.6.1
|
|
react: 15.6.1
|
|
dev: false
|
|
id: registry.npmjs.org/react-dnd/2.5.4
|
|
peerDependencies: &ref_11
|
|
react: '1'
|
|
resolution:
|
|
integrity: sha512-y9YmnusURc+3KPgvhYKvZ9oCucj51MSZWODyaeV0KFU0cquzA7dCD1g/OIYUKtNoZ+MXtacDngkdud2TklMSjw==
|
|
/react-dnd@2.5.4(react@15.6.2):
|
|
dependencies:
|
|
disposables: 1.0.2
|
|
dnd-core: 2.5.4
|
|
hoist-non-react-statics: 2.5.0
|
|
invariant: 2.2.3
|
|
lodash: 4.15.0
|
|
prop-types: 15.6.1
|
|
react: 15.6.2
|
|
dev: false
|
|
id: registry.npmjs.org/react-dnd/2.5.4
|
|
peerDependencies: *ref_11
|
|
resolution:
|
|
integrity: sha512-y9YmnusURc+3KPgvhYKvZ9oCucj51MSZWODyaeV0KFU0cquzA7dCD1g/OIYUKtNoZ+MXtacDngkdud2TklMSjw==
|
|
`,
|
|
}
|
|
await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
})
|
|
|
|
const lockfileContent = fs.readFileSync(path.join(projectPath, WANTED_LOCKFILE), 'utf8')
|
|
expect(lockfileContent).not.toMatch('&')
|
|
expect(lockfileContent).not.toMatch('*')
|
|
})
|
|
|
|
test('writeLockfiles() does not fail if the lockfile has undefined properties', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
'is-negative': '1.0.0',
|
|
'is-positive': '1.0.0',
|
|
},
|
|
specifiers: {
|
|
'is-negative': '^1.0.0',
|
|
'is-positive': '^1.0.0',
|
|
},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {
|
|
'/is-negative@1.0.0': {
|
|
// eslint-disable-next-line
|
|
dependencies: undefined as any,
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
'/is-positive@1.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
'/is-positive@2.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
},
|
|
}
|
|
await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
})
|
|
})
|
|
|
|
test('writeLockfiles() when useGitBranchLockfile', async () => {
|
|
const branchName: string = 'branch'
|
|
jest.mocked(getCurrentBranch).mockReturnValue(Promise.resolve(branchName))
|
|
const projectPath = temporaryDirectory()
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
foo: '1.0.0',
|
|
},
|
|
specifiers: {
|
|
foo: '^1.0.0',
|
|
},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {
|
|
'/foo@1.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
},
|
|
}
|
|
|
|
await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
useGitBranchLockfile: true,
|
|
})
|
|
expect(fs.existsSync(path.join(projectPath, WANTED_LOCKFILE))).toBeFalsy()
|
|
expect(fs.existsSync(path.join(projectPath, `pnpm-lock.${branchName}.yaml`))).toBeTruthy()
|
|
})
|
|
|
|
test('writeLockfiles() preserves env document prefix in pnpm-lock.yaml', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const envDoc = '---\nlockfileVersion: env-1.0\nimporters:\n .:\n configDependencies:\n typescript: 5.0.0\n\n---\n'
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
'is-positive': '1.0.0',
|
|
},
|
|
specifiers: {
|
|
'is-positive': '^1.0.0',
|
|
},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {
|
|
'is-positive@1.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
},
|
|
}
|
|
|
|
// Write lockfile with an env document prefix already present
|
|
fs.writeFileSync(path.join(projectPath, WANTED_LOCKFILE), envDoc + 'lockfileVersion: "9.0"\n')
|
|
|
|
await writeLockfiles({
|
|
currentLockfile: wantedLockfile,
|
|
currentLockfileDir: projectPath,
|
|
wantedLockfile,
|
|
wantedLockfileDir: projectPath,
|
|
})
|
|
|
|
const written = fs.readFileSync(path.join(projectPath, WANTED_LOCKFILE), 'utf8')
|
|
// The env document should be preserved at the top
|
|
expect(written.startsWith('---\n')).toBe(true)
|
|
expect(written).toContain('configDependencies')
|
|
expect(written).toContain('typescript: 5.0.0')
|
|
|
|
// The main lockfile should still be readable
|
|
const lockfile = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
|
|
expect(lockfile).toBeTruthy()
|
|
expect(lockfile!.importers['.' as ProjectId].dependencies).toEqual({ 'is-positive': '1.0.0' })
|
|
})
|
|
|
|
test('writeWantedLockfile() preserves env document prefix', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const envDoc = '---\nlockfileVersion: env-1.0\nimporters:\n .:\n configDependencies:\n typescript: 5.0.0\n\n---\n'
|
|
const wantedLockfile = {
|
|
importers: {
|
|
'.': {
|
|
dependencies: {
|
|
'is-positive': '1.0.0',
|
|
},
|
|
specifiers: {
|
|
'is-positive': '^1.0.0',
|
|
},
|
|
},
|
|
},
|
|
lockfileVersion: LOCKFILE_VERSION,
|
|
packages: {
|
|
'is-positive@1.0.0': {
|
|
resolution: {
|
|
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
|
|
},
|
|
},
|
|
},
|
|
}
|
|
|
|
// Pre-seed with env document
|
|
fs.writeFileSync(path.join(projectPath, WANTED_LOCKFILE), envDoc + 'lockfileVersion: "9.0"\n')
|
|
|
|
await writeWantedLockfile(projectPath, wantedLockfile)
|
|
|
|
const written = fs.readFileSync(path.join(projectPath, WANTED_LOCKFILE), 'utf8')
|
|
expect(written.startsWith('---\n')).toBe(true)
|
|
expect(written).toContain('typescript: 5.0.0')
|
|
|
|
// Main lockfile should be readable
|
|
const lockfile = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
|
|
expect(lockfile!.importers['.' as ProjectId].dependencies).toEqual({ 'is-positive': '1.0.0' })
|
|
})
|
|
|
|
test('readWantedLockfile() skips env document in combined lockfile', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
const envDoc = '---\nlockfileVersion: env-1.0\nimporters:\n .:\n configDependencies:\n typescript: 5.0.0\n\n---\n'
|
|
const mainDoc = `lockfileVersion: '${LOCKFILE_VERSION}'
|
|
importers:
|
|
.:
|
|
dependencies:
|
|
is-positive:
|
|
version: 1.0.0
|
|
specifier: ^1.0.0
|
|
packages:
|
|
is-positive@1.0.0:
|
|
resolution:
|
|
integrity: sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=
|
|
`
|
|
fs.writeFileSync(path.join(projectPath, WANTED_LOCKFILE), envDoc + mainDoc)
|
|
|
|
const lockfile = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
|
|
expect(lockfile).toBeTruthy()
|
|
expect(lockfile!.lockfileVersion).toBe(LOCKFILE_VERSION)
|
|
expect(lockfile!.importers['.' as ProjectId].dependencies).toEqual({ 'is-positive': '1.0.0' })
|
|
})
|
|
|
|
test('readWantedLockfile() returns null for env-only lockfile with no main document', async () => {
|
|
const projectPath = temporaryDirectory()
|
|
fs.writeFileSync(path.join(projectPath, WANTED_LOCKFILE), '---\nlockfileVersion: env-1.0\n')
|
|
|
|
const lockfile = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
|
|
expect(lockfile).toBeNull()
|
|
})
|