diff --git a/.changeset/clean-swans-listen.md b/.changeset/clean-swans-listen.md new file mode 100644 index 000000000..445b409a3 --- /dev/null +++ b/.changeset/clean-swans-listen.md @@ -0,0 +1,5 @@ +--- +"@pnpm/plugin-commands-patching": minor +--- + +apply existing patch file when re-patch [#5632](https://github.com/pnpm/pnpm/issues/5632) diff --git a/.changeset/eleven-fireants-warn.md b/.changeset/eleven-fireants-warn.md new file mode 100644 index 000000000..24be308ad --- /dev/null +++ b/.changeset/eleven-fireants-warn.md @@ -0,0 +1,5 @@ +--- +"@pnpm/parse-wanted-dependency": minor +--- + +Export `ParseWantedDependencyResult`. diff --git a/.changeset/flat-eagles-tease.md b/.changeset/flat-eagles-tease.md new file mode 100644 index 000000000..874b5c23f --- /dev/null +++ b/.changeset/flat-eagles-tease.md @@ -0,0 +1,5 @@ +--- +"pnpm": minor +--- + +When patching a dependency that is already patched, the existing patch is applied to the dependency, so that the new edit are applied on top of the existing ones. To ignore the existing patches, run the patch command with the `--ignore-existing` option [#5632](https://github.com/pnpm/pnpm/issues/5632). diff --git a/.changeset/fuzzy-birds-joke.md b/.changeset/fuzzy-birds-joke.md new file mode 100644 index 000000000..c3d85bd42 --- /dev/null +++ b/.changeset/fuzzy-birds-joke.md @@ -0,0 +1,5 @@ +--- +"@pnpm/patching.apply-patch": major +--- + +Initial release. diff --git a/exec/build-modules/package.json b/exec/build-modules/package.json index 33ad6fe9d..130feb90b 100644 --- a/exec/build-modules/package.json +++ b/exec/build-modules/package.json @@ -36,16 +36,15 @@ "dependencies": { "@pnpm/calc-dep-state": "workspace:*", "@pnpm/core-loggers": "workspace:*", - "@pnpm/error": "workspace:*", "@pnpm/fs.hard-link-dir": "workspace:*", "@pnpm/graph-sequencer": "1.0.0", "@pnpm/lifecycle": "workspace:*", "@pnpm/link-bins": "workspace:*", + "@pnpm/patching.apply-patch": "workspace:*", "@pnpm/read-package-json": "workspace:*", "@pnpm/store-controller-types": "workspace:*", "@pnpm/types": "workspace:*", "p-defer": "^3.0.0", - "patch-package": "^6.5.1", "ramda": "npm:@pnpm/ramda@0.28.1", "run-groups": "^3.0.1" }, diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts index 8c7a7d641..90954e2b5 100644 --- a/exec/build-modules/src/index.ts +++ b/exec/build-modules/src/index.ts @@ -1,16 +1,15 @@ import path from 'path' import { calcDepState, DepsStateCache } from '@pnpm/calc-dep-state' import { skippedOptionalDependencyLogger } from '@pnpm/core-loggers' -import { PnpmError } from '@pnpm/error' import { runPostinstallHooks } from '@pnpm/lifecycle' import { linkBins, linkBinsOfPackages } from '@pnpm/link-bins' import { logger } from '@pnpm/logger' import { hardLinkDir } from '@pnpm/fs.hard-link-dir' import { readPackageJsonFromDir, safeReadPackageJsonFromDir } from '@pnpm/read-package-json' import { StoreController } from '@pnpm/store-controller-types' +import { applyPatchToDir } from '@pnpm/patching.apply-patch' import { DependencyManifest } from '@pnpm/types' import pDefer, { DeferredPromise } from 'p-defer' -import { applyPatch } from 'patch-package/dist/applyPatches' import pickBy from 'ramda/src/pickBy' import runGroups from 'run-groups' import { buildSequence, DependenciesGraph, DependenciesGraphNode } from './buildSequence' @@ -106,7 +105,7 @@ async function buildDependency ( await linkBinsOfDependencies(depNode, depGraph, opts) const isPatched = depNode.patchFile?.path != null if (isPatched) { - applyPatchToDep(depNode.dir, depNode.patchFile!.path) + applyPatchToDir({ patchedDir: depNode.dir, patchFilePath: depNode.patchFile!.path }) } const hasSideEffects = !opts.ignoreScripts && await runPostinstallHooks({ depPath, @@ -179,21 +178,6 @@ async function buildDependency ( } } -function applyPatchToDep (patchDir: string, patchFilePath: string) { - // 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() - process.chdir(patchDir) - const success = applyPatch({ - patchFilePath, - patchDir, - }) - process.chdir(cwd) - if (!success) { - throw new PnpmError('PATCH_FAILED', `Could not apply patch ${patchFilePath} to ${patchDir}`) - } -} - export async function linkBinsOfDependencies ( depNode: DependenciesGraphNode, depGraph: DependenciesGraph, diff --git a/exec/build-modules/tsconfig.json b/exec/build-modules/tsconfig.json index a6cbebe40..1295f003d 100644 --- a/exec/build-modules/tsconfig.json +++ b/exec/build-modules/tsconfig.json @@ -19,10 +19,10 @@ "path": "../../packages/core-loggers" }, { - "path": "../../packages/error" + "path": "../../packages/types" }, { - "path": "../../packages/types" + "path": "../../patching/apply-patch" }, { "path": "../../pkg-manager/link-bins" diff --git a/packages/parse-wanted-dependency/src/index.ts b/packages/parse-wanted-dependency/src/index.ts index ae812c7a4..bc3343824 100644 --- a/packages/parse-wanted-dependency/src/index.ts +++ b/packages/parse-wanted-dependency/src/index.ts @@ -1,13 +1,18 @@ import validateNpmPackageName from 'validate-npm-package-name' -interface ParsedWantedDependency { +export interface ParsedWantedDependency { alias: string pref: string } -export function parseWantedDependency ( - rawWantedDependency: string -): Partial & (Omit | Omit | ParsedWantedDependency) { +export type ParseWantedDependencyResult = Partial & +( + Omit + | Omit + | ParsedWantedDependency +) + +export function parseWantedDependency (rawWantedDependency: string): ParseWantedDependencyResult { const versionDelimiter = rawWantedDependency.indexOf('@', 1) // starting from 1 to skip the @ that marks scope if (versionDelimiter !== -1) { const alias = rawWantedDependency.slice(0, versionDelimiter) diff --git a/packages/plugin-commands-patching/package.json b/packages/plugin-commands-patching/package.json index 94d4c8fbb..df178029c 100644 --- a/packages/plugin-commands-patching/package.json +++ b/packages/plugin-commands-patching/package.json @@ -42,6 +42,7 @@ "@pnpm/cli-utils": "workspace:*", "@pnpm/config": "workspace:*", "@pnpm/error": "workspace:*", + "@pnpm/patching.apply-patch": "workspace:*", "@pnpm/parse-wanted-dependency": "workspace:*", "@pnpm/pick-registry-for-package": "workspace:*", "@pnpm/plugin-commands-installation": "workspace:*", diff --git a/packages/plugin-commands-patching/src/patch.ts b/packages/plugin-commands-patching/src/patch.ts index f500cccd9..c5f4c7d41 100644 --- a/packages/plugin-commands-patching/src/patch.ts +++ b/packages/plugin-commands-patching/src/patch.ts @@ -1,4 +1,6 @@ import fs from 'fs' +import path from 'path' +import { applyPatchToDir } from '@pnpm/patching.apply-patch' import { docsUrl } from '@pnpm/cli-utils' import { Config, types as allTypes } from '@pnpm/config' import { LogBase } from '@pnpm/logger' @@ -9,14 +11,14 @@ import pick from 'ramda/src/pick' import renderHelp from 'render-help' import tempy from 'tempy' import { PnpmError } from '@pnpm/error' -import { writePackage } from './writePackage' +import { writePackage, ParseWantedDependencyResult } from './writePackage' export function rcOptionsTypes () { return pick([], allTypes) } export function cliOptionsTypes () { - return { ...rcOptionsTypes(), 'edit-dir': String } + return { ...rcOptionsTypes(), 'edit-dir': String, 'ignore-existing': Boolean } } export const shorthands = { @@ -35,6 +37,10 @@ export function help () { description: 'The package that needs to be modified will be extracted to this directory', name: '--edit-dir', }, + { + description: 'Ignore existing patch files when patching', + name: '--ignore-existing', + }, ], }], url: docsUrl('patch'), @@ -42,9 +48,10 @@ export function help () { }) } -export type PatchCommandOptions = Pick & CreateStoreControllerOptions & { +export type PatchCommandOptions = Pick & CreateStoreControllerOptions & { editDir?: string reporter?: (logObj: LogBase) => void + ignoreExisting?: boolean } export async function handler (opts: PatchCommandOptions, params: string[]) { @@ -52,8 +59,43 @@ export async function handler (opts: PatchCommandOptions, params: string[]) { throw new PnpmError('PATCH_EDIT_DIR_EXISTS', `The target directory already exists: '${opts.editDir}'`) } const editDir = opts.editDir ?? tempy.directory() - await writePackage(params[0], editDir, opts) + const patchedDep = await writePackage(params[0], editDir, opts) + if (!opts.ignoreExisting && opts.rootProjectManifest?.pnpm?.patchedDependencies) { + tryPatchWithExistingPatchFile({ + patchedDep, + patchedDir: editDir, + patchedDependencies: opts.rootProjectManifest.pnpm.patchedDependencies, + lockfileDir: opts.lockfileDir ?? opts.dir ?? process.cwd(), + }) + } return `You can now edit the following folder: ${editDir} Once you're done with your changes, run "pnpm patch-commit ${editDir}"` } + +function tryPatchWithExistingPatchFile ( + { + patchedDep, + patchedDir, + patchedDependencies, + lockfileDir, + }: { + patchedDep: ParseWantedDependencyResult + patchedDir: string + patchedDependencies: Record + lockfileDir: string + } +) { + if (!patchedDep.alias || !patchedDep.pref) { + return + } + const existingPatchFile = patchedDependencies[`${patchedDep.alias}@${patchedDep.pref}`] + if (!existingPatchFile) { + return + } + const existingPatchFilePath = path.resolve(lockfileDir, existingPatchFile) + if (!fs.existsSync(existingPatchFilePath)) { + throw new PnpmError('PATCH_FILE_NOT_FOUND', `Unable to find patch file ${existingPatchFilePath}`) + } + applyPatchToDir({ patchedDir, patchFilePath: existingPatchFilePath }) +} diff --git a/packages/plugin-commands-patching/src/writePackage.ts b/packages/plugin-commands-patching/src/writePackage.ts index cc6fc5bad..b6bb3eba1 100644 --- a/packages/plugin-commands-patching/src/writePackage.ts +++ b/packages/plugin-commands-patching/src/writePackage.ts @@ -4,11 +4,13 @@ import { CreateStoreControllerOptions, } from '@pnpm/store-connection-manager' import { pickRegistryForPackage } from '@pnpm/pick-registry-for-package' -import { parseWantedDependency } from '@pnpm/parse-wanted-dependency' +import { parseWantedDependency, ParseWantedDependencyResult } from '@pnpm/parse-wanted-dependency' + +export { ParseWantedDependencyResult } export type WritePackageOptions = CreateStoreControllerOptions & Pick -export async function writePackage (pkg: string, dest: string, opts: WritePackageOptions) { +export async function writePackage (pkg: string, dest: string, opts: WritePackageOptions): Promise { const dep = parseWantedDependency(pkg) const store = await createOrConnectStoreController({ ...opts, @@ -26,4 +28,5 @@ export async function writePackage (pkg: string, dest: string, opts: WritePackag filesResponse, force: true, }) + return dep } diff --git a/packages/plugin-commands-patching/test/patch.test.ts b/packages/plugin-commands-patching/test/patch.test.ts index 95d5229b5..cab11e74a 100644 --- a/packages/plugin-commands-patching/test/patch.test.ts +++ b/packages/plugin-commands-patching/test/patch.test.ts @@ -112,6 +112,78 @@ describe('patch and commit', () => { expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching') }) + + test('should reuse existing patch file by default', async () => { + let output = await patch.handler(defaultPatchOption, ['is-positive@1.0.0']) + let patchDir = getPatchDirFromPatchOutput(output) + + fs.appendFileSync(path.join(patchDir, 'index.js'), '// test patching', 'utf8') + fs.unlinkSync(path.join(patchDir, 'license')) + + await patchCommit.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + }, [patchDir]) + + const { manifest } = await readProjectManifest(process.cwd()) + expect(manifest.pnpm?.patchedDependencies).toStrictEqual({ + 'is-positive@1.0.0': 'patches/is-positive@1.0.0.patch', + }) + expect(fs.existsSync('patches/is-positive@1.0.0.patch')).toBe(true) + + // re-patch + output = await patch.handler({ ...defaultPatchOption, rootProjectManifest: manifest }, ['is-positive@1.0.0']) + patchDir = getPatchDirFromPatchOutput(output) + + expect(fs.existsSync(patchDir)).toBe(true) + expect(fs.existsSync(path.join(patchDir, 'license'))).toBe(false) + expect(fs.readFileSync(path.join(patchDir, 'index.js'), 'utf8')).toContain('// test patching') + }) + + test('if the patch file is not existed when patching, should throw an error', async () => { + const { writeProjectManifest, manifest } = await readProjectManifest(process.cwd()) + await writeProjectManifest({ + ...manifest, + pnpm: { + patchedDependencies: { + 'is-positive@1.0.0': 'patches/not-found.patch', + }, + }, + }) + + try { + await patch.handler(defaultPatchOption, ['is-positive@1.0.0']) + } catch (err: any) { // eslint-disable-line + expect(err.code).toBe('ERR_PNPM_PATCH_FILE_NOT_FOUND') + } + }) + + test('should ignore patch files with --ignore-patches', async () => { + let output = await patch.handler(defaultPatchOption, ['is-positive@1.0.0']) + let patchDir = getPatchDirFromPatchOutput(output) + + fs.appendFileSync(path.join(patchDir, 'index.js'), '// test patching', 'utf8') + fs.unlinkSync(path.join(patchDir, 'license')) + + await patchCommit.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + }, [patchDir]) + + const { manifest } = await readProjectManifest(process.cwd()) + expect(manifest.pnpm?.patchedDependencies).toStrictEqual({ + 'is-positive@1.0.0': 'patches/is-positive@1.0.0.patch', + }) + expect(fs.existsSync('patches/is-positive@1.0.0.patch')).toBe(true) + + // re-patch with --ignore-patches + output = await patch.handler({ ...defaultPatchOption, ignoreExisting: true }, ['is-positive@1.0.0']) + patchDir = getPatchDirFromPatchOutput(output) + + expect(fs.existsSync(patchDir)).toBe(true) + expect(fs.existsSync(path.join(patchDir, 'license'))).toBe(true) + expect(fs.readFileSync(path.join(patchDir, 'index.js'), 'utf8')).not.toContain('// test patching') + }) }) describe('patching should work when there is a no EOL in the patched file', () => { diff --git a/packages/plugin-commands-patching/tsconfig.json b/packages/plugin-commands-patching/tsconfig.json index 07ed6bf6c..a856e4835 100644 --- a/packages/plugin-commands-patching/tsconfig.json +++ b/packages/plugin-commands-patching/tsconfig.json @@ -21,6 +21,9 @@ { "path": "../../config/pick-registry-for-package" }, + { + "path": "../../patching/apply-patch" + }, { "path": "../../pkg-manager/plugin-commands-installation" }, diff --git a/patching/apply-patch/README.md b/patching/apply-patch/README.md new file mode 100644 index 000000000..4f1a8a9d5 --- /dev/null +++ b/patching/apply-patch/README.md @@ -0,0 +1,17 @@ +# @pnpm/patching.apply-patch + +> Apply a patch to a directory + + +[![npm version](https://img.shields.io/npm/v/@pnpm/patching.apply-patch.svg)](https://www.npmjs.com/package/@pnpm/patching.apply-patch) + + +## Installation + +```sh +pnpm add @pnpm/patching.apply-patch +``` + +## License + +MIT diff --git a/patching/apply-patch/package.json b/patching/apply-patch/package.json new file mode 100644 index 000000000..e8c181bfe --- /dev/null +++ b/patching/apply-patch/package.json @@ -0,0 +1,42 @@ +{ + "name": "@pnpm/patching.apply-patch", + "version": "0.0.0", + "description": "Apply a patch to a directory", + "main": "lib/index.js", + "types": "lib/index.d.ts", + "files": [ + "lib", + "!*.map" + ], + "engines": { + "node": ">=14.6" + }, + "scripts": { + "test": "pnpm run compile", + "prepublishOnly": "pnpm run compile", + "compile": "tsc --build && pnpm run lint --fix", + "lint": "eslint src/**/*.ts" + }, + "repository": "https://github.com/pnpm/pnpm/blob/main/patching/apply-patch", + "keywords": [ + "pnpm7", + "pnpm", + "patch" + ], + "license": "MIT", + "bugs": { + "url": "https://github.com/pnpm/pnpm/issues" + }, + "homepage": "https://github.com/pnpm/pnpm/blob/main/patching/apply-patch#readme", + "dependencies": { + "@pnpm/error": "workspace:*", + "patch-package": "^6.5.1" + }, + "devDependencies": { + "@pnpm/patching.apply-patch": "workspace:*" + }, + "funding": "https://opencollective.com/pnpm", + "exports": { + ".": "./lib/index.js" + } +} diff --git a/patching/apply-patch/src/index.ts b/patching/apply-patch/src/index.ts new file mode 100644 index 000000000..8e3d140df --- /dev/null +++ b/patching/apply-patch/src/index.ts @@ -0,0 +1,22 @@ +import { PnpmError } from '@pnpm/error' +import { applyPatch } from 'patch-package/dist/applyPatches' + +export interface ApplyPatchToDirOpts { + patchedDir: string + patchFilePath: string +} + +export function applyPatchToDir (opts: ApplyPatchToDirOpts) { + // 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() + process.chdir(opts.patchedDir) + const success = applyPatch({ + patchDir: opts.patchedDir, + patchFilePath: opts.patchFilePath, + }) + process.chdir(cwd) + if (!success) { + throw new PnpmError('PATCH_FAILED', `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}`) + } +} diff --git a/patching/apply-patch/tsconfig.json b/patching/apply-patch/tsconfig.json new file mode 100644 index 000000000..019cba19e --- /dev/null +++ b/patching/apply-patch/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "@pnpm/tsconfig", + "compilerOptions": { + "outDir": "lib", + "rootDir": "src" + }, + "include": [ + "src/**/*.ts", + "../../__typings__/**/*.d.ts" + ], + "references": [ + { + "path": "../../packages/error" + } + ] +} diff --git a/patching/apply-patch/tsconfig.lint.json b/patching/apply-patch/tsconfig.lint.json new file mode 100644 index 000000000..1bbe71197 --- /dev/null +++ b/patching/apply-patch/tsconfig.lint.json @@ -0,0 +1,8 @@ +{ + "extends": "./tsconfig.json", + "include": [ + "src/**/*.ts", + "test/**/*.ts", + "../../__typings__/**/*.d.ts" + ] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ebf1e05d9..1649a354d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -897,9 +897,6 @@ importers: '@pnpm/core-loggers': specifier: workspace:* version: link:../../packages/core-loggers - '@pnpm/error': - specifier: workspace:* - version: link:../../packages/error '@pnpm/fs.hard-link-dir': specifier: workspace:* version: link:../../fs/hard-link-dir @@ -915,6 +912,9 @@ importers: '@pnpm/logger': specifier: ^5.0.0 version: 5.0.0 + '@pnpm/patching.apply-patch': + specifier: workspace:* + version: link:../../patching/apply-patch '@pnpm/read-package-json': specifier: workspace:* version: link:../../pkg-manifest/read-package-json @@ -927,9 +927,6 @@ importers: p-defer: specifier: ^3.0.0 version: 3.0.0 - patch-package: - specifier: ^6.5.1 - version: 6.5.1 ramda: specifier: npm:@pnpm/ramda@0.28.1 version: /@pnpm/ramda@0.28.1 @@ -2394,6 +2391,9 @@ importers: '@pnpm/parse-wanted-dependency': specifier: workspace:* version: link:../parse-wanted-dependency + '@pnpm/patching.apply-patch': + specifier: workspace:* + version: link:../../patching/apply-patch '@pnpm/pick-registry-for-package': specifier: workspace:* version: link:../../config/pick-registry-for-package @@ -2504,6 +2504,19 @@ importers: specifier: workspace:* version: 'link:' + patching/apply-patch: + dependencies: + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error + patch-package: + specifier: ^6.5.1 + version: 6.5.1 + devDependencies: + '@pnpm/patching.apply-patch': + specifier: workspace:* + version: 'link:' + pkg-manager/client: dependencies: '@pnpm/default-resolver': diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 4c39c34b7..be9d0d1c6 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -14,6 +14,7 @@ packages: - packages/* - pkg-manager/* - pkg-manifest/* + - patching/* - pnpm - pnpm/artifacts/* - releasing/*