diff --git a/.changeset/fix-dlx-prompt-approve-builds.md b/.changeset/fix-dlx-prompt-approve-builds.md new file mode 100644 index 0000000000..9567a350df --- /dev/null +++ b/.changeset/fix-dlx-prompt-approve-builds.md @@ -0,0 +1,8 @@ +--- +"@pnpm/building.commands": patch +"@pnpm/exec.commands": minor +"@pnpm/installing.commands": patch +"pnpm": patch +--- + +`pnpm dlx` (and `pnpx`/`pnx`/`pnpm create`) now runs the same interactive `approve-builds` prompt as `pnpm add -g` when the package being launched depends on transitive packages with install scripts. Previously, the v11 `strictDepBuilds` default made dlx fail with `ERR_PNPM_IGNORED_BUILDS` and required users to re-run with `--allow-build=` for every offending dependency. dlx also now removes the partially-populated cache directory when the install fails, so a subsequent run starts clean instead of reusing a broken install whose builds were silently skipped [#11444](https://github.com/pnpm/pnpm/issues/11444). diff --git a/building/commands/src/index.ts b/building/commands/src/index.ts index 0f3953a5a7..38153677d1 100644 --- a/building/commands/src/index.ts +++ b/building/commands/src/index.ts @@ -1,3 +1,3 @@ export { rebuild, type RebuildCommandOpts } from './build/index.js' export type { ApproveBuildsCommandOpts } from './policy/approveBuilds.js' -export { approveBuilds, ignoredBuilds } from './policy/index.js' +export { approveBuilds, getAutomaticallyIgnoredBuilds, ignoredBuilds } from './policy/index.js' diff --git a/building/commands/src/policy/index.ts b/building/commands/src/policy/index.ts index ca1722dd39..539af012f2 100644 --- a/building/commands/src/policy/index.ts +++ b/building/commands/src/policy/index.ts @@ -1,5 +1,6 @@ import * as approveBuilds from './approveBuilds.js' import * as ignoredBuilds from './ignoredBuilds.js' export type { ApproveBuildsCommandOpts } from './approveBuilds.js' +export { getAutomaticallyIgnoredBuilds } from './getAutomaticallyIgnoredBuilds.js' export { approveBuilds, ignoredBuilds } diff --git a/exec/commands/package.json b/exec/commands/package.json index 143f753ed1..230d5e7963 100644 --- a/exec/commands/package.json +++ b/exec/commands/package.json @@ -34,6 +34,7 @@ }, "dependencies": { "@pnpm/bins.resolver": "workspace:*", + "@pnpm/building.commands": "workspace:*", "@pnpm/catalogs.resolver": "workspace:*", "@pnpm/cli.command": "workspace:*", "@pnpm/cli.common-cli-options-help": "workspace:*", diff --git a/exec/commands/src/create.ts b/exec/commands/src/create.ts index 1d4ef30509..a66ffc17ab 100644 --- a/exec/commands/src/create.ts +++ b/exec/commands/src/create.ts @@ -1,3 +1,4 @@ +import type { CommandHandlerMap } from '@pnpm/cli.command' import { docsUrl } from '@pnpm/cli.utils' import { PnpmError } from '@pnpm/error' import { renderHelp } from 'render-help' @@ -6,7 +7,7 @@ import * as dlx from './dlx.js' export const commandNames = ['create'] -export async function handler (_opts: dlx.DlxCommandOptions, params: string[]): Promise<{ exitCode: number } | string> { +export async function handler (_opts: dlx.DlxCommandOptions, params: string[], commands?: CommandHandlerMap): Promise<{ exitCode: number } | string> { // If the first argument is --help or -h, we show the help message. if (params[0] === '--help' || params[0] === '-h') { return help() @@ -23,7 +24,7 @@ export async function handler (_opts: dlx.DlxCommandOptions, params: string[]): } const createPackageName = convertToCreateName(packageName) - return dlx.handler(_opts, [createPackageName, ...packageArgs]) + return dlx.handler(_opts, [createPackageName, ...packageArgs], commands) } export function rcOptionsTypes (): Record { diff --git a/exec/commands/src/dlx.ts b/exec/commands/src/dlx.ts index d4d61552fa..9786386a27 100644 --- a/exec/commands/src/dlx.ts +++ b/exec/commands/src/dlx.ts @@ -3,10 +3,12 @@ import path from 'node:path' import util from 'node:util' import { getBinsFromPackageManifest } from '@pnpm/bins.resolver' +import { getAutomaticallyIgnoredBuilds } from '@pnpm/building.commands' import { type CatalogResolver, resolveFromCatalog, } from '@pnpm/catalogs.resolver' +import type { CommandHandlerMap } from '@pnpm/cli.command' import { OUTPUT_OPTIONS } from '@pnpm/cli.common-cli-options-help' import { docsUrl, readProjectManifestOnly } from '@pnpm/cli.utils' import { type Config, types } from '@pnpm/config.reader' @@ -30,6 +32,14 @@ export const skipPackageManagerCheck = true export const commandNames = ['dlx'] +/** + * Test-only env var. When set, the dlx ignored-builds recovery path bypasses + * the TTY check and forwards `all: true` so `approve-builds` skips its + * multiselect and confirm prompts. Mirrors the same env var honored by + * `promptApproveGlobalBuilds` for global installs. Not for production use. + */ +const AUTO_APPROVE_FOR_TESTS_ENV = 'PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS' + export const shorthands: Record = { c: '--shell-mode', } @@ -88,7 +98,8 @@ export type DlxCommandOptions = { export async function handler ( opts: DlxCommandOptions, - [command, ...args]: string[] + [command, ...args]: string[], + commands?: CommandHandlerMap ): Promise<{ exitCode: number, output?: string }> { if (!command && (!opts.package || opts.package.length === 0)) { return { exitCode: 1, output: help() } @@ -147,15 +158,22 @@ export async function handler ( supportedArchitectures: opts.supportedArchitectures, }) if (!cacheExists) { + const allowBuilds = Object.fromEntries([...resolvedPkgAliases, ...(opts.allowBuild ?? [])].map(pkg => [pkg, true])) try { fs.mkdirSync(cachedDir, { recursive: true }) await add.handler({ ...opts, + // Mirror the global install flow: dlx prompts via `approve-builds` + // when transitive deps have skipped build scripts, so it must not let + // strictDepBuilds (the v11 default) turn that into a hard error. + // Without this, `pnpm dlx ` cannot launch packages whose bin + // depends on a postinstall step (e.g. native modules). + strictDepBuilds: false, enableGlobalVirtualStore: opts.enableGlobalVirtualStore ?? true, bin: path.join(cachedDir, 'node_modules/.bin'), dir: cachedDir, lockfileDir: cachedDir, - allowBuilds: Object.fromEntries([...resolvedPkgAliases, ...(opts.allowBuild ?? [])].map(pkg => [pkg, true])), + allowBuilds, rootProjectManifestDir: cachedDir, saveProd: true, // dlx will be looking for the package in the "dependencies" field! saveDev: false, @@ -164,6 +182,7 @@ export async function handler ( symlink: true, workspaceDir: undefined, }, resolvedPkgs) + await promptApproveDlxBuilds({ cachedDir, allowBuilds, inheritedOpts: opts }, commands) try { await symlinkDir(cachedDir, cacheLink, { overwrite: true }) } catch (error) { @@ -182,10 +201,14 @@ export async function handler ( // directory swaps). If another process completed the cache in the meantime, // use that instead of failing. const completedDir = getValidCacheDir(cacheLink, opts.dlxCacheMaxAge) - if (completedDir == null) { + if (completedDir != null) { + cachedDir = completedDir + } else { + // Drop the partially-populated cache so a subsequent dlx run starts + // clean instead of reusing a broken install. + await fs.promises.rm(cachedDir, { recursive: true, force: true }) throw err } - cachedDir = completedDir } } const binsDir = path.join(cachedDir, 'node_modules/.bin') @@ -252,6 +275,52 @@ function scopeless (pkgName: string): string { return pkgName } +/** + * After a dlx install with `strictDepBuilds: false`, check whether any + * transitive dependencies had their build scripts skipped and, if so, run + * the same `approve-builds` flow that `pnpm add -g` uses. Mirrors + * `promptApproveGlobalBuilds` in @pnpm/global.commands. + * + * In non-interactive mode (no TTY, no commands map) this is a no-op and + * the install is persisted to the dlx cache with builds skipped — same + * behavior as `pnpm add -g` in CI. Users who need the skipped scripts + * to run can re-invoke dlx with `--allow-build=`, which produces a + * different cache key and forces a fresh install. + */ +async function promptApproveDlxBuilds ( + opts: { + cachedDir: string + allowBuilds: Record + inheritedOpts: object + }, + commands?: CommandHandlerMap +): Promise { + if (!commands?.['approve-builds']) return + const autoApproveForTests = process.env[AUTO_APPROVE_FOR_TESTS_ENV] === '1' + if (!autoApproveForTests && !process.stdin.isTTY) return + const { automaticallyIgnoredBuilds } = await getAutomaticallyIgnoredBuilds({ + dir: opts.cachedDir, + lockfileDir: opts.cachedDir, + }) + if (!automaticallyIgnoredBuilds?.length) return + await commands['approve-builds']({ + ...opts.inheritedOpts, + dir: opts.cachedDir, + lockfileDir: opts.cachedDir, + rootProjectManifestDir: opts.cachedDir, + modulesDir: undefined, + workspaceDir: undefined, + allProjects: undefined, + selectedProjectsGraph: undefined, + workspacePackagePatterns: undefined, + rootProjectManifest: undefined, + global: false, + pending: false, + allowBuilds: opts.allowBuilds, + all: autoApproveForTests ? true : undefined, + }, [], commands) +} + function findCache (opts: { packages: string[] cacheDir: string diff --git a/exec/commands/test/create.ts b/exec/commands/test/create.ts index db6892f0e5..c2d5b14e1c 100644 --- a/exec/commands/test/create.ts +++ b/exec/commands/test/create.ts @@ -26,13 +26,13 @@ it( ...DEFAULT_OPTS, dir: process.cwd(), }, ['some-app']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-some-app']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-some-app'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['create_no_dash']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-create_no_dash']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-create_no_dash'], undefined) } ) @@ -43,13 +43,13 @@ it( ...DEFAULT_OPTS, dir: process.cwd(), }, ['create-some-app']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-some-app']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-some-app'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['create-']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-'], undefined) } ) @@ -60,13 +60,13 @@ it( ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope/some-app']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-some-app']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-some-app'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope/create_no_dash']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-create_no_dash']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-create_no_dash'], undefined) } ) @@ -77,13 +77,13 @@ it( ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope/create-some-app']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-some-app']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-some-app'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope/create-']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-'], undefined) } ) @@ -92,7 +92,7 @@ it('infers a package name from a plain scope', async () => { ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create'], undefined) }) it('passes the remaining arguments to `dlx`', async () => { @@ -100,7 +100,7 @@ it('passes the remaining arguments to `dlx`', async () => { ...DEFAULT_OPTS, dir: process.cwd(), }, ['some-app', 'directory/', '--silent']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-some-app', 'directory/', '--silent']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-some-app', 'directory/', '--silent'], undefined) }) it( @@ -110,35 +110,35 @@ it( ...DEFAULT_OPTS, dir: process.cwd(), }, ['foo@2.0.0']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-foo@2.0.0']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-foo@2.0.0'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['foo@latest']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-foo@latest']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['create-foo@latest'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope@2.0.0']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create@2.0.0']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create@2.0.0'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope@next']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create@next']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create@next'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope/foo@2.0.0']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-foo@2.0.0']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-foo@2.0.0'], undefined) await create.handler({ ...DEFAULT_OPTS, dir: process.cwd(), }, ['@scope/create-a@2.0.0']) - expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-a@2.0.0']) + expect(dlx.handler).toHaveBeenCalledWith(expect.anything(), ['@scope/create-a@2.0.0'], undefined) } ) diff --git a/exec/commands/test/dlx.e2e.ts b/exec/commands/test/dlx.e2e.ts index 5679ecd95e..cfb98c063c 100644 --- a/exec/commands/test/dlx.e2e.ts +++ b/exec/commands/test/dlx.e2e.ts @@ -10,9 +10,12 @@ const { getSystemNodeVersion: originalGetSystemNodeVersion } = await import('@pn jest.unstable_mockModule('@pnpm/engine.runtime.system-node-version', () => ({ getSystemNodeVersion: jest.fn(originalGetSystemNodeVersion), })) -const { add: originalAdd } = await import('@pnpm/installing.commands') +const installingCommands = await import('@pnpm/installing.commands') +const { add: originalAdd } = installingCommands jest.unstable_mockModule('@pnpm/installing.commands', () => ({ + ...installingCommands, add: { + ...originalAdd, handler: jest.fn(originalAdd.handler), }, })) @@ -20,6 +23,7 @@ jest.unstable_mockModule('@pnpm/installing.commands', () => ({ const systemNodeVersion = await import('@pnpm/engine.runtime.system-node-version') const { add } = await import('@pnpm/installing.commands') const { dlx } = await import('@pnpm/exec.commands') +const { approveBuilds } = await import('@pnpm/building.commands') const testOnWindowsOnly = process.platform === 'win32' ? test : test.skip @@ -388,6 +392,72 @@ test('dlx builds the packages passed via --allow-build', async () => { expect(fs.existsSync(path.join(builtPkg2Path, 'generated-by-install.js'))).toBeTruthy() }) +// Regression test for https://github.com/pnpm/pnpm/issues/11444. +// +// dlx mirrors the global install flow: it overrides `strictDepBuilds` +// internally so the install never throws ERR_PNPM_IGNORED_BUILDS, then +// runs the same interactive `approve-builds` prompt that `pnpm add -g` +// uses when transitive deps have skipped build scripts. The user can +// opt in to the builds without retrying with `--allow-build=`. +// +// Without a TTY (and without the test escape hatch below), the prompt is +// skipped and dlx proceeds with build scripts skipped — same behavior +// as `pnpm add -g` in CI. +test('dlx does not error on ignored builds in non-interactive mode', async () => { + prepareEmpty() + + await dlx.handler({ + ...DEFAULT_OPTS, + enableGlobalVirtualStore: false, + strictDepBuilds: true, + dir: path.resolve('project'), + storeDir: path.resolve('store'), + cacheDir: path.resolve('cache'), + dlxCacheMaxAge: Infinity, + }, ['@pnpm.e2e/has-bin-and-needs-build']) + + // Cache is populated even though build scripts were skipped — the + // package is installed so the bin can run if it does not depend on + // the skipped script. + const dlxCacheDir = path.resolve('cache', 'dlx', createCacheKey('@pnpm.e2e/has-bin-and-needs-build@1.0.0'), 'pkg') + expect(fs.existsSync(path.join(dlxCacheDir, 'package.json'))).toBe(true) +}) + +// Regression test for https://github.com/pnpm/pnpm/issues/11444. +// +// `PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS=1` lets the test drive the +// approve-builds flow non-interactively: dlx skips the TTY check and +// forwards `all: true` to approve-builds, which approves every pending +// build without prompting and re-runs install. The build artifacts must +// end up in the dlx cache. +test('dlx prompts to approve ignored builds when invoked with a commands map', async () => { + prepareEmpty() + + const prevAutoApprove = process.env.PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS + process.env.PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS = '1' + try { + await dlx.handler({ + ...DEFAULT_OPTS, + enableGlobalVirtualStore: false, + strictDepBuilds: true, + dir: path.resolve('project'), + storeDir: path.resolve('store'), + cacheDir: path.resolve('cache'), + dlxCacheMaxAge: Infinity, + }, ['@pnpm.e2e/has-bin-and-needs-build'], { 'approve-builds': approveBuilds.handler }) + } finally { + if (prevAutoApprove === undefined) { + delete process.env.PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS + } else { + process.env.PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS = prevAutoApprove + } + } + + const dlxCacheDir = path.resolve('cache', 'dlx', createCacheKey('@pnpm.e2e/has-bin-and-needs-build@1.0.0'), 'pkg') + const builtPkg2Path = path.join(dlxCacheDir, 'node_modules/.pnpm/@pnpm.e2e+install-script-example@1.0.0/node_modules/@pnpm.e2e/install-script-example') + expect(fs.existsSync(path.join(builtPkg2Path, 'generated-by-install.js'))).toBe(true) +}) + test('dlx should fail when the requested package does not meet the minimum age requirement', async () => { prepareEmpty() diff --git a/exec/commands/tsconfig.json b/exec/commands/tsconfig.json index 7044d0dba7..cfd305a983 100644 --- a/exec/commands/tsconfig.json +++ b/exec/commands/tsconfig.json @@ -18,6 +18,9 @@ { "path": "../../bins/resolver" }, + { + "path": "../../building/commands" + }, { "path": "../../catalogs/resolver" }, diff --git a/installing/commands/src/install.ts b/installing/commands/src/install.ts index fff1d15fd6..a539e0f15b 100644 --- a/installing/commands/src/install.ts +++ b/installing/commands/src/install.ts @@ -353,7 +353,7 @@ export type InstallCommandOptions = Pick> +} & Partial> export async function handler (opts: InstallCommandOptions & { _calledFromLink?: boolean }, _params?: string[], commands?: CommandHandlerMap): Promise { if (opts.global && !opts._calledFromLink) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5e2166725e..8b5d7ae376 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4128,6 +4128,9 @@ importers: '@pnpm/bins.resolver': specifier: workspace:* version: link:../../bins/resolver + '@pnpm/building.commands': + specifier: workspace:* + version: link:../../building/commands '@pnpm/catalogs.resolver': specifier: workspace:* version: link:../../catalogs/resolver