From 7b1c189f2eb0a555aed06cd2d7cb552eb73f7aa4 Mon Sep 17 00:00:00 2001 From: Johan Quan Vo <31811643+Qanlevon@users.noreply.github.com> Date: Tue, 27 Jan 2026 23:07:43 +0700 Subject: [PATCH] feat!: remove deprecated patch options (#10505) * refactor: remove allowNonAppliedPatches * refactor: remove ignorePatchFailures * refactor: remove `strict` field in groupPatchedDependencies * test: update test failure in package patching * test: fix * docs: update changesets --------- Co-authored-by: Zoltan Kochan --- .changeset/remove-deprecated-patch-options.md | 18 + .../config/src/getOptionsFromRootManifest.ts | 16 +- .../test/getOptionsFromRootManifest.test.ts | 56 -- exec/build-modules/src/index.ts | 9 +- packages/types/src/package.ts | 2 - patching/apply-patch/src/index.ts | 9 +- patching/apply-patch/test/applyPatchToDir.ts | 59 +-- .../config/src/groupPatchedDependencies.ts | 9 +- patching/config/test/getPatchInfo.test.ts | 13 - .../test/groupPatchedDependencies.test.ts | 23 +- patching/config/test/index.test.ts | 7 +- .../plugin-commands-patching/src/patch.ts | 5 +- .../test/patch.test.ts | 8 +- patching/types/src/index.ts | 1 - .../core/src/install/extendInstallOptions.ts | 2 - pkg-manager/core/src/install/index.ts | 1 - pkg-manager/core/test/install/patch.ts | 58 +- pkg-manager/headless/src/index.ts | 2 - .../test/patchedDependencies.ts | 6 +- pnpm/test/patch/ignorePatchFailures.ts | 500 ------------------ .../test/fixtures/is-positive.patch | 4 +- 21 files changed, 52 insertions(+), 756 deletions(-) create mode 100644 .changeset/remove-deprecated-patch-options.md delete mode 100644 pnpm/test/patch/ignorePatchFailures.ts diff --git a/.changeset/remove-deprecated-patch-options.md b/.changeset/remove-deprecated-patch-options.md new file mode 100644 index 0000000000..86c89fb67f --- /dev/null +++ b/.changeset/remove-deprecated-patch-options.md @@ -0,0 +1,18 @@ +--- +"@pnpm/plugin-commands-installation": major +"@pnpm/plugin-commands-deploy": major +"@pnpm/plugin-commands-patching": major +"@pnpm/package-bins": major +"@pnpm/patching.apply-patch": major +"@pnpm/patching.config": major +"@pnpm/patching.types": major +"@pnpm/headless": major +"@pnpm/build-modules": major +"@pnpm/core": major +"@pnpm/types": major +"@pnpm/config": major +"pnpm": major +--- + +Removed the deprecated `allowNonAppliedPatches` completely in favor of `allowUnusedPatches`. +Remove `ignorePatchFailures` so all patch application failures should throw an error. diff --git a/config/config/src/getOptionsFromRootManifest.ts b/config/config/src/getOptionsFromRootManifest.ts index e21f2e1995..8da77bb206 100644 --- a/config/config/src/getOptionsFromRootManifest.ts +++ b/config/config/src/getOptionsFromRootManifest.ts @@ -9,13 +9,11 @@ import { type ProjectManifest, type PnpmSettings, } from '@pnpm/types' -import { map as mapValues, omit, pick } from 'ramda' -import { globalWarn } from '@pnpm/logger' +import { map as mapValues, pick } from 'ramda' export type OptionsFromRootManifest = { allowedDeprecatedVersions?: AllowedDeprecatedVersions allowUnusedPatches?: boolean - ignorePatchFailures?: boolean overrides?: Record packageExtensions?: Record ignoredOptionalDependencies?: string[] @@ -29,13 +27,11 @@ export type OptionsFromRootManifest = { export function getOptionsFromRootManifest (manifestDir: string, manifest: ProjectManifest): OptionsFromRootManifest { const settings: OptionsFromRootManifest = getOptionsFromPnpmSettings(manifestDir, { ...pick([ - 'allowNonAppliedPatches', 'allowBuilds', 'allowUnusedPatches', 'allowedDeprecatedVersions', 'auditConfig', 'configDependencies', - 'ignorePatchFailures', 'ignoredOptionalDependencies', 'overrides', 'packageExtensions', @@ -57,8 +53,7 @@ export function getOptionsFromRootManifest (manifestDir: string, manifest: Proje } export function getOptionsFromPnpmSettings (manifestDir: string | undefined, pnpmSettings: PnpmSettings, manifest?: ProjectManifest): OptionsFromRootManifest { - const renamedKeys = ['allowNonAppliedPatches'] as const satisfies Array - const settings: OptionsFromRootManifest = omit(renamedKeys, replaceEnvInSettings(pnpmSettings)) + const settings: OptionsFromRootManifest = replaceEnvInSettings(pnpmSettings) if (settings.overrides) { if (Object.keys(settings.overrides).length === 0) { delete settings.overrides @@ -73,13 +68,6 @@ export function getOptionsFromPnpmSettings (manifestDir: string | undefined, pnp settings.patchedDependencies[dep] = path.join(manifestDir, patchFile) } } - if (pnpmSettings.allowNonAppliedPatches != null) { - globalWarn('allowNonAppliedPatches is deprecated, use allowUnusedPatches instead.') - settings.allowUnusedPatches ??= pnpmSettings.allowNonAppliedPatches - } - if (pnpmSettings.ignorePatchFailures != null) { - settings.ignorePatchFailures = pnpmSettings.ignorePatchFailures - } return settings } diff --git a/config/config/test/getOptionsFromRootManifest.test.ts b/config/config/test/getOptionsFromRootManifest.test.ts index 6351445c9e..04ad6a4042 100644 --- a/config/config/test/getOptionsFromRootManifest.test.ts +++ b/config/config/test/getOptionsFromRootManifest.test.ts @@ -99,62 +99,6 @@ test('getOptionsFromRootManifest() should return allowBuilds', () => { expect(options.allowBuilds).toStrictEqual({ electron: true }) }) -test('getOptionsFromRootManifest() should derive allowUnusedPatches from allowNonAppliedPatches (legacy behavior)', () => { - expect(getOptionsFromRootManifest(process.cwd(), { - pnpm: { - allowNonAppliedPatches: false, - }, - })).toStrictEqual({ - allowUnusedPatches: false, - }) - - expect(getOptionsFromRootManifest(process.cwd(), { - pnpm: { - allowNonAppliedPatches: true, - }, - })).toStrictEqual({ - allowUnusedPatches: true, - }) -}) - -test('allowUnusedPatches should override allowNonAppliedPatches', () => { - expect(getOptionsFromRootManifest(process.cwd(), { - pnpm: { - allowNonAppliedPatches: false, - allowUnusedPatches: false, - }, - })).toStrictEqual({ - allowUnusedPatches: false, - }) - - expect(getOptionsFromRootManifest(process.cwd(), { - pnpm: { - allowNonAppliedPatches: true, - allowUnusedPatches: false, - }, - })).toStrictEqual({ - allowUnusedPatches: false, - }) - - expect(getOptionsFromRootManifest(process.cwd(), { - pnpm: { - allowNonAppliedPatches: false, - allowUnusedPatches: false, - }, - })).toStrictEqual({ - allowUnusedPatches: false, - }) - - expect(getOptionsFromRootManifest(process.cwd(), { - pnpm: { - allowNonAppliedPatches: true, - allowUnusedPatches: false, - }, - })).toStrictEqual({ - allowUnusedPatches: false, - }) -}) - test('getOptionsFromRootManifest() should return patchedDependencies', () => { const options = getOptionsFromRootManifest(process.cwd(), { pnpm: { diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts index 869a8fc750..b054ecfcd4 100644 --- a/exec/build-modules/src/index.ts +++ b/exec/build-modules/src/index.ts @@ -29,7 +29,6 @@ export async function buildModules ( rootDepPaths: T[], opts: { allowBuild?: AllowBuild - ignorePatchFailures?: boolean childConcurrency?: number depsToBuild?: Set depsStateCache: DepsStateCache @@ -112,7 +111,6 @@ async function buildDependency ( depPath: T, depGraph: DependenciesGraph, opts: { - ignorePatchFailures?: boolean extraBinPaths?: string[] extraNodePaths?: string[] extraEnv?: Record @@ -147,11 +145,8 @@ async function buildDependency ( await linkBinsOfDependencies(depNode, depGraph, opts) let isPatched = false if (depNode.patch) { - const { file, strict } = depNode.patch - // `strict` is a legacy property which was kept to preserve backward compatibility. - // Once a major version of pnpm is released, `strict` should be removed completely. - const allowFailure: boolean = opts.ignorePatchFailures ?? !strict - isPatched = applyPatchToDir({ allowFailure, patchedDir: depNode.dir, patchFilePath: file.path }) + const { file } = depNode.patch + isPatched = applyPatchToDir({ patchedDir: depNode.dir, patchFilePath: file.path }) } const hasSideEffects = !opts.ignoreScripts && await runPostinstallHooks({ depPath, diff --git a/packages/types/src/package.ts b/packages/types/src/package.ts index 7ebfd221eb..4c87f9263c 100644 --- a/packages/types/src/package.ts +++ b/packages/types/src/package.ts @@ -161,9 +161,7 @@ export interface PnpmSettings { ignoredOptionalDependencies?: string[] peerDependencyRules?: PeerDependencyRules allowedDeprecatedVersions?: AllowedDeprecatedVersions - allowNonAppliedPatches?: boolean // deprecated: use allowUnusedPatches instead allowUnusedPatches?: boolean - ignorePatchFailures?: boolean patchedDependencies?: Record updateConfig?: { ignoreDependencies?: string[] diff --git a/patching/apply-patch/src/index.ts b/patching/apply-patch/src/index.ts index 1a9535d235..e070de55e5 100644 --- a/patching/apply-patch/src/index.ts +++ b/patching/apply-patch/src/index.ts @@ -1,9 +1,7 @@ import { PnpmError } from '@pnpm/error' import { applyPatch } from '@pnpm/patch-package/dist/applyPatches.js' -import { globalWarn } from '@pnpm/logger' export interface ApplyPatchToDirOpts { - allowFailure?: boolean patchedDir: string patchFilePath: string } @@ -27,12 +25,7 @@ export function applyPatchToDir (opts: ApplyPatchToDirOpts): boolean { process.chdir(cwd) } if (!success) { - const message = `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}` - if (opts.allowFailure) { - globalWarn(message) - } else { - throw new PnpmError('PATCH_FAILED', message) - } + throw new PnpmError('PATCH_FAILED', `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}`) } return success } diff --git a/patching/apply-patch/test/applyPatchToDir.ts b/patching/apply-patch/test/applyPatchToDir.ts index 34539fed39..2bf683309d 100644 --- a/patching/apply-patch/test/applyPatchToDir.ts +++ b/patching/apply-patch/test/applyPatchToDir.ts @@ -27,15 +27,13 @@ function prepareDirToPatch () { return dir } -describe('applyPatchToDir() without allowFailure', () => { - const allowFailure = false +describe('applyPatchToDir()', () => { it('should succeed when patch is applicable', () => { const patchFilePath = f.find('applicable.patch') const successfullyPatched = f.find('successfully-patched.txt') const patchedDir = prepareDirToPatch() expect( applyPatchToDir({ - allowFailure, patchFilePath, patchedDir, }) @@ -48,7 +46,6 @@ describe('applyPatchToDir() without allowFailure', () => { const patchedDir = prepareDirToPatch() expect(() => { applyPatchToDir({ - allowFailure, patchFilePath, patchedDir, }) @@ -59,7 +56,6 @@ describe('applyPatchToDir() without allowFailure', () => { const patchFilePath = f.find('invalid.patch') expect(() => { applyPatchToDir({ - allowFailure, patchFilePath, patchedDir: tempDir(), }) @@ -68,59 +64,6 @@ describe('applyPatchToDir() without allowFailure', () => { it('should fail if the patch file is not found', () => { expect(() => { applyPatchToDir({ - allowFailure, - patchFilePath: 'does-not-exist.patch', - patchedDir: tempDir(), - }) - }).toThrow('Patch file not found') - }) -}) - -describe('applyPatchToDir() with allowFailure', () => { - const allowFailure = true - it('should succeed when patch is applicable', () => { - const patchFilePath = f.find('applicable.patch') - const successfullyPatched = f.find('successfully-patched.txt') - const patchedDir = prepareDirToPatch() - expect( - applyPatchToDir({ - allowFailure, - patchFilePath, - patchedDir, - }) - ).toBe(true) - const patchTarget = path.join(patchedDir, 'patch-target.txt') - expect(fs.readFileSync(patchTarget, 'utf-8')).toBe(fs.readFileSync(successfullyPatched, 'utf-8')) - }) - it('should warn when patch fails to apply', () => { - const patchFilePath = f.find('non-applicable.patch') - const patchedDir = prepareDirToPatch() - expect( - applyPatchToDir({ - allowFailure, - patchFilePath, - patchedDir, - }) - ).toBe(false) - expect(jest.mocked(globalWarn).mock.calls).toStrictEqual([[ - `Could not apply patch ${patchFilePath} to ${patchedDir}`, - ]]) - expect(fs.readFileSync(path.join(patchedDir, 'patch-target.txt'), 'utf-8')).toBe(fs.readFileSync(f.find('patch-target.txt'), 'utf-8')) - }) - it('should fail on invalid patch', () => { - const patchFilePath = f.find('invalid.patch') - expect(() => { - applyPatchToDir({ - allowFailure, - patchFilePath, - patchedDir: tempDir(), - }) - }).toThrow(`Applying patch "${patchFilePath}" failed: hunk header integrity check failed`) - }) - it('should fail if the patch file is not found', () => { - expect(() => { - applyPatchToDir({ - allowFailure, patchFilePath: 'does-not-exist.patch', patchedDir: tempDir(), }) diff --git a/patching/config/src/groupPatchedDependencies.ts b/patching/config/src/groupPatchedDependencies.ts index 96b8cc186f..041d1a7768 100644 --- a/patching/config/src/groupPatchedDependencies.ts +++ b/patching/config/src/groupPatchedDependencies.ts @@ -22,7 +22,7 @@ export function groupPatchedDependencies (patchedDependencies: Record { hash: '00000000000000000000000000000000', }, key: 'foo', - strict: true, }, }, } satisfies PatchGroupRecord @@ -83,7 +80,6 @@ test('exact version patches override version range patches, version range patche hash: '00000000000000000000000000000000', }, key: 'foo@1.0.0', - strict: true, }, '1.1.0': { file: { @@ -91,7 +87,6 @@ test('exact version patches override version range patches, version range patche hash: '00000000000000000000000000000000', }, key: 'foo@1.1.0', - strict: true, }, }, range: [ @@ -103,7 +98,6 @@ test('exact version patches override version range patches, version range patche hash: '00000000000000000000000000000000', }, key: 'foo@1', - strict: true, }, }, { @@ -114,7 +108,6 @@ test('exact version patches override version range patches, version range patche hash: '00000000000000000000000000000000', }, key: 'foo@2', - strict: true, }, }, ], @@ -124,7 +117,6 @@ test('exact version patches override version range patches, version range patche hash: '00000000000000000000000000000000', }, key: 'foo', - strict: true, }, }, } satisfies PatchGroupRecord @@ -150,7 +142,6 @@ test('getPatchInfo(_, name, version) throws an error when name@version matches m hash: '00000000000000000000000000000000', }, key: 'foo@>=1.0.0 <3.0.0', - strict: true, }, }, { @@ -161,7 +152,6 @@ test('getPatchInfo(_, name, version) throws an error when name@version matches m hash: '00000000000000000000000000000000', }, key: 'foo@>=2.0.0', - strict: true, }, }, ], @@ -185,7 +175,6 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m hash: '00000000000000000000000000000000', }, key: 'foo@>=1.0.0 <3.0.0', - strict: true, }, }, range: [ @@ -197,7 +186,6 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m hash: '00000000000000000000000000000000', }, key: 'foo@>=1.0.0 <3.0.0', - strict: true, }, }, { @@ -208,7 +196,6 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m hash: '00000000000000000000000000000000', }, key: 'foo@>=2.0.0', - strict: true, }, }, ], diff --git a/patching/config/test/groupPatchedDependencies.test.ts b/patching/config/test/groupPatchedDependencies.test.ts index f7042afcae..f61ca98214 100644 --- a/patching/config/test/groupPatchedDependencies.test.ts +++ b/patching/config/test/groupPatchedDependencies.test.ts @@ -53,17 +53,16 @@ test('groups patchedDependencies according to names, match types, and versions', path: 'patches/mixed-style.patch', }, } satisfies Record - const info = (strict: boolean, key: keyof typeof patchedDependencies): ExtendedPatchInfo => ({ - strict, + const info = (key: keyof typeof patchedDependencies): ExtendedPatchInfo => ({ key, file: patchedDependencies[key], }) expect(_groupPatchedDependencies(patchedDependencies)).toStrictEqual({ 'exact-version-only': { exact: { - '0.0.0': info(true, 'exact-version-only@0.0.0'), - '1.2.3': info(true, 'exact-version-only@1.2.3'), - '2.1.0': info(true, 'exact-version-only@2.1.0'), + '0.0.0': info('exact-version-only@0.0.0'), + '1.2.3': info('exact-version-only@1.2.3'), + '2.1.0': info('exact-version-only@2.1.0'), }, range: [], all: undefined, @@ -73,11 +72,11 @@ test('groups patchedDependencies according to names, match types, and versions', range: [ { version: '~1.2.0', - patch: info(true, 'version-range-only@~1.2.0'), + patch: info('version-range-only@~1.2.0'), }, { version: '4', - patch: info(true, 'version-range-only@4'), + patch: info('version-range-only@4'), }, ], all: undefined, @@ -85,24 +84,24 @@ test('groups patchedDependencies according to names, match types, and versions', 'star-version-range': { exact: {}, range: [], - all: info(true, 'star-version-range@*'), + all: info('star-version-range@*'), }, 'without-versions': { exact: {}, range: [], - all: info(false, 'without-versions'), + all: info('without-versions'), }, 'mixed-style': { exact: { - '0.1.2': info(true, 'mixed-style@0.1.2'), + '0.1.2': info('mixed-style@0.1.2'), }, range: [ { version: '1.x.x', - patch: info(true, 'mixed-style@1.x.x'), + patch: info('mixed-style@1.x.x'), }, ], - all: info(false, 'mixed-style'), + all: info('mixed-style'), }, } as PatchGroupRecord) }) diff --git a/patching/config/test/index.test.ts b/patching/config/test/index.test.ts index 7d30cdc5d1..04a79a17e3 100644 --- a/patching/config/test/index.test.ts +++ b/patching/config/test/index.test.ts @@ -8,7 +8,7 @@ test('getPatchInfo(undefined, ...) returns undefined', () => { expect(getPatchInfo(undefined, 'foo', '1.0.0')).toBeUndefined() }) -test('getPatchInfo(_, name, version) returns strict=true if name@version exists', () => { +test('getPatchInfo(_, name, version) if name@version exists', () => { expect(_getPatchInfo({ 'foo@1.0.0': { path: 'patches/foo@1.0.0.patch', @@ -20,11 +20,10 @@ test('getPatchInfo(_, name, version) returns strict=true if name@version exists' hash: expect.any(String), }, key: 'foo@1.0.0', - strict: true, }) }) -test('getPatchInfo(_, name, version) returns strict=false if name exists and name@version does not exist', () => { +test('getPatchInfo(_, name, version) if name exists but name@version does not exist', () => { expect(_getPatchInfo({ foo: { path: 'patches/foo.patch', @@ -36,7 +35,6 @@ test('getPatchInfo(_, name, version) returns strict=false if name exists and nam hash: expect.any(String), }, key: 'foo', - strict: false, }) }) @@ -56,7 +54,6 @@ test('getPatchInfo(_, name, version) prioritizes name@version over name if both hash: expect.any(String), }, key: 'foo@1.0.0', - strict: true, }) }) diff --git a/patching/plugin-commands-patching/src/patch.ts b/patching/plugin-commands-patching/src/patch.ts index 866c07c9eb..37a3f0a97b 100644 --- a/patching/plugin-commands-patching/src/patch.ts +++ b/patching/plugin-commands-patching/src/patch.ts @@ -108,7 +108,6 @@ export async function handler (opts: PatchCommandOptions, params: string[]): Pro if (!opts.ignoreExisting && opts.patchedDependencies) { tryPatchWithExistingPatchFile({ - allowFailure: patchedDep.applyToAll, patchedDep, patchedDir: editDir, patchedDependencies: opts.patchedDependencies, @@ -129,13 +128,11 @@ To commit your changes, run: function tryPatchWithExistingPatchFile ( { - allowFailure, patchedDep: { applyToAll, alias, bareSpecifier }, patchedDir, patchedDependencies, lockfileDir, }: { - allowFailure: boolean patchedDep: GetPatchedDependencyResult patchedDir: string patchedDependencies: Record @@ -157,5 +154,5 @@ function tryPatchWithExistingPatchFile ( if (!fs.existsSync(existingPatchFilePath)) { throw new PnpmError('PATCH_FILE_NOT_FOUND', `Unable to find patch file ${existingPatchFilePath}`) } - applyPatchToDir({ patchedDir, patchFilePath: existingPatchFilePath, allowFailure }) + applyPatchToDir({ patchedDir, patchFilePath: existingPatchFilePath }) } diff --git a/patching/plugin-commands-patching/test/patch.test.ts b/patching/plugin-commands-patching/test/patch.test.ts index 0b5820f277..658f865b7a 100644 --- a/patching/plugin-commands-patching/test/patch.test.ts +++ b/patching/plugin-commands-patching/test/patch.test.ts @@ -849,9 +849,9 @@ describe('prompt to choose version', () => { expect(path.basename(patchDir)).toMatch(/^chalk@\d+\.\d+\.\d+$/) expect(fs.existsSync(patchDir)).toBe(true) expect(JSON.parse(fs.readFileSync(path.join(patchDir, 'package.json'), 'utf8')).version).toBe('5.3.0') - expect(fs.existsSync(path.join(patchDir, 'source/index.js'))).toBe(true) + expect(fs.existsSync(path.join(patchDir, 'license'))).toBe(true) - fs.appendFileSync(path.join(patchDir, 'source/index.js'), '// test patching', 'utf8') + fs.appendFileSync(path.join(patchDir, 'license'), '\ntest patching', 'utf8') await patchCommit.handler({ ...DEFAULT_OPTS, cacheDir, @@ -868,8 +868,8 @@ describe('prompt to choose version', () => { }) const patchContent = fs.readFileSync('patches/chalk.patch', 'utf8') expect(patchContent).toContain('diff --git') - expect(patchContent).toContain('// test patching') - expect(fs.readFileSync('node_modules/.pnpm/@pnpm.e2e+requires-chalk-530@1.0.0/node_modules/chalk/source/index.js', 'utf8')).toContain('// test patching') + expect(patchContent).toContain('test patching') + expect(fs.readFileSync('node_modules/.pnpm/@pnpm.e2e+requires-chalk-530@1.0.0/node_modules/chalk/license', 'utf8')).toContain('test patching') }) }) diff --git a/patching/types/src/index.ts b/patching/types/src/index.ts index 08ef4d1328..5dea224c91 100644 --- a/patching/types/src/index.ts +++ b/patching/types/src/index.ts @@ -5,7 +5,6 @@ export interface PatchFile { // TODO: replace all occurrences of PatchInfo with PatchFile before the next major version is released export interface PatchInfo { - strict: boolean file: PatchFile } diff --git a/pkg-manager/core/src/install/extendInstallOptions.ts b/pkg-manager/core/src/install/extendInstallOptions.ts index b614115429..c0f6cfe512 100644 --- a/pkg-manager/core/src/install/extendInstallOptions.ts +++ b/pkg-manager/core/src/install/extendInstallOptions.ts @@ -118,7 +118,6 @@ export interface StrictInstallOptions { modulesCacheMaxAge: number peerDependencyRules: PeerDependencyRules allowedDeprecatedVersions: AllowedDeprecatedVersions - ignorePatchFailures?: boolean allowUnusedPatches: boolean preferSymlinkedExecutables: boolean resolutionMode: 'highest' | 'time-based' | 'lowest-direct' @@ -183,7 +182,6 @@ const defaults = (opts: InstallOptions): StrictInstallOptions => { return { allowedDeprecatedVersions: {}, allowUnusedPatches: false, - ignorePatchFailures: undefined, autoInstallPeers: true, autoInstallPeersFromHighestMatch: false, catalogs: {}, diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index bea9333516..ca4bf10da1 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -1373,7 +1373,6 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { } ignoredBuilds = (await buildModules(dependenciesGraph, rootNodes, { allowBuild: opts.allowBuild, - ignorePatchFailures: opts.ignorePatchFailures, childConcurrency: opts.childConcurrency, depsStateCache, depsToBuild: new Set(result.newDepPaths), diff --git a/pkg-manager/core/test/install/patch.ts b/pkg-manager/core/test/install/patch.ts index 930068d3fb..e1c51c47f0 100644 --- a/pkg-manager/core/test/install/patch.ts +++ b/pkg-manager/core/test/install/patch.ts @@ -551,62 +551,7 @@ test('patch package should fail when the version range patch fails to apply', as expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') }) -test('patch package should print a warning when the patch fails to apply and ignorePatchFailures is set to true', async () => { - prepareEmpty() - const reporter = jest.fn() - const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') - - const patchedDependencies = { - 'is-positive@3.1.0': patchPath, - } - const opts = testDefaults({ - fastUnpack: false, - ignorePatchFailures: true, - sideEffectsCacheRead: true, - sideEffectsCacheWrite: true, - patchedDependencies, - reporter, - }, {}, {}, { packageImportMethod: 'hardlink' }) - await install({ - dependencies: { - 'is-positive': '3.1.0', - }, - }, opts) - - expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') - expect(reporter).toHaveBeenCalledWith(expect.objectContaining({ - message: expect.stringMatching(/Could not apply patch/), - })) -}) - -test('patch package should print a warning when the name-only patch fails to apply (legacy behavior)', async () => { - prepareEmpty() - const reporter = jest.fn() - const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') - - const patchedDependencies = { - 'is-positive': patchPath, - } - const opts = testDefaults({ - fastUnpack: false, - sideEffectsCacheRead: true, - sideEffectsCacheWrite: true, - patchedDependencies, - reporter, - }, {}, {}, { packageImportMethod: 'hardlink' }) - await install({ - dependencies: { - 'is-positive': '3.1.0', - }, - }, opts) - - expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') - expect(reporter).toHaveBeenCalledWith(expect.objectContaining({ - message: expect.stringMatching(/Could not apply patch/), - })) -}) - -test('patch package should fail when the name-only range patch fails to apply and ignorePatchFailures is explicitly set to false', async () => { +test('patch package should fail when the name-only range patch fails to apply', async () => { prepareEmpty() const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') @@ -615,7 +560,6 @@ test('patch package should fail when the name-only range patch fails to apply an } const opts = testDefaults({ fastUnpack: false, - ignorePatchFailures: false, sideEffectsCacheRead: true, sideEffectsCacheWrite: true, patchedDependencies, diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index d5fa79191f..c7efbe1f1f 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -103,7 +103,6 @@ export interface Project { } export interface HeadlessOptions { - ignorePatchFailures?: boolean allowBuilds?: Record autoInstallPeers?: boolean childConcurrency?: number @@ -536,7 +535,6 @@ export async function headlessInstall (opts: HeadlessOptions): Promise { +test('bare package name as a patchedDependencies key should apply to all possible versions and error on non-applicable versions', async () => { const patchFixture = f.find('patchedDependencies/console-log-replace-3rd-line.patch') prepareEmpty() @@ -111,7 +111,7 @@ test('bare package name as a patchedDependencies key should apply to all possibl const rootProjectManifest = addPatch('@pnpm.e2e/console-log', patchFixture, 'patches/console-log.patch') - await install.handler({ + const promise = install.handler({ ...DEFAULT_OPTS, dir: process.cwd(), frozenLockfile: false, @@ -119,8 +119,8 @@ test('bare package name as a patchedDependencies key should apply to all possibl }) // the common patch does not apply to v1 + await expect(promise).rejects.toThrow(`Could not apply patch ${path.resolve('patches/console-log.patch')}`) expect(patchedFileContent(1)).toBe(unpatchedFileContent(1)) - expect(globalWarn).toHaveBeenCalledWith(expect.stringContaining(`Could not apply patch ${path.resolve('patches/console-log.patch')}`)) // the common patch applies to v2 { diff --git a/pnpm/test/patch/ignorePatchFailures.ts b/pnpm/test/patch/ignorePatchFailures.ts deleted file mode 100644 index c778c6830a..0000000000 --- a/pnpm/test/patch/ignorePatchFailures.ts +++ /dev/null @@ -1,500 +0,0 @@ -import fs from 'fs' -import { preparePackages } from '@pnpm/prepare' -import { fixtures } from '@pnpm/test-fixtures' -import { sync as writeYamlFile } from 'write-yaml-file' -import { execPnpmSync } from '../utils/index.js' - -const f = fixtures(import.meta.dirname) - -describe('ignorePatchFailures=undefined (necessary for backward-compatibility)', () => { - test('errors on exact version patch failures', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@1.0.0': patchFile, // should succeed - 'is-positive@3.1.0': patchFile, // should fail - }, - }) - - // pnpm install should fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) - - test('errors on version range patch failures', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@>=1': patchFile, - }, - }) - - // pnpm install should fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) - - test('errors on star version range patch failures', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@*': patchFile, - }, - }) - - // pnpm install should fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) - - test('allows name-only patches to fail with a warning (legacy behavior)', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive': patchFile, - }, - }) - - // pnpm install should not fail - const { stdout } = execPnpmSync(['install'], { expectSuccess: true }) - - // pnpm install should print a warning about patch application failure - expect(stdout.toString()).toContain('Could not apply patch') - - // the patch should apply to is-positive@1.0.0 - expect(fs.readFileSync('foo/node_modules/is-positive/index.js', 'utf-8')).toContain('// patched') - - // the patch should not apply to is-positive@3.2.1 - expect(fs.readFileSync('bar/node_modules/is-positive/index.js', 'utf-8')).not.toContain('// patched') - }) -}) - -describe('ignorePatchFailures=true', () => { - test('allows exact version patches to fail with a warning', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@1.0.0': patchFile, // should succeed - 'is-positive@3.1.0': patchFile, // should fail - }, - ignorePatchFailures: true, - }) - - // pnpm install should not fail - const { stdout } = execPnpmSync(['install'], { expectSuccess: true }) - - // pnpm install should print a warning about patch application failure - expect(stdout.toString()).toContain('Could not apply patch') - - // the patch should apply to is-positive@1.0.0 - expect(fs.readFileSync('foo/node_modules/is-positive/index.js', 'utf-8')).toContain('// patched') - - // the patch should not apply to is-positive@3.2.1 - expect(fs.readFileSync('bar/node_modules/is-positive/index.js', 'utf-8')).not.toContain('// patched') - }) - - test('allows version range patches to fail with a warning', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@>=1': patchFile, - }, - ignorePatchFailures: true, - }) - - // pnpm install should not fail - const { stdout } = execPnpmSync(['install'], { expectSuccess: true }) - - // pnpm install should print a warning about patch application failure - expect(stdout.toString()).toContain('Could not apply patch') - - // the patch should apply to is-positive@1.0.0 - expect(fs.readFileSync('foo/node_modules/is-positive/index.js', 'utf-8')).toContain('// patched') - - // the patch should not apply to is-positive@3.2.1 - expect(fs.readFileSync('bar/node_modules/is-positive/index.js', 'utf-8')).not.toContain('// patched') - }) - - test('allows star version range patches to fail with a warning', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@*': patchFile, - }, - ignorePatchFailures: true, - }) - - // pnpm install should not fail - const { stdout } = execPnpmSync(['install'], { expectSuccess: true }) - - // pnpm install should print a warning about patch application failure - expect(stdout.toString()).toContain('Could not apply patch') - - // the patch should apply to is-positive@1.0.0 - expect(fs.readFileSync('foo/node_modules/is-positive/index.js', 'utf-8')).toContain('// patched') - - // the patch should not apply to is-positive@3.2.1 - expect(fs.readFileSync('bar/node_modules/is-positive/index.js', 'utf-8')).not.toContain('// patched') - }) - - test('allows name-only patches to fail with a warning', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive': patchFile, - }, - ignorePatchFailures: true, - }) - - // pnpm install should not fail - const { stdout } = execPnpmSync(['install'], { expectSuccess: true }) - - // pnpm install should print a warning about patch application failure - expect(stdout.toString()).toContain('Could not apply patch') - - // the patch should apply to is-positive@1.0.0 - expect(fs.readFileSync('foo/node_modules/is-positive/index.js', 'utf-8')).toContain('// patched') - - // the patch should not apply to is-positive@3.2.1 - expect(fs.readFileSync('bar/node_modules/is-positive/index.js', 'utf-8')).not.toContain('// patched') - }) -}) - -describe('ignorePatchFailures=false', () => { - test('errors on exact version patch failures', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@1.0.0': patchFile, // should succeed - 'is-positive@3.1.0': patchFile, // should fail - }, - ignorePatchFailures: false, - }) - - // pnpm install should fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) - - test('errors on version range patch failures', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@>=1': patchFile, - }, - ignorePatchFailures: false, - }) - - // pnpm install not fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) - - test('errors on star version range patch failures', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive@*': patchFile, - }, - ignorePatchFailures: false, - }) - - // pnpm install not fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) - - test('allows name-only patches to fail with a warning', async () => { - preparePackages([ - { - name: 'foo', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '1.0.0', // is-positive@1.0.0.patch should succeed - }, - }, - { - name: 'bar', - version: '0.0.0', - private: true, - dependencies: { - 'is-positive': '3.1.0', // is-positive@1.0.0.patch should fail - }, - }, - ]) - - const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') - - writeYamlFile('pnpm-workspace.yaml', { - packages: ['**', '!store/**'], - patchedDependencies: { - 'is-positive': patchFile, - }, - ignorePatchFailures: false, - }) - - // pnpm install should fail - const { status, stdout } = execPnpmSync(['install']) - expect(status).not.toBe(0) - expect(stdout.toString()).toContain('ERR_PNPM_PATCH_FAILED') - const errorLines = stdout.toString().split('\n').filter(line => line.includes('ERR_PNPM_PATCH_FAILED')) - expect(errorLines).toStrictEqual([expect.stringContaining(patchFile)]) - expect(errorLines).toStrictEqual([expect.stringContaining('is-positive@3.1.0')]) - }) -}) diff --git a/releasing/plugin-commands-deploy/test/fixtures/is-positive.patch b/releasing/plugin-commands-deploy/test/fixtures/is-positive.patch index e2c1424221..0a145ff782 100644 --- a/releasing/plugin-commands-deploy/test/fixtures/is-positive.patch +++ b/releasing/plugin-commands-deploy/test/fixtures/is-positive.patch @@ -9,8 +9,8 @@ diff --git a/package.json b/package.json index 5feb15ba194c74ad48a2ee15abec9887ec1f9e83..27e24feff1bbfc20b3735c23b332bfdc16803362 100644 --- a/package.json +++ b/package.json -@@ -16,6 +16,7 @@ - "test": "xo && ava" +@@ -19,6 +19,7 @@ + "test": "node test.js" }, "files": [ + "PATCH.txt",