fix: a modified patch should update the deps on install (#4918)

close #4916

.changeset/strange-peas-tease.md

@@ -0,0 +1,9 @@
+---
+"@pnpm/build-modules": patch
+"@pnpm/core": patch
+"@pnpm/headless": patch
+"@pnpm/lockfile-types": patch
+"@pnpm/resolve-dependencies": patch
+---
+
+Update the dependencies when a patch file is modified.

.changeset/two-seals-add.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/types": minor
+---
+
+Add PatchFile type.

packages/build-modules/src/buildSequence.ts

@@ -1,5 +1,5 @@
 import graphSequencer from '@pnpm/graph-sequencer'
-import { PackageManifest } from '@pnpm/types'
+import { PackageManifest, PatchFile } from '@pnpm/types'
 import filter from 'ramda/src/filter'
 
 export interface DependenciesGraphNode {
@@ -16,10 +16,7 @@ export interface DependenciesGraphNode {
   optionalDependencies: Set<string>
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   requiresBuild?: boolean | any // this is a durty workaround added in https://github.com/pnpm/pnpm/pull/4898
-  patchFile?: {
-    hash: string
-    path: string
-  }
+  patchFile?: PatchFile
 }
 
 export interface DependenciesGraph {

packages/core/package.json

@@ -19,6 +19,7 @@
     "@pnpm/calc-dep-state": "workspace:3.0.0",
     "@pnpm/constants": "workspace:6.1.0",
     "@pnpm/core-loggers": "workspace:7.0.4",
+    "@pnpm/crypto.base32-hash": "workspace:1.0.0",
     "@pnpm/error": "workspace:3.0.1",
     "@pnpm/filter-lockfile": "workspace:6.0.7",
     "@pnpm/get-context": "workspace:6.2.1",

packages/core/src/install/index.ts

@@ -10,6 +10,7 @@ import {
   stageLogger,
   summaryLogger,
 } from '@pnpm/core-loggers'
+import { createBase32HashFromFile } from '@pnpm/crypto.base32-hash'
 import PnpmError from '@pnpm/error'
 import getContext, { PnpmContext, ProjectOptions } from '@pnpm/get-context'
 import headless, { Project } from '@pnpm/headless'
@@ -26,6 +27,7 @@ import {
   writeLockfiles,
   writeWantedLockfile,
   cleanGitBranchLockfiles,
+  PatchFile,
 } from '@pnpm/lockfile-file'
 import { writePnpFile } from '@pnpm/lockfile-to-pnp'
 import { extendProjectsWithTargetDirs } from '@pnpm/lockfile-utils'
@@ -234,13 +236,22 @@ export async function mutateModules (
       )
     }
     const packageExtensionsChecksum = isEmpty(opts.packageExtensions ?? {}) ? undefined : createObjectChecksum(opts.packageExtensions!)
+    const patchedDependencies = opts.ignorePackageManifest
+      ? ctx.wantedLockfile.patchedDependencies
+      : (opts.patchedDependencies ? await calcPatchHashes(opts.patchedDependencies, opts.lockfileDir) : {})
+    const patchedDependenciesWithResolvedPath = patchedDependencies
+      ? fromPairs(Object.entries(patchedDependencies).map(([key, patchFile]) => [key, {
+        hash: patchFile.hash,
+        path: path.join(opts.lockfileDir, patchFile.path),
+      }]))
+      : undefined
     let needsFullResolution = !maybeOpts.ignorePackageManifest &&
       lockfileIsUpToDate(ctx.wantedLockfile, {
         overrides: opts.overrides,
         neverBuiltDependencies: opts.neverBuiltDependencies,
         onlyBuiltDependencies: opts.onlyBuiltDependencies,
         packageExtensionsChecksum,
-        patchedDependencies: opts.patchedDependencies,
+        patchedDependencies,
       }) ||
       opts.fixLockfile
     if (needsFullResolution) {
@@ -248,7 +259,7 @@ export async function mutateModules (
       ctx.wantedLockfile.neverBuiltDependencies = opts.neverBuiltDependencies
       ctx.wantedLockfile.onlyBuiltDependencies = opts.onlyBuiltDependencies
       ctx.wantedLockfile.packageExtensionsChecksum = packageExtensionsChecksum
-      ctx.wantedLockfile.patchedDependencies = opts.patchedDependencies
+      ctx.wantedLockfile.patchedDependencies = patchedDependencies
     }
     const frozenLockfile = opts.frozenLockfile ||
       opts.frozenLockfileIfExists && ctx.existsWantedLockfile
@@ -296,6 +307,7 @@ export async function mutateModules (
               nodeVersion: opts.nodeVersion,
               pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '',
             },
+            patchedDependencies: patchedDependenciesWithResolvedPath,
             projects: ctx.projects as Project[],
             prunedAt: ctx.modulesFile?.prunedAt,
             pruneVirtualStore,
@@ -479,12 +491,28 @@ export async function mutateModules (
       pruneVirtualStore,
       scriptsOpts,
       updateLockfileMinorVersion: true,
+      patchedDependencies: patchedDependenciesWithResolvedPath,
     })
 
     return result.projects
   }
 }
 
+async function calcPatchHashes (patches: Record<string, string>, lockfileDir: string) {
+  return fromPairs(await Promise.all(
+    Object.entries(patches).map(async ([key, patchFileRelativePath]) => {
+      const patchFilePath = path.join(lockfileDir, patchFileRelativePath)
+      return [
+        key,
+        {
+          hash: await createBase32HashFromFile(patchFilePath),
+          path: patchFileRelativePath,
+        },
+      ]
+    })
+  ))
+}
+
 function lockfileIsUpToDate (
   lockfile: Lockfile,
   {
@@ -498,7 +526,7 @@ function lockfileIsUpToDate (
     onlyBuiltDependencies?: string[]
     overrides?: Record<string, string>
     packageExtensionsChecksum?: string
-    patchedDependencies?: Record<string, string>
+    patchedDependencies?: Record<string, PatchFile>
   }) {
   return !equals(lockfile.overrides ?? {}, overrides ?? {}) ||
     !equals((lockfile.neverBuiltDependencies ?? []).sort(), (neverBuiltDependencies ?? []).sort()) ||
@@ -652,7 +680,8 @@ interface InstallFunctionResult {
 type InstallFunction = (
   projects: ImporterToUpdate[],
   ctx: PnpmContext<DependenciesMutation>,
-  opts: StrictInstallOptions & {
+  opts: Omit<StrictInstallOptions, 'patchedDependencies'> & {
+    patchedDependencies?: Record<string, PatchFile>
     makePartialCurrentLockfile: boolean
     needsFullResolution: boolean
     neverBuiltDependencies?: string[]

packages/core/test/install/patch.ts

@@ -34,7 +34,12 @@ test('patch package', async () => {
 
   const patchFileHash = 'jnbpamcxayl5i4ehrkoext3any'
   const lockfile = await project.readLockfile()
-  expect(lockfile.patchedDependencies).toStrictEqual(patchedDependencies)
+  expect(lockfile.patchedDependencies).toStrictEqual({
+    'is-positive@1.0.0': {
+      path: patchedDependencies['is-positive@1.0.0'],
+      hash: patchFileHash,
+    },
+  })
   expect(lockfile.packages[`/is-positive/1.0.0_${patchFileHash}`]).toBeTruthy()
 
   const filesIndexFile = path.join(opts.storeDir, 'files/c7/1ccf199e0fdae37aad13946b937d67bcd35fa111b84d21b3a19439cfdc2812c5d8da8a735e94c2a1ccb77b4583808ee8405313951e7146ac83ede3671dc292-index.json')
@@ -113,3 +118,31 @@ test('patch package throws an exception if not all patches are applied', async (
     }, opts)
   ).rejects.toThrow('The following patches were not applied: is-negative@1.0.0')
 })
+
+test('the patched package is updated if the patch is modified', async () => {
+  prepareEmpty()
+  f.copy('patch-pkg', 'patches')
+  const patchPath = path.resolve('patches', 'is-positive@1.0.0.patch')
+
+  const patchedDependencies = {
+    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
+  }
+  const opts = await testDefaults({
+    fastUnpack: false,
+    sideEffectsCacheRead: true,
+    sideEffectsCacheWrite: true,
+    patchedDependencies,
+  }, {}, {}, { packageImportMethod: 'hardlink' })
+  const manifest = {
+    dependencies: {
+      'is-positive': '1.0.0',
+    },
+  }
+  await install(manifest, opts)
+
+  const patchContent = fs.readFileSync(patchPath, 'utf8')
+  fs.writeFileSync(patchPath, patchContent.replace('// patched', '// edited patch'), 'utf8')
+
+  await install(manifest, opts)
+  expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// edited patch')
+})

packages/core/tsconfig.json

@@ -39,6 +39,9 @@
     {
       "path": "../core-loggers"
     },
+    {
+      "path": "../crypto.base32-hash"
+    },
     {
       "path": "../dependency-path"
     },

packages/headless/package.json

@@ -71,7 +71,6 @@
     "@pnpm/calc-dep-state": "workspace:3.0.0",
     "@pnpm/constants": "workspace:6.1.0",
     "@pnpm/core-loggers": "workspace:7.0.4",
-    "@pnpm/crypto.base32-hash": "workspace:1.0.0",
     "@pnpm/error": "workspace:3.0.1",
     "@pnpm/filter-lockfile": "workspace:6.0.7",
     "@pnpm/hoist": "workspace:6.1.5",

packages/headless/src/index.ts

@@ -30,6 +30,7 @@ import {
   readCurrentLockfile,
   readWantedLockfile,
   writeCurrentLockfile,
+  PatchFile,
 } from '@pnpm/lockfile-file'
 import { writePnpFile } from '@pnpm/lockfile-to-pnp'
 import {
@@ -111,6 +112,7 @@ export interface HeadlessOptions {
   lockfileDir: string
   modulesDir?: string
   virtualStoreDir?: string
+  patchedDependencies?: Record<string, PatchFile>
   scriptsPrependNodePath?: boolean | 'warn-only'
   scriptShell?: string
   shellEmulator?: boolean

packages/headless/src/lockfileToDepGraph.ts

@@ -3,7 +3,6 @@ import { WANTED_LOCKFILE } from '@pnpm/constants'
 import {
   progressLogger,
 } from '@pnpm/core-loggers'
-import { createBase32HashFromFile } from '@pnpm/crypto.base32-hash'
 import {
   Lockfile,
   PackageSnapshot,
@@ -16,7 +15,7 @@ import {
 import logger from '@pnpm/logger'
 import { IncludedDependencies } from '@pnpm/modules-yaml'
 import packageIsInstallable from '@pnpm/package-is-installable'
-import { Registries } from '@pnpm/types'
+import { PatchFile, Registries } from '@pnpm/types'
 import {
   FetchPackageToStoreFunction,
   PackageFilesResponse,
@@ -45,10 +44,7 @@ export interface DependenciesGraphNode {
   prepare: boolean
   hasBin: boolean
   filesIndexFile: string
-  patchFile?: {
-    path: string
-    hash: string
-  }
+  patchFile?: PatchFile
 }
 
 export interface DependenciesGraph {
@@ -63,6 +59,7 @@ export interface LockfileToDepGraphOptions {
   lockfileDir: string
   nodeVersion: string
   pnpmVersion: string
+  patchedDependencies?: Record<string, PatchFile>
   registries: Registries
   sideEffectsCacheRead: boolean
   skipped: Set<string>
@@ -180,7 +177,7 @@ export default async function lockfileToDepGraph (
           optionalDependencies: new Set(Object.keys(pkgSnapshot.optionalDependencies ?? {})),
           prepare: pkgSnapshot.prepare === true,
           requiresBuild: pkgSnapshot.requiresBuild === true,
-          patchFile: await tryReadPatchFile(opts.lockfileDir, lockfile, pkgName, pkgVersion),
+          patchFile: opts.patchedDependencies?.[`${pkgName}@${pkgVersion}`],
         }
         pkgSnapshotByLocation[dir] = pkgSnapshot
       })
@@ -220,18 +217,6 @@ export default async function lockfileToDepGraph (
   return { graph, directDependenciesByImporterId }
 }
 
-export async function tryReadPatchFile (lockfileDir: string, lockfile: Lockfile, pkgName: string, pkgVersion: string) {
-  const patchFileRelativePath = lockfile.patchedDependencies?.[`${pkgName}@${pkgVersion}`]
-  if (!patchFileRelativePath) {
-    return undefined
-  }
-  const patchFilePath = path.join(lockfileDir, patchFileRelativePath)
-  return {
-    path: patchFilePath,
-    hash: await createBase32HashFromFile(patchFilePath),
-  }
-}
-
 async function getChildrenPaths (
   ctx: {
     graph: DependenciesGraph

packages/headless/src/lockfileToHoistedDepGraph.ts

@@ -11,7 +11,7 @@ import {
 } from '@pnpm/lockfile-utils'
 import { IncludedDependencies } from '@pnpm/modules-yaml'
 import packageIsInstallable from '@pnpm/package-is-installable'
-import { Registries } from '@pnpm/types'
+import { PatchFile, Registries } from '@pnpm/types'
 import {
   FetchPackageToStoreFunction,
   StoreController,
@@ -23,7 +23,6 @@ import {
   DepHierarchy,
   DirectDependenciesByImporterId,
   LockfileToDepGraphResult,
-  tryReadPatchFile,
 } from './lockfileToDepGraph'
 
 export interface LockfileToHoistedDepGraphOptions {
@@ -36,6 +35,7 @@ export interface LockfileToHoistedDepGraphOptions {
   nodeVersion: string
   pnpmVersion: string
   registries: Registries
+  patchedDependencies?: Record<string, PatchFile>
   sideEffectsCacheRead: boolean
   skipped: Set<string>
   storeController: StoreController
@@ -209,7 +209,7 @@ async function fetchDeps (
       optionalDependencies: new Set(Object.keys(pkgSnapshot.optionalDependencies ?? {})),
       prepare: pkgSnapshot.prepare === true,
       requiresBuild: pkgSnapshot.requiresBuild === true,
-      patchFile: await tryReadPatchFile(opts.lockfileDir, opts.lockfile, pkgName, pkgVersion),
+      patchFile: opts.patchedDependencies?.[`${pkgName}@${pkgVersion}`],
     }
     opts.pkgLocationByDepPath[depPath] = dir
     depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies)

packages/headless/tsconfig.json

@@ -33,9 +33,6 @@
     {
       "path": "../core-loggers"
     },
-    {
-      "path": "../crypto.base32-hash"
-    },
     {
       "path": "../dependency-path"
     },

packages/lockfile-types/src/index.ts

@@ -1,4 +1,6 @@
-import { DependenciesMeta } from '@pnpm/types'
+import { DependenciesMeta, PatchFile } from '@pnpm/types'
+
+export { PatchFile }
 
 export interface Lockfile {
   importers: Record<string, ProjectSnapshot>
@@ -8,7 +10,7 @@ export interface Lockfile {
   onlyBuiltDependencies?: string[]
   overrides?: Record<string, string>
   packageExtensionsChecksum?: string
-  patchedDependencies?: Record<string, string>
+  patchedDependencies?: Record<string, PatchFile>
 }
 
 export interface ProjectSnapshot {

packages/resolve-dependencies/package.json

@@ -31,7 +31,6 @@
   "dependencies": {
     "@pnpm/constants": "workspace:6.1.0",
     "@pnpm/core-loggers": "workspace:7.0.4",
-    "@pnpm/crypto.base32-hash": "workspace:1.0.0",
     "@pnpm/error": "workspace:3.0.1",
     "@pnpm/lockfile-types": "workspace:4.1.0",
     "@pnpm/lockfile-utils": "workspace:4.0.6",

packages/resolve-dependencies/src/index.ts

@@ -104,7 +104,7 @@ export default async function (
 
   // We only check whether patches were applied in cases when the whole lockfile was reanalyzed.
   if (opts.patchedDependencies && (opts.forceFullResolution || !opts.wantedLockfile.packages?.length)) {
-    verifyPatches(opts.patchedDependencies, appliedPatches)
+    verifyPatches(Object.keys(opts.patchedDependencies), appliedPatches)
   }
 
   const linkedDependenciesByProjectId: Record<string, LinkedDependency[]> = {}
@@ -239,8 +239,8 @@ export default async function (
   }
 }
 
-function verifyPatches (patchedDependencies: Record<string, string>, appliedPatches: Set<string>) {
-  const nonAppliedPatches: string[] = Object.keys(patchedDependencies).filter((patchKey) => !appliedPatches.has(patchKey))
+function verifyPatches (patchedDependencies: string[], appliedPatches: Set<string>) {
+  const nonAppliedPatches: string[] = patchedDependencies.filter((patchKey) => !appliedPatches.has(patchKey))
   if (nonAppliedPatches.length) {
     throw new PnpmError('PATCH_NOT_APPLIED', `The following patches were not applied: ${nonAppliedPatches.join(', ')}`, {
       hint: 'Either remove them from "patchedDependencies" or update them to much packages in your dependencies.',

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -4,7 +4,6 @@ import {
   progressLogger,
   skippedOptionalDependencyLogger,
 } from '@pnpm/core-loggers'
-import { createBase32HashFromFile } from '@pnpm/crypto.base32-hash'
 import PnpmError from '@pnpm/error'
 import {
   Lockfile,
@@ -34,6 +33,7 @@ import {
   Dependencies,
   DependencyManifest,
   PackageManifest,
+  PatchFile,
   PeerDependenciesMeta,
   ReadPackageHook,
   Registries,
@@ -131,7 +131,7 @@ export interface ResolutionContext {
   resolvedPackagesByDepPath: ResolvedPackagesByDepPath
   outdatedDependencies: {[pkgId: string]: string}
   childrenByParentDepPath: ChildrenByParentDepPath
-  patchedDependencies?: Record<string, string>
+  patchedDependencies?: Record<string, PatchFile>
   pendingNodes: PendingNode[]
   wantedLockfile: Lockfile
   currentLockfile: Lockfile
@@ -181,11 +181,6 @@ export type PkgAddress = {
   isLinkedDependency: undefined
 })
 
-export interface PatchFile {
-  path: string
-  hash: string
-}
-
 export interface ResolvedPackage {
   id: string
   resolution: Resolution
@@ -925,9 +920,8 @@ async function resolveDependency (
     })
 
     const nameAndVersion = `${pkg.name}@${pkg.version}`
-    let patchPath = ctx.patchedDependencies?.[nameAndVersion]
-    if (patchPath) {
-      patchPath = path.join(ctx.lockfileDir, patchPath)
+    const patchFile = ctx.patchedDependencies?.[nameAndVersion]
+    if (patchFile) {
       ctx.appliedPatches.add(nameAndVersion)
     }
 
@@ -937,7 +931,7 @@ async function resolveDependency (
       depPath,
       force: ctx.force,
       hasBin,
-      patchPath,
+      patchFile,
       pkg,
       pkgResponse,
       prepare,
@@ -1029,7 +1023,7 @@ async function getResolvedPackage (
     depPath: string
     force: boolean
     hasBin: boolean
-    patchPath?: string
+    patchFile?: PatchFile
     pkg: PackageManifest
     pkgResponse: PackageResponse
     prepare: boolean
@@ -1064,9 +1058,7 @@ async function getResolvedPackage (
     name: options.pkg.name,
     optional: options.wantedDependency.optional,
     optionalDependencies: new Set(Object.keys(options.pkg.optionalDependencies ?? {})),
-    patchFile: options.patchPath
-      ? { path: options.patchPath, hash: await createBase32HashFromFile(options.patchPath) }
-      : undefined,
+    patchFile: options.patchFile,
     peerDependencies: peerDependencies ?? {},
     peerDependenciesMeta: options.pkg.peerDependenciesMeta,
     prepare: options.prepare,

packages/resolve-dependencies/src/resolveDependencyTree.ts

@@ -1,4 +1,4 @@
-import { Lockfile } from '@pnpm/lockfile-types'
+import { Lockfile, PatchFile } from '@pnpm/lockfile-types'
 import { PreferredVersions, Resolution, WorkspacePackages } from '@pnpm/resolver-base'
 import { StoreController } from '@pnpm/store-controller-types'
 import {
@@ -67,7 +67,7 @@ export interface ResolveDependenciesOptions {
   }
   nodeVersion: string
   registries: Registries
-  patchedDependencies?: Record<string, string>
+  patchedDependencies?: Record<string, PatchFile>
   pnpmVersion: string
   preferredVersions?: PreferredVersions
   preferWorkspacePackages?: boolean

packages/resolve-dependencies/tsconfig.json

@@ -15,9 +15,6 @@
     {
       "path": "../core-loggers"
     },
-    {
-      "path": "../crypto.base32-hash"
-    },
     {
       "path": "../dependency-path"
     },

packages/types/src/misc.ts

@@ -13,3 +13,8 @@ export interface Registries {
 }
 
 export type HoistedDependencies = Record<string, Record<string, 'public' | 'private'>>
+
+export interface PatchFile {
+  path: string
+  hash: string
+}

pnpm-lock.yaml

@@ -405,6 +405,7 @@ importers:
       '@pnpm/constants': workspace:6.1.0
       '@pnpm/core': workspace:5.5.0
       '@pnpm/core-loggers': workspace:7.0.4
+      '@pnpm/crypto.base32-hash': workspace:1.0.0
       '@pnpm/error': workspace:3.0.1
       '@pnpm/filter-lockfile': workspace:6.0.7
       '@pnpm/get-context': workspace:6.2.1
@@ -484,6 +485,7 @@ importers:
       '@pnpm/calc-dep-state': link:../calc-dep-state
       '@pnpm/constants': link:../constants
       '@pnpm/core-loggers': link:../core-loggers
+      '@pnpm/crypto.base32-hash': link:../crypto.base32-hash
       '@pnpm/error': link:../error
       '@pnpm/filter-lockfile': link:../filter-lockfile
       '@pnpm/get-context': link:../get-context
@@ -1124,7 +1126,6 @@ importers:
       '@pnpm/client': workspace:7.1.6
       '@pnpm/constants': workspace:6.1.0
       '@pnpm/core-loggers': workspace:7.0.4
-      '@pnpm/crypto.base32-hash': workspace:1.0.0
       '@pnpm/error': workspace:3.0.1
       '@pnpm/filter-lockfile': workspace:6.0.7
       '@pnpm/headless': workspace:18.3.0
@@ -1173,7 +1174,6 @@ importers:
       '@pnpm/calc-dep-state': link:../calc-dep-state
       '@pnpm/constants': link:../constants
       '@pnpm/core-loggers': link:../core-loggers
-      '@pnpm/crypto.base32-hash': link:../crypto.base32-hash
       '@pnpm/error': link:../error
       '@pnpm/filter-lockfile': link:../filter-lockfile
       '@pnpm/hoist': link:../hoist
@@ -3291,7 +3291,6 @@ importers:
     specifiers:
       '@pnpm/constants': workspace:6.1.0
       '@pnpm/core-loggers': workspace:7.0.4
-      '@pnpm/crypto.base32-hash': workspace:1.0.0
       '@pnpm/error': workspace:3.0.1
       '@pnpm/lockfile-types': workspace:4.1.0
       '@pnpm/lockfile-utils': workspace:4.0.6
@@ -3327,7 +3326,6 @@ importers:
     dependencies:
       '@pnpm/constants': link:../constants
       '@pnpm/core-loggers': link:../core-loggers
-      '@pnpm/crypto.base32-hash': link:../crypto.base32-hash
       '@pnpm/error': link:../error
       '@pnpm/lockfile-types': link:../lockfile-types
       '@pnpm/lockfile-utils': link:../lockfile-utils