diff --git a/.changeset/common-poems-cut.md b/.changeset/common-poems-cut.md new file mode 100644 index 0000000000..30cfa9d7cb --- /dev/null +++ b/.changeset/common-poems-cut.md @@ -0,0 +1,36 @@ +--- +"@pnpm/resolve-dependencies": major +"@pnpm/headless": major +"@pnpm/deps.graph-builder": major +"@pnpm/patching.config": major +"@pnpm/patching.types": minor +"@pnpm/core": patch +"pnpm": minor +--- + +Add an ability to patch dependencies by version ranges. Exact versions override version ranges, which in turn override name-only patches. Version range `*` is the same as name-only, except that patch application failure will not be ignored. + +For example: + +```yaml +patchedDependencies: + foo: patches/foo-1.patch + foo@^2.0.0: patches/foo-2.patch + foo@2.1.0: patches/foo-3.patch +``` + +The above configuration would apply `patches/foo-3.patch` to `foo@2.1.0`, `patches/foo-2.patch` to all `foo` versions which satisfy `^2.0.0` except `2.1.0`, and `patches/foo-1.patch` to the remaining `foo` versions. + +> [!WARNING] +> The version ranges should not overlap. If you want to specialize a sub range, make sure to exclude it from the other keys. For example: +> +> ```yaml +> # pnpm-workspace.yaml +> patchedDependencies: +> # the specialized sub range +> 'foo@2.2.0-2.8.0': patches/foo.2.2.0-2.8.0.patch +> # the more general patch, excluding the sub range above +> 'foo@>=2.0.0 <2.2.0 || >2.8.0': 'patches/foo.gte2.patch +> ``` +> +> In most cases, however, it's sufficient to just define an exact version to override the range. diff --git a/.changeset/metal-ravens-judge.md b/.changeset/metal-ravens-judge.md new file mode 100644 index 0000000000..c8151956a9 --- /dev/null +++ b/.changeset/metal-ravens-judge.md @@ -0,0 +1,14 @@ +--- +"@pnpm/resolve-dependencies": major +"@pnpm/config": minor +"@pnpm/core": major +"@pnpm/types": minor +"@pnpm/headless": minor +"@pnpm/patching.config": minor +"@pnpm/patching.types": patch +"@pnpm/plugin-commands-script-runners": patch +"@pnpm/build-modules": minor +"pnpm": minor +--- + +Rename `pnpm.allowNonAppliedPatches` to `pnpm.allowUnusedPatches`. The old name is still supported but it would print a deprecation warning message. diff --git a/.changeset/rotten-clubs-mate.md b/.changeset/rotten-clubs-mate.md new file mode 100644 index 0000000000..a27cd4fddc --- /dev/null +++ b/.changeset/rotten-clubs-mate.md @@ -0,0 +1,20 @@ +--- +"@pnpm/resolve-dependencies": major +"@pnpm/config": minor +"@pnpm/core": major +"@pnpm/types": minor +"@pnpm/headless": minor +"@pnpm/patching.config": minor +"@pnpm/patching.types": patch +"@pnpm/plugin-commands-script-runners": patch +"@pnpm/build-modules": minor +"pnpm": minor +--- + +Add `pnpm.ignorePatchFailures` to manage whether pnpm would ignore patch application failures. + +If `ignorePatchFailures` is not set, pnpm would throw an error when patches with exact versions or version ranges fail to apply, and it would ignore failures from name-only patches. + +If `ignorePatchFailures` is explicitly set to `false`, pnpm would throw an error when any type of patch fails to apply. + +If `ignorePatchFailures` is explicitly set to `true`, pnpm would print a warning when any type of patch fails to apply. diff --git a/config/config/package.json b/config/config/package.json index 85a1b43bac..4e7127cea1 100644 --- a/config/config/package.json +++ b/config/config/package.json @@ -59,6 +59,9 @@ "realpath-missing": "catalog:", "which": "catalog:" }, + "peerDependencies": { + "@pnpm/logger": ">=5.1.0 <1001.0.0" + }, "devDependencies": { "@pnpm/config": "workspace:*", "@pnpm/prepare": "workspace:*", diff --git a/config/config/src/getOptionsFromRootManifest.ts b/config/config/src/getOptionsFromRootManifest.ts index f143e5d65c..16df132406 100644 --- a/config/config/src/getOptionsFromRootManifest.ts +++ b/config/config/src/getOptionsFromRootManifest.ts @@ -9,11 +9,14 @@ import { type PnpmSettings, } from '@pnpm/types' import mapValues from 'ramda/src/map' +import omit from 'ramda/src/omit' import pick from 'ramda/src/pick' +import { globalWarn } from '@pnpm/logger' export type OptionsFromRootManifest = { allowedDeprecatedVersions?: AllowedDeprecatedVersions - allowNonAppliedPatches?: boolean + allowUnusedPatches?: boolean + ignorePatchFailures?: boolean overrides?: Record neverBuiltDependencies?: string[] onlyBuiltDependencies?: string[] @@ -29,11 +32,13 @@ export type OptionsFromRootManifest = { export function getOptionsFromRootManifest (manifestDir: string, manifest: ProjectManifest): OptionsFromRootManifest { const settings: OptionsFromRootManifest = getOptionsFromPnpmSettings(manifestDir, { ...pick([ - 'allowNonAppliedPatches', 'allowedDeprecatedVersions', + 'allowNonAppliedPatches', + 'allowUnusedPatches', 'configDependencies', 'ignoredBuiltDependencies', 'ignoredOptionalDependencies', + 'ignorePatchFailures', 'neverBuiltDependencies', 'onlyBuiltDependencies', 'onlyBuiltDependenciesFile', @@ -55,7 +60,8 @@ export function getOptionsFromRootManifest (manifestDir: string, manifest: Proje } export function getOptionsFromPnpmSettings (manifestDir: string, pnpmSettings: PnpmSettings, manifest?: ProjectManifest): OptionsFromRootManifest { - const settings: OptionsFromRootManifest = { ...pnpmSettings } + const renamedKeys = ['allowNonAppliedPatches'] as const satisfies Array + const settings: OptionsFromRootManifest = omit(renamedKeys, pnpmSettings) if (settings.overrides) { if (Object.keys(settings.overrides).length === 0) { delete settings.overrides @@ -73,6 +79,13 @@ export function getOptionsFromPnpmSettings (manifestDir: string, pnpmSettings: P 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 0f98e37854..bdacb8d66d 100644 --- a/config/config/test/getOptionsFromRootManifest.test.ts +++ b/config/config/test/getOptionsFromRootManifest.test.ts @@ -93,6 +93,62 @@ test('getOptionsFromRootManifest() should return the list from onlyBuiltDependen expect(options.onlyBuiltDependencies).toStrictEqual(['electron']) }) +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/deps/graph-builder/src/lockfileToDepGraph.ts b/deps/graph-builder/src/lockfileToDepGraph.ts index 3ea90a5014..46560ec09a 100644 --- a/deps/graph-builder/src/lockfileToDepGraph.ts +++ b/deps/graph-builder/src/lockfileToDepGraph.ts @@ -15,8 +15,8 @@ import { import { logger } from '@pnpm/logger' import { type IncludedDependencies } from '@pnpm/modules-yaml' import { packageIsInstallable } from '@pnpm/package-is-installable' -import { getPatchInfo } from '@pnpm/patching.config' -import { type PatchFile, type PatchInfo } from '@pnpm/patching.types' +import { type PatchGroupRecord, getPatchInfo } from '@pnpm/patching.config' +import { type PatchInfo } from '@pnpm/patching.types' import { type DepPath, type SupportedArchitectures, type Registries, type PkgIdWithPatchHash, type ProjectId } from '@pnpm/types' import { type PkgRequestFetchResult, @@ -63,7 +63,7 @@ export interface LockfileToDepGraphOptions { lockfileDir: string nodeVersion: string pnpmVersion: string - patchedDependencies?: Record + patchedDependencies?: PatchGroupRecord registries: Registries sideEffectsCacheRead: boolean skipped: Set diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts index dcb8e40aba..312c648be9 100644 --- a/exec/build-modules/src/index.ts +++ b/exec/build-modules/src/index.ts @@ -23,6 +23,7 @@ export async function buildModules ( rootDepPaths: T[], opts: { allowBuild?: (pkgName: string) => boolean + ignorePatchFailures?: boolean ignoredBuiltDependencies?: string[] childConcurrency?: number depsToBuild?: Set @@ -103,6 +104,7 @@ async function buildDependency ( depPath: T, depGraph: DependenciesGraph, opts: { + ignorePatchFailures?: boolean extraBinPaths?: string[] extraNodePaths?: string[] extraEnv?: Record @@ -138,7 +140,10 @@ async function buildDependency ( let isPatched = false if (depNode.patch) { const { file, strict } = depNode.patch - isPatched = applyPatchToDir({ allowFailure: !strict, patchedDir: depNode.dir, patchFilePath: file.path }) + // `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 hasSideEffects = !opts.ignoreScripts && await runPostinstallHooks({ depPath, diff --git a/packages/types/src/package.ts b/packages/types/src/package.ts index f5d64688d5..63c375c118 100644 --- a/packages/types/src/package.ts +++ b/packages/types/src/package.ts @@ -144,7 +144,9 @@ export interface PnpmSettings { ignoredOptionalDependencies?: string[] peerDependencyRules?: PeerDependencyRules allowedDeprecatedVersions?: AllowedDeprecatedVersions - allowNonAppliedPatches?: boolean + allowNonAppliedPatches?: boolean // deprecated: use allowUnusedPatches instead + allowUnusedPatches?: boolean + ignorePatchFailures?: boolean patchedDependencies?: Record updateConfig?: { ignoreDependencies?: string[] diff --git a/patching/config/package.json b/patching/config/package.json index 8cec26981b..d2ec0f097e 100644 --- a/patching/config/package.json +++ b/patching/config/package.json @@ -31,10 +31,17 @@ "lint": "eslint \"src/**/*.ts\" \"test/**/*.ts\"" }, "dependencies": { - "@pnpm/patching.types": "workspace:*" + "@pnpm/dependency-path": "workspace:*", + "@pnpm/error": "workspace:*", + "@pnpm/patching.types": "workspace:*", + "semver": "catalog:" + }, + "peerDependencies": { + "@pnpm/logger": ">=5.1.0 <1001.0.0" }, "devDependencies": { - "@pnpm/patching.config": "workspace:*" + "@pnpm/patching.config": "workspace:*", + "@types/semver": "catalog:" }, "engines": { "node": ">=18.12" diff --git a/patching/config/src/allPatchKeys.ts b/patching/config/src/allPatchKeys.ts new file mode 100644 index 0000000000..2e0d470248 --- /dev/null +++ b/patching/config/src/allPatchKeys.ts @@ -0,0 +1,16 @@ +import { type PatchGroupRecord } from '@pnpm/patching.types' + +export function * allPatchKeys (patchedDependencies: PatchGroupRecord): Generator { + for (const name in patchedDependencies) { + const group = patchedDependencies[name] + for (const version in group.exact) { + yield group.exact[version].key + } + for (const item of group.range) { + yield item.patch.key + } + if (group.all) { + yield group.all.key + } + } +} diff --git a/patching/config/src/getPatchInfo.ts b/patching/config/src/getPatchInfo.ts new file mode 100644 index 0000000000..dd509139f1 --- /dev/null +++ b/patching/config/src/getPatchInfo.ts @@ -0,0 +1,39 @@ +import { PnpmError } from '@pnpm/error' +import { type ExtendedPatchInfo, type PatchGroupRangeItem, type PatchGroupRecord } from '@pnpm/patching.types' +import { satisfies } from 'semver' + +class PatchKeyConflictError extends PnpmError { + constructor ( + pkgName: string, + pkgVersion: string, + satisfied: Array> + ) { + const pkgId = `${pkgName}@${pkgVersion}` + const satisfiedVersions = satisfied.map(({ version }) => version) + const message = `Unable to choose between ${satisfied.length} version ranges to patch ${pkgId}: ${satisfiedVersions.join(', ')}` + super('PATCH_KEY_CONFLICT', message, { + hint: `Explicitly set the exact version (${pkgId}) to resolve conflict`, + }) + } +} + +export function getPatchInfo ( + patchFileGroups: PatchGroupRecord | undefined, + pkgName: string, + pkgVersion: string +): ExtendedPatchInfo | undefined { + if (!patchFileGroups?.[pkgName]) return undefined + + const exactVersion = patchFileGroups[pkgName].exact[pkgVersion] + if (exactVersion) return exactVersion + + const satisfied = patchFileGroups[pkgName].range.filter(item => satisfies(pkgVersion, item.version)) + if (satisfied.length > 1) { + throw new PatchKeyConflictError(pkgName, pkgVersion, satisfied) + } + if (satisfied.length === 1) { + return satisfied[0].patch + } + + return patchFileGroups[pkgName].all +} diff --git a/patching/config/src/groupPatchedDependencies.ts b/patching/config/src/groupPatchedDependencies.ts new file mode 100644 index 0000000000..96b8cc186f --- /dev/null +++ b/patching/config/src/groupPatchedDependencies.ts @@ -0,0 +1,49 @@ +import * as dp from '@pnpm/dependency-path' +import { PnpmError } from '@pnpm/error' +import { type PatchFile, type PatchGroup, type PatchGroupRecord } from '@pnpm/patching.types' +import { validRange } from 'semver' + +export function groupPatchedDependencies (patchedDependencies: Record): PatchGroupRecord { + const result: PatchGroupRecord = {} + function getGroup (name: string): PatchGroup { + let group: PatchGroup | undefined = result[name] + if (group) return group + group = { + exact: {}, + range: [], + all: undefined, + } + result[name] = group + return group + } + + for (const key in patchedDependencies) { + const file = patchedDependencies[key] + const { name, version, nonSemverVersion } = dp.parse(key) + + if (name && version) { + getGroup(name).exact[version] = { strict: true, file, key } + continue + } + + if (name && nonSemverVersion) { + if (!validRange(nonSemverVersion)) { + throw new PnpmError('PATCH_NON_SEMVER_RANGE', `${nonSemverVersion} is not a valid semantic version range.`) + } + if (nonSemverVersion.trim() === '*') { + getGroup(name).all = { strict: true, file, key } + } else { + getGroup(name).range.push({ + version: nonSemverVersion, + patch: { strict: true, file, key }, + }) + } + continue + } + + // Set `strict` to `false` to preserve backward compatibility. + getGroup(key).all = { strict: false, file, key } + } + + return result +} diff --git a/patching/config/src/index.ts b/patching/config/src/index.ts index 498dacc532..a4cf54823f 100644 --- a/patching/config/src/index.ts +++ b/patching/config/src/index.ts @@ -1,29 +1,11 @@ -import { type PatchFile, type PatchInfo } from '@pnpm/patching.types' - -export interface ExtendedPatchInfo extends PatchInfo { - key: string -} - -export function getPatchInfo ( - patchedDependencies: Record | undefined, - pkgName: string, - pkgVersion: string -): ExtendedPatchInfo | undefined { - if (!patchedDependencies) return undefined - const pkgNameAndVersion = `${pkgName}@${pkgVersion}` - if (patchedDependencies[pkgNameAndVersion]) { - return { - file: patchedDependencies[pkgNameAndVersion], - key: pkgNameAndVersion, - strict: true, - } - } - if (patchedDependencies[pkgName]) { - return { - file: patchedDependencies[pkgName], - key: pkgName, - strict: false, - } - } - return undefined -} +export { + type ExtendedPatchInfo, + type PatchFile, + type PatchInfo, + type PatchGroup, + type PatchGroupRangeItem, + type PatchGroupRecord, +} from '@pnpm/patching.types' +export { groupPatchedDependencies } from './groupPatchedDependencies' +export { getPatchInfo } from './getPatchInfo' +export { type VerifyPatchesOptions, verifyPatches } from './verifyPatches' diff --git a/patching/config/src/verifyPatches.ts b/patching/config/src/verifyPatches.ts new file mode 100644 index 0000000000..1715f0162b --- /dev/null +++ b/patching/config/src/verifyPatches.ts @@ -0,0 +1,31 @@ +import { PnpmError } from '@pnpm/error' +import { globalWarn } from '@pnpm/logger' +import { type PatchGroupRecord } from '@pnpm/patching.types' +import { allPatchKeys } from './allPatchKeys' + +export interface VerifyPatchesOptions { + patchedDependencies: PatchGroupRecord + appliedPatches: Set + allowUnusedPatches: boolean +} + +export function verifyPatches ({ + patchedDependencies, + appliedPatches, + allowUnusedPatches, +}: VerifyPatchesOptions): void { + const unusedPatches: string[] = [] + for (const patchKey of allPatchKeys(patchedDependencies)) { + if (!appliedPatches.has(patchKey)) unusedPatches.push(patchKey) + } + + if (!unusedPatches.length) return + const message = `The following patches were not used: ${unusedPatches.join(', ')}` + if (allowUnusedPatches) { + globalWarn(message) + return + } + throw new PnpmError('UNUSED_PATCH', message, { + hint: 'Either remove them from "patchedDependencies" or update them to match packages in your dependencies.', + }) +} diff --git a/patching/config/test/getPatchInfo.test.ts b/patching/config/test/getPatchInfo.test.ts new file mode 100644 index 0000000000..3ff5e54938 --- /dev/null +++ b/patching/config/test/getPatchInfo.test.ts @@ -0,0 +1,219 @@ +import { getPatchInfo } from '../src/getPatchInfo' +import { type PatchGroupRecord } from '../src/index' + +test('getPatchInfo(undefined, ...) returns undefined', () => { + expect(getPatchInfo(undefined, 'foo', '1.0.0')).toBeUndefined() +}) + +test('getPatchInfo() returns an exact version patch if the name and version match', () => { + const patchedDependencies = { + foo: { + exact: { + '1.0.0': { + file: { + path: 'patches/foo@1.0.0.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@1.0.0', + strict: true, + }, + }, + range: [], + all: undefined, + }, + } satisfies PatchGroupRecord + expect(getPatchInfo(patchedDependencies, 'foo', '1.0.0')).toStrictEqual(patchedDependencies.foo.exact['1.0.0']) + expect(getPatchInfo(patchedDependencies, 'foo', '1.1.0')).toBeUndefined() + expect(getPatchInfo(patchedDependencies, 'foo', '2.0.0')).toBeUndefined() + expect(getPatchInfo(patchedDependencies, 'bar', '1.0.0')).toBeUndefined() +}) + +test('getPatchInfo() returns a range version patch if the name matches and the version satisfied', () => { + const patchedDependencies = { + foo: { + exact: {}, + range: [{ + version: '1', + patch: { + file: { + path: 'patches/foo@1.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@1', + strict: true, + }, + }], + all: undefined, + }, + } satisfies PatchGroupRecord + expect(getPatchInfo(patchedDependencies, 'foo', '1.0.0')).toStrictEqual(patchedDependencies.foo.range[0].patch) + expect(getPatchInfo(patchedDependencies, 'foo', '1.1.0')).toStrictEqual(patchedDependencies.foo.range[0].patch) + expect(getPatchInfo(patchedDependencies, 'foo', '2.0.0')).toBeUndefined() + expect(getPatchInfo(patchedDependencies, 'bar', '1.0.0')).toBeUndefined() +}) + +test('getPatchInfo() returns name-only patch if the name matches', () => { + const patchedDependencies = { + foo: { + exact: {}, + range: [], + all: { + file: { + path: 'patches/foo.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo', + strict: true, + }, + }, + } satisfies PatchGroupRecord + expect(getPatchInfo(patchedDependencies, 'foo', '1.0.0')).toStrictEqual(patchedDependencies.foo.all) + expect(getPatchInfo(patchedDependencies, 'foo', '1.1.0')).toStrictEqual(patchedDependencies.foo.all) + expect(getPatchInfo(patchedDependencies, 'foo', '2.0.0')).toStrictEqual(patchedDependencies.foo.all) + expect(getPatchInfo(patchedDependencies, 'bar', '1.0.0')).toBeUndefined() +}) + +test('exact version patches override version range patches, version range patches override name-only patches', () => { + const patchedDependencies = { + foo: { + exact: { + '1.0.0': { + file: { + path: 'patches/foo@1.0.0.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@1.0.0', + strict: true, + }, + '1.1.0': { + file: { + path: 'patches/foo@1.1.0.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@1.1.0', + strict: true, + }, + }, + range: [ + { + version: '1', + patch: { + file: { + path: 'patches/foo@1.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@1', + strict: true, + }, + }, + { + version: '2', + patch: { + file: { + path: 'patches/foo@2.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@2', + strict: true, + }, + }, + ], + all: { + file: { + path: 'patches/foo.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo', + strict: true, + }, + }, + } satisfies PatchGroupRecord + expect(getPatchInfo(patchedDependencies, 'foo', '1.0.0')).toStrictEqual(patchedDependencies.foo.exact['1.0.0']) + expect(getPatchInfo(patchedDependencies, 'foo', '1.1.0')).toStrictEqual(patchedDependencies.foo.exact['1.1.0']) + expect(getPatchInfo(patchedDependencies, 'foo', '1.1.1')).toStrictEqual(patchedDependencies.foo.range[0].patch) + expect(getPatchInfo(patchedDependencies, 'foo', '2.0.0')).toStrictEqual(patchedDependencies.foo.range[1].patch) + expect(getPatchInfo(patchedDependencies, 'foo', '2.1.0')).toStrictEqual(patchedDependencies.foo.range[1].patch) + expect(getPatchInfo(patchedDependencies, 'foo', '3.0.0')).toStrictEqual(patchedDependencies.foo.all) + expect(getPatchInfo(patchedDependencies, 'bar', '1.0.0')).toBeUndefined() +}) + +test('getPatchInfo(_, name, version) throws an error when name@version matches more than one version range patches', () => { + const patchedDependencies = { + foo: { + exact: {}, + range: [ + { + version: '>=1.0.0 <3.0.0', + patch: { + file: { + path: 'patches/foo_a.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@>=1.0.0 <3.0.0', + strict: true, + }, + }, + { + version: '>=2.0.0', + patch: { + file: { + path: 'patches/foo_b.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@>=2.0.0', + strict: true, + }, + }, + ], + all: undefined, + }, + } satisfies PatchGroupRecord + expect(() => getPatchInfo(patchedDependencies, 'foo', '2.1.0')).toThrow(expect.objectContaining({ + code: 'ERR_PNPM_PATCH_KEY_CONFLICT', + message: 'Unable to choose between 2 version ranges to patch foo@2.1.0: >=1.0.0 <3.0.0, >=2.0.0', + hint: 'Explicitly set the exact version (foo@2.1.0) to resolve conflict', + })) +}) + +test('getPatchInfo(_, name, version) does not throw an error when name@version matches an exact version patch and more than one version range patches', () => { + const patchedDependencies = { + foo: { + exact: { + '2.1.0': { + file: { + path: 'patches/foo_a.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@>=1.0.0 <3.0.0', + strict: true, + }, + }, + range: [ + { + version: '>=1.0.0 <3.0.0', + patch: { + file: { + path: 'patches/foo_b.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@>=1.0.0 <3.0.0', + strict: true, + }, + }, + { + version: '>=2.0.0', + patch: { + file: { + path: 'patches/foo_c.patch', + hash: '00000000000000000000000000000000', + }, + key: 'foo@>=2.0.0', + strict: true, + }, + }, + ], + all: undefined, + }, + } satisfies PatchGroupRecord + expect(getPatchInfo(patchedDependencies, 'foo', '2.1.0')).toStrictEqual(patchedDependencies.foo.exact['2.1.0']) +}) diff --git a/patching/config/test/groupPatchedDependencies.test.ts b/patching/config/test/groupPatchedDependencies.test.ts new file mode 100644 index 0000000000..91d2d07087 --- /dev/null +++ b/patching/config/test/groupPatchedDependencies.test.ts @@ -0,0 +1,119 @@ +import { type ExtendedPatchInfo, type PatchFile, type PatchGroupRecord } from '@pnpm/patching.types' +import { groupPatchedDependencies } from '../src/groupPatchedDependencies' + +function sanitizePatchGroupRecord (patchGroups: PatchGroupRecord): PatchGroupRecord { + for (const name in patchGroups) { + patchGroups[name].range.sort((a, b) => a.version.localeCompare(b.version)) + } + return patchGroups +} + +const _groupPatchedDependencies: typeof groupPatchedDependencies = patchedDependencies => sanitizePatchGroupRecord(groupPatchedDependencies(patchedDependencies)) + +test('groups patchedDependencies according to names, match types, and versions', () => { + const patchedDependencies = { + 'exact-version-only@0.0.0': { + hash: '00000000000000000000000000000000', + path: 'patches/exact-version-only@2.10.patch', + }, + 'exact-version-only@1.2.3': { + hash: '00000000000000000000000000000000', + path: 'patches/exact-version-only@1.2.3.patch', + }, + 'exact-version-only@2.1.0': { + hash: '00000000000000000000000000000000', + path: 'patches/exact-version-only@2.10.patch', + }, + 'version-range-only@~1.2.0': { + hash: '00000000000000000000000000000000', + path: 'patches/version-range-only@~1.2.0.patch', + }, + 'version-range-only@4': { + hash: '00000000000000000000000000000000', + path: 'patches/version-range-only@4.patch', + }, + 'star-version-range@*': { + hash: '00000000000000000000000000000000', + path: 'patches/star-version-range.patch', + }, + 'without-versions': { + hash: '00000000000000000000000000000000', + path: 'patches/without-versions.patch', + }, + 'mixed-style@0.1.2': { + hash: '00000000000000000000000000000000', + path: 'patches/mixed-style@0.1.2.patch', + }, + 'mixed-style@1.x.x': { + hash: '00000000000000000000000000000000', + path: 'patches/mixed-style@1.x.x.patch', + }, + 'mixed-style': { + hash: '00000000000000000000000000000000', + path: 'patches/mixed-style.patch', + }, + } satisfies Record + const info = (strict: boolean, key: keyof typeof patchedDependencies): ExtendedPatchInfo => ({ + strict, + 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'), + }, + range: [], + all: undefined, + }, + 'version-range-only': { + exact: {}, + range: [ + { + version: '~1.2.0', + patch: info(true, 'version-range-only@~1.2.0'), + }, + { + version: '4', + patch: info(true, 'version-range-only@4'), + }, + ], + all: undefined, + }, + 'star-version-range': { + exact: {}, + range: [], + all: info(true, 'star-version-range@*'), + }, + 'without-versions': { + exact: {}, + range: [], + all: info(false, 'without-versions'), + }, + 'mixed-style': { + exact: { + '0.1.2': info(true, 'mixed-style@0.1.2'), + }, + range: [ + { + version: '1.x.x', + patch: info(true, 'mixed-style@1.x.x'), + }, + ], + all: info(false, 'mixed-style'), + }, + } as PatchGroupRecord) +}) + +test('errors on invalid version range', async () => { + expect(() => _groupPatchedDependencies({ + 'foo@link:packages/foo': { + hash: '00000000000000000000000000000000', + path: 'patches/foo.patch', + }, + })).toThrow(expect.objectContaining({ + code: 'ERR_PNPM_PATCH_NON_SEMVER_RANGE', + })) +}) diff --git a/patching/config/test/index.test.ts b/patching/config/test/index.test.ts index 1dfb723dd6..23d9380164 100644 --- a/patching/config/test/index.test.ts +++ b/patching/config/test/index.test.ts @@ -1,11 +1,15 @@ -import { getPatchInfo } from '../src/index' +import { type PatchFile } from '@pnpm/patching.types' +import { getPatchInfo, groupPatchedDependencies } from '../src/index' + +const _getPatchInfo = (patchedDependencies: Record, name: string, version: string) => + getPatchInfo(groupPatchedDependencies(patchedDependencies), name, version) test('getPatchInfo(undefined, ...) returns undefined', () => { expect(getPatchInfo(undefined, 'foo', '1.0.0')).toBeUndefined() }) test('getPatchInfo(_, name, version) returns strict=true if name@version exists', () => { - expect(getPatchInfo({ + expect(_getPatchInfo({ 'foo@1.0.0': { path: 'patches/foo@1.0.0.patch', hash: '00000000000000000000000000000000', @@ -21,7 +25,7 @@ test('getPatchInfo(_, name, version) returns strict=true if name@version exists' }) test('getPatchInfo(_, name, version) returns strict=false if name exists and name@version does not exist', () => { - expect(getPatchInfo({ + expect(_getPatchInfo({ foo: { path: 'patches/foo.patch', hash: '00000000000000000000000000000000', @@ -37,7 +41,7 @@ test('getPatchInfo(_, name, version) returns strict=false if name exists and nam }) test('getPatchInfo(_, name, version) prioritizes name@version over name if both exist', () => { - expect(getPatchInfo({ + expect(_getPatchInfo({ foo: { path: 'patches/foo.patch', hash: '00000000000000000000000000000000', @@ -57,7 +61,7 @@ test('getPatchInfo(_, name, version) prioritizes name@version over name if both }) test('getPatchInfo(_, name, version) does not access wrong name', () => { - expect(getPatchInfo({ + expect(_getPatchInfo({ 'bar@1.0.0': { path: 'patches/bar@1.0.0.patch', hash: '00000000000000000000000000000000', @@ -66,7 +70,7 @@ test('getPatchInfo(_, name, version) does not access wrong name', () => { }) test('getPatchInfo(_, name, version) does not access wrong version', () => { - expect(getPatchInfo({ + expect(_getPatchInfo({ 'foo@2.0.0': { path: 'patches/foo@2.0.0.patch', hash: '00000000000000000000000000000000', diff --git a/patching/config/tsconfig.json b/patching/config/tsconfig.json index 64c25a32e3..90e70b4fee 100644 --- a/patching/config/tsconfig.json +++ b/patching/config/tsconfig.json @@ -9,6 +9,12 @@ "../../__typings__/**/*.d.ts" ], "references": [ + { + "path": "../../packages/dependency-path" + }, + { + "path": "../../packages/error" + }, { "path": "../types" } diff --git a/patching/types/src/index.ts b/patching/types/src/index.ts index 0614c9185d..08ef4d1328 100644 --- a/patching/types/src/index.ts +++ b/patching/types/src/index.ts @@ -3,7 +3,30 @@ export interface PatchFile { hash: string } +// TODO: replace all occurrences of PatchInfo with PatchFile before the next major version is released export interface PatchInfo { strict: boolean file: PatchFile } + +export interface ExtendedPatchInfo extends PatchInfo { + key: string +} + +export interface PatchGroupRangeItem { + version: string + patch: ExtendedPatchInfo +} + +/** A group of {@link ExtendedPatchInfo}s which correspond to a package name. */ +export interface PatchGroup { + /** Maps exact versions to {@link ExtendedPatchInfo}. */ + exact: Record + /** Pairs of version ranges and {@link ExtendedPatchInfo}. */ + range: PatchGroupRangeItem[] + /** The {@link ExtendedPatchInfo} without exact versions or version ranges. */ + all: ExtendedPatchInfo | undefined +} + +/** Maps package names to their corresponding groups. */ +export type PatchGroupRecord = Record diff --git a/pkg-manager/core/package.json b/pkg-manager/core/package.json index 589301d47e..b460d89147 100644 --- a/pkg-manager/core/package.json +++ b/pkg-manager/core/package.json @@ -92,6 +92,7 @@ "@pnpm/package-requester": "workspace:*", "@pnpm/parse-overrides": "workspace:*", "@pnpm/parse-wanted-dependency": "workspace:*", + "@pnpm/patching.config": "workspace:*", "@pnpm/pkg-manager.direct-dep-linker": "workspace:*", "@pnpm/read-modules-dir": "workspace:*", "@pnpm/read-project-manifest": "workspace:*", diff --git a/pkg-manager/core/src/getPeerDependencyIssues.ts b/pkg-manager/core/src/getPeerDependencyIssues.ts index f02711b152..79964f81ca 100644 --- a/pkg-manager/core/src/getPeerDependencyIssues.ts +++ b/pkg-manager/core/src/getPeerDependencyIssues.ts @@ -61,7 +61,7 @@ export async function getPeerDependencyIssues ( { currentLockfile: ctx.currentLockfile, allowedDeprecatedVersions: {}, - allowNonAppliedPatches: false, + allowUnusedPatches: false, catalogs: opts.catalogs, defaultUpdateDepth: -1, dedupePeerDependents: opts.dedupePeerDependents, diff --git a/pkg-manager/core/src/install/extendInstallOptions.ts b/pkg-manager/core/src/install/extendInstallOptions.ts index 60d0a84498..94418b77b2 100644 --- a/pkg-manager/core/src/install/extendInstallOptions.ts +++ b/pkg-manager/core/src/install/extendInstallOptions.ts @@ -112,7 +112,8 @@ export interface StrictInstallOptions { enableModulesDir: boolean modulesCacheMaxAge: number allowedDeprecatedVersions: AllowedDeprecatedVersions - allowNonAppliedPatches: boolean + ignorePatchFailures?: boolean + allowUnusedPatches: boolean preferSymlinkedExecutables: boolean resolutionMode: 'highest' | 'time-based' | 'lowest-direct' resolvePeersFromWorkspaceRoot: boolean @@ -169,7 +170,8 @@ const defaults = (opts: InstallOptions): StrictInstallOptions => { } return { allowedDeprecatedVersions: {}, - allowNonAppliedPatches: false, + allowUnusedPatches: false, + ignorePatchFailures: undefined, autoInstallPeers: true, autoInstallPeersFromHighestMatch: false, childConcurrency: 5, diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index 0056c35c38..a9c9cd3b42 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -36,7 +36,6 @@ import { writeLockfiles, writeWantedLockfile, cleanGitBranchLockfiles, - type PatchFile, type CatalogSnapshots, } from '@pnpm/lockfile.fs' import { writePnpFile } from '@pnpm/lockfile-to-pnp' @@ -46,6 +45,7 @@ import { getPreferredVersionsFromLockfileAndManifests } from '@pnpm/lockfile.pre import { logger, globalInfo, streamParser } from '@pnpm/logger' import { getAllDependenciesFromManifest, getAllUniqueSpecs } from '@pnpm/manifest-utils' import { writeModulesManifest } from '@pnpm/modules-yaml' +import { type PatchGroupRecord, groupPatchedDependencies } from '@pnpm/patching.config' import { safeReadProjectManifestOnly } from '@pnpm/read-project-manifest' import { getWantedDependencies, @@ -367,6 +367,7 @@ export async function mutateModules ( path: path.join(opts.lockfileDir, patchFile.path), }), patchedDependencies) : undefined + const patchGroups = patchedDependenciesWithResolvedPath && groupPatchedDependencies(patchedDependenciesWithResolvedPath) const frozenLockfile = opts.frozenLockfile || opts.frozenLockfileIfExists && ctx.existsNonEmptyWantedLockfile let outdatedLockfileSettings = false @@ -418,7 +419,7 @@ export async function mutateModules ( const frozenInstallResult = await tryFrozenInstall({ frozenLockfile, needsFullResolution, - patchedDependenciesWithResolvedPath, + patchGroups, upToDateLockfileMajorVersion, }) if (frozenInstallResult !== null) { @@ -543,7 +544,7 @@ export async function mutateModules ( pruneVirtualStore, scriptsOpts, updateLockfileMinorVersion: true, - patchedDependencies: patchedDependenciesWithResolvedPath, + patchedDependencies: patchGroups, }) return { @@ -582,12 +583,12 @@ export async function mutateModules ( async function tryFrozenInstall ({ frozenLockfile, needsFullResolution, - patchedDependenciesWithResolvedPath, + patchGroups, upToDateLockfileMajorVersion, }: { frozenLockfile: boolean needsFullResolution: boolean - patchedDependenciesWithResolvedPath?: Record + patchGroups?: PatchGroupRecord upToDateLockfileMajorVersion: boolean }): Promise { const isFrozenInstallPossible = @@ -692,7 +693,7 @@ Note that in CI environments, this setting is enabled by default.`, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, currentHoistedLocations: ctx.modulesFile?.hoistedLocations, - patchedDependencies: patchedDependenciesWithResolvedPath, + patchedDependencies: patchGroups, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, prunedAt: ctx.modulesFile?.prunedAt, @@ -919,7 +920,7 @@ type InstallFunction = ( projects: ImporterToUpdate[], ctx: PnpmContext, opts: Omit & { - patchedDependencies?: Record + patchedDependencies?: PatchGroupRecord makePartialCurrentLockfile: boolean needsFullResolution: boolean neverBuiltDependencies?: string[] @@ -1031,7 +1032,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { projects, { allowedDeprecatedVersions: opts.allowedDeprecatedVersions, - allowNonAppliedPatches: opts.allowNonAppliedPatches, + allowUnusedPatches: opts.allowUnusedPatches, autoInstallPeers: opts.autoInstallPeers, autoInstallPeersFromHighestMatch: opts.autoInstallPeersFromHighestMatch, catalogs: opts.catalogs, @@ -1203,6 +1204,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { } ignoredBuilds = (await buildModules(dependenciesGraph, rootNodes, { allowBuild, + ignorePatchFailures: opts.ignorePatchFailures, ignoredBuiltDependencies: opts.ignoredBuiltDependencies, childConcurrency: opts.childConcurrency, depsStateCache, diff --git a/pkg-manager/core/test/install/patch.ts b/pkg-manager/core/test/install/patch.ts index 44329cc689..32f6d055ee 100644 --- a/pkg-manager/core/test/install/patch.ts +++ b/pkg-manager/core/test/install/patch.ts @@ -14,7 +14,7 @@ import { testDefaults } from '../utils' const f = fixtures(__dirname) -test('patch package', async () => { +test('patch package with exact version', async () => { const reporter = sinon.spy() const project = prepareEmpty() const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') @@ -109,7 +109,102 @@ test('patch package', async () => { expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') }) -test('patch package reports warning if not all patches are applied and allowNonAppliedPatches is set', async () => { +test('patch package with version range', async () => { + const reporter = sinon.spy() + const project = prepareEmpty() + const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') + + const patchedDependencies = { + 'is-positive@1': patchPath, + } + const opts = testDefaults({ + neverBuiltDependencies: undefined, + onlyBuiltDependencies: [], + fastUnpack: false, + sideEffectsCacheRead: true, + sideEffectsCacheWrite: true, + patchedDependencies, + reporter, + }, {}, {}, { packageImportMethod: 'hardlink' }) + await install({ + dependencies: { + 'is-positive': '1.0.0', + }, + }, opts) + + expect(reporter.calledWithMatch({ + packageNames: [], + level: 'debug', + name: 'pnpm:ignored-scripts', + } as IgnoredScriptsLog)).toBeTruthy() + + expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// patched') + + const patchFileHash = await createHexHashFromFile(patchPath) + const lockfile = project.readLockfile() + expect(lockfile.patchedDependencies).toStrictEqual({ + 'is-positive@1': { + path: path.relative(process.cwd(), patchedDependencies['is-positive@1']).replaceAll('\\', '/'), + hash: patchFileHash, + }, + }) + expect(lockfile.snapshots[`is-positive@1.0.0(patch_hash=${patchFileHash})`]).toBeTruthy() + + const filesIndexFile = path.join(opts.storeDir, 'index/c7/1ccf199e0fdae37aad13946b937d67bcd35fa111b84d21b3a19439cfdc2812-is-positive@1.0.0.json') + const filesIndex = loadJsonFile.sync(filesIndexFile) + const sideEffectsKey = `${ENGINE_NAME};patch=${patchFileHash}` + const patchedFileIntegrity = filesIndex.sideEffects?.[sideEffectsKey].added?.['index.js']?.integrity + expect(patchedFileIntegrity).toBeTruthy() + const originalFileIntegrity = filesIndex.files['index.js'].integrity + expect(originalFileIntegrity).toBeTruthy() + // The integrity of the original file differs from the integrity of the patched file + expect(originalFileIntegrity).not.toEqual(patchedFileIntegrity) + + // The same with frozen lockfile + rimraf('node_modules') + await install({ + dependencies: { + 'is-positive': '1.0.0', + }, + }, { + ...opts, + frozenLockfile: true, + }) + expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// patched') + + // The same with frozen lockfile and hoisted node_modules + rimraf('node_modules') + await install({ + dependencies: { + 'is-positive': '1.0.0', + }, + }, { + ...opts, + frozenLockfile: true, + nodeLinker: 'hoisted', + }) + expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// patched') + + process.chdir('..') + fs.mkdirSync('project2') + process.chdir('project2') + + await install({ + dependencies: { + 'is-positive': '1.0.0', + }, + }, testDefaults({ + fastUnpack: false, + sideEffectsCacheRead: true, + sideEffectsCacheWrite: true, + offline: true, + }, {}, {}, { packageImportMethod: 'hardlink' })) + + // The original file did not break, when a patched version was created + expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') +}) + +test('patch package reports warning if not all patches are applied and allowUnusedPatches is set', async () => { prepareEmpty() const reporter = jest.fn() const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') @@ -123,7 +218,7 @@ test('patch package reports warning if not all patches are applied and allowNonA sideEffectsCacheRead: true, sideEffectsCacheWrite: true, patchedDependencies, - allowNonAppliedPatches: true, + allowUnusedPatches: true, reporter, }, {}, {}, { packageImportMethod: 'hardlink' }) await install({ @@ -134,7 +229,7 @@ test('patch package reports warning if not all patches are applied and allowNonA expect(reporter).toHaveBeenCalledWith( expect.objectContaining({ level: 'warn', - message: 'The following patches were not applied: is-negative@1.0.0', + message: 'The following patches were not used: is-negative@1.0.0', }) ) }) @@ -159,7 +254,7 @@ test('patch package throws an exception if not all patches are applied', async ( 'is-positive': '1.0.0', }, }, opts) - ).rejects.toThrow('The following patches were not applied: is-negative@1.0.0') + ).rejects.toThrow('The following patches were not used: is-negative@1.0.0') }) test('the patched package is updated if the patch is modified', async () => { @@ -399,7 +494,7 @@ test('patch package when the patched package has no dependencies and appears mul ].sort()) }) -test('patch package should fail when the patch could not be applied', async () => { +test('patch package should fail when the exact version patch fails to apply', async () => { prepareEmpty() const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') @@ -420,3 +515,103 @@ test('patch package should fail when the patch could not be applied', async () = expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') }) + +test('patch package should fail when the version range patch fails to apply', async () => { + prepareEmpty() + const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') + + const patchedDependencies = { + 'is-positive@>=3': patchPath, + } + const opts = testDefaults({ + fastUnpack: false, + sideEffectsCacheRead: true, + sideEffectsCacheWrite: true, + patchedDependencies, + }, {}, {}, { packageImportMethod: 'hardlink' }) + await expect(install({ + dependencies: { + 'is-positive': '3.1.0', + }, + }, opts)).rejects.toThrow(/Could not apply patch/) + + 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 () => { + prepareEmpty() + const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') + + const patchedDependencies = { + 'is-positive': patchPath, + } + const opts = testDefaults({ + fastUnpack: false, + ignorePatchFailures: false, + sideEffectsCacheRead: true, + sideEffectsCacheWrite: true, + patchedDependencies, + }, {}, {}, { packageImportMethod: 'hardlink' }) + await expect(install({ + dependencies: { + 'is-positive': '3.1.0', + }, + }, opts)).rejects.toThrow(/Could not apply patch/) + + expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).not.toContain('// patched') +}) diff --git a/pkg-manager/core/tsconfig.json b/pkg-manager/core/tsconfig.json index e866619939..c92a6c8b75 100644 --- a/pkg-manager/core/tsconfig.json +++ b/pkg-manager/core/tsconfig.json @@ -126,6 +126,9 @@ { "path": "../../packages/which-version-is-pinned" }, + { + "path": "../../patching/config" + }, { "path": "../../pkg-manifest/manifest-utils" }, diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index 49d494cdee..09c365e0c8 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -28,7 +28,6 @@ import { linkBins, linkBinsOfPackages } from '@pnpm/link-bins' import { getLockfileImporterId, type LockfileObject, - type PatchFile, readCurrentLockfile, readWantedLockfile, writeLockfiles, @@ -50,6 +49,7 @@ import { type Modules, writeModulesManifest, } from '@pnpm/modules-yaml' +import { type PatchGroupRecord } from '@pnpm/patching.config' import { type HoistingLimits } from '@pnpm/real-hoist' import { readPackageJsonFromDir } from '@pnpm/read-package-json' import { readProjectManifestOnly, safeReadProjectManifestOnly } from '@pnpm/read-project-manifest' @@ -107,6 +107,7 @@ export interface Project { } export interface HeadlessOptions { + ignorePatchFailures?: boolean neverBuiltDependencies?: string[] ignoredBuiltDependencies?: string[] onlyBuiltDependencies?: string[] @@ -143,7 +144,7 @@ export interface HeadlessOptions { modulesDir?: string virtualStoreDir?: string virtualStoreDirMaxLength: number - patchedDependencies?: Record + patchedDependencies?: PatchGroupRecord scriptsPrependNodePath?: boolean | 'warn-only' scriptShell?: string shellEmulator?: boolean @@ -531,6 +532,7 @@ export async function headlessInstall (opts: HeadlessOptions): Promise + patchedDependencies?: PatchGroupRecord sideEffectsCacheRead: boolean skipped: Set storeController: StoreController diff --git a/pkg-manager/resolve-dependencies/src/index.ts b/pkg-manager/resolve-dependencies/src/index.ts index 8ee2340930..5c5b0688b3 100644 --- a/pkg-manager/resolve-dependencies/src/index.ts +++ b/pkg-manager/resolve-dependencies/src/index.ts @@ -1,9 +1,7 @@ import path from 'path' -import { PnpmError } from '@pnpm/error' import { packageManifestLogger, } from '@pnpm/core-loggers' -import { globalWarn } from '@pnpm/logger' import { type LockfileObject, type ProjectSnapshot, @@ -13,6 +11,7 @@ import { getSpecFromPackageManifest, type PinnedVersion, } from '@pnpm/manifest-utils' +import { verifyPatches } from '@pnpm/patching.config' import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json' import { type DependenciesField, @@ -116,7 +115,7 @@ export async function resolveDependencies ( preserveWorkspaceProtocol: boolean saveWorkspaceProtocol: 'rolling' | boolean lockfileIncludeTarballUrl?: boolean - allowNonAppliedPatches?: boolean + allowUnusedPatches?: boolean } ): Promise { const _toResolveImporter = toResolveImporter.bind(null, { @@ -148,9 +147,9 @@ export async function resolveDependencies ( Object.keys(opts.wantedLockfile.importers).length === importers.length ) { verifyPatches({ - patchedDependencies: Object.keys(opts.patchedDependencies), + patchedDependencies: opts.patchedDependencies, appliedPatches, - allowNonAppliedPatches: opts.allowNonAppliedPatches, + allowUnusedPatches: opts.allowUnusedPatches, }) } @@ -328,29 +327,6 @@ export async function resolveDependencies ( } } -function verifyPatches ( - { - patchedDependencies, - appliedPatches, - allowNonAppliedPatches, - }: { - patchedDependencies: string[] - appliedPatches: Set - allowNonAppliedPatches: boolean - } -): void { - const nonAppliedPatches: string[] = patchedDependencies.filter((patchKey) => !appliedPatches.has(patchKey)) - if (!nonAppliedPatches.length) return - const message = `The following patches were not applied: ${nonAppliedPatches.join(', ')}` - if (allowNonAppliedPatches) { - globalWarn(message) - return - } - throw new PnpmError('PATCH_NOT_APPLIED', message, { - hint: 'Either remove them from "patchedDependencies" or update them to match packages in your dependencies.', - }) -} - function addDirectDependenciesToLockfile ( newManifest: ProjectManifest, projectSnapshot: ProjectSnapshot, diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index 0d25293efc..e616fb2840 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -16,7 +16,7 @@ import { pkgSnapshotToResolution, } from '@pnpm/lockfile.utils' import { logger } from '@pnpm/logger' -import { getPatchInfo } from '@pnpm/patching.config' +import { type PatchGroupRecord, getPatchInfo } from '@pnpm/patching.config' import { pickRegistryForPackage } from '@pnpm/pick-registry-for-package' import { type DirectoryResolution, @@ -42,7 +42,7 @@ import { } from '@pnpm/types' import * as dp from '@pnpm/dependency-path' import { getPreferredVersionsFromLockfileAndManifests } from '@pnpm/lockfile.preferred-versions' -import { type PatchFile, type PatchInfo } from '@pnpm/patching.types' +import { type PatchInfo } from '@pnpm/patching.types' import normalizePath from 'normalize-path' import exists from 'path-exists' import pDefer from 'p-defer' @@ -148,7 +148,7 @@ export interface ResolutionContext { resolvedPkgsById: ResolvedPkgsById outdatedDependencies: Record childrenByParentId: ChildrenByParentId - patchedDependencies?: Record + patchedDependencies?: PatchGroupRecord pendingNodes: PendingNode[] wantedLockfile: LockfileObject currentLockfile: LockfileObject diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts index 723a9b0408..7457f380de 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts @@ -1,7 +1,7 @@ import { resolveFromCatalog } from '@pnpm/catalogs.resolver' import { type Catalogs } from '@pnpm/catalogs.types' import { type LockfileObject } from '@pnpm/lockfile.types' -import { type PatchFile } from '@pnpm/patching.types' +import { type PatchGroupRecord } from '@pnpm/patching.config' import { type PreferredVersions, type Resolution, type WorkspacePackages } from '@pnpm/resolver-base' import { type StoreController } from '@pnpm/store-controller-types' import { @@ -98,7 +98,7 @@ export interface ResolveDependenciesOptions { autoInstallPeers?: boolean autoInstallPeersFromHighestMatch?: boolean allowedDeprecatedVersions: AllowedDeprecatedVersions - allowNonAppliedPatches: boolean + allowUnusedPatches: boolean catalogs?: Catalogs currentLockfile: LockfileObject dedupePeerDependents?: boolean @@ -112,7 +112,7 @@ export interface ResolveDependenciesOptions { } nodeVersion?: string registries: Registries - patchedDependencies?: Record + patchedDependencies?: PatchGroupRecord pnpmVersion: string preferredVersions?: PreferredVersions preferWorkspacePackages?: boolean diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1eac5170c9..3bfa39b51c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1460,6 +1460,9 @@ importers: '@pnpm/git-utils': specifier: workspace:* version: link:../../packages/git-utils + '@pnpm/logger': + specifier: '>=5.1.0 <1001.0.0' + version: 1000.0.0 '@pnpm/matcher': specifier: workspace:* version: link:../matcher @@ -4150,13 +4153,28 @@ importers: patching/config: dependencies: + '@pnpm/dependency-path': + specifier: workspace:* + version: link:../../packages/dependency-path + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error + '@pnpm/logger': + specifier: '>=5.1.0 <1001.0.0' + version: 1000.0.0 '@pnpm/patching.types': specifier: workspace:* version: link:../types + semver: + specifier: 'catalog:' + version: 7.7.1 devDependencies: '@pnpm/patching.config': specifier: workspace:* version: 'link:' + '@types/semver': + specifier: 'catalog:' + version: 7.5.3 patching/plugin-commands-patching: dependencies: @@ -4456,6 +4474,9 @@ importers: '@pnpm/parse-wanted-dependency': specifier: workspace:* version: link:../../packages/parse-wanted-dependency + '@pnpm/patching.config': + specifier: workspace:* + version: link:../../patching/config '@pnpm/pkg-manager.direct-dep-linker': specifier: workspace:* version: link:../direct-dep-linker diff --git a/pnpm/test/__fixtures__/patch-pkg/is-positive@1.0.0.patch b/pnpm/test/__fixtures__/patch-pkg/is-positive@1.0.0.patch new file mode 100644 index 0000000000..53bee08fdd --- /dev/null +++ b/pnpm/test/__fixtures__/patch-pkg/is-positive@1.0.0.patch @@ -0,0 +1,11 @@ +diff --git a/index.js b/index.js +index 8e020ca..ff3aee4 100644 +--- a/index.js ++++ b/index.js +@@ -5,5 +5,6 @@ module.exports = function (n) { + throw new TypeError('Expected a number'); + } + ++ // patched + return n >= 0; + }; diff --git a/pnpm/test/patch/allowUnusedPatches.ts b/pnpm/test/patch/allowUnusedPatches.ts new file mode 100644 index 0000000000..275380b34e --- /dev/null +++ b/pnpm/test/patch/allowUnusedPatches.ts @@ -0,0 +1,68 @@ +import { preparePackages } from '@pnpm/prepare' +import { fixtures } from '@pnpm/test-fixtures' +import { sync as writeYamlFile } from 'write-yaml-file' +import { execPnpmSync } from '../utils' + +const f = fixtures(__dirname) + +test('allowUnusedPatches=false errors on unused patches', async () => { + preparePackages([ + { + name: 'foo', + version: '0.0.0', + private: true, + }, + { + name: 'bar', + version: '0.0.0', + private: true, + }, + ]) + + const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') + + writeYamlFile('pnpm-workspace.yaml', { + allowUnusedPatches: false, + 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_UNUSED_PATCH') + expect(stdout.toString()).toContain('The following patches were not used: is-positive') +}) + +test('allowUnusedPatches=true warns about unused patches', async () => { + preparePackages([ + { + name: 'foo', + version: '0.0.0', + private: true, + }, + { + name: 'bar', + version: '0.0.0', + private: true, + }, + ]) + + const patchFile = f.find('patch-pkg/is-positive@1.0.0.patch') + + writeYamlFile('pnpm-workspace.yaml', { + allowUnusedPatches: true, + packages: ['**', '!store/**'], + patchedDependencies: { + 'is-positive': patchFile, + }, + }) + + // pnpm install should not fail + const { stdout } = execPnpmSync(['install'], { expectSuccess: true }) + + // pnpm install should print a warning regarding unused patches + expect(stdout.toString()).toContain('The following patches were not used: is-positive') +}) diff --git a/pnpm/test/patch/ignorePatchFailures.ts b/pnpm/test/patch/ignorePatchFailures.ts new file mode 100644 index 0000000000..87b9d838b9 --- /dev/null +++ b/pnpm/test/patch/ignorePatchFailures.ts @@ -0,0 +1,500 @@ +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' + +const f = fixtures(__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')]) + }) +})