From aeb06caae9585d14a51afdfd97e8494b31d72383 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 8 Mar 2026 19:26:48 +0100 Subject: [PATCH] refactor: simplify patchedDependencies lockfile format (#10911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: simplify patchedDependencies lockfile format to map selectors to hashes Remove the `path` field from patchedDependencies in the lockfile, changing the format from `Record` to `Record` (selector → hash). The path was never consumed from the lockfile — patch file paths come from user config, not the lockfile. Co-Authored-By: Claude Opus 4.6 * fix: migrate old patchedDependencies format when reading lockfile When reading a lockfile with the old `{ path, hash }` format for patchedDependencies, extract just the hash string. This ensures backwards compatibility with existing lockfiles. Co-Authored-By: Claude Opus 4.6 * fix: carry patchFilePath through patch groups for runtime patch application The previous commit removed `path` from the lockfile format but also accidentally dropped it from the runtime PatchInfo type. This broke patch application since `applyPatchToDir` needs the file path. - Add optional `patchFilePath` to `PatchInfo` for runtime use - Build patch groups with resolved file paths in install - Fix `build-modules` to use `patchFilePath` instead of `file.path` - Fix `calcPatchHashes` call site in `checkDepsStatus` (extra arg) Co-Authored-By: Claude Opus 4.6 * fix: update remaining references to old PatchFile type - Update getPatchInfo tests to use { hash, key } instead of { file, key } - Fix createDeployFiles to handle patchedDependencies as hash strings - Fix configurationalDependencies test assertion Co-Authored-By: Claude Opus 4.6 * fix: throw when patch exists but patchFilePath is missing Also guard against undefined patchedDependencies entry when ignorePackageManifest is true. Co-Authored-By: Claude Opus 4.6 * fix: don't join lockfileDir with already-absolute patch file paths opts.patchedDependencies values are already absolute paths, so path.join(opts.lockfileDir, absolutePath) created invalid doubled paths like /project/home/runner/work/pnpm/... Co-Authored-By: Claude Opus 4.6 * fix: use path.resolve for patch file paths and address Copilot review - Use path.resolve instead of path.join to correctly handle both relative and absolute patch file paths - Use PnpmError instead of plain Error for missing patch file path - Only copy patchedDependencies to deploy output when manifest provides the patch file paths Co-Authored-By: Claude Opus 4.6 * fix: pass rootProjectManifest in deploy patchedDependencies test The test was missing rootProjectManifest, so createDeployFiles could not find the manifest's patchedDependencies to propagate to the deploy output. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- deps/status/src/checkDepsStatus.ts | 3 +- exec/build-modules/package.json | 1 + exec/build-modules/src/index.ts | 12 +++- exec/build-modules/tsconfig.json | 3 + lockfile/fs/src/lockfileFormatConverters.ts | 10 +++ lockfile/fs/test/sortLockfileKeys.test.ts | 12 ++-- .../settings-checker/src/calcPatchHashes.ts | 9 +-- .../src/getOutdatedLockfileSetting.ts | 4 +- lockfile/types/src/index.ts | 5 +- .../config/src/groupPatchedDependencies.ts | 15 +++-- patching/config/src/index.ts | 1 - patching/config/test/getPatchInfo.test.ts | 65 ++++--------------- .../test/groupPatchedDependencies.test.ts | 63 +++++------------- patching/config/test/index.test.ts | 48 +++----------- patching/types/src/index.ts | 9 +-- pkg-manager/core/src/install/index.ts | 20 +++--- pkg-manager/core/src/install/link.ts | 2 +- pkg-manager/core/test/install/patch.ts | 20 ++---- pkg-manager/headless/src/index.ts | 2 +- .../headless/src/linkHoistedModules.ts | 2 +- .../src/resolveDependencies.ts | 2 +- pnpm-lock.yaml | 3 + pnpm/test/configurationalDependencies.test.ts | 3 +- .../src/createDeployFiles.ts | 19 +++--- .../test/shared-lockfile.test.ts | 15 ++--- 25 files changed, 123 insertions(+), 225 deletions(-) diff --git a/deps/status/src/checkDepsStatus.ts b/deps/status/src/checkDepsStatus.ts index 1fc7a6d5e7..ea5d22a000 100644 --- a/deps/status/src/checkDepsStatus.ts +++ b/deps/status/src/checkDepsStatus.ts @@ -466,7 +466,6 @@ async function assertWantedLockfileUpToDate ( linkWorkspacePackages, getManifestsByDir, getWorkspacePackages, - rootDir, rootManifestOptions, } = ctx @@ -482,7 +481,7 @@ async function assertWantedLockfileUpToDate ( patchedDependencies, pnpmfileChecksum, ] = await Promise.all([ - calcPatchHashes(rootManifestOptions?.patchedDependencies ?? {}, rootDir), + calcPatchHashes(rootManifestOptions?.patchedDependencies ?? {}), config.hooks?.calculatePnpmfileChecksum?.(), ]) diff --git a/exec/build-modules/package.json b/exec/build-modules/package.json index fd5da41084..d064169a17 100644 --- a/exec/build-modules/package.json +++ b/exec/build-modules/package.json @@ -38,6 +38,7 @@ "@pnpm/core-loggers": "workspace:*", "@pnpm/dependency-path": "workspace:*", "@pnpm/deps.graph-sequencer": "workspace:*", + "@pnpm/error": "workspace:*", "@pnpm/fs.hard-link-dir": "workspace:*", "@pnpm/lifecycle": "workspace:*", "@pnpm/link-bins": "workspace:*", diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts index dcd7fad452..bc00fc9df9 100644 --- a/exec/build-modules/src/index.ts +++ b/exec/build-modules/src/index.ts @@ -5,6 +5,7 @@ import util from 'util' import { calcDepState, type DepsStateCache } from '@pnpm/calc-dep-state' import { getWorkspaceConcurrency } from '@pnpm/config' import { skippedOptionalDependencyLogger } from '@pnpm/core-loggers' +import { PnpmError } from '@pnpm/error' import { runPostinstallHooks } from '@pnpm/lifecycle' import { linkBins, linkBinsOfPackages } from '@pnpm/link-bins' import { logger } from '@pnpm/logger' @@ -166,8 +167,13 @@ async function buildDependency ( await linkBinsOfDependencies(depNode, depGraph, opts) let isPatched = false if (depNode.patch) { - const { file } = depNode.patch - isPatched = applyPatchToDir({ patchedDir: depNode.dir, patchFilePath: file.path }) + if (!depNode.patch.patchFilePath) { + throw new PnpmError('PATCH_FILE_PATH_MISSING', + `Cannot apply patch for ${depPath}: patch file path is missing`, + { hint: 'Ensure the package is listed in patchedDependencies configuration' } + ) + } + isPatched = applyPatchToDir({ patchedDir: depNode.dir, patchFilePath: depNode.patch.patchFilePath }) } const hasSideEffects = !opts.ignoreScripts && await runPostinstallHooks({ depPath, @@ -191,7 +197,7 @@ async function buildDependency ( if ((isPatched || hasSideEffects) && opts.sideEffectsCacheWrite) { try { const sideEffectsCacheKey = calcDepState(depGraph, opts.depsStateCache, depPath, { - patchFileHash: depNode.patch?.file.hash, + patchFileHash: depNode.patch?.hash, includeDepGraphHash: hasSideEffects, }) await opts.storeController.upload(depNode.dir, { diff --git a/exec/build-modules/tsconfig.json b/exec/build-modules/tsconfig.json index 4e8257451c..d937d2f114 100644 --- a/exec/build-modules/tsconfig.json +++ b/exec/build-modules/tsconfig.json @@ -27,6 +27,9 @@ { "path": "../../packages/dependency-path" }, + { + "path": "../../packages/error" + }, { "path": "../../packages/logger" }, diff --git a/lockfile/fs/src/lockfileFormatConverters.ts b/lockfile/fs/src/lockfileFormatConverters.ts index dd49b947a1..fe959b8f7a 100644 --- a/lockfile/fs/src/lockfileFormatConverters.ts +++ b/lockfile/fs/src/lockfileFormatConverters.ts @@ -150,11 +150,21 @@ export function convertToLockfileObject (lockfile: LockfileFile): LockfileObject } return { ...omit(['snapshots'], rest), + patchedDependencies: migratePatchedDependencies(rest.patchedDependencies), packages, importers: mapValues(importers ?? {}, revertProjectSnapshot), } } +function migratePatchedDependencies (patchedDependencies: Record | undefined): Record | undefined { + if (!patchedDependencies) return undefined + const result: Record = {} + for (const [key, value] of Object.entries(patchedDependencies)) { + result[key] = typeof value === 'string' ? value : value.hash + } + return result +} + function convertProjectSnapshotToInlineSpecifiersFormat ( projectSnapshot: ProjectSnapshot ): LockfileFileProjectSnapshot { diff --git a/lockfile/fs/test/sortLockfileKeys.test.ts b/lockfile/fs/test/sortLockfileKeys.test.ts index fb3211b2be..bdd0ca6907 100644 --- a/lockfile/fs/test/sortLockfileKeys.test.ts +++ b/lockfile/fs/test/sortLockfileKeys.test.ts @@ -31,9 +31,9 @@ test('sorts keys alphabetically', () => { }, }, patchedDependencies: { - zzz: { path: 'foo', hash: 'bar' }, - bar: { path: 'foo', hash: 'bar' }, - aaa: { path: 'foo', hash: 'bar' }, + zzz: 'bar', + bar: 'bar', + aaa: 'bar', }, }) @@ -66,9 +66,9 @@ test('sorts keys alphabetically', () => { }, }, patchedDependencies: { - aaa: { path: 'foo', hash: 'bar' }, - bar: { path: 'foo', hash: 'bar' }, - zzz: { path: 'foo', hash: 'bar' }, + aaa: 'bar', + bar: 'bar', + zzz: 'bar', }, }) expect(Object.keys(normalizedLockfile.importers?.foo.dependencies ?? {})).toStrictEqual(['aaa', 'bar', 'zzz']) diff --git a/lockfile/settings-checker/src/calcPatchHashes.ts b/lockfile/settings-checker/src/calcPatchHashes.ts index 9f9d6d3667..c19ea40099 100644 --- a/lockfile/settings-checker/src/calcPatchHashes.ts +++ b/lockfile/settings-checker/src/calcPatchHashes.ts @@ -1,13 +1,8 @@ -import path from 'path' import pMapValues from 'p-map-values' import { createHexHashFromFile } from '@pnpm/crypto.hash' -import type { PatchFile } from '@pnpm/lockfile.types' -export async function calcPatchHashes (patches: Record, lockfileDir: string): Promise> { +export async function calcPatchHashes (patches: Record): Promise> { return pMapValues.default(async (patchFilePath) => { - return { - hash: await createHexHashFromFile(patchFilePath), - path: path.relative(lockfileDir, patchFilePath).replaceAll('\\', '/'), - } + return createHexHashFromFile(patchFilePath) }, patches) } diff --git a/lockfile/settings-checker/src/getOutdatedLockfileSetting.ts b/lockfile/settings-checker/src/getOutdatedLockfileSetting.ts index e1683ebe29..ac7a0cf88b 100644 --- a/lockfile/settings-checker/src/getOutdatedLockfileSetting.ts +++ b/lockfile/settings-checker/src/getOutdatedLockfileSetting.ts @@ -1,5 +1,5 @@ import type { Catalogs } from '@pnpm/catalogs.types' -import type { LockfileObject, PatchFile } from '@pnpm/lockfile.types' +import type { LockfileObject } from '@pnpm/lockfile.types' import { allCatalogsAreUpToDate } from '@pnpm/lockfile.verification' import { equals } from 'ramda' @@ -32,7 +32,7 @@ export function getOutdatedLockfileSetting ( catalogs?: Catalogs overrides?: Record packageExtensionsChecksum?: string - patchedDependencies?: Record + patchedDependencies?: Record ignoredOptionalDependencies?: string[] autoInstallPeers?: boolean excludeLinksFromLockfile?: boolean diff --git a/lockfile/types/src/index.ts b/lockfile/types/src/index.ts index d92949dfa0..728af8a1a3 100644 --- a/lockfile/types/src/index.ts +++ b/lockfile/types/src/index.ts @@ -1,8 +1,7 @@ -import type { PatchFile } from '@pnpm/patching.types' import type { DependenciesMeta, DepPath, ProjectId } from '@pnpm/types' import type { PlatformAssetTarget } from '@pnpm/resolver-base' -export type { PatchFile, ProjectId } +export type { ProjectId } export * from './lockfileFileTypes.js' @@ -19,7 +18,7 @@ export interface LockfileBase { lockfileVersion: string overrides?: Record packageExtensionsChecksum?: string - patchedDependencies?: Record + patchedDependencies?: Record pnpmfileChecksum?: string settings?: LockfileSettings time?: Record diff --git a/patching/config/src/groupPatchedDependencies.ts b/patching/config/src/groupPatchedDependencies.ts index 0a01008ef7..6d69a61a00 100644 --- a/patching/config/src/groupPatchedDependencies.ts +++ b/patching/config/src/groupPatchedDependencies.ts @@ -1,9 +1,9 @@ import * as dp from '@pnpm/dependency-path' import { PnpmError } from '@pnpm/error' -import type { PatchFile, PatchGroup, PatchGroupRecord } from '@pnpm/patching.types' +import type { PatchGroup, PatchGroupRecord, PatchInfo } from '@pnpm/patching.types' import { validRange } from 'semver' -export function groupPatchedDependencies (patchedDependencies: Record): PatchGroupRecord { +export function groupPatchedDependencies (patchedDependencies: Record): PatchGroupRecord { const result: PatchGroupRecord = {} function getGroup (name: string): PatchGroup { let group: PatchGroup | undefined = result[name] @@ -18,11 +18,12 @@ export function groupPatchedDependencies (patchedDependencies: Record { exact: {}, range: [], all: { - file: { - path: 'patches/foo.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo', }, }, @@ -75,17 +66,11 @@ test('exact version patches override version range patches, version range patche foo: { exact: { '1.0.0': { - file: { - path: 'patches/foo@1.0.0.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@1.0.0', }, '1.1.0': { - file: { - path: 'patches/foo@1.1.0.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@1.1.0', }, }, @@ -93,29 +78,20 @@ test('exact version patches override version range patches, version range patche { version: '1', patch: { - file: { - path: 'patches/foo@1.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@1', }, }, { version: '2', patch: { - file: { - path: 'patches/foo@2.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@2', }, }, ], all: { - file: { - path: 'patches/foo.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo', }, }, @@ -137,20 +113,14 @@ test('getPatchInfo(_, name, version) throws an error when name@version matches m { version: '>=1.0.0 <3.0.0', patch: { - file: { - path: 'patches/foo_a.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@>=1.0.0 <3.0.0', }, }, { version: '>=2.0.0', patch: { - file: { - path: 'patches/foo_b.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@>=2.0.0', }, }, @@ -170,10 +140,7 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m foo: { exact: { '2.1.0': { - file: { - path: 'patches/foo_a.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@>=1.0.0 <3.0.0', }, }, @@ -181,20 +148,14 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m { version: '>=1.0.0 <3.0.0', patch: { - file: { - path: 'patches/foo_b.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@>=1.0.0 <3.0.0', }, }, { version: '>=2.0.0', patch: { - file: { - path: 'patches/foo_c.patch', - hash: '00000000000000000000000000000000', - }, + hash: '00000000000000000000000000000000', key: 'foo@>=2.0.0', }, }, diff --git a/patching/config/test/groupPatchedDependencies.test.ts b/patching/config/test/groupPatchedDependencies.test.ts index 60ddf6f17a..52d54083c0 100644 --- a/patching/config/test/groupPatchedDependencies.test.ts +++ b/patching/config/test/groupPatchedDependencies.test.ts @@ -1,4 +1,4 @@ -import type { ExtendedPatchInfo, PatchFile, PatchGroupRecord } from '@pnpm/patching.types' +import type { ExtendedPatchInfo, PatchGroupRecord } from '@pnpm/patching.types' import { groupPatchedDependencies } from '../src/groupPatchedDependencies.js' function sanitizePatchGroupRecord (patchGroups: PatchGroupRecord): PatchGroupRecord { @@ -11,51 +11,21 @@ function sanitizePatchGroupRecord (patchGroups: PatchGroupRecord): PatchGroupRec 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 patchedDependencies: Record = { + 'exact-version-only@0.0.0': '00000000000000000000000000000000', + 'exact-version-only@1.2.3': '00000000000000000000000000000000', + 'exact-version-only@2.1.0': '00000000000000000000000000000000', + 'version-range-only@~1.2.0': '00000000000000000000000000000000', + 'version-range-only@4': '00000000000000000000000000000000', + 'star-version-range@*': '00000000000000000000000000000000', + 'without-versions': '00000000000000000000000000000000', + 'mixed-style@0.1.2': '00000000000000000000000000000000', + 'mixed-style@1.x.x': '00000000000000000000000000000000', + 'mixed-style': '00000000000000000000000000000000', + } const info = (key: keyof typeof patchedDependencies): ExtendedPatchInfo => ({ key, - file: patchedDependencies[key], + hash: patchedDependencies[key], }) expect(_groupPatchedDependencies(patchedDependencies)).toStrictEqual({ 'exact-version-only': { @@ -108,10 +78,7 @@ test('groups patchedDependencies according to names, match types, and versions', test('errors on invalid version range', async () => { expect(() => _groupPatchedDependencies({ - 'foo@link:packages/foo': { - hash: '00000000000000000000000000000000', - path: 'patches/foo.patch', - }, + 'foo@link:packages/foo': '00000000000000000000000000000000', })).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 14330b6fa7..b4980a4e2b 100644 --- a/patching/config/test/index.test.ts +++ b/patching/config/test/index.test.ts @@ -1,7 +1,6 @@ -import type { PatchFile } from '@pnpm/patching.types' import { getPatchInfo, groupPatchedDependencies } from '../src/index.js' -const _getPatchInfo = (patchedDependencies: Record, name: string, version: string) => +const _getPatchInfo = (patchedDependencies: Record, name: string, version: string) => getPatchInfo(groupPatchedDependencies(patchedDependencies), name, version) test('getPatchInfo(undefined, ...) returns undefined', () => { @@ -10,67 +9,40 @@ test('getPatchInfo(undefined, ...) returns undefined', () => { test('getPatchInfo(_, name, version) if name@version exists', () => { expect(_getPatchInfo({ - 'foo@1.0.0': { - path: 'patches/foo@1.0.0.patch', - hash: '00000000000000000000000000000000', - }, + 'foo@1.0.0': '00000000000000000000000000000000', }, 'foo', '1.0.0')).toStrictEqual({ - file: { - path: 'patches/foo@1.0.0.patch', - hash: expect.any(String), - }, + hash: expect.any(String), key: 'foo@1.0.0', }) }) test('getPatchInfo(_, name, version) if name exists but name@version does not exist', () => { expect(_getPatchInfo({ - foo: { - path: 'patches/foo.patch', - hash: '00000000000000000000000000000000', - }, + foo: '00000000000000000000000000000000', }, 'foo', '1.0.0')).toStrictEqual({ - file: { - path: 'patches/foo.patch', - hash: expect.any(String), - }, + hash: expect.any(String), key: 'foo', }) }) test('getPatchInfo(_, name, version) prioritizes name@version over name if both exist', () => { expect(_getPatchInfo({ - foo: { - path: 'patches/foo.patch', - hash: '00000000000000000000000000000000', - }, - 'foo@1.0.0': { - path: 'patches/foo@1.0.0.patch', - hash: '00000000000000000000000000000000', - }, + foo: '00000000000000000000000000000000', + 'foo@1.0.0': '00000000000000000000000000000000', }, 'foo', '1.0.0')).toStrictEqual({ - file: { - path: 'patches/foo@1.0.0.patch', - hash: expect.any(String), - }, + hash: expect.any(String), key: 'foo@1.0.0', }) }) test('getPatchInfo(_, name, version) does not access wrong name', () => { expect(_getPatchInfo({ - 'bar@1.0.0': { - path: 'patches/bar@1.0.0.patch', - hash: '00000000000000000000000000000000', - }, + 'bar@1.0.0': '00000000000000000000000000000000', }, 'foo', '1.0.0')).toBeUndefined() }) test('getPatchInfo(_, name, version) does not access wrong version', () => { expect(_getPatchInfo({ - 'foo@2.0.0': { - path: 'patches/foo@2.0.0.patch', - hash: '00000000000000000000000000000000', - }, + 'foo@2.0.0': '00000000000000000000000000000000', }, 'foo', '1.0.0')).toBeUndefined() }) diff --git a/patching/types/src/index.ts b/patching/types/src/index.ts index 5dea224c91..b060a26362 100644 --- a/patching/types/src/index.ts +++ b/patching/types/src/index.ts @@ -1,11 +1,6 @@ -export interface PatchFile { - path: string - hash: string -} - -// TODO: replace all occurrences of PatchInfo with PatchFile before the next major version is released export interface PatchInfo { - file: PatchFile + hash: string + patchFilePath?: string } export interface ExtendedPatchInfo extends PatchInfo { diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index 8e3e3168df..26bd79bfae 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -425,14 +425,18 @@ export async function mutateModules ( const pnpmfileChecksum = await opts.hooks.calculatePnpmfileChecksum?.() const patchedDependencies = opts.ignorePackageManifest ? ctx.wantedLockfile.patchedDependencies - : (opts.patchedDependencies ? await calcPatchHashes(opts.patchedDependencies, opts.lockfileDir) : {}) - const patchedDependenciesWithResolvedPath = patchedDependencies - ? mapValues((patchFile) => ({ - hash: patchFile.hash, - path: path.join(opts.lockfileDir, patchFile.path), - }), patchedDependencies) - : undefined - const patchGroups = patchedDependenciesWithResolvedPath && groupPatchedDependencies(patchedDependenciesWithResolvedPath) + : (opts.patchedDependencies ? await calcPatchHashes(opts.patchedDependencies) : {}) + const patchGroupInput = opts.patchedDependencies + ? Object.fromEntries( + Object.entries(patchedDependencies ?? {}).map(([key, hash]) => { + const patchFilePath = opts.patchedDependencies![key] + ? path.resolve(opts.lockfileDir, opts.patchedDependencies![key]) + : undefined + return [key, { hash, patchFilePath }] + }) + ) + : patchedDependencies + const patchGroups = patchGroupInput ? groupPatchedDependencies(patchGroupInput) : undefined const frozenLockfile = opts.frozenLockfile || opts.frozenLockfileIfExists && ctx.existsNonEmptyWantedLockfile let outdatedLockfileSettings = false diff --git a/pkg-manager/core/src/install/link.ts b/pkg-manager/core/src/install/link.ts index bb20393941..74a53251bb 100644 --- a/pkg-manager/core/src/install/link.ts +++ b/pkg-manager/core/src/install/link.ts @@ -467,7 +467,7 @@ async function linkAllPkgs ( if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) { sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.depPath, { includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built - patchFileHash: depNode.patch?.file.hash, + patchFileHash: depNode.patch?.hash, }) } } diff --git a/pkg-manager/core/test/install/patch.ts b/pkg-manager/core/test/install/patch.ts index 63b49682af..c5e95f54b2 100644 --- a/pkg-manager/core/test/install/patch.ts +++ b/pkg-manager/core/test/install/patch.ts @@ -54,10 +54,7 @@ test('patch package with exact version', async () => { const patchFileHash = await createHexHashFromFile(patchPath) const lockfile = project.readLockfile() expect(lockfile.patchedDependencies).toStrictEqual({ - 'is-positive@1.0.0': { - path: path.relative(process.cwd(), patchedDependencies['is-positive@1.0.0']).replaceAll('\\', '/'), - hash: patchFileHash, - }, + 'is-positive@1.0.0': patchFileHash, }) expect(lockfile.snapshots[`is-positive@1.0.0(patch_hash=${patchFileHash})`]).toBeTruthy() @@ -154,10 +151,7 @@ test('patch package with version range', async () => { 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, - }, + 'is-positive@1': patchFileHash, }) expect(lockfile.snapshots[`is-positive@1.0.0(patch_hash=${patchFileHash})`]).toBeTruthy() @@ -326,10 +320,7 @@ test('patch package when scripts are ignored', async () => { const patchFileHash = await createHexHashFromFile(patchPath) const lockfile = project.readLockfile() expect(lockfile.patchedDependencies).toStrictEqual({ - 'is-positive@1.0.0': { - path: path.relative(process.cwd(), patchedDependencies['is-positive@1.0.0']).replaceAll('\\', '/'), - hash: patchFileHash, - }, + 'is-positive@1.0.0': patchFileHash, }) expect(lockfile.snapshots[`is-positive@1.0.0(patch_hash=${patchFileHash})`]).toBeTruthy() @@ -419,10 +410,7 @@ test('patch package when the package is not in allowBuilds list', async () => { const patchFileHash = await createHexHashFromFile(patchPath) const lockfile = project.readLockfile() expect(lockfile.patchedDependencies).toStrictEqual({ - 'is-positive@1.0.0': { - path: path.relative(process.cwd(), patchedDependencies['is-positive@1.0.0']).replaceAll('\\', '/'), - hash: patchFileHash, - }, + 'is-positive@1.0.0': patchFileHash, }) expect(lockfile.snapshots[`is-positive@1.0.0(patch_hash=${patchFileHash})`]).toBeTruthy() diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index c408d49d83..be2101c24f 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -890,7 +890,7 @@ async function linkAllPkgs ( if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) { sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.dir, { includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built - patchFileHash: depNode.patch?.file.hash, + patchFileHash: depNode.patch?.hash, }) } } diff --git a/pkg-manager/headless/src/linkHoistedModules.ts b/pkg-manager/headless/src/linkHoistedModules.ts index beda7d25b2..119f4f88fe 100644 --- a/pkg-manager/headless/src/linkHoistedModules.ts +++ b/pkg-manager/headless/src/linkHoistedModules.ts @@ -118,7 +118,7 @@ async function linkAllPkgsInOrder ( if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) { sideEffectsCacheKey = _calcDepState(dir, { includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built - patchFileHash: depNode.patch?.file.hash, + patchFileHash: depNode.patch?.hash, }) } } diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index 17966be40b..0dc595fb0a 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -1480,7 +1480,7 @@ async function resolveDependency ( const patch = getPatchInfo(ctx.patchedDependencies, pkg.name, pkg.version) if (patch) { ctx.appliedPatches.add(patch.key) - pkgIdWithPatchHash = `${pkgIdWithPatchHash}(patch_hash=${patch.file.hash})` as PkgIdWithPatchHash + pkgIdWithPatchHash = `${pkgIdWithPatchHash}(patch_hash=${patch.hash})` as PkgIdWithPatchHash } // We are building the dependency tree only until there are new packages diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index afd407c044..bf9873ba22 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2507,6 +2507,9 @@ importers: '@pnpm/deps.graph-sequencer': specifier: workspace:* version: link:../../deps/graph-sequencer + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error '@pnpm/fs.hard-link-dir': specifier: workspace:* version: link:../../fs/hard-link-dir diff --git a/pnpm/test/configurationalDependencies.test.ts b/pnpm/test/configurationalDependencies.test.ts index 45a6b4c144..047024aea3 100644 --- a/pnpm/test/configurationalDependencies.test.ts +++ b/pnpm/test/configurationalDependencies.test.ts @@ -35,8 +35,7 @@ test('patch from configuration dependency is applied via updateConfig hook', asy expect(fs.existsSync('node_modules/@pnpm.e2e/foo/index.js')).toBeTruthy() const lockfile = project.readLockfile() - // The patch path goes through the global virtual store since config deps are symlinked there - expect(lockfile.patchedDependencies['@pnpm.e2e/foo'].path).toContain('@pnpm.e2e/has-patch-for-foo/@pnpm.e2e__foo@100.0.0.patch') + expect(lockfile.patchedDependencies['@pnpm.e2e/foo']).toEqual(expect.any(String)) }) test('catalog applied by configurational dependency hook', async () => { diff --git a/releasing/plugin-commands-deploy/src/createDeployFiles.ts b/releasing/plugin-commands-deploy/src/createDeployFiles.ts index 1beff93d69..2a19cc8db0 100644 --- a/releasing/plugin-commands-deploy/src/createDeployFiles.ts +++ b/releasing/plugin-commands-deploy/src/createDeployFiles.ts @@ -152,17 +152,14 @@ export function createDeployFiles ({ } if (lockfile.patchedDependencies) { - result.lockfile.patchedDependencies = {} - result.manifest.pnpm!.patchedDependencies = {} - - for (const name in lockfile.patchedDependencies) { - const patchInfo = lockfile.patchedDependencies[name] - const resolvedPath = path.resolve(rootProjectManifestDir, patchInfo.path) - const relativePath = normalizePath(path.relative(deployDir, resolvedPath)) - result.manifest.pnpm!.patchedDependencies[name] = relativePath - result.lockfile.patchedDependencies[name] = { - hash: patchInfo.hash, - path: relativePath, + const manifestPatchedDeps = rootProjectManifest?.pnpm?.patchedDependencies + if (manifestPatchedDeps) { + result.lockfile.patchedDependencies = { ...lockfile.patchedDependencies } + result.manifest.pnpm!.patchedDependencies = {} + for (const name in manifestPatchedDeps) { + const resolvedPath = path.resolve(rootProjectManifestDir, manifestPatchedDeps[name]) + const relativePath = normalizePath(path.relative(deployDir, resolvedPath)) + result.manifest.pnpm!.patchedDependencies[name] = relativePath } } } diff --git a/releasing/plugin-commands-deploy/test/shared-lockfile.test.ts b/releasing/plugin-commands-deploy/test/shared-lockfile.test.ts index ea32a590ee..e2fd741a0f 100644 --- a/releasing/plugin-commands-deploy/test/shared-lockfile.test.ts +++ b/releasing/plugin-commands-deploy/test/shared-lockfile.test.ts @@ -4,7 +4,7 @@ import url from 'url' import { install } from '@pnpm/plugin-commands-installation' import { assertProject } from '@pnpm/assert-project' import { preparePackages } from '@pnpm/prepare' -import type { PatchFile, LockfileFile, LockfilePackageSnapshot } from '@pnpm/lockfile.types' +import type { LockfileFile, LockfilePackageSnapshot } from '@pnpm/lockfile.types' import { filterPackagesFromDir } from '@pnpm/workspace.filter-packages-from-dir' import { fixtures } from '@pnpm/test-fixtures' import type { ProjectManifest } from '@pnpm/types' @@ -839,6 +839,8 @@ test('deploy with a shared lockfile should correctly handle patchedDependencies' dir: process.cwd(), patchedDependencies, recursive: true, + rootProjectManifest: preparedManifests.root, + rootProjectManifestDir: process.cwd(), selectedProjectsGraph, sharedWorkspaceLockfile: true, lockfileDir: process.cwd(), @@ -850,13 +852,10 @@ test('deploy with a shared lockfile should correctly handle patchedDependencies' const lockfile = project.readLockfile() expect(lockfile.patchedDependencies).toStrictEqual({ - 'is-positive': { - hash: expect.any(String), - path: '../__patches__/is-positive.patch', - }, - } as Record) + 'is-positive': expect.any(String), + }) - const patchFile = lockfile.patchedDependencies['is-positive'] + const patchHash = lockfile.patchedDependencies['is-positive'] const manifest = readPackageJson('deploy') as ProjectManifest expect(manifest).toStrictEqual({ @@ -878,7 +877,7 @@ test('deploy with a shared lockfile should correctly handle patchedDependencies' expect(project1Name).toBeDefined() if (process.platform !== 'win32') { expect(fs.realpathSync(`deploy/node_modules/.pnpm/${project1Name}/node_modules/is-positive`)).toBe( - path.resolve(`deploy/node_modules/.pnpm/is-positive@1.0.0_patch_hash=${patchFile.hash}/node_modules/is-positive`) + path.resolve(`deploy/node_modules/.pnpm/is-positive@1.0.0_patch_hash=${patchHash}/node_modules/is-positive`) ) } expect(