feat(patching): apply patch to all versions (#8337)

Related issue: #5686
pnpm · Aug 1, 2024 · cb006df · cb006df
1 parent e7f6330
commit cb006df
Show file tree

Hide file tree

Showing 55 changed files with 1,279 additions and 116 deletions.
diff --git a/.changeset/dull-goats-fly.md b/.changeset/dull-goats-fly.md
@@ -0,0 +1,31 @@
+---
+"@pnpm/build-modules": major
+"@pnpm/core": minor
+"@pnpm/deps.graph-builder": major
+"@pnpm/headless": minor
+"@pnpm/lockfile.types": patch
+"@pnpm/patching.apply-patch": minor
+"@pnpm/patching.config": major
+"@pnpm/patching.types": major
+"@pnpm/plugin-commands-patching": minor
+"@pnpm/resolve-dependencies": major
+"@pnpm/types": major
+"pnpm": minor
+---
+
+Add ability to apply patch to all versions:
+If the key of `pnpm.patchedDependencies` is a package name without a version (e.g. `pkg`), pnpm will attempt to apply the patch to all versions of
+the package, failure will be skipped.
+If it is a package name and an exact version (e.g. `pkg@x.y.z`), pnpm will attempt to apply the patch to that exact version only, failure will
+cause pnpm to fail.
+
+If there's only one version of `pkg` installed, `pnpm patch pkg` and subsequent `pnpm patch-commit $edit_dir` will create an entry named `pkg` in
+`pnpm.patchedDependencies`. And pnpm will attempt to apply this patch to other versions of `pkg` in the future.
+
+If there's multiple versions of `pkg` installed, `pnpm patch pkg` will ask which version to edit and whether to attempt to apply the patch to all.
+If the user chooses to apply the patch to all, `pnpm patch-commit $edit_dir` would create a `pkg` entry in `pnpm.patchedDependencies`.
+If the user chooses not to apply the patch to all, `pnpm patch-commit $edit_dir` would create a `pkg@x.y.z` entry in `pnpm.patchedDependencies` with
+`x.y.z` being the version the user chose to edit.
+
+If the user runs `pnpm patch pkg@x.y.z` with `x.y.z` being the exact version of `pkg` that has been installed, `pnpm patch-commit $edit_dir` will always
+create a `pkg@x.y.z` entry in `pnpm.patchedDependencies`.
diff --git a/deps/graph-builder/package.json b/deps/graph-builder/package.json
@@ -46,6 +46,8 @@
     "@pnpm/lockfile.utils": "workspace:*",
     "@pnpm/modules-yaml": "workspace:*",
     "@pnpm/package-is-installable": "workspace:*",
+    "@pnpm/patching.config": "workspace:*",
+    "@pnpm/patching.types": "workspace:*",
     "@pnpm/store-controller-types": "workspace:*",
     "@pnpm/types": "workspace:*",
     "path-exists": "catalog:",

diff --git a/deps/graph-builder/src/lockfileToDepGraph.ts b/deps/graph-builder/src/lockfileToDepGraph.ts
@@ -15,7 +15,9 @@ import {
 import { logger } from '@pnpm/logger'
 import { type IncludedDependencies } from '@pnpm/modules-yaml'
 import { packageIsInstallable } from '@pnpm/package-is-installable'
-import { type DepPath, type SupportedArchitectures, type PatchFile, type Registries, type PkgIdWithPatchHash, type ProjectId } from '@pnpm/types'
+import { getPatchInfo } from '@pnpm/patching.config'
+import { type PatchFile, type PatchInfo } from '@pnpm/patching.types'
+import { type DepPath, type SupportedArchitectures, type Registries, type PkgIdWithPatchHash, type ProjectId } from '@pnpm/types'
 import {
   type PkgRequestFetchResult,
   type FetchResponse,
@@ -44,7 +46,7 @@ export interface DependenciesGraphNode {
   requiresBuild?: boolean
   hasBin: boolean
   filesIndexFile?: string
-  patchFile?: PatchFile
+  patch?: PatchInfo
 }
 
 export interface DependenciesGraph {
@@ -100,6 +102,7 @@ export async function lockfileToDepGraph (
   const directDependenciesByImporterId: DirectDependenciesByImporterId = {}
   if (lockfile.packages != null) {
     const pkgSnapshotByLocation: Record<string, PackageSnapshot> = {}
+    const _getPatchInfo = getPatchInfo.bind(null, opts.patchedDependencies)
     await Promise.all(
       (Object.entries(lockfile.packages) as Array<[DepPath, PackageSnapshot]>).map(async ([depPath, pkgSnapshot]) => {
         if (opts.skipped.has(depPath)) return
@@ -196,7 +199,7 @@ export async function lockfileToDepGraph (
           name: pkgName,
           optional: !!pkgSnapshot.optional,
           optionalDependencies: new Set(Object.keys(pkgSnapshot.optionalDependencies ?? {})),
-          patchFile: opts.patchedDependencies?.[`${pkgName}@${pkgVersion}`],
+          patch: _getPatchInfo(pkgName, pkgVersion),
         }
         pkgSnapshotByLocation[dir] = pkgSnapshot
       })

diff --git a/deps/graph-builder/tsconfig.json b/deps/graph-builder/tsconfig.json
@@ -30,6 +30,12 @@
     {
       "path": "../../packages/types"
     },
+    {
+      "path": "../../patching/config"
+    },
+    {
+      "path": "../../patching/types"
+    },
     {
       "path": "../../pkg-manager/modules-yaml"
     },

diff --git a/exec/build-modules/package.json b/exec/build-modules/package.json
@@ -41,6 +41,7 @@
     "@pnpm/lifecycle": "workspace:*",
     "@pnpm/link-bins": "workspace:*",
     "@pnpm/patching.apply-patch": "workspace:*",
+    "@pnpm/patching.types": "workspace:*",
     "@pnpm/read-package-json": "workspace:*",
     "@pnpm/store-controller-types": "workspace:*",
     "@pnpm/types": "workspace:*",

diff --git a/exec/build-modules/src/buildSequence.ts b/exec/build-modules/src/buildSequence.ts
@@ -1,5 +1,6 @@
 import { graphSequencer } from '@pnpm/deps.graph-sequencer'
-import { type PkgIdWithPatchHash, type DepPath, type PackageManifest, type PatchFile } from '@pnpm/types'
+import { type PatchInfo } from '@pnpm/patching.types'
+import { type PkgIdWithPatchHash, type DepPath, type PackageManifest } from '@pnpm/types'
 import filter from 'ramda/src/filter'
 
 export interface DependenciesGraphNode<T extends string> {
@@ -18,7 +19,7 @@ export interface DependenciesGraphNode<T extends string> {
   optionalDependencies: Set<string>
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   requiresBuild?: boolean | any // this is a dirty workaround added in https://github.com/pnpm/pnpm/pull/4898
-  patchFile?: PatchFile
+  patch?: PatchInfo
 }
 
 export type DependenciesGraph<T extends string> = Record<T, DependenciesGraphNode<T>>
@@ -41,7 +42,7 @@ export function buildSequence<T extends string> (
 }
 
 function getSubgraphToBuild<T extends string> (
-  graph: Record<string, Pick<DependenciesGraphNode<T>, 'children' | 'requiresBuild' | 'patchFile'>>,
+  graph: Record<string, Pick<DependenciesGraphNode<T>, 'children' | 'requiresBuild' | 'patch'>>,
   entryNodes: T[],
   nodesToBuild: Set<T>,
   walked: Set<T>
@@ -54,7 +55,7 @@ function getSubgraphToBuild<T extends string> (
     walked.add(depPath)
     const childShouldBeBuilt = getSubgraphToBuild(graph, Object.values(node.children), nodesToBuild, walked) ||
       node.requiresBuild ||
-      node.patchFile != null
+      node.patch != null
     if (childShouldBeBuilt) {
       nodesToBuild.add(depPath)
       currentShouldBeBuilt = true

diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts
@@ -67,7 +67,7 @@ export async function buildModules<T extends string> (
   const groups = chunks.map((chunk) => {
     chunk = chunk.filter((depPath) => {
       const node = depGraph[depPath]
-      return (node.requiresBuild || node.patchFile != null) && !node.isBuilt
+      return (node.requiresBuild || node.patch != null) && !node.isBuilt
     })
     if (opts.depsToBuild != null) {
       chunk = chunk.filter((depPath) => opts.depsToBuild!.has(depPath))
@@ -127,9 +127,10 @@ async function buildDependency<T extends string> (
   }
   try {
     await linkBinsOfDependencies(depNode, depGraph, opts)
-    const isPatched = depNode.patchFile?.path != null
-    if (isPatched) {
-      applyPatchToDir({ patchedDir: depNode.dir, patchFilePath: depNode.patchFile!.path })
+    let isPatched = false
+    if (depNode.patch) {
+      const { file, strict } = depNode.patch
+      isPatched = applyPatchToDir({ allowFailure: !strict, patchedDir: depNode.dir, patchFilePath: file.path })
     }
     const hasSideEffects = !opts.ignoreScripts && await runPostinstallHooks({
       depPath,
@@ -148,7 +149,7 @@ async function buildDependency<T extends string> (
     if ((isPatched || hasSideEffects) && opts.sideEffectsCacheWrite) {
       try {
         const sideEffectsCacheKey = calcDepState(depGraph, opts.depsStateCache, depPath, {
-          patchFileHash: depNode.patchFile?.hash,
+          patchFileHash: depNode.patch?.file.hash,
           isBuilt: hasSideEffects,
         })
         await opts.storeController.upload(depNode.dir, {

diff --git a/exec/build-modules/tsconfig.json b/exec/build-modules/tsconfig.json
@@ -27,6 +27,9 @@
     {
       "path": "../../patching/apply-patch"
     },
+    {
+      "path": "../../patching/types"
+    },
     {
       "path": "../../pkg-manager/link-bins"
     },

diff --git a/lockfile/types/package.json b/lockfile/types/package.json
@@ -30,6 +30,7 @@
   },
   "funding": "https://opencollective.com/pnpm",
   "dependencies": {
+    "@pnpm/patching.types": "workspace:*",
     "@pnpm/types": "workspace:*"
   },
   "devDependencies": {

diff --git a/lockfile/types/src/index.ts b/lockfile/types/src/index.ts
@@ -1,4 +1,5 @@
-import { type DependenciesMeta, type DepPath, type PatchFile, type ProjectId } from '@pnpm/types'
+import { type PatchFile } from '@pnpm/patching.types'
+import { type DependenciesMeta, type DepPath, type ProjectId } from '@pnpm/types'
 
 export type { PatchFile, ProjectId }
 

diff --git a/lockfile/types/tsconfig.json b/lockfile/types/tsconfig.json
@@ -11,6 +11,9 @@
   "references": [
     {
       "path": "../../packages/types"
+    },
+    {
+      "path": "../../patching/types"
     }
   ]
 }
diff --git a/packages/types/src/misc.ts b/packages/types/src/misc.ts
@@ -27,11 +27,6 @@ export interface SslConfig {
 
 export type HoistedDependencies = Record<DepPath | ProjectId, Record<string, 'public' | 'private'>>
 
-export interface PatchFile {
-  path: string
-  hash: string
-}
-
 export type PkgResolutionId = string & { __brand: 'PkgResolutionId' }
 
 export type PkgId = string & { __brand: 'PkgId' }

diff --git a/patching/apply-patch/__fixtures__/applicable.patch b/patching/apply-patch/__fixtures__/applicable.patch
@@ -0,0 +1,9 @@
+diff --git a/patch-target.txt b/patch-target.txt
+index 27a8dc9..31cbe49 100644
+--- a/patch-target.txt
++++ b/patch-target.txt
+@@ -1,3 +1,3 @@
+ first line of patch-target
+ second line of patch-target
+-third line of patch-target
++final line of patch-target
diff --git a/patching/apply-patch/__fixtures__/non-applicable.patch b/patching/apply-patch/__fixtures__/non-applicable.patch
@@ -0,0 +1,9 @@
+diff --git a/patch-target.txt b/patch-target.txt
+index 27a8dc9..31cbe49 100644
+--- a/patch-target.txt
++++ b/patch-target.txt
+@@ -1,3 +1,3 @@
+ first line of patch-target
+ second line of patch-target
+-FINAL line of patch-target
++final line of patch-target
diff --git a/patching/apply-patch/__fixtures__/patch-target.txt b/patching/apply-patch/__fixtures__/patch-target.txt
@@ -0,0 +1,3 @@
+first line of patch-target
+second line of patch-target
+third line of patch-target
diff --git a/patching/apply-patch/__fixtures__/successfully-patched.txt b/patching/apply-patch/__fixtures__/successfully-patched.txt
@@ -0,0 +1,3 @@
+first line of patch-target
+second line of patch-target
+final line of patch-target
diff --git a/patching/apply-patch/package.json b/patching/apply-patch/package.json
@@ -29,6 +29,9 @@
     "url": "https://github.com/pnpm/pnpm/issues"
   },
   "homepage": "https://github.com/pnpm/pnpm/blob/main/patching/apply-patch#readme",
+  "peerDependencies": {
+    "@pnpm/logger": "catalog:"
+  },
   "dependencies": {
     "@pnpm/error": "workspace:*",
     "@pnpm/patch-package": "catalog:"

diff --git a/patching/apply-patch/src/index.ts b/patching/apply-patch/src/index.ts
@@ -1,12 +1,14 @@
 import { PnpmError } from '@pnpm/error'
 import { applyPatch } from '@pnpm/patch-package/dist/applyPatches'
+import { globalWarn } from '@pnpm/logger'
 
 export interface ApplyPatchToDirOpts {
+  allowFailure?: boolean
   patchedDir: string
   patchFilePath: string
 }
 
-export function applyPatchToDir (opts: ApplyPatchToDirOpts): void {
+export function applyPatchToDir (opts: ApplyPatchToDirOpts): boolean {
   // Ideally, we would just run "patch" or "git apply".
   // However, "patch" is not available on Windows and "git apply" is hard to execute on a subdirectory of an existing repository
   const cwd = process.cwd()
@@ -25,6 +27,12 @@ export function applyPatchToDir (opts: ApplyPatchToDirOpts): void {
     process.chdir(cwd)
   }
   if (!success) {
-    throw new PnpmError('PATCH_FAILED', `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}`)
+    const message = `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}`
+    if (opts.allowFailure) {
+      globalWarn(message)
+    } else {
+      throw new PnpmError('PATCH_FAILED', message)
+    }
   }
+  return success
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -27,6 +27,9 @@ @@
         {
           "path": "../../patching/apply-patch"
         },
+        {
+          "path": "../../patching/types"
+        },
         {
           "path": "../../pkg-manager/link-bins"
         },
@@ Expand Down @@