Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: print more info when lockfile is outdated #6536

Merged
merged 4 commits into from May 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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