Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(patch): allow non-applied patches #5354

Merged
merged 5 commits into from Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/angry-hounds-bow.md
@@ -0,0 +1,23 @@
---
"@pnpm/core": minor
"@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:

```json
{
"name": "foo",
"version": "1.0.0",
"pnpm": {
"patchedDependencies": {
"express@4.18.1": "patches/express@4.18.1.patch"
},
"allowNonAppliedPatches": true
}
}
```

1 change: 1 addition & 0 deletions packages/core/src/getPeerDependencyIssues.ts
Expand Up @@ -51,6 +51,7 @@ export async function getPeerDependencyIssues (
{
currentLockfile: ctx.currentLockfile,
allowedDeprecatedVersions: {},
allowNonAppliedPatches: false,
defaultUpdateDepth: -1,
dryRun: true,
engineStrict: false,
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/install/extendInstallOptions.ts
Expand Up @@ -98,6 +98,7 @@ export interface StrictInstallOptions {
modulesCacheMaxAge: number
peerDependencyRules: PeerDependencyRules
allowedDeprecatedVersions: AllowedDeprecatedVersions
allowNonAppliedPatches: boolean
preferSymlinkedExecutables: boolean
resolutionMode: 'highest' | 'time-based'

Expand All @@ -124,6 +125,7 @@ const defaults = async (opts: InstallOptions) => {
}
return {
allowedDeprecatedVersions: {},
allowNonAppliedPatches: false,
autoInstallPeers: false,
childConcurrency: 5,
depth: 0,
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/install/index.ts
Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions packages/core/test/install/patch.ts
Expand Up @@ -96,6 +96,36 @@ 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 reporter = jest.fn()
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,
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',
})
)
})

zkochan marked this conversation as resolved.
Show resolved Hide resolved
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')
Expand Down
Expand Up @@ -7,6 +7,7 @@ import {

export default function getOptionsFromRootManifest (manifest: ProjectManifest): {
allowedDeprecatedVersions?: AllowedDeprecatedVersions
allowNonAppliedPatches?: boolean
overrides?: Record<string, string>
neverBuiltDependencies?: string[]
onlyBuiltDependencies?: string[]
Expand All @@ -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,
Expand Down
32 changes: 26 additions & 6 deletions packages/resolve-dependencies/src/index.ts
Expand Up @@ -4,6 +4,7 @@ import PnpmError from '@pnpm/error'
import {
packageManifestLogger,
} from '@pnpm/core-loggers'
import { globalWarn } from '@pnpm/logger'
import {
Lockfile,
ProjectSnapshot,
Expand Down Expand Up @@ -83,6 +84,7 @@ export default async function (
preserveWorkspaceProtocol: boolean
saveWorkspaceProtocol: 'rolling' | boolean
lockfileIncludeTarballUrl?: boolean
allowNonAppliedPatches?: boolean
}
) {
const _toResolveImporter = toResolveImporter.bind(null, {
Expand Down Expand Up @@ -110,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)
verifyPatches({
patchedDependencies: Object.keys(opts.patchedDependencies),
appliedPatches,
allowNonAppliedPatches: opts.allowNonAppliedPatches,
})
}

const linkedDependenciesByProjectId: Record<string, LinkedDependency[]> = {}
Expand Down Expand Up @@ -266,13 +272,27 @@ export default async function (
}
}

function verifyPatches (patchedDependencies: string[], appliedPatches: Set<string>) {
function verifyPatches (
{
patchedDependencies,
appliedPatches,
allowNonAppliedPatches,
}: {
patchedDependencies: string[]
appliedPatches: Set<string>
allowNonAppliedPatches: boolean
}
): void {
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.',
})
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 (
Expand Down
Expand Up @@ -62,6 +62,7 @@ export interface ResolveDependenciesOptions {
autoInstallPeers?: boolean
allowBuild?: (pkgName: string) => boolean
allowedDeprecatedVersions: AllowedDeprecatedVersions
allowNonAppliedPatches: boolean
currentLockfile: Lockfile
dryRun: boolean
engineStrict: boolean
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/package.ts
Expand Up @@ -129,6 +129,7 @@ export type ProjectManifest = BaseManifest & {
packageExtensions?: Record<string, PackageExtension>
peerDependencyRules?: PeerDependencyRules
allowedDeprecatedVersions?: AllowedDeprecatedVersions
allowNonAppliedPatches?: boolean
patchedDependencies?: Record<string, string>
}
private?: boolean
Expand Down