fix: never skip resolution if the overrides were changed

Never skip resolution if the overrides were changed,
even if frozenLockfile is true.

PR #3079

.changeset/blue-snakes-rhyme.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/filter-lockfile": patch
+---
+
+Include all the properties of the filtered lockfile.

.changeset/polite-dragons-press.md

@@ -0,0 +1,5 @@
+---
+"supi": patch
+---
+
+Resolution should never be skipped if the overrides were updated.

.changeset/proud-pigs-judge.md

@@ -0,0 +1,5 @@
+---
+"supi": patch
+---
+
+Installation should fail if the overrides in the lockfile don't match the ones in the package.json and the frozenLockfile option is on.

.changeset/small-rockets-love.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/lockfile-file": patch
+---
+
+An empty overrides field should be removed from the lockfile before saving.

packages/filter-lockfile/src/filterLockfile.ts

@@ -22,11 +22,11 @@ export default function filterLockfile (
     pairs = pairs.filter(([depPath, pkg]) => !pkg.optional)
   }
   return {
+    ...lockfile,
     importers: Object.keys(lockfile.importers).reduce((acc, importerId) => {
       acc[importerId] = filterImporter(lockfile.importers[importerId], opts.include)
       return acc
     }, {}),
-    lockfileVersion: lockfile.lockfileVersion,
     packages: R.fromPairs(pairs),
   }
 }

packages/filter-lockfile/src/filterLockfileByImporters.ts

@@ -41,8 +41,8 @@ export default function filterByImporters (
   }, { ...lockfile.importers })
 
   return {
+    ...lockfile,
     importers,
-    lockfileVersion: lockfile.lockfileVersion,
     packages,
   }
 }

packages/filter-lockfile/src/filterLockfileByImportersAndEngine.ts

@@ -67,8 +67,8 @@ export default function filterByImportersAndEngine (
   }, { ...lockfile.importers })
 
   return {
+    ...lockfile,
     importers,
-    lockfileVersion: lockfile.lockfileVersion,
     packages,
   }
 }

packages/lockfile-file/src/write.ts

@@ -73,8 +73,9 @@ function isEmptyLockfile (lockfile: Lockfile) {
 type LockfileFile = Omit<Lockfile, 'importers'> & Partial<ProjectSnapshot> & Partial<Pick<Lockfile, 'importers'>>
 
 function normalizeLockfile (lockfile: Lockfile, forceSharedFormat: boolean) {
+  let lockfileToSave!: LockfileFile
   if (!forceSharedFormat && R.equals(R.keys(lockfile.importers), ['.'])) {
-    const lockfileToSave: LockfileFile = {
+    lockfileToSave = {
       ...lockfile,
       ...lockfile.importers['.'],
     }
@@ -87,9 +88,8 @@ function normalizeLockfile (lockfile: Lockfile, forceSharedFormat: boolean) {
     if (R.isEmpty(lockfileToSave.packages) || !lockfileToSave.packages) {
       delete lockfileToSave.packages
     }
-    return lockfileToSave
   } else {
-    const lockfileToSave = {
+    lockfileToSave = {
       ...lockfile,
       importers: R.keys(lockfile.importers).reduce((acc, alias) => {
         const importer = lockfile.importers[alias]
@@ -108,8 +108,11 @@ function normalizeLockfile (lockfile: Lockfile, forceSharedFormat: boolean) {
     if (R.isEmpty(lockfileToSave.packages) || !lockfileToSave.packages) {
       delete lockfileToSave.packages
     }
-    return lockfileToSave
   }
+  if (lockfileToSave.overrides && R.isEmpty(lockfileToSave.overrides)) {
+    delete lockfileToSave.overrides
+  }
+  return lockfileToSave
 }
 
 export default async function writeLockfiles (

packages/supi/src/install/allProjectsAreUpToDate.ts

@@ -19,7 +19,6 @@ import semver = require('semver')
 export default function allProjectsAreUpToDate (
   projects: Array<ProjectOptions & { id: string }>,
   opts: {
-    overrides?: Record<string, string>
     linkWorkspacePackages: boolean
     wantedLockfile: Lockfile
     workspacePackages: WorkspacePackages
@@ -32,7 +31,7 @@ export default function allProjectsAreUpToDate (
     manifestsByDir,
     workspacePackages: opts.workspacePackages,
   })
-  return R.equals(opts.wantedLockfile.overrides ?? {}, opts.overrides ?? {}) && pEvery(projects, (project) => {
+  return pEvery(projects, (project) => {
     const importer = opts.wantedLockfile.importers[project.id]
     return !hasLocalTarballDepsInRoot(importer) &&
       _satisfiesPackageManifest(project.manifest, project.id) &&

packages/supi/src/install/index.ts

@@ -175,6 +175,8 @@ export async function mutateModules (
   return result
 
   async function _install (): Promise<Array<{ rootDir: string, manifest: ProjectManifest }>> {
+    const needsFullResolution = !R.equals(ctx.wantedLockfile.overrides ?? {}, overrides ?? {})
+    ctx.wantedLockfile.overrides = overrides
     const frozenLockfile = opts.frozenLockfile ||
       opts.frozenLockfileIfExists && ctx.existsWantedLockfile
     if (
@@ -184,18 +186,21 @@ export async function mutateModules (
       installsOnly &&
       (
         frozenLockfile ||
+        !needsFullResolution &&
         opts.preferFrozenLockfile &&
         (!opts.pruneLockfileImporters || Object.keys(ctx.wantedLockfile.importers).length === ctx.projects.length) &&
         ctx.existsWantedLockfile &&
         ctx.wantedLockfile.lockfileVersion === LOCKFILE_VERSION &&
         await allProjectsAreUpToDate(ctx.projects, {
           linkWorkspacePackages: opts.linkWorkspacePackagesDepth >= 0,
-          overrides,
           wantedLockfile: ctx.wantedLockfile,
           workspacePackages: opts.workspacePackages,
         })
       )
     ) {
+      if (needsFullResolution) {
+        throw new PnpmError('FROZEN_LOCKFILE_WITH_OUTDATED_LOCKFILE', 'Cannot perform a frozen installation because the lockfile needs updates')
+      }
       if (!ctx.existsWantedLockfile) {
         if (ctx.projects.some((project) => pkgHasDependencies(project.manifest))) {
           throw new Error(`Headless installation requires a ${WANTED_LOCKFILE} file`)
@@ -428,6 +433,7 @@ export async function mutateModules (
       ...opts,
       currentLockfileIsUpToDate: !ctx.existsWantedLockfile || ctx.currentLockfileIsUpToDate,
       makePartialCurrentLockfile,
+      needsFullResolution,
       overrides,
       update: opts.update || !installsOnly,
       updateLockfileMinorVersion: true,
@@ -573,6 +579,7 @@ async function installInContext (
   ctx: PnpmContext<DependenciesMutation>,
   opts: StrictInstallOptions & {
     makePartialCurrentLockfile: boolean
+    needsFullResolution: boolean
     overrides?: Record<string, string>
     updateLockfileMinorVersion: boolean
     preferredVersions?: PreferredVersions
@@ -600,15 +607,6 @@ async function installInContext (
       }
     }
   }
-  const overridesChanged = !R.equals(opts.overrides ?? {}, ctx.wantedLockfile.overrides ?? {})
-  if (!R.isEmpty(opts.overrides ?? {})) {
-    ctx.wantedLockfile.overrides = opts.overrides
-  } else {
-    delete ctx.wantedLockfile.overrides
-    // We were only setting the resolutions field in pnpm v5.10.0, which was never latest,
-    // so we can probably remove this line safely in future pnpm versions.
-    delete ctx.wantedLockfile['resolutions']
-  }
 
   await Promise.all(
     projects
@@ -637,7 +635,7 @@ async function installInContext (
   const forceFullResolution = ctx.wantedLockfile.lockfileVersion !== LOCKFILE_VERSION ||
     !opts.currentLockfileIsUpToDate ||
     opts.force ||
-    overridesChanged ||
+    opts.needsFullResolution ||
     ctx.lockfileHadConflicts
   const _toResolveImporter = toResolveImporter.bind(null, {
     defaultUpdateDepth: (opts.update || opts.updateMatching) ? opts.depth : -1,

packages/supi/test/install/overrides.ts

@@ -1,3 +1,4 @@
+import PnpmError from '@pnpm/error'
 import { prepareEmpty } from '@pnpm/prepare'
 import { addDistTag } from '@pnpm/registry-mock'
 import { addDependenciesToPackage, mutateModules } from 'supi'
@@ -32,6 +33,8 @@ test('versions are replaced with versions specified through pnpm.overrides field
       'bar@^100.0.0': '100.1.0',
       'dep-of-pkg-with-1-dep': '101.0.0',
     })
+    const currentLockfile = await project.readCurrentLockfile()
+    expect(lockfile.overrides).toStrictEqual(currentLockfile.overrides)
   }
 
   // The lockfile is updated if the overrides are changed
@@ -54,7 +57,45 @@ test('versions are replaced with versions specified through pnpm.overrides field
       'bar@^100.0.0': '100.0.0',
       'dep-of-pkg-with-1-dep': '101.0.0',
     })
+    const currentLockfile = await project.readCurrentLockfile()
+    expect(lockfile.overrides).toStrictEqual(currentLockfile.overrides)
   }
+
+  await mutateModules([
+    {
+      buildIndex: 0,
+      manifest,
+      mutation: 'install',
+      rootDir: process.cwd(),
+    },
+  ], await testDefaults({ frozenLockfile: true }))
+
+  {
+    const lockfile = await project.readLockfile()
+    expect(lockfile.overrides).toStrictEqual({
+      'foobarqar>foo': 'npm:qar@100.0.0',
+      'bar@^100.0.0': '100.0.0',
+      'dep-of-pkg-with-1-dep': '101.0.0',
+    })
+    const currentLockfile = await project.readCurrentLockfile()
+    expect(lockfile.overrides).toStrictEqual(currentLockfile.overrides)
+  }
+
+  manifest.pnpm!.overrides!['bar@^100.0.0'] = '100.0.1'
+  await expect(
+    mutateModules([
+      {
+        buildIndex: 0,
+        manifest,
+        mutation: 'install',
+        rootDir: process.cwd(),
+      },
+    ], await testDefaults({ frozenLockfile: true }))
+  ).rejects.toThrow(
+    new PnpmError('FROZEN_LOCKFILE_WITH_OUTDATED_LOCKFILE',
+      'Cannot perform a frozen installation because the lockfile needs updates'
+    )
+  )
 })
 
 test('versions are replaced with versions specified through "resolutions" field (for Yarn compatibility)', async () => {