From 22f4d7265fcd9a62e1130c76b7c61010203c3868 Mon Sep 17 00:00:00 2001 From: larrybahr-ocelot Date: Tue, 13 Sep 2022 16:54:32 -0500 Subject: [PATCH 1/5] feat(patch): allow non-applied patches add flag to make non-applied patches a warning instead of a fatal error close #5234 --- .changeset/angry-hounds-bow.md | 8 +++++++ packages/core/src/getPeerDependencyIssues.ts | 1 + .../core/src/install/extendInstallOptions.ts | 2 ++ packages/core/src/install/index.ts | 1 + packages/core/test/install/patch.ts | 22 +++++++++++++++++++ .../src/getOptionsFromRootManifest.ts | 3 +++ packages/resolve-dependencies/src/index.ts | 21 +++++++++++++----- .../src/resolveDependencyTree.ts | 1 + packages/types/src/package.ts | 1 + 9 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 .changeset/angry-hounds-bow.md diff --git a/.changeset/angry-hounds-bow.md b/.changeset/angry-hounds-bow.md new file mode 100644 index 00000000000..0bed19434b5 --- /dev/null +++ b/.changeset/angry-hounds-bow.md @@ -0,0 +1,8 @@ +--- +"@pnpm/core": minor +"@pnpm/plugin-commands-installation": minor +"@pnpm/resolve-dependencies": minor +"@pnpm/types": minor +--- + +add flag to make non-applied patches a warning instead of a fatal error diff --git a/packages/core/src/getPeerDependencyIssues.ts b/packages/core/src/getPeerDependencyIssues.ts index 2fb95a9ad2e..db545de53c9 100644 --- a/packages/core/src/getPeerDependencyIssues.ts +++ b/packages/core/src/getPeerDependencyIssues.ts @@ -51,6 +51,7 @@ export async function getPeerDependencyIssues ( { currentLockfile: ctx.currentLockfile, allowedDeprecatedVersions: {}, + allowNonAppliedPatches: false, defaultUpdateDepth: -1, dryRun: true, engineStrict: false, diff --git a/packages/core/src/install/extendInstallOptions.ts b/packages/core/src/install/extendInstallOptions.ts index c3f27191e48..b1d29a5b037 100644 --- a/packages/core/src/install/extendInstallOptions.ts +++ b/packages/core/src/install/extendInstallOptions.ts @@ -98,6 +98,7 @@ export interface StrictInstallOptions { modulesCacheMaxAge: number peerDependencyRules: PeerDependencyRules allowedDeprecatedVersions: AllowedDeprecatedVersions + allowNonAppliedPatches: boolean preferSymlinkedExecutables: boolean resolutionMode: 'highest' | 'time-based' @@ -124,6 +125,7 @@ const defaults = async (opts: InstallOptions) => { } return { allowedDeprecatedVersions: {}, + allowNonAppliedPatches: false, autoInstallPeers: false, childConcurrency: 5, depth: 0, diff --git a/packages/core/src/install/index.ts b/packages/core/src/install/index.ts index 57e8766b6da..153a2f77421 100644 --- a/packages/core/src/install/index.ts +++ b/packages/core/src/install/index.ts @@ -819,6 +819,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { { allowBuild: createAllowBuildFunction(opts), allowedDeprecatedVersions: opts.allowedDeprecatedVersions, + allowNonAppliedPatches: opts.allowNonAppliedPatches, autoInstallPeers: opts.autoInstallPeers, currentLockfile: ctx.currentLockfile, defaultUpdateDepth: (opts.update || (opts.updateMatching != null)) ? opts.depth : -1, diff --git a/packages/core/test/install/patch.ts b/packages/core/test/install/patch.ts index aacbd9121ac..407dd860229 100644 --- a/packages/core/test/install/patch.ts +++ b/packages/core/test/install/patch.ts @@ -96,6 +96,28 @@ 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 () => { + prepareEmpty() + const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') + + const patchedDependencies = { + 'is-positive@1.0.0': path.relative(process.cwd(), patchPath), + 'is-negative@1.0.0': path.relative(process.cwd(), patchPath), + } + const opts = await testDefaults({ + fastUnpack: false, + sideEffectsCacheRead: true, + sideEffectsCacheWrite: true, + patchedDependencies, + allowNonAppliedPatches: true, + }, {}, {}, { packageImportMethod: 'hardlink' }) + await install({ + dependencies: { + 'is-positive': '1.0.0', + }, + }, opts) +}) + test('patch package throws an exception if not all patches are applied', async () => { prepareEmpty() const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') diff --git a/packages/plugin-commands-installation/src/getOptionsFromRootManifest.ts b/packages/plugin-commands-installation/src/getOptionsFromRootManifest.ts index 7239f1d17d6..dc82ec70d30 100644 --- a/packages/plugin-commands-installation/src/getOptionsFromRootManifest.ts +++ b/packages/plugin-commands-installation/src/getOptionsFromRootManifest.ts @@ -7,6 +7,7 @@ import { export default function getOptionsFromRootManifest (manifest: ProjectManifest): { allowedDeprecatedVersions?: AllowedDeprecatedVersions + allowNonAppliedPatches?: boolean overrides?: Record neverBuiltDependencies?: string[] onlyBuiltDependencies?: string[] @@ -23,9 +24,11 @@ export default function getOptionsFromRootManifest (manifest: ProjectManifest): const packageExtensions = manifest.pnpm?.packageExtensions const peerDependencyRules = manifest.pnpm?.peerDependencyRules const allowedDeprecatedVersions = manifest.pnpm?.allowedDeprecatedVersions + const allowNonAppliedPatches = manifest.pnpm?.allowNonAppliedPatches const patchedDependencies = manifest.pnpm?.patchedDependencies const settings = { allowedDeprecatedVersions, + allowNonAppliedPatches, overrides, neverBuiltDependencies, packageExtensions, diff --git a/packages/resolve-dependencies/src/index.ts b/packages/resolve-dependencies/src/index.ts index 8daac805a9a..93fa7cea614 100644 --- a/packages/resolve-dependencies/src/index.ts +++ b/packages/resolve-dependencies/src/index.ts @@ -4,6 +4,7 @@ import PnpmError from '@pnpm/error' import { packageManifestLogger, } from '@pnpm/core-loggers' +import logger from '@pnpm/logger' import { Lockfile, ProjectSnapshot, @@ -83,6 +84,7 @@ export default async function ( preserveWorkspaceProtocol: boolean saveWorkspaceProtocol: 'rolling' | boolean lockfileIncludeTarballUrl?: boolean + allowNonAppliedPatches?: boolean } ) { const _toResolveImporter = toResolveImporter.bind(null, { @@ -110,7 +112,7 @@ export default async function ( (opts.forceFullResolution || !opts.wantedLockfile.packages?.length) && Object.keys(opts.wantedLockfile.importers).length === importers.length ) { - verifyPatches(Object.keys(opts.patchedDependencies), appliedPatches) + verifyPatches(Object.keys(opts.patchedDependencies), appliedPatches, opts.allowNonAppliedPatches) } const linkedDependenciesByProjectId: Record = {} @@ -266,12 +268,21 @@ export default async function ( } } -function verifyPatches (patchedDependencies: string[], appliedPatches: Set) { +function verifyPatches (patchedDependencies: string[], appliedPatches: Set, allowNonAppliedPatches: boolean = false) { const nonAppliedPatches: string[] = patchedDependencies.filter((patchKey) => !appliedPatches.has(patchKey)) if (nonAppliedPatches.length) { - throw new PnpmError('PATCH_NOT_APPLIED', `The following patches were not applied: ${nonAppliedPatches.join(', ')}`, { - hint: 'Either remove them from "patchedDependencies" or update them to match packages in your dependencies.', - }) + const code = 'PATCH_NOT_APPLIED' + const message = `The following patches were not applied: ${nonAppliedPatches.join(', ')}` + if (allowNonAppliedPatches) { + logger.warn({ + prefix: code, + message, + }) + } else { + throw new PnpmError(code, message, { + hint: 'Either remove them from "patchedDependencies" or update them to match packages in your dependencies.', + }) + } } } diff --git a/packages/resolve-dependencies/src/resolveDependencyTree.ts b/packages/resolve-dependencies/src/resolveDependencyTree.ts index f9a1d332497..758389b07ad 100644 --- a/packages/resolve-dependencies/src/resolveDependencyTree.ts +++ b/packages/resolve-dependencies/src/resolveDependencyTree.ts @@ -62,6 +62,7 @@ export interface ResolveDependenciesOptions { autoInstallPeers?: boolean allowBuild?: (pkgName: string) => boolean allowedDeprecatedVersions: AllowedDeprecatedVersions + allowNonAppliedPatches: boolean currentLockfile: Lockfile dryRun: boolean engineStrict: boolean diff --git a/packages/types/src/package.ts b/packages/types/src/package.ts index cdecea1b3e4..016fd10f738 100644 --- a/packages/types/src/package.ts +++ b/packages/types/src/package.ts @@ -129,6 +129,7 @@ export type ProjectManifest = BaseManifest & { packageExtensions?: Record peerDependencyRules?: PeerDependencyRules allowedDeprecatedVersions?: AllowedDeprecatedVersions + allowNonAppliedPatches?: boolean patchedDependencies?: Record } private?: boolean From 91e538aecba99316105b9caa1d8271a91a2e23c5 Mon Sep 17 00:00:00 2001 From: larrybahr-ocelot Date: Wed, 14 Sep 2022 12:44:02 -0500 Subject: [PATCH 2/5] feat(patch): allow non-applied patches update logging --- packages/core/test/install/patch.ts | 8 ++++++++ packages/resolve-dependencies/src/index.ts | 10 +++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/core/test/install/patch.ts b/packages/core/test/install/patch.ts index 407dd860229..78cb202fa38 100644 --- a/packages/core/test/install/patch.ts +++ b/packages/core/test/install/patch.ts @@ -98,6 +98,7 @@ test('patch package', async () => { test('patch package reports warning if not all patches are applied and allowNonAppliedPatches is set', async () => { prepareEmpty() + const reporter = jest.fn() const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch') const patchedDependencies = { @@ -110,12 +111,19 @@ test('patch package reports warning if not all patches are applied and allowNonA sideEffectsCacheWrite: true, patchedDependencies, allowNonAppliedPatches: true, + reporter, }, {}, {}, { packageImportMethod: 'hardlink' }) await install({ dependencies: { 'is-positive': '1.0.0', }, }, opts) + expect(reporter).toBeCalledWith( + expect.objectContaining({ + level: 'warn', + message: 'The following patches were not applied: is-negative@1.0.0', + }) + ) }) test('patch package throws an exception if not all patches are applied', async () => { diff --git a/packages/resolve-dependencies/src/index.ts b/packages/resolve-dependencies/src/index.ts index 93fa7cea614..df242093d41 100644 --- a/packages/resolve-dependencies/src/index.ts +++ b/packages/resolve-dependencies/src/index.ts @@ -4,7 +4,7 @@ import PnpmError from '@pnpm/error' import { packageManifestLogger, } from '@pnpm/core-loggers' -import logger from '@pnpm/logger' +import globalWarn from '@pnpm/logger' import { Lockfile, ProjectSnapshot, @@ -271,15 +271,11 @@ export default async function ( function verifyPatches (patchedDependencies: string[], appliedPatches: Set, allowNonAppliedPatches: boolean = false) { const nonAppliedPatches: string[] = patchedDependencies.filter((patchKey) => !appliedPatches.has(patchKey)) if (nonAppliedPatches.length) { - const code = 'PATCH_NOT_APPLIED' const message = `The following patches were not applied: ${nonAppliedPatches.join(', ')}` if (allowNonAppliedPatches) { - logger.warn({ - prefix: code, - message, - }) + globalWarn(message) } else { - throw new PnpmError(code, message, { + throw new PnpmError('PATCH_NOT_APPLIED', message, { hint: 'Either remove them from "patchedDependencies" or update them to match packages in your dependencies.', }) } From 5224452f6bc047a7e6b86e17c2cb90d9933a42bb Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Thu, 15 Sep 2022 23:54:04 +0300 Subject: [PATCH 3/5] fix: global warn --- packages/resolve-dependencies/src/index.ts | 37 +++++++++++++++------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/resolve-dependencies/src/index.ts b/packages/resolve-dependencies/src/index.ts index df242093d41..8b98f1094ff 100644 --- a/packages/resolve-dependencies/src/index.ts +++ b/packages/resolve-dependencies/src/index.ts @@ -4,7 +4,7 @@ import PnpmError from '@pnpm/error' import { packageManifestLogger, } from '@pnpm/core-loggers' -import globalWarn from '@pnpm/logger' +import { globalWarn } from '@pnpm/logger' import { Lockfile, ProjectSnapshot, @@ -112,7 +112,11 @@ export default async function ( (opts.forceFullResolution || !opts.wantedLockfile.packages?.length) && Object.keys(opts.wantedLockfile.importers).length === importers.length ) { - verifyPatches(Object.keys(opts.patchedDependencies), appliedPatches, opts.allowNonAppliedPatches) + verifyPatches({ + patchedDependencies: Object.keys(opts.patchedDependencies), + appliedPatches, + allowNonAppliedPatches: opts.allowNonAppliedPatches, + }) } const linkedDependenciesByProjectId: Record = {} @@ -268,18 +272,27 @@ export default async function ( } } -function verifyPatches (patchedDependencies: string[], appliedPatches: Set, allowNonAppliedPatches: boolean = false) { +function verifyPatches ( + { + patchedDependencies, + appliedPatches, + allowNonAppliedPatches, + }: { + patchedDependencies: string[] + appliedPatches: Set + allowNonAppliedPatches: boolean + } +): void { const nonAppliedPatches: string[] = patchedDependencies.filter((patchKey) => !appliedPatches.has(patchKey)) - if (nonAppliedPatches.length) { - const message = `The following patches were not applied: ${nonAppliedPatches.join(', ')}` - if (allowNonAppliedPatches) { - globalWarn(message) - } else { - throw new PnpmError('PATCH_NOT_APPLIED', message, { - hint: 'Either remove them from "patchedDependencies" or update them to match packages in your dependencies.', - }) - } + 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.', + }) } async function finishLockfileUpdates ( From faaa7882d39949f05a2888d24c6dbacdeefaebd8 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Fri, 16 Sep 2022 00:18:12 +0300 Subject: [PATCH 4/5] docs: update changesets --- .changeset/angry-hounds-bow.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.changeset/angry-hounds-bow.md b/.changeset/angry-hounds-bow.md index 0bed19434b5..67255421a20 100644 --- a/.changeset/angry-hounds-bow.md +++ b/.changeset/angry-hounds-bow.md @@ -5,4 +5,18 @@ "@pnpm/types": minor --- -add flag to make non-applied patches a warning instead of a fatal error +A new setting supported in the pnpm section of the `package.json` file: `allowNonAppliedPatches`. When it is set to `true`, non-applied patches will not cause an error, just a warning will be printed. For example: + +```json +{ + "name": "foo", + "version": "1.0.0", + "pnpm": { + "patchedDependencies": { + "express@4.18.1": "patches/express@4.18.1.patch" + }, + "allowNonAppliedPatches": true + } +} +``` + From 83bae289617d7ce0d16b959c1a5049025dfd9ebe Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Fri, 16 Sep 2022 00:22:50 +0300 Subject: [PATCH 5/5] docs: update changesets --- .changeset/angry-hounds-bow.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/angry-hounds-bow.md b/.changeset/angry-hounds-bow.md index 67255421a20..1344d16b088 100644 --- a/.changeset/angry-hounds-bow.md +++ b/.changeset/angry-hounds-bow.md @@ -3,6 +3,7 @@ "@pnpm/plugin-commands-installation": minor "@pnpm/resolve-dependencies": minor "@pnpm/types": minor +"pnpm": minor --- A new setting supported in the pnpm section of the `package.json` file: `allowNonAppliedPatches`. When it is set to `true`, non-applied patches will not cause an error, just a warning will be printed. For example: