From 9fc552d37aa7ae38b2743e664844b9b033f06bf6 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 21 Mar 2026 12:50:46 +0100 Subject: [PATCH] fix: update GVS symlinks after approve-builds by running install (#11043) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #11042 - **Root cause**: When `enableGlobalVirtualStore` is true and `allowBuilds` is not configured, `createAllowBuildFunction()` returned `undefined`, causing all GVS hashes to include `ENGINE_NAME`. When `approve-builds` later configured `allowBuilds`, the hash didn't change because the engine was already included. - **Fix**: Default `allowBuilds` to `{}` in GVS mode so hashes are engine-agnostic by default, and have `approve-builds` call `install.handler()` in GVS mode instead of the low-level `install()` function, so it properly handles workspaces and updates symlinks. - **Refactor**: Broke circular dependencies between `building/commands`, `installing/commands`, and `global/commands` using dependency injection via a `commands` map passed as the third argument to command handlers. Added `CommandHandler` and `CommandHandlerMap` types to `@pnpm/cli.command`. ## Changes ### Architecture - Command handlers now receive a `commands` map as an optional third argument `(opts, params, commands?)` - The CLI dispatcher in `main.ts` passes the full commands map to every handler - Handlers that need other commands (e.g., `globalAdd` needs `approve-builds`, `recursive` needs `rebuild`) access them from this map - This replaces direct cross-package imports that would create circular dependencies ### Packages changed - `@pnpm/cli.command` — new `CommandHandler` and `CommandHandlerMap` types - `@pnpm/building.commands` — `approve-builds` uses `install.handler` for GVS - `@pnpm/global.commands` — removed `building/commands` dependency; receives `approve-builds` via commands map - `@pnpm/installing.commands` — receives `rebuild` via commands map instead of direct import - `@pnpm/installing.deps-installer` / `@pnpm/installing.deps-restorer` — default `allowBuilds` to `{}` in GVS mode - `pnpm` CLI — dispatcher passes commands map to all handlers --- .changeset/fix-gvs-approve-builds.md | 9 ++++ building/commands/package.json | 2 + building/commands/src/build/rebuild.ts | 2 +- building/commands/src/index.ts | 3 +- building/commands/src/policy/approveBuilds.ts | 17 +++++-- .../test/policy/approveBuilds.test.ts | 30 ++++++------ building/commands/tsconfig.json | 6 +++ cli/command/src/index.ts | 5 ++ config/reader/src/index.ts | 7 +++ engine/pm/commands/package.json | 1 + .../commands/src/self-updater/installPnpm.ts | 17 +++++-- engine/pm/commands/tsconfig.json | 3 ++ global/commands/package.json | 2 +- global/commands/src/globalAdd.ts | 13 +++-- global/commands/src/globalUpdate.ts | 18 +++---- global/commands/tsconfig.json | 2 +- installing/commands/package.json | 1 - installing/commands/src/add.ts | 8 +++- installing/commands/src/dedupe.ts | 4 +- installing/commands/src/install.ts | 4 +- installing/commands/src/installDeps.ts | 2 + installing/commands/src/recursive.ts | 5 +- installing/commands/src/update/index.ts | 21 ++++---- installing/commands/tsconfig.json | 3 -- .../src/install/extendInstallOptions.ts | 7 ++- .../test/install/globalVirtualStore.ts | 48 +++++++++++++++++++ installing/deps-restorer/src/index.ts | 3 ++ pnpm-lock.yaml | 16 +++++-- pnpm/src/cmd/index.ts | 8 ++-- pnpm/src/main.ts | 3 +- pnpm/test/install/globalVirtualStore.ts | 47 ++++++++++++++++++ 31 files changed, 245 insertions(+), 72 deletions(-) create mode 100644 .changeset/fix-gvs-approve-builds.md diff --git a/.changeset/fix-gvs-approve-builds.md b/.changeset/fix-gvs-approve-builds.md new file mode 100644 index 0000000000..dd0920859e --- /dev/null +++ b/.changeset/fix-gvs-approve-builds.md @@ -0,0 +1,9 @@ +--- +"@pnpm/building.commands": patch +"@pnpm/installing.commands": patch +"@pnpm/installing.deps-installer": patch +"@pnpm/installing.deps-restorer": patch +"pnpm": patch +--- + +In GVS mode, `pnpm approve-builds` now runs a full install instead of rebuild. This ensures that GVS hash directories and symlinks are updated correctly after changing `allowBuilds`, preventing build artifact contamination of engine-agnostic directories [#11042](https://github.com/pnpm/pnpm/issues/11042). diff --git a/building/commands/package.json b/building/commands/package.json index 2199c2fc28..9928302f72 100644 --- a/building/commands/package.json +++ b/building/commands/package.json @@ -33,12 +33,14 @@ }, "dependencies": { "@pnpm/building.after-install": "workspace:*", + "@pnpm/cli.command": "workspace:*", "@pnpm/cli.common-cli-options-help": "workspace:*", "@pnpm/cli.utils": "workspace:*", "@pnpm/config.reader": "workspace:*", "@pnpm/config.writer": "workspace:*", "@pnpm/deps.path": "workspace:*", "@pnpm/error": "workspace:*", + "@pnpm/installing.commands": "workspace:*", "@pnpm/installing.modules-yaml": "workspace:*", "@pnpm/prepare-temp-dir": "workspace:*", "@pnpm/store.connection-manager": "workspace:*", diff --git a/building/commands/src/build/rebuild.ts b/building/commands/src/build/rebuild.ts index ab19233e76..4d94b52c69 100644 --- a/building/commands/src/build/rebuild.ts +++ b/building/commands/src/build/rebuild.ts @@ -55,7 +55,7 @@ For options that may be used with `-r`, see "pnpm help recursive"', shortAlias: '-r', }, { - description: 'Rebuild packages that were not build during installation. Packages are not build when installing with the --ignore-scripts flag', + description: 'Rebuild packages that were not built during installation. Packages are not built when installing with the --ignore-scripts flag', name: '--pending', }, { diff --git a/building/commands/src/index.ts b/building/commands/src/index.ts index 4132611c2c..0f3953a5a7 100644 --- a/building/commands/src/index.ts +++ b/building/commands/src/index.ts @@ -1,4 +1,3 @@ -export { rebuild } from './build/index.js' -export type { RebuildCommandOpts } from './build/rebuild.js' +export { rebuild, type RebuildCommandOpts } from './build/index.js' export type { ApproveBuildsCommandOpts } from './policy/approveBuilds.js' export { approveBuilds, ignoredBuilds } from './policy/index.js' diff --git a/building/commands/src/policy/approveBuilds.ts b/building/commands/src/policy/approveBuilds.ts index cc9bf9aff2..34648b5fae 100644 --- a/building/commands/src/policy/approveBuilds.ts +++ b/building/commands/src/policy/approveBuilds.ts @@ -1,8 +1,9 @@ -import { rebuild, type RebuildCommandOpts } from '@pnpm/building.commands' +import type { CommandHandlerMap } from '@pnpm/cli.command' import type { Config } from '@pnpm/config.reader' import { writeSettings } from '@pnpm/config.writer' import { parse } from '@pnpm/deps.path' import { PnpmError } from '@pnpm/error' +import { install } from '@pnpm/installing.commands' import { type StrictModules, writeModulesManifest } from '@pnpm/installing.modules-yaml' import { globalInfo } from '@pnpm/logger' import { lexCompare } from '@pnpm/util.lex-comparator' @@ -10,9 +11,10 @@ import chalk from 'chalk' import enquirer from 'enquirer' import { renderHelp } from 'render-help' +import { rebuild, type RebuildCommandOpts } from '../build/index.js' import { getAutomaticallyIgnoredBuilds } from './getAutomaticallyIgnoredBuilds.js' -export type ApproveBuildsCommandOpts = Pick & { all?: boolean, global?: boolean } +export type ApproveBuildsCommandOpts = Pick & { all?: boolean, global?: boolean } export const commandNames = ['approve-builds'] @@ -49,7 +51,7 @@ export function rcOptionsTypes (): Record { return {} } -export async function handler (opts: ApproveBuildsCommandOpts & RebuildCommandOpts, params: string[] = []): Promise { +export async function handler (opts: ApproveBuildsCommandOpts & RebuildCommandOpts, params: string[] = [], commands?: CommandHandlerMap): Promise { if (opts.global) { throw new PnpmError( 'APPROVE_BUILDS_NOT_SUPPORTED_WITH_GLOBAL', @@ -201,6 +203,15 @@ Do you approve?`, await writeModulesManifest(modulesDir, modulesManifest as StrictModules) } if (buildPackages.length) { + if (opts.enableGlobalVirtualStore) { + await install.handler({ + ...opts, + allowBuilds, + frozenLockfile: true, + optimisticRepeatInstall: false, + } as any, [], commands) // eslint-disable-line @typescript-eslint/no-explicit-any + return + } return rebuild.handler({ ...opts, allowBuilds, diff --git a/building/commands/test/policy/approveBuilds.test.ts b/building/commands/test/policy/approveBuilds.test.ts index 5ee58fd12e..3fe4d49482 100644 --- a/building/commands/test/policy/approveBuilds.test.ts +++ b/building/commands/test/policy/approveBuilds.test.ts @@ -23,7 +23,7 @@ const prompt = jest.mocked(enquirer.prompt) const REGISTRY = `http://localhost:${REGISTRY_MOCK_PORT}/` const pnpmBin = path.join(import.meta.dirname, '../../../../pnpm/bin/pnpm.mjs') -async function execPnpmInstall (): Promise { +async function execPnpmInstall (opts?: { enableGlobalVirtualStore?: boolean }): Promise { await execa('node', [ pnpmBin, 'install', @@ -31,7 +31,7 @@ async function execPnpmInstall (): Promise { `--cache-dir=${path.resolve('cache')}`, `--registry=${REGISTRY}`, '--config.strict-dep-builds=false', - '--config.enable-global-virtual-store=false', + `--config.enable-global-virtual-store=${opts?.enableGlobalVirtualStore ?? false}`, ]) } @@ -70,7 +70,7 @@ async function approveSomeBuilds (opts?: ApproveBuildsOptions) { build: true, }) - await approveBuilds.handler({ ...config, ...opts }) + await approveBuilds.handler({ ...config, ...opts }, [], {}) } async function approveNoBuilds (opts?: ApproveBuildsOptions) { @@ -81,7 +81,7 @@ async function approveNoBuilds (opts?: ApproveBuildsOptions) { result: [], }) - await approveBuilds.handler({ ...config, ...opts }) + await approveBuilds.handler({ ...config, ...opts }, [], {}) } test('approve selected build', async () => { @@ -155,7 +155,7 @@ test("works when root project manifest doesn't exist in a workspace", async () = result: [{ value: '@pnpm.e2e/pre-and-postinstall-scripts-example' }], }) prompt.mockResolvedValueOnce({ build: true }) - await approveBuilds.handler({ ...config, workspaceDir, rootProjectManifestDir: workspaceDir }) + await approveBuilds.handler({ ...config, workspaceDir, rootProjectManifestDir: workspaceDir }, [], {}) expect(readYamlFileSync(workspaceManifestFile)).toStrictEqual({ packages: ['packages/*'], @@ -203,7 +203,7 @@ test('approve all builds with --all flag', async () => { const config = await getApproveBuildsConfig() prompt.mockClear() - await approveBuilds.handler({ ...config, all: true }) + await approveBuilds.handler({ ...config, all: true }, [], {}) expect(prompt).not.toHaveBeenCalled() @@ -230,7 +230,7 @@ test('approve builds via positional arguments', async () => { const config = await getApproveBuildsConfig() prompt.mockClear() - await approveBuilds.handler(config, ['@pnpm.e2e/pre-and-postinstall-scripts-example']) + await approveBuilds.handler(config, ['@pnpm.e2e/pre-and-postinstall-scripts-example'], {}) expect(prompt).not.toHaveBeenCalled() @@ -263,7 +263,7 @@ test('deny builds via !pkg positional arguments', async () => { await approveBuilds.handler(config, [ '@pnpm.e2e/pre-and-postinstall-scripts-example', '!@pnpm.e2e/install-script-example', - ]) + ], {}) expect(prompt).not.toHaveBeenCalled() @@ -291,7 +291,7 @@ test('deny-only via !pkg keeps other builds pending', async () => { prompt.mockClear() await approveBuilds.handler(config, [ '!@pnpm.e2e/install-script-example', - ]) + ], {}) expect(prompt).not.toHaveBeenCalled() @@ -319,7 +319,7 @@ test('positional arguments with unknown package throws error', async () => { const config = await getApproveBuildsConfig() await expect( - approveBuilds.handler(config, ['@pnpm.e2e/nonexistent-package']) + approveBuilds.handler(config, ['@pnpm.e2e/nonexistent-package'], {}) ).rejects.toThrow('not awaiting approval') }) @@ -334,7 +334,7 @@ test('!pkg with unknown package throws error', async () => { const config = await getApproveBuildsConfig() await expect( - approveBuilds.handler(config, ['!@pnpm.e2e/nonexistent-package']) + approveBuilds.handler(config, ['!@pnpm.e2e/nonexistent-package'], {}) ).rejects.toThrow('not awaiting approval') }) @@ -352,7 +352,7 @@ test('contradictory arguments throw error', async () => { approveBuilds.handler(config, [ '@pnpm.e2e/pre-and-postinstall-scripts-example', '!@pnpm.e2e/pre-and-postinstall-scripts-example', - ]) + ], {}) ).rejects.toThrow('both approved and denied') }) @@ -367,7 +367,7 @@ test('--all with positional arguments throws error', async () => { const config = await getApproveBuildsConfig() await expect( - approveBuilds.handler({ ...config, all: true }, ['@pnpm.e2e/pre-and-postinstall-scripts-example']) + approveBuilds.handler({ ...config, all: true }, ['@pnpm.e2e/pre-and-postinstall-scripts-example'], {}) ).rejects.toThrow('Cannot use --all with positional arguments') }) @@ -401,7 +401,7 @@ test('positional args preserve existing allowBuilds entries', async () => { allowBuilds: { '@pnpm.e2e/existing-package': true, }, - }, ['@pnpm.e2e/pre-and-postinstall-scripts-example']) + }, ['@pnpm.e2e/pre-and-postinstall-scripts-example'], {}) const manifest = readYamlFileSync(workspaceManifestFile) // eslint-disable-line expect(manifest.allowBuilds['@pnpm.e2e/existing-package']).toBe(true) @@ -449,7 +449,7 @@ test('should retain existing allowBuilds entries when approving builds', async ( '@pnpm.e2e/test': false, '@pnpm.e2e/install-script-example': true, }, - }) + }, [], {}) expect(readYamlFileSync(workspaceManifestFile)).toStrictEqual({ packages: ['packages/*'], diff --git a/building/commands/tsconfig.json b/building/commands/tsconfig.json index 46f6423a93..19063084ba 100644 --- a/building/commands/tsconfig.json +++ b/building/commands/tsconfig.json @@ -24,6 +24,9 @@ { "path": "../../__utils__/test-ipc-server" }, + { + "path": "../../cli/command" + }, { "path": "../../cli/common-cli-options-help" }, @@ -54,6 +57,9 @@ { "path": "../../deps/path" }, + { + "path": "../../installing/commands" + }, { "path": "../../installing/modules-yaml" }, diff --git a/cli/command/src/index.ts b/cli/command/src/index.ts index c012da0941..635b9aaf6b 100644 --- a/cli/command/src/index.ts +++ b/cli/command/src/index.ts @@ -4,3 +4,8 @@ export type CompletionFunc = ( options: Record, params: string[] ) => Promise + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type CommandHandler = (opts: any, params: string[], commands?: CommandHandlerMap) => any + +export type CommandHandlerMap = Record diff --git a/config/reader/src/index.ts b/config/reader/src/index.ts index 6cf9e8da58..3388bfc699 100644 --- a/config/reader/src/index.ts +++ b/config/reader/src/index.ts @@ -383,6 +383,13 @@ export async function getConfig (opts: { } else if (!pnpmConfig.bin) { pnpmConfig.bin = path.join(pnpmConfig.dir, 'node_modules', '.bin') } + // Default allowBuilds to {} when GVS is enabled so that GVS hashes + // are engine-agnostic when no build policy is configured. Without + // this, allowBuilds is undefined which makes createAllowBuildFunction + // return undefined, causing all hashes to include ENGINE_NAME. + if (pnpmConfig.enableGlobalVirtualStore && pnpmConfig.allowBuilds == null) { + pnpmConfig.allowBuilds = {} + } pnpmConfig.packageManager = packageManager if (!opts.ignoreLocalSettings) { diff --git a/engine/pm/commands/package.json b/engine/pm/commands/package.json index 5871c9fdfe..e4f5036842 100644 --- a/engine/pm/commands/package.json +++ b/engine/pm/commands/package.json @@ -32,6 +32,7 @@ }, "dependencies": { "@pnpm/bins.linker": "workspace:*", + "@pnpm/building.policy": "workspace:*", "@pnpm/cli.meta": "workspace:*", "@pnpm/cli.utils": "workspace:*", "@pnpm/config.reader": "workspace:*", diff --git a/engine/pm/commands/src/self-updater/installPnpm.ts b/engine/pm/commands/src/self-updater/installPnpm.ts index 09801a76f8..68db4644a6 100644 --- a/engine/pm/commands/src/self-updater/installPnpm.ts +++ b/engine/pm/commands/src/self-updater/installPnpm.ts @@ -3,6 +3,7 @@ import path from 'node:path' import util from 'node:util' import { linkBins } from '@pnpm/bins.linker' +import { createAllowBuildFunction } from '@pnpm/building.policy' import { getCurrentPackageName } from '@pnpm/cli.meta' import { iterateHashedGraphNodes, @@ -23,6 +24,10 @@ import type { StoreController } from '@pnpm/store.controller' import type { DepPath, ProjectId, ProjectRootDir, Registries } from '@pnpm/types' import { symlinkDir } from 'symlink-dir' +// @pnpm/exe has platform-specific binaries, so its GVS hash must +// include ENGINE_NAME for correct per-platform resolution. +const PNPM_ALLOW_BUILDS: Record = { '@pnpm/exe': true } + export interface InstallPnpmResult { binDir: string baseDir: string @@ -82,7 +87,7 @@ export async function installPnpmToStore ( const globalVirtualStoreDir = path.join(opts.storeDir, 'links') // Compute the GVS hash for the pnpm package to find its path - const pnpmGvsPath = findPnpmGvsPath(wantedLockfile, currentPkgName, globalVirtualStoreDir) + const pnpmGvsPath = findPnpmGvsPath(wantedLockfile, currentPkgName, globalVirtualStoreDir, PNPM_ALLOW_BUILDS) const pnpmPkgDir = path.join(pnpmGvsPath, 'node_modules', currentPkgName) const binDir = path.join(pnpmGvsPath, 'bin') @@ -102,6 +107,7 @@ export async function installPnpmToStore ( try { await installFromLockfile(tmpInstallDir, binDir, { wantedLockfile, + allowBuilds: PNPM_ALLOW_BUILDS, storeController: opts.storeController, storeDir: opts.storeDir, registries: opts.registries, @@ -126,11 +132,13 @@ function noop (_message: string) {} function findPnpmGvsPath ( lockfile: LockfileObject, pkgName: string, - globalVirtualStoreDir: string + globalVirtualStoreDir: string, + allowBuilds?: Record ): string { const graph = lockfileToDepGraph(lockfile) const pkgMetaIterator = iteratePkgMeta(lockfile, graph) - for (const { hash, pkgMeta } of iterateHashedGraphNodes(graph, pkgMetaIterator)) { + const allowBuild = createAllowBuildFunction({ allowBuilds }) + for (const { hash, pkgMeta } of iterateHashedGraphNodes(graph, pkgMetaIterator, allowBuild)) { if (pkgMeta.name === pkgName) { return path.join(globalVirtualStoreDir, hash) } @@ -181,6 +189,7 @@ async function installPnpmToGlobalDir ( if (wantedLockfile != null && opts.storeController != null && opts.storeDir != null) { await installFromLockfile(installDir, binDir, { wantedLockfile, + allowBuilds: PNPM_ALLOW_BUILDS, storeController: opts.storeController, storeDir: opts.storeDir, registries: opts.registries as Registries, @@ -215,6 +224,7 @@ async function installFromLockfile ( binDir: string, opts: { wantedLockfile: LockfileObject + allowBuilds?: Record storeController: StoreController storeDir: string registries: Registries @@ -234,6 +244,7 @@ async function installFromLockfile ( registries: opts.registries, enableGlobalVirtualStore: true, globalVirtualStoreDir: path.join(opts.storeDir, 'links'), + allowBuilds: opts.allowBuilds, ignoreScripts: true, ignoreDepScripts: true, force: false, diff --git a/engine/pm/commands/tsconfig.json b/engine/pm/commands/tsconfig.json index 82ab1a2a3b..e32594c69c 100644 --- a/engine/pm/commands/tsconfig.json +++ b/engine/pm/commands/tsconfig.json @@ -16,6 +16,9 @@ { "path": "../../../bins/linker" }, + { + "path": "../../../building/policy" + }, { "path": "../../../cli/meta" }, diff --git a/global/commands/package.json b/global/commands/package.json index b2a8129a3d..7c99fa800b 100644 --- a/global/commands/package.json +++ b/global/commands/package.json @@ -35,7 +35,7 @@ "@pnpm/bins.linker": "workspace:*", "@pnpm/bins.remover": "workspace:*", "@pnpm/bins.resolver": "workspace:*", - "@pnpm/building.commands": "workspace:*", + "@pnpm/cli.command": "workspace:*", "@pnpm/cli.utils": "workspace:*", "@pnpm/config.matcher": "workspace:*", "@pnpm/config.reader": "workspace:*", diff --git a/global/commands/src/globalAdd.ts b/global/commands/src/globalAdd.ts index f0e27bedfc..f869e5c493 100644 --- a/global/commands/src/globalAdd.ts +++ b/global/commands/src/globalAdd.ts @@ -3,7 +3,7 @@ import path from 'node:path' import { linkBinsOfPackages } from '@pnpm/bins.linker' import { removeBin } from '@pnpm/bins.remover' -import { approveBuilds } from '@pnpm/building.commands' +import type { CommandHandlerMap } from '@pnpm/cli.command' import { cleanOrphanedInstallDirs, createGlobalCacheKey, @@ -17,10 +17,8 @@ import type { CreateStoreControllerOptions } from '@pnpm/store.connection-manage import { isSubdir } from 'is-subdir' import { symlinkDir } from 'symlink-dir' -import { installGlobalPackages } from './installGlobalPackages.js' - -type ApproveBuildsHandlerOpts = Parameters[0] import { checkGlobalBinConflicts } from './checkGlobalBinConflicts.js' +import { installGlobalPackages } from './installGlobalPackages.js' import { readInstalledPackages } from './readInstalledPackages.js' export type GlobalAddOptions = CreateStoreControllerOptions & { @@ -37,7 +35,8 @@ export type GlobalAddOptions = CreateStoreControllerOptions & { export async function handleGlobalAdd ( opts: GlobalAddOptions, - params: string[] + params: string[], + commands: CommandHandlerMap ): Promise { // Resolve relative path selectors to absolute paths before the working // directory is changed to the global install dir, otherwise "." or @@ -94,7 +93,7 @@ export async function handleGlobalAdd ( // If any packages had their builds skipped, prompt the user to approve them // (reuses the same interactive flow as `pnpm approve-builds`) if (ignoredBuilds?.size && process.stdin.isTTY) { - await approveBuilds.handler({ + await commands['approve-builds']({ ...opts, modulesDir: path.join(installDir, 'node_modules'), dir: installDir, @@ -105,7 +104,7 @@ export async function handleGlobalAdd ( global: false, pending: false, allowBuilds, - } as ApproveBuildsHandlerOpts) + }, [], commands) } // Read resolved aliases from the installed package.json diff --git a/global/commands/src/globalUpdate.ts b/global/commands/src/globalUpdate.ts index 283c33cd3a..baab3021a0 100644 --- a/global/commands/src/globalUpdate.ts +++ b/global/commands/src/globalUpdate.ts @@ -3,7 +3,7 @@ import path from 'node:path' import { linkBinsOfPackages } from '@pnpm/bins.linker' import { removeBin } from '@pnpm/bins.remover' -import { approveBuilds } from '@pnpm/building.commands' +import type { CommandHandlerMap } from '@pnpm/cli.command' import { cleanOrphanedInstallDirs, createInstallDir, @@ -16,10 +16,8 @@ import type { CreateStoreControllerOptions } from '@pnpm/store.connection-manage import { isSubdir } from 'is-subdir' import { symlinkDir } from 'symlink-dir' -import { installGlobalPackages } from './installGlobalPackages.js' - -type ApproveBuildsHandlerOpts = Parameters[0] import { checkGlobalBinConflicts } from './checkGlobalBinConflicts.js' +import { installGlobalPackages } from './installGlobalPackages.js' import { readInstalledPackages } from './readInstalledPackages.js' export type GlobalUpdateOptions = CreateStoreControllerOptions & { @@ -35,7 +33,8 @@ export type GlobalUpdateOptions = CreateStoreControllerOptions & { export async function handleGlobalUpdate ( opts: GlobalUpdateOptions, - params: string[] + params: string[], + commands: CommandHandlerMap ): Promise { const globalDir = opts.globalPkgDir! const globalBinDir = opts.bin! @@ -62,7 +61,7 @@ export async function handleGlobalUpdate ( // Update each package group sequentially to avoid overwhelming the system for (const pkg of packagesToUpdate) { - await updateGlobalPackageGroup(opts, globalDir, globalBinDir, pkg) // eslint-disable-line no-await-in-loop + await updateGlobalPackageGroup(opts, globalDir, globalBinDir, pkg, commands) // eslint-disable-line no-await-in-loop } return undefined } @@ -71,7 +70,8 @@ async function updateGlobalPackageGroup ( opts: GlobalUpdateOptions, globalDir: string, globalBinDir: string, - pkg: GlobalPackageInfo + pkg: GlobalPackageInfo, + commands: CommandHandlerMap ): Promise { const installDir = createInstallDir(globalDir) @@ -113,7 +113,7 @@ async function updateGlobalPackageGroup ( // If any packages had their builds skipped, prompt the user to approve them // (reuses the same interactive flow as `pnpm approve-builds`) if (ignoredBuilds?.size && process.stdin.isTTY) { - await approveBuilds.handler({ + await commands['approve-builds']({ ...opts, modulesDir: path.join(installDir, 'node_modules'), dir: installDir, @@ -124,7 +124,7 @@ async function updateGlobalPackageGroup ( global: false, pending: false, allowBuilds, - } as ApproveBuildsHandlerOpts) + }, [], commands) } // Check for bin name conflicts with other global packages diff --git a/global/commands/tsconfig.json b/global/commands/tsconfig.json index 5e7a0cd1ab..d5e158d4d6 100644 --- a/global/commands/tsconfig.json +++ b/global/commands/tsconfig.json @@ -19,7 +19,7 @@ "path": "../../bins/resolver" }, { - "path": "../../building/commands" + "path": "../../cli/command" }, { "path": "../../cli/utils" diff --git a/installing/commands/package.json b/installing/commands/package.json index eafeb472ab..1b27891dd0 100644 --- a/installing/commands/package.json +++ b/installing/commands/package.json @@ -33,7 +33,6 @@ }, "dependencies": { "@pnpm/building.after-install": "workspace:*", - "@pnpm/building.commands": "workspace:*", "@pnpm/catalogs.types": "workspace:*", "@pnpm/cli.command": "workspace:*", "@pnpm/cli.common-cli-options-help": "workspace:*", diff --git a/installing/commands/src/add.ts b/installing/commands/src/add.ts index 3ee33ef0b7..c817897d09 100644 --- a/installing/commands/src/add.ts +++ b/installing/commands/src/add.ts @@ -1,3 +1,4 @@ +import type { CommandHandlerMap } from '@pnpm/cli.command' import { FILTERING, OPTIONS, UNIVERSAL_OPTIONS } from '@pnpm/cli.common-cli-options-help' import { docsUrl } from '@pnpm/cli.utils' import { types as allTypes } from '@pnpm/config.reader' @@ -209,7 +210,8 @@ export type AddCommandOptions = InstallCommandOptions & { export async function handler ( opts: AddCommandOptions, - params: string[] + params: string[], + commands?: CommandHandlerMap ): Promise { if (opts.cliOptions['save'] === false) { throw new PnpmError('OPTION_NOT_SUPPORTED', 'The "add" command currently does not support the no-save option') @@ -251,7 +253,7 @@ export async function handler ( if (params.includes('pnpm') || params.includes('@pnpm/exe')) { throw new PnpmError('GLOBAL_PNPM_INSTALL', 'Use the "pnpm self-update" command to install or update pnpm') } - return handleGlobalAdd(opts, params) + return handleGlobalAdd(opts, params, commands ?? {}) } const include = { @@ -296,6 +298,7 @@ export async function handler ( return installDeps({ ...opts, allowBuilds: mergedAllowBuilds, + rebuildHandler: commands?.rebuild, fetchFullMetadata: getFetchFullMetadata(opts), include, includeDirect: include, @@ -303,6 +306,7 @@ export async function handler ( } return installDeps({ ...opts, + rebuildHandler: commands?.rebuild, fetchFullMetadata: getFetchFullMetadata(opts), include, includeDirect: include, diff --git a/installing/commands/src/dedupe.ts b/installing/commands/src/dedupe.ts index e2201bee72..2b4eac9377 100644 --- a/installing/commands/src/dedupe.ts +++ b/installing/commands/src/dedupe.ts @@ -1,3 +1,4 @@ +import type { CommandHandlerMap } from '@pnpm/cli.command' import { OPTIONS, UNIVERSAL_OPTIONS } from '@pnpm/cli.common-cli-options-help' import { docsUrl } from '@pnpm/cli.utils' import { dedupeDiffCheck } from '@pnpm/installing.dedupe.check' @@ -52,7 +53,7 @@ export interface DedupeCommandOptions extends InstallCommandOptions { readonly check?: boolean } -export async function handler (opts: DedupeCommandOptions): Promise { +export async function handler (opts: DedupeCommandOptions, _params?: string[], commands?: CommandHandlerMap): Promise { const include = { dependencies: opts.production !== false, devDependencies: opts.dev !== false, @@ -60,6 +61,7 @@ export async function handler (opts: DedupeCommandOptions): Promise { } return installDeps({ ...opts, + rebuildHandler: commands?.rebuild, dedupe: true, include, includeDirect: include, diff --git a/installing/commands/src/install.ts b/installing/commands/src/install.ts index ceb3830470..669523b6a7 100644 --- a/installing/commands/src/install.ts +++ b/installing/commands/src/install.ts @@ -1,3 +1,4 @@ +import type { CommandHandlerMap } from '@pnpm/cli.command' import { FILTERING, OPTIONS, OUTPUT_OPTIONS, UNIVERSAL_OPTIONS } from '@pnpm/cli.common-cli-options-help' import { docsUrl } from '@pnpm/cli.utils' import { type Config, types as allTypes } from '@pnpm/config.reader' @@ -349,7 +350,7 @@ export type InstallCommandOptions = Pick> -export async function handler (opts: InstallCommandOptions & { _calledFromLink?: boolean }): Promise { +export async function handler (opts: InstallCommandOptions & { _calledFromLink?: boolean }, _params?: string[], commands?: CommandHandlerMap): Promise { if (opts.global && !opts._calledFromLink) { throw new PnpmError('GLOBAL_INSTALL_NOT_SUPPORTED', '"pnpm install -g" is not supported. Use "pnpm add -g " to install global packages.') @@ -361,6 +362,7 @@ export async function handler (opts: InstallCommandOptions & { _calledFromLink?: } const installDepsOptions: InstallDepsOptions = { ...opts, + rebuildHandler: commands?.rebuild, frozenLockfileIfExists: opts.frozenLockfileIfExists ?? ( opts.ci && !opts.lockfileOnly && typeof opts.frozenLockfile === 'undefined' && diff --git a/installing/commands/src/installDeps.ts b/installing/commands/src/installDeps.ts index ff397ab4ac..313ec57062 100644 --- a/installing/commands/src/installDeps.ts +++ b/installing/commands/src/installDeps.ts @@ -1,6 +1,7 @@ import path from 'node:path' import { buildProjects } from '@pnpm/building.after-install' +import type { CommandHandler } from '@pnpm/cli.command' import { readProjectManifestOnly, tryReadProjectManifest, @@ -149,6 +150,7 @@ export type InstallDepsOptions = Pick> diff --git a/installing/commands/src/recursive.ts b/installing/commands/src/recursive.ts index 468e1569ad..92c17c9ad1 100755 --- a/installing/commands/src/recursive.ts +++ b/installing/commands/src/recursive.ts @@ -1,8 +1,8 @@ import { promises as fs } from 'node:fs' import path from 'node:path' -import { rebuild } from '@pnpm/building.commands' import type { Catalogs } from '@pnpm/catalogs.types' +import type { CommandHandler } from '@pnpm/cli.command' import { type RecursiveSummary, throwOnCommandFail, @@ -90,6 +90,7 @@ export type RecursiveOptions = CreateStoreControllerOptions & Pick & { + rebuildHandler?: CommandHandler include?: IncludedDependencies includeDirect?: IncludedDependencies latest?: boolean @@ -463,7 +464,7 @@ export async function recursive ( cmdFullName === 'update' ) ) { - await rebuild.handler({ + await opts.rebuildHandler?.({ ...opts, pending: opts.pending === true, skipIfHasSideEffectsCache: true, diff --git a/installing/commands/src/update/index.ts b/installing/commands/src/update/index.ts index 2426cfbfa0..cf2bfa0d43 100644 --- a/installing/commands/src/update/index.ts +++ b/installing/commands/src/update/index.ts @@ -1,4 +1,4 @@ -import type { CompletionFunc } from '@pnpm/cli.command' +import type { CommandHandler, CommandHandlerMap, CompletionFunc } from '@pnpm/cli.command' import { FILTERING, OPTIONS, UNIVERSAL_OPTIONS } from '@pnpm/cli.common-cli-options-help' import { docsUrl, @@ -168,7 +168,8 @@ export type UpdateCommandOptions = InstallCommandOptions & { export async function handler ( opts: UpdateCommandOptions, - params: string[] = [] + params: string[] = [], + commands?: CommandHandlerMap ): Promise { if (opts.global) { if (!opts.bin) { @@ -176,17 +177,19 @@ export async function handler ( hint: 'Run "pnpm setup" to create it automatically, or set the global-bin-dir setting, or the PNPM_HOME env variable. The global bin directory should be in the PATH.', }) } - return handleGlobalUpdate(opts, params) + return handleGlobalUpdate(opts, params, commands ?? {}) } + const rebuildHandler = commands?.rebuild if (opts.interactive) { - return interactiveUpdate(params, opts) + return interactiveUpdate(params, opts, rebuildHandler) } - return update(params, opts) as Promise + return update(params, opts, rebuildHandler) as Promise } async function interactiveUpdate ( input: string[], - opts: UpdateCommandOptions + opts: UpdateCommandOptions, + rebuildHandler?: CommandHandler ): Promise { const include = makeIncludeDependenciesFromCLI(opts.cliOptions) const projects = (opts.selectedProjectsGraph != null) @@ -276,12 +279,13 @@ async function interactiveUpdate ( } as any) as any // eslint-disable-line @typescript-eslint/no-explicit-any const updatePkgNames = pluck('value', updateDependencies as ChoiceRow[]) - return update(updatePkgNames, opts) as Promise + return update(updatePkgNames, opts, rebuildHandler) as Promise } async function update ( dependencies: string[], - opts: UpdateCommandOptions + opts: UpdateCommandOptions, + rebuildHandler?: CommandHandler ): Promise { if (opts.latest) { const dependenciesWithTags = dependencies.filter((name) => parseUpdateParam(name).versionSpec != null) @@ -307,6 +311,7 @@ async function update ( } return installDeps({ ...opts, + rebuildHandler, allowNew: false, depth, ignoreCurrentSpecifiers: false, diff --git a/installing/commands/tsconfig.json b/installing/commands/tsconfig.json index a10ceac266..09cb23474d 100644 --- a/installing/commands/tsconfig.json +++ b/installing/commands/tsconfig.json @@ -24,9 +24,6 @@ { "path": "../../building/after-install" }, - { - "path": "../../building/commands" - }, { "path": "../../catalogs/types" }, diff --git a/installing/deps-installer/src/install/extendInstallOptions.ts b/installing/deps-installer/src/install/extendInstallOptions.ts index c385410b5d..c0a3770880 100644 --- a/installing/deps-installer/src/install/extendInstallOptions.ts +++ b/installing/deps-installer/src/install/extendInstallOptions.ts @@ -337,8 +337,11 @@ export function extendOptions ( } extendedOpts.registries = normalizeRegistries(extendedOpts.registries) extendedOpts.rawConfig['registry'] = extendedOpts.registries.default - if (extendedOpts.enableGlobalVirtualStore && extendedOpts.virtualStoreDir == null) { - extendedOpts.virtualStoreDir = path.join(extendedOpts.storeDir, 'links') + if (extendedOpts.enableGlobalVirtualStore) { + if (extendedOpts.virtualStoreDir == null) { + extendedOpts.virtualStoreDir = path.join(extendedOpts.storeDir, 'links') + } + extendedOpts.allowBuilds ??= {} } extendedOpts.globalVirtualStoreDir = extendedOpts.enableGlobalVirtualStore ? extendedOpts.virtualStoreDir! diff --git a/installing/deps-installer/test/install/globalVirtualStore.ts b/installing/deps-installer/test/install/globalVirtualStore.ts index 236c6ecc56..ecd8767fcd 100644 --- a/installing/deps-installer/test/install/globalVirtualStore.ts +++ b/installing/deps-installer/test/install/globalVirtualStore.ts @@ -286,6 +286,54 @@ test('GVS successful build creates package directory with build artifacts', asyn } }) +test('GVS: approve-builds scenario — install with no builds, then reinstall with allowBuilds', async () => { + prepareEmpty() + const globalVirtualStoreDir = path.resolve('links') + const manifest = { + dependencies: { + '@pnpm.e2e/pre-and-postinstall-scripts-example': '1.0.0', + }, + } + + // Step 1: Install with builds NOT approved (simulating first `pnpm install`) + await install(manifest, testDefaults({ + enableGlobalVirtualStore: true, + virtualStoreDir: globalVirtualStoreDir, + fastUnpack: false, + allowBuilds: {}, + })) + + const pkgVersionDir = path.join(globalVirtualStoreDir, '@pnpm.e2e/pre-and-postinstall-scripts-example/1.0.0') + const hashBefore = fs.readdirSync(pkgVersionDir) + expect(hashBefore).toHaveLength(1) + + // Build artifacts should NOT be present + expect(fs.existsSync(path.resolve('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js'))).toBeFalsy() + + // Step 2: Reinstall with allowBuilds changed (simulating what approve-builds does) + await install(manifest, testDefaults({ + enableGlobalVirtualStore: true, + virtualStoreDir: globalVirtualStoreDir, + fastUnpack: false, + allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true }, + })) + + // Step 3: Verify the hash changed and build artifacts are in the new directory + const hashesAfter = fs.readdirSync(pkgVersionDir) + const newHash = hashesAfter.find((h) => h !== hashBefore[0]) + expect(newHash).toBeDefined() + expect(newHash).not.toBe(hashBefore[0]) + + // Build artifacts in new hash directory + const newPkgDir = path.join(pkgVersionDir, newHash!, 'node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example') + expect(fs.existsSync(path.join(newPkgDir, 'generated-by-postinstall.js'))).toBeTruthy() + expect(fs.existsSync(path.join(newPkgDir, 'generated-by-preinstall.js'))).toBeTruthy() + + // Build artifacts accessible via node_modules + expect(fs.existsSync(path.resolve('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js'))).toBeTruthy() + expect(fs.existsSync(path.resolve('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js'))).toBeTruthy() +}) + test('GVS build failure cleans up broken package directory', async () => { prepareEmpty() const globalVirtualStoreDir = path.resolve('links') diff --git a/installing/deps-restorer/src/index.ts b/installing/deps-restorer/src/index.ts index 060af82074..61296f7080 100644 --- a/installing/deps-restorer/src/index.ts +++ b/installing/deps-restorer/src/index.ts @@ -333,6 +333,9 @@ export async function headlessInstall (opts: HeadlessOptions): Promise CommandResponse | Promise // eslint-disable-line @typescript-eslint/no-explicit-any + (opts: PnpmOptions | any, params: string[], commands?: CommandHandlerMap) => CommandResponse | Promise // eslint-disable-line @typescript-eslint/no-explicit-any ) | ( - (opts: PnpmOptions | any, params: string[]) => void // eslint-disable-line @typescript-eslint/no-explicit-any + (opts: PnpmOptions | any, params: string[], commands?: CommandHandlerMap) => void // eslint-disable-line @typescript-eslint/no-explicit-any ) | ( - (opts: PnpmOptions | any, params: string[]) => Promise // eslint-disable-line @typescript-eslint/no-explicit-any + (opts: PnpmOptions | any, params: string[], commands?: CommandHandlerMap) => Promise // eslint-disable-line @typescript-eslint/no-explicit-any ) export interface CommandDefinition { diff --git a/pnpm/src/main.ts b/pnpm/src/main.ts index 526abd33ec..1b17d387df 100644 --- a/pnpm/src/main.ts +++ b/pnpm/src/main.ts @@ -307,7 +307,8 @@ export async function main (inputArgv: string[]): Promise { // TypeScript doesn't currently infer that the type of config // is `Omit` after the `delete config.reporter` statement config as Omit, - cliParams + cliParams, + pnpmCmds ) try { if (result instanceof Promise) { diff --git a/pnpm/test/install/globalVirtualStore.ts b/pnpm/test/install/globalVirtualStore.ts index d6fd686fe0..0d67e3a684 100644 --- a/pnpm/test/install/globalVirtualStore.ts +++ b/pnpm/test/install/globalVirtualStore.ts @@ -2,6 +2,7 @@ import fs from 'node:fs' import path from 'node:path' import { prepare } from '@pnpm/prepare' +import { readYamlFileSync } from 'read-yaml-file' import { writeYamlFileSync } from 'write-yaml-file' import { execPnpm } from '../utils/index.js' @@ -28,3 +29,49 @@ test('using a global virtual store', async () => { expect(fs.existsSync(path.join(globalVirtualStoreDir, '@pnpm.e2e/pkg-with-1-dep/100.0.0', files[0], 'node_modules/@pnpm.e2e/pkg-with-1-dep/package.json'))).toBeTruthy() expect(fs.existsSync(path.join(globalVirtualStoreDir, '@pnpm.e2e/pkg-with-1-dep/100.0.0', files[0], 'node_modules/@pnpm.e2e/dep-of-pkg-with-1-dep/package.json'))).toBeTruthy() }) + +test('approve-builds updates GVS symlinks and runs builds at correct hash directory', async () => { + prepare({ + dependencies: { + '@pnpm.e2e/pre-and-postinstall-scripts-example': '1.0.0', + }, + }) + const storeDir = path.resolve('store') + const globalVirtualStoreDir = path.join(storeDir, 'v11/links') + writeYamlFileSync(path.resolve('pnpm-workspace.yaml'), { + enableGlobalVirtualStore: true, + storeDir, + }) + + // Step 1: Install with GVS, builds NOT approved + await execPnpm(['install', '--config.strict-dep-builds=false']) + + const pkgVersionDir = path.join(globalVirtualStoreDir, '@pnpm.e2e/pre-and-postinstall-scripts-example/1.0.0') + const hashBefore = fs.readdirSync(pkgVersionDir) + expect(hashBefore).toHaveLength(1) + + // Build artifacts should NOT be present + expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')).toBeFalsy() + + // Step 2: approve-builds — updates config then runs install in GVS mode + await execPnpm(['approve-builds', '--all']) + + // Step 3: Verify GVS hash changed (new engine-specific directory) + const hashesAfter = fs.readdirSync(pkgVersionDir) + const newHash = hashesAfter.find((h) => h !== hashBefore[0]) + expect(newHash).toBeDefined() + expect(newHash).not.toBe(hashBefore[0]) + + // Build artifacts should be in the new hash directory + const newPkgDir = path.join(pkgVersionDir, newHash!, 'node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example') + expect(fs.existsSync(path.join(newPkgDir, 'generated-by-postinstall.js'))).toBeTruthy() + expect(fs.existsSync(path.join(newPkgDir, 'generated-by-preinstall.js'))).toBeTruthy() + + // Build artifacts should be accessible through node_modules + expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')).toBeTruthy() + expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')).toBeTruthy() + + // allowBuilds should be persisted in workspace manifest + const workspaceManifest = readYamlFileSync(path.resolve('pnpm-workspace.yaml')) // eslint-disable-line + expect(workspaceManifest.allowBuilds?.['@pnpm.e2e/pre-and-postinstall-scripts-example']).toBe(true) +})