Skip to content

Commit

Permalink
fix: print more info when lockfile is outdated (#6536)
Browse files Browse the repository at this point in the history
ref #6526
  • Loading branch information
zkochan committed May 14, 2023
1 parent b3b5320 commit d58cdb9
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 36 deletions.
5 changes: 5 additions & 0 deletions .changeset/ninety-eels-begin.md
@@ -0,0 +1,5 @@
---
"@pnpm/lockfile-utils": major
---

Return details about the reason why the lockfile doesn't satisfy the manifest.
6 changes: 6 additions & 0 deletions .changeset/rotten-rivers-ring.md
@@ -0,0 +1,6 @@
---
"@pnpm/headless": patch
"pnpm": patch
---

When installation fails because the lockfile is not up-to-date with the `package.json` file(s), print out what are the differences [#6536](https://github.com/pnpm/pnpm/pull/6536).
47 changes: 34 additions & 13 deletions lockfile/lockfile-utils/src/satisfiesPackageManifest.ts
Expand Up @@ -14,8 +14,8 @@ export function satisfiesPackageManifest (
},
importer: ProjectSnapshot | undefined,
pkg: ProjectManifest
) {
if (!importer) return false
): { satisfies: boolean, detailedReason?: string } {
if (!importer) return { satisfies: false, detailedReason: 'no importer' }
let existingDeps: Record<string, string> = { ...pkg.devDependencies, ...pkg.dependencies, ...pkg.optionalDependencies }
if (opts?.autoInstallPeers) {
pkg = {
Expand All @@ -36,13 +36,24 @@ export function satisfiesPackageManifest (
existingDeps = pickNonLinkedDeps(existingDeps)
specs = pickNonLinkedDeps(specs)
}
if (
!equals(existingDeps, specs) ||
importer.publishDirectory !== pkg.publishConfig?.directory
) {
return false
if (!equals(existingDeps, specs)) {
return {
satisfies: false,
detailedReason: `specifiers in the lockfile (${JSON.stringify(specs)}) don't match specs in package.json (${JSON.stringify(existingDeps)})`,
}
}
if (importer.publishDirectory !== pkg.publishConfig?.directory) {
return {
satisfies: false,
detailedReason: `"publishDirectory" in the lockfile (${importer.publishDirectory ?? 'undefined'}) doesn't match "publishConfig.directory" in package.json (${pkg.publishConfig?.directory ?? 'undefined'})`,
}
}
if (!equals(pkg.dependenciesMeta ?? {}, importer.dependenciesMeta ?? {})) {
return {
satisfies: false,
detailedReason: `importer dependencies meta (${JSON.stringify(importer.dependenciesMeta)}) doesn't match package manifest dependencies meta (${JSON.stringify(pkg.dependenciesMeta)})`,
}
}
if (!equals(pkg.dependenciesMeta ?? {}, importer.dependenciesMeta ?? {})) return false
for (const depField of DEPENDENCIES_FIELDS) {
const importerDeps = importer[depField] ?? {}
let pkgDeps: Record<string, string> = pkg[depField] ?? {}
Expand All @@ -69,15 +80,25 @@ export function satisfiesPackageManifest (
default:
throw new Error(`Unknown dependency type "${depField as string}"`)
}
if (pkgDepNames.length !== Object.keys(importerDeps).length &&
pkgDepNames.length !== countOfNonLinkedDeps(importerDeps)) {
return false
if (
pkgDepNames.length !== Object.keys(importerDeps).length &&
pkgDepNames.length !== countOfNonLinkedDeps(importerDeps)
) {
return {
satisfies: false,
detailedReason: `"${depField}" in the lockfile (${JSON.stringify(importerDeps)}) doesn't match the same field in package.json (${JSON.stringify(pkgDeps)})`,
}
}
for (const depName of pkgDepNames) {
if (!importerDeps[depName] || importer.specifiers?.[depName] !== pkgDeps[depName]) return false
if (!importerDeps[depName] || importer.specifiers?.[depName] !== pkgDeps[depName]) {
return {
satisfies: false,
detailedReason: `importer ${depField}.${depName} specifier ${importer.specifiers[depName]} don't match package manifest specifier (${pkgDeps[depName]})`,
}
}
}
}
return true
return { satisfies: true }
}

function countOfNonLinkedDeps (lockfileDeps: { [depName: string]: string }): number {
Expand Down
61 changes: 41 additions & 20 deletions lockfile/lockfile-utils/test/satisfiesPackageManifest.ts
Expand Up @@ -16,7 +16,7 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.0.0' },
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })
expect(satisfiesPackageManifest(
{},
{
Expand All @@ -28,7 +28,7 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.0.0' },
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })
expect(satisfiesPackageManifest(
{},
{
Expand All @@ -39,7 +39,7 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
devDependencies: { foo: '^1.0.0' },
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })
expect(satisfiesPackageManifest(
{},
{
Expand All @@ -50,7 +50,7 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
optionalDependencies: { foo: '^1.0.0' },
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })
expect(satisfiesPackageManifest(
{},
{
Expand All @@ -61,7 +61,10 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
optionalDependencies: { foo: '^1.0.0' },
}
)).toBe(false)
)).toStrictEqual({
satisfies: false,
detailedReason: '"optionalDependencies" in the lockfile ({}) doesn\'t match the same field in package.json ({"foo":"^1.0.0"})',
})
expect(satisfiesPackageManifest(
{},
{
Expand All @@ -72,7 +75,10 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.1.0' },
}
)).toBe(false)
)).toStrictEqual({
satisfies: false,
detailedReason: 'specifiers in the lockfile ({"foo":"^1.0.0"}) don\'t match specs in package.json ({"foo":"^1.1.0"})',
})
expect(satisfiesPackageManifest(
{},
{
Expand All @@ -83,7 +89,10 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.0.0', bar: '2.0.0' },
}
)).toBe(false)
)).toStrictEqual({
satisfies: false,
detailedReason: 'specifiers in the lockfile ({"foo":"^1.0.0"}) don\'t match specs in package.json ({"foo":"^1.0.0","bar":"2.0.0"})',
})

expect(satisfiesPackageManifest(
{},
Expand All @@ -95,7 +104,10 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.0.0', bar: '2.0.0' },
}
)).toBe(false)
)).toStrictEqual({
satisfies: false,
detailedReason: '"dependencies" in the lockfile ({"foo":"1.0.0"}) doesn\'t match the same field in package.json ({"foo":"^1.0.0","bar":"2.0.0"})',
})

{
const importer = {
Expand All @@ -120,7 +132,7 @@ test('satisfiesPackageManifest()', () => {
bar: '2.0.0',
},
}
expect(satisfiesPackageManifest({}, importer, pkg)).toBe(true)
expect(satisfiesPackageManifest({}, importer, pkg)).toStrictEqual({ satisfies: true })
}

{
Expand All @@ -140,7 +152,10 @@ test('satisfiesPackageManifest()', () => {
bar: '2.0.0',
},
}
expect(satisfiesPackageManifest({}, importer, pkg)).toBe(false)
expect(satisfiesPackageManifest({}, importer, pkg)).toStrictEqual({
satisfies: false,
detailedReason: 'specifiers in the lockfile ({"bar":"2.0.0","qar":"^1.0.0"}) don\'t match specs in package.json ({"bar":"2.0.0"})',
})
}

{
Expand All @@ -159,7 +174,10 @@ test('satisfiesPackageManifest()', () => {
bar: '2.0.0',
},
}
expect(satisfiesPackageManifest({}, importer, pkg)).toBe(false)
expect(satisfiesPackageManifest({}, importer, pkg)).toStrictEqual({
satisfies: false,
detailedReason: '"dependencies" in the lockfile ({"bar":"2.0.0","qar":"1.0.0"}) doesn\'t match the same field in package.json ({"bar":"2.0.0"})',
})
}

expect(satisfiesPackageManifest(
Expand All @@ -172,7 +190,7 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.0.0' },
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })

expect(satisfiesPackageManifest(
{},
Expand All @@ -181,7 +199,7 @@ test('satisfiesPackageManifest()', () => {
...DEFAULT_PKG_FIELDS,
dependencies: { foo: '^1.0.0' },
}
)).toBe(false)
)).toStrictEqual({ satisfies: false, detailedReason: 'no importer' })

expect(satisfiesPackageManifest(
{},
Expand All @@ -202,7 +220,7 @@ test('satisfiesPackageManifest()', () => {
foo: '1.0.0',
},
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })

expect(satisfiesPackageManifest(
{},
Expand All @@ -224,7 +242,7 @@ test('satisfiesPackageManifest()', () => {
},
dependenciesMeta: {},
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })

expect(satisfiesPackageManifest(
{ autoInstallPeers: true },
Expand All @@ -247,7 +265,7 @@ test('satisfiesPackageManifest()', () => {
bar: '^1.0.0',
},
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })

expect(satisfiesPackageManifest(
{ autoInstallPeers: true },
Expand Down Expand Up @@ -284,7 +302,7 @@ test('satisfiesPackageManifest()', () => {
qar: '^1.0.0',
},
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })

expect(satisfiesPackageManifest(
{},
Expand All @@ -306,7 +324,7 @@ test('satisfiesPackageManifest()', () => {
directory: 'dist',
},
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })

expect(satisfiesPackageManifest(
{},
Expand All @@ -328,7 +346,10 @@ test('satisfiesPackageManifest()', () => {
directory: 'lib',
},
}
)).toBe(false)
)).toStrictEqual({
satisfies: false,
detailedReason: '"publishDirectory" in the lockfile (dist) doesn\'t match "publishConfig.directory" in package.json (lib)',
})

expect(satisfiesPackageManifest(
{
Expand All @@ -349,5 +370,5 @@ test('satisfiesPackageManifest()', () => {
bar: 'link:../bar',
},
}
)).toBe(true)
)).toStrictEqual({ satisfies: true })
})
2 changes: 1 addition & 1 deletion pkg-manager/core/src/install/allProjectsAreUpToDate.ts
Expand Up @@ -39,7 +39,7 @@ export async function allProjectsAreUpToDate (
return pEvery(projects, (project) => {
const importer = opts.wantedLockfile.importers[project.id]
return !hasLocalTarballDepsInRoot(importer) &&
_satisfiesPackageManifest(importer, project.manifest) &&
_satisfiesPackageManifest(importer, project.manifest).satisfies &&
_linkedPackagesAreUpToDate({
dir: project.rootDir,
manifest: project.manifest,
Expand Down
8 changes: 6 additions & 2 deletions pkg-manager/headless/src/index.ts
Expand Up @@ -200,11 +200,15 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
excludeLinksFromLockfile: opts.excludeLinksFromLockfile,
})
for (const { id, manifest, rootDir } of selectedProjects) {
if (!_satisfiesPackageManifest(wantedLockfile.importers[id], manifest)) {
const { satisfies, detailedReason } = _satisfiesPackageManifest(wantedLockfile.importers[id], manifest)
if (!satisfies) {
throw new PnpmError('OUTDATED_LOCKFILE',
`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is not up to date with ` +
path.relative(lockfileDir, path.join(rootDir, 'package.json')), {
hint: 'Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"',
hint: `Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
${detailedReason ?? ''}`,
})
}
}
Expand Down

0 comments on commit d58cdb9

Please sign in to comment.