mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-31 12:10:49 -04:00
fix(installing.commands): forward pnpm install flags to pacquet (#11781)
* fix(installing.commands): forward `pnpm install` flags to pacquet When the install engine is delegated to pacquet via configDependencies, pnpm hard-coded the args to `install --frozen-lockfile --reporter=ndjson` and silently dropped the user's other CLI flags. `pnpm install --no-runtime` therefore still installed the workspace's runtime devDependency, clobbering the Node version the surrounding tooling had set up — visible as the `Verify Node version` failure on PR #11765 where setup-pnpm provisions Node 24.0.0 but pacquet then materializes node 24.6.0. Pacquet's `install` subcommand already mirrors pnpm's surface for the common flags (`--no-runtime`, `--prod`, `--dev`, `--no-optional`, `--node-linker`, `--offline`, `--prefer-offline`, `--cpu`/`--os`/`--libc`). Forward the user's argv verbatim when the command is `install`/`i`; `add`/`update`/`dedupe` still don't forward — their flag surfaces don't line up with pacquet's `install`. * fix(installing.commands): pass --ignore-manifest-check to pacquet `pnpm up` / `add` / `remove` were aborting with `pacquet_package_manager::outdated_lockfile` whenever pacquet was declared in `configDependencies`. After resolving and writing the updated lockfile, pnpm hands materialization off to pacquet but hasn't yet written the post-mutation `package.json` — that write happens after `mutateModules` returns. Pacquet's frozen-lockfile freshness gate then saw the new lockfile paired with the pre-mutation manifest and refused to install. Pass pacquet's new `--ignore-manifest-check` flag (pacquet PR #11811) on every delegation. The flag is narrow: it only skips `satisfies_package_manifest`. Settings drift like `overrides` is still enforced, and pnpm already re-validated the lockfile before delegating, so re-checking the manifest here was redundant work that only ever fired false positives on the mutate-then-materialize path. Requires a pacquet release that ships the flag; bump `PACQUET_VERSION` in `pnpm/test/install/pacquet.ts` once it does, or the existing e2e tests will fail against pacquet 0.2.2-9 (which doesn't recognize the flag and clap would reject). Closes #11797. --- Written by an agent (Claude Code, claude-opus-4-7). * fix: update pacquet in tests * fix(installing.commands): strip positionals + always-injected flags when forwarding to pacquet `collectForwardedFlags` checked `argv[0] === 'install'` to find the command token to strip. Any global flag the user typed before `install` (e.g. `--config.registry=...` in the e2e test) shifted the token out of position, so the function returned the full argv and pacquet saw `install` twice — `error: unexpected argument 'install' found`. Use the parsed argv that `@pnpm/cli.parse-cli-args` already produced: `remain` lists positionals (the `install`/`i` token and nothing else on this code path, since `isInstallCommand` is only true when no package params are present), and `original` preserves the user's exact tokens. Drop positionals + the flags we always inject (`--reporter=ndjson`, `--frozen-lockfile`, `--ignore-manifest-check`) so clap doesn't reject duplicates either. `original` over `cooked` deliberately: nopt's `cooked` splits `--key=value` into two tokens, which would break pacquet's `--config.<key>=<value>` parser (it requires the `=` form). * fix(installing.commands): make argv.cooked/remain optional on InstallCommandOptions Widening these to required broke test fixtures elsewhere (publish/pack/ deprecate/dist-tag/deploy) that construct minimal `argv: { original }` options for code paths that never reach pacquet. Only the pacquet delegation actually reads `remain`, so make the two new fields optional on the shared options type and supply a default at the runPacquet call site. The runtime path through main.ts already populates all three. * fix(installing.commands): strip any user-supplied --reporter when forwarding to pacquet Pacquet's `--reporter` is a clap value option with last-value-wins semantics, so `pnpm install --reporter=silent` (or `--reporter silent` two-token form) reached pacquet and overrode the `--reporter=ndjson` pnpm injects, breaking the NDJSON-to- streamParser plumbing the default reporter depends on. The previous filter only matched the exact `--reporter=ndjson` token. Walk argv with a lookahead so both `--reporter=<value>` and `--reporter <value>` are dropped without consuming an adjacent flag. * fix(installing.commands): drop negated/value forms of always-injected flags `collectForwardedFlags` only matched the exact positive tokens `--frozen-lockfile` and `--ignore-manifest-check`, so a user typing `pnpm install --no-frozen-lockfile` (or `--frozen-lockfile=false`) forwarded the negation to pacquet, which then saw both our injected `--frozen-lockfile` and the user's `--no-frozen-lockfile` and crashed clap with "unexpected argument". Match every shape the user can write the same flag in: positive, `--no-` negated, and any `=value` form. Can't blindly strip `--no-` either way — pacquet has flags whose literal name starts with `no-` (`--no-runtime`, `--no-optional`); those must still forward. The user's `--no-frozen-lockfile` intent is honored upstream — pnpm did a fresh resolve before delegating; pacquet's role here is just lockfile-driven materialization, which is always frozen. * fix(installing.commands): match positionals by index, hide reporter from dropped-flags warning `collectForwardedFlags` matched positionals via `new Set(argv.remain)`, which strips by value: a flag value that happened to equal a positional token (e.g. `pnpm install --node-linker install`) was wrongly dropped from the forwarded list, costing pacquet the value of `--node-linker`. Walk `argv.original` with a subsequence pointer into `argv.remain` so only the actual positional indexes get skipped. `collectDroppedFlags` still surfaced `--reporter foo` / `--reporter=foo` in the "may not be honored" warning on `add`/`update`/`dedupe`, but pnpm honors reporter selection itself before delegation — so the warning was misleading. Route both helpers through the same `isAlwaysInjected` check and consume `--reporter` and its value the same way `collectForwardedFlags` already does.
This commit is contained in:
Zoltan Kochan
5
.changeset/fix-pacquet-outdated-lockfile-on-update.md
Normal file
5
.changeset/fix-pacquet-outdated-lockfile-on-update.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
Fixed `pnpm up` (and `pnpm add` / `pnpm remove`) failing with `pacquet_package_manager::outdated_lockfile` when pacquet is declared in `configDependencies`. pnpm now passes `--ignore-manifest-check` to pacquet so its `--frozen-lockfile` check doesn't fire against the (pre-mutation) `package.json` pnpm hasn't written yet [#11797](https://github.com/pnpm/pnpm/issues/11797). Requires a pacquet release that supports the flag — bump `PACQUET_VERSION` in the e2e tests once it ships.
|
||||
6
.changeset/forward-install-flags-to-pacquet.md
Normal file
6
.changeset/forward-install-flags-to-pacquet.md
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@pnpm/installing.commands": patch
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
When the install engine is delegated to pacquet via `configDependencies`, the user's CLI flags passed to `pnpm install` (e.g. `--no-runtime`, `--prod`, `--dev`, `--no-optional`, `--node-linker`, `--cpu`/`--os`/`--libc`, `--offline`, `--prefer-offline`) are now forwarded to pacquet's `install` subcommand verbatim. Previously pacquet was invoked with a fixed argument list, so flags like `--no-runtime` were silently dropped. Flag forwarding is gated on the command being `install`/`i`; `add`, `update`, and `dedupe` still don't forward (their flag surface doesn't line up with pacquet's `install`).
|
||||
@@ -350,7 +350,9 @@ export type InstallCommandOptions = Pick<Config,
|
||||
| 'selectedProjectsGraph'
|
||||
> & CreateStoreControllerOptions & Partial<Pick<Config, 'globalPkgDir'>> & {
|
||||
argv: {
|
||||
cooked?: string[]
|
||||
original: string[]
|
||||
remain?: string[]
|
||||
}
|
||||
fixLockfile?: boolean
|
||||
frozenLockfileIfExists?: boolean
|
||||
@@ -388,6 +390,7 @@ export async function handler (opts: InstallCommandOptions & { _calledFromLink?:
|
||||
include,
|
||||
includeDirect: include,
|
||||
fetchFullMetadata: getFetchFullMetadata(opts),
|
||||
isInstallCommand: true,
|
||||
}
|
||||
if (opts.resolutionOnly) {
|
||||
installDepsOptions.lockfileOnly = true
|
||||
|
||||
@@ -127,7 +127,9 @@ export type InstallDepsOptions = Pick<Config,
|
||||
> & Partial<Pick<Config, 'ci'>>
|
||||
& CreateStoreControllerOptions & {
|
||||
argv: {
|
||||
cooked?: string[]
|
||||
original: string[]
|
||||
remain?: string[]
|
||||
}
|
||||
allowNew?: boolean
|
||||
forceFullResolution?: boolean
|
||||
@@ -160,6 +162,13 @@ export type InstallDepsOptions = Pick<Config,
|
||||
rebuildHandler?: CommandHandler
|
||||
pnpmfile: string[]
|
||||
packageVulnerabilityAudit?: PackageVulnerabilityAudit
|
||||
/**
|
||||
* `true` when this call originated from `pnpm install` (or `pnpm i`),
|
||||
* `false`/`undefined` for `add`, `update`, `dedupe`, etc. Used to gate
|
||||
* which pnpm CLI flags are safe to forward to pacquet's `install`
|
||||
* subcommand — see `runPacquet.ts`'s `noRuntime` opt.
|
||||
*/
|
||||
isInstallCommand?: boolean
|
||||
} & Partial<Pick<Config, 'pnpmHomeDir' | 'strictDepBuilds'>>
|
||||
|
||||
export async function installDeps (
|
||||
@@ -216,7 +225,8 @@ export async function installDeps (
|
||||
? makeRunPacquet({
|
||||
lockfileDir: opts.lockfileDir ?? opts.dir,
|
||||
packageName: pacquetConfigDepName,
|
||||
argv: opts.argv.original,
|
||||
argv: { original: opts.argv.original, remain: opts.argv.remain ?? [] },
|
||||
isInstallCommand: opts.isInstallCommand === true,
|
||||
})
|
||||
: undefined
|
||||
const includeDirect = opts.includeDirect ?? {
|
||||
|
||||
@@ -27,27 +27,43 @@ export interface MakeRunPacquetOpts {
|
||||
*/
|
||||
packageName: 'pacquet' | '@pnpm/pacquet'
|
||||
/**
|
||||
* The user's original `pnpm` argv (`process.argv.slice(2)`). Not
|
||||
* forwarded to pacquet — we only inspect it to warn about flags
|
||||
* The parsed pnpm argv from `@pnpm/cli.parse-cli-args` — `original`
|
||||
* preserves the user's exact tokens (so `--key=value` stays joined,
|
||||
* which pacquet's `--config.<key>=<value>` parser requires), and
|
||||
* `remain` lists the positionals (the `install`/`i` command token
|
||||
* among them). When `isInstallCommand` is true we forward
|
||||
* `original` minus positionals to pacquet's own `install`
|
||||
* subcommand; otherwise we only inspect it to warn about flags
|
||||
* pacquet won't see.
|
||||
*/
|
||||
argv: string[]
|
||||
argv: {
|
||||
original: string[]
|
||||
remain: string[]
|
||||
}
|
||||
/**
|
||||
* `true` when the user invoked `pnpm install` (or `pnpm i`). Gates
|
||||
* flag forwarding: pacquet's `install` subcommand mirrors pnpm's
|
||||
* surface closely enough that the user's flags are safe to pass
|
||||
* along on that command, but not from `add`/`update`/`dedupe` (whose
|
||||
* own flag surface doesn't line up with pacquet's `install`).
|
||||
*/
|
||||
isInstallCommand: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the install-engine callback `mutateModules` invokes when
|
||||
* `configDependencies` declares pacquet. Returns `undefined` when no
|
||||
* pacquet binary is on disk — the caller falls back to the JS path in
|
||||
* that case.
|
||||
* `configDependencies` declares pacquet.
|
||||
*
|
||||
* The callback spawns the pacquet binary installed under
|
||||
* `node_modules/.pnpm-config/pacquet` and forwards the user's own
|
||||
* pnpm CLI flags. Pacquet's NDJSON stderr is parsed line-by-line and
|
||||
* the valid JSON records are re-emitted on pnpm's global
|
||||
* `streamParser` so `@pnpm/cli.default-reporter` renders pacquet's
|
||||
* events the same way it renders pnpm's own. Non-JSON stderr lines
|
||||
* (panic backtraces, unexpected diagnostics) are forwarded to the
|
||||
* real stderr verbatim so they reach the user.
|
||||
* `node_modules/.pnpm-config/pacquet`. From `pnpm install`/`pnpm i` it
|
||||
* forwards the user's own pnpm CLI flags to pacquet's `install`
|
||||
* subcommand; from `add`/`update`/`dedupe` it doesn't forward (warning
|
||||
* instead). Pacquet's NDJSON stderr is parsed line-by-line and the
|
||||
* valid JSON records are re-emitted on pnpm's global `streamParser` so
|
||||
* `@pnpm/cli.default-reporter` renders pacquet's events the same way it
|
||||
* renders pnpm's own. Non-JSON stderr lines (panic backtraces,
|
||||
* unexpected diagnostics) are forwarded to the real stderr verbatim so
|
||||
* they reach the user.
|
||||
*/
|
||||
/** Args the deps-installer passes per pacquet invocation. */
|
||||
export interface RunPacquetCallOpts {
|
||||
@@ -67,15 +83,27 @@ export interface RunPacquetCallOpts {
|
||||
export function makeRunPacquet (opts: MakeRunPacquetOpts): (callOpts?: RunPacquetCallOpts) => Promise<void> {
|
||||
return async (callOpts) => {
|
||||
const pacquetBin = resolvePacquetBin(opts.lockfileDir, opts.packageName)
|
||||
// Always the same fixed args. We don't forward pnpm's CLI flags
|
||||
// even though pacquet's `install` subcommand mirrors most of them:
|
||||
// pnpm has commands like `add` and `update` that carry flags
|
||||
// pacquet's `install` doesn't recognize (e.g., `--save-dev`,
|
||||
// `--save-peer`), and clap would reject them. The settings users
|
||||
// care about live in `pnpm-workspace.yaml` / `.npmrc`, which
|
||||
// pacquet reads on its own.
|
||||
const args = ['--reporter=ndjson', 'install', '--frozen-lockfile']
|
||||
const droppedFlags = collectDroppedFlags(opts.argv)
|
||||
// From `pnpm install`/`pnpm i` we forward the user's flags through to
|
||||
// pacquet's own `install` subcommand verbatim — pacquet mirrors pnpm's
|
||||
// surface closely enough on that command that they're safe to pass
|
||||
// along. From `add`/`update`/`dedupe` we don't forward anything: those
|
||||
// commands carry flags pacquet's `install` doesn't recognize
|
||||
// (`--save-dev`, `--save-peer`, etc.) which clap would reject. Either
|
||||
// way pacquet picks up the settings users care about from
|
||||
// `pnpm-workspace.yaml` / `.npmrc` on its own, so a non-install
|
||||
// delegation isn't broken by the omission.
|
||||
const forwardedFlags = opts.isInstallCommand ? collectForwardedFlags(opts.argv) : []
|
||||
// `--ignore-manifest-check` tells pacquet to skip its per-importer
|
||||
// `package.json` ↔ `pnpm-lock.yaml` freshness gate. pnpm just
|
||||
// resolved and wrote the lockfile itself; on `pnpm up` / `add` /
|
||||
// `remove` the manifest on disk is still the pre-mutation copy
|
||||
// (pnpm writes it after `mutateModules` returns), so pacquet's own
|
||||
// check would always fire here. See
|
||||
// https://github.com/pnpm/pnpm/issues/11797. The flag is narrow
|
||||
// (only the manifest check); settings drift like `overrides` is
|
||||
// still enforced and was already re-validated by pnpm.
|
||||
const args = ['--reporter=ndjson', 'install', '--frozen-lockfile', '--ignore-manifest-check', ...forwardedFlags]
|
||||
const droppedFlags = opts.isInstallCommand ? [] : collectDroppedFlags(opts.argv)
|
||||
if (droppedFlags.length > 0) {
|
||||
logger.warn({
|
||||
message: `The following CLI flags are not forwarded to pacquet and may not be honored: ${droppedFlags.join(' ')}. Move the equivalent settings into pnpm-workspace.yaml (or .npmrc for auth/registry) if pacquet needs them.`,
|
||||
@@ -154,24 +182,97 @@ function resolvePacquetBin (lockfileDir: string, packageName: 'pacquet' | '@pnpm
|
||||
}
|
||||
|
||||
/**
|
||||
* Pull the CLI flags out of pnpm's argv so we can warn about them
|
||||
* before pacquet runs. We don't forward any of them — pacquet always
|
||||
* gets `install --frozen-lockfile --reporter=ndjson` — but most are
|
||||
* handled by pnpm itself before delegation (`--save-dev` rewrites
|
||||
* `package.json`, `--filter` selects projects, etc.) so listing them
|
||||
* to the user makes the "not forwarded" surface concrete.
|
||||
* From `pnpm install`/`pnpm i`, return everything in argv that should
|
||||
* ride along to pacquet's own `install` subcommand. Drops the
|
||||
* positionals nopt classified (`install` / `i`, plus anything users
|
||||
* typed positionally) since pacquet's `install` doesn't accept any —
|
||||
* leaving them in produces `error: unexpected argument 'install'
|
||||
* found`. Pacquet's clap parser walks the same `--prod`, `--dev`,
|
||||
* `--no-optional`, `--no-runtime`, `--node-linker`, `--offline`,
|
||||
* `--prefer-offline`, `--cpu`, `--os`, `--libc`, `--frozen-lockfile`
|
||||
* surface pnpm itself accepts on `install`, so the flags don't need
|
||||
* reshaping.
|
||||
*
|
||||
* Flags we explicitly emit ourselves (`--frozen-lockfile`,
|
||||
* `--reporter=ndjson`) are filtered out: they're honored, so warning
|
||||
* about them would be misleading. `--config.*` is filtered too —
|
||||
* those configure pnpm's runtime and aren't intended for the install
|
||||
* engine.
|
||||
* Flags we always inject ourselves (`--frozen-lockfile`,
|
||||
* `--ignore-manifest-check`) are dropped in every form the user can
|
||||
* type them — positive (`--frozen-lockfile`), negated
|
||||
* (`--no-frozen-lockfile`), and any `=value` form. Pacquet's clap
|
||||
* defines these as plain `#[clap(long)] bool` flags, so a duplicate
|
||||
* `--frozen-lockfile` or a conflicting `--no-frozen-lockfile`
|
||||
* crashes the parser with "used multiple times" / "unexpected
|
||||
* argument". The user's `--no-frozen-lockfile` intent is already
|
||||
* honored upstream (pnpm did a fresh resolve before delegating);
|
||||
* pacquet's role here is just lockfile-driven materialization.
|
||||
*
|
||||
* `--reporter` is stripped in any form (`--reporter=foo`,
|
||||
* `--reporter foo`): pacquet's `reporter` is a clap value option
|
||||
* with last-value-wins semantics, so a user-supplied value would
|
||||
* override our `--reporter=ndjson` and break the
|
||||
* NDJSON-to-streamParser plumbing the default reporter relies on.
|
||||
*/
|
||||
function collectDroppedFlags (argv: string[]): string[] {
|
||||
return argv.filter((arg) => {
|
||||
if (!arg.startsWith('-')) return false
|
||||
if (arg === '--frozen-lockfile' || arg === '--reporter=ndjson') return false
|
||||
if (arg.startsWith('--config.')) return false
|
||||
return true
|
||||
})
|
||||
function collectForwardedFlags (argv: { original: string[], remain: string[] }): string[] {
|
||||
const result: string[] = []
|
||||
// `argv.remain` is the ordered subsequence of positionals nopt
|
||||
// extracted from `original`. Match by index rather than by value so
|
||||
// an option's value that happens to equal a positional (e.g.
|
||||
// `--node-linker install`) isn't mistaken for the positional itself.
|
||||
let positionalIdx = 0
|
||||
for (let i = 0; i < argv.original.length; i++) {
|
||||
const arg = argv.original[i]
|
||||
if (positionalIdx < argv.remain.length && arg === argv.remain[positionalIdx]) {
|
||||
positionalIdx++
|
||||
continue
|
||||
}
|
||||
if (isAlwaysInjected(arg)) continue
|
||||
if (arg.startsWith('--reporter=')) continue
|
||||
if (arg === '--reporter') {
|
||||
// Consume the next token as the reporter value (`--reporter foo`).
|
||||
i++
|
||||
continue
|
||||
}
|
||||
result.push(arg)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
const ALWAYS_INJECTED_FLAGS = ['frozen-lockfile', 'ignore-manifest-check'] as const
|
||||
|
||||
function isAlwaysInjected (arg: string): boolean {
|
||||
for (const name of ALWAYS_INJECTED_FLAGS) {
|
||||
if (arg === `--${name}` || arg === `--no-${name}`) return true
|
||||
if (arg.startsWith(`--${name}=`) || arg.startsWith(`--no-${name}=`)) return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* From a non-install command (`add`, `update`, `dedupe`, ...), pull the
|
||||
* CLI flags out of pnpm's argv so we can warn that pacquet won't see
|
||||
* them. They're still handled by pnpm itself before delegation
|
||||
* (`--save-dev` rewrites `package.json`, `--filter` selects projects,
|
||||
* etc.) so listing them to the user makes the "not forwarded" surface
|
||||
* concrete.
|
||||
*
|
||||
* Flags pnpm itself honors before delegation are filtered out —
|
||||
* warning about them would be misleading: `--frozen-lockfile` and
|
||||
* `--ignore-manifest-check` in every shape (positive / negated /
|
||||
* `=value`); `--reporter` in every shape (`--reporter=foo`,
|
||||
* `--reporter foo`); and `--config.*` (configures pnpm's runtime,
|
||||
* not the install engine).
|
||||
*/
|
||||
function collectDroppedFlags (argv: { original: string[] }): string[] {
|
||||
const result: string[] = []
|
||||
for (let i = 0; i < argv.original.length; i++) {
|
||||
const arg = argv.original[i]
|
||||
if (!arg.startsWith('-')) continue
|
||||
if (isAlwaysInjected(arg)) continue
|
||||
if (arg.startsWith('--config.')) continue
|
||||
if (arg.startsWith('--reporter=')) continue
|
||||
if (arg === '--reporter') {
|
||||
i++
|
||||
continue
|
||||
}
|
||||
result.push(arg)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
@@ -11,7 +11,7 @@ import { execPnpm, execPnpmSync } from '../utils/index.js'
|
||||
// version known to ship the `configDependencies` integration surface this
|
||||
// PR depends on; tests are gated on the public registry being reachable.
|
||||
const PUBLIC_REGISTRY = '--config.registry=https://registry.npmjs.org/'
|
||||
const PACQUET_VERSION = '0.2.2-9'
|
||||
const PACQUET_VERSION = '0.2.2'
|
||||
|
||||
// Each test runs two or three installs against the public registry; raise
|
||||
// the per-test timeout above jest's 5s default to allow for cold caches.
|
||||
@@ -126,9 +126,9 @@ test.skip('`pnpm update <pkg>` resolves a new version with pnpm and materializes
|
||||
|
||||
// Skipped until pacquet ships a release built with the updated
|
||||
// `generate-packages.mjs` (this PR's change) so the `@pnpm/pacquet`
|
||||
// scoped alias actually exists on npm. The pinned `0.2.2-9` doesn't
|
||||
// publish that mirror yet. Re-enable when the next pacquet release
|
||||
// ships under both names.
|
||||
// scoped alias actually exists on npm. The pinned PACQUET_VERSION
|
||||
// above doesn't publish that mirror yet. Re-enable when the next
|
||||
// pacquet release ships under both names.
|
||||
test.skip('the `@pnpm/pacquet` scoped alias is recognized in configDependencies', async () => {
|
||||
await prepareWithPacquet({
|
||||
manifest: { dependencies: { 'is-positive': '3.1.0' } },
|
||||
|
||||
Reference in New Issue
Block a user