diff --git a/.changeset/fix-11587-global-isolated-per-arg.md b/.changeset/fix-11587-global-isolated-per-arg.md new file mode 100644 index 0000000000..3aae4a1b68 --- /dev/null +++ b/.changeset/fix-11587-global-isolated-per-arg.md @@ -0,0 +1,12 @@ +--- +"@pnpm/global.commands": patch +"@pnpm/installing.deps-installer": patch +"pnpm": patch +--- + +`pnpm add -g` now installs each space-separated package into its own isolated directory by default. To bundle multiple packages into the same isolated install (so that they share dependencies and are removed together), pass them as a comma-separated list. For example: + +- `pnpm add -g foo bar` installs `foo` and `bar` as two independent globals — removing one does not affect the other. +- `pnpm add -g foo,bar qar` bundles `foo` and `bar` into a single isolated install while `qar` is installed on its own. + +Related: [#11587](https://github.com/pnpm/pnpm/issues/11587). diff --git a/global/commands/package.json b/global/commands/package.json index ae0851088d..9b677f6f7d 100644 --- a/global/commands/package.json +++ b/global/commands/package.json @@ -39,6 +39,7 @@ "@pnpm/cli.utils": "workspace:*", "@pnpm/config.matcher": "workspace:*", "@pnpm/config.reader": "workspace:*", + "@pnpm/core-loggers": "workspace:*", "@pnpm/deps.inspection.list": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/global.packages": "workspace:*", diff --git a/global/commands/src/globalAdd.ts b/global/commands/src/globalAdd.ts index 068d7d00c9..c21eac4439 100644 --- a/global/commands/src/globalAdd.ts +++ b/global/commands/src/globalAdd.ts @@ -4,6 +4,7 @@ import path from 'node:path' import { linkBinsOfPackages } from '@pnpm/bins.linker' import { removeBin } from '@pnpm/bins.remover' import type { CommandHandlerMap } from '@pnpm/cli.command' +import { summaryLogger } from '@pnpm/core-loggers' import { cleanOrphanedInstallDirs, createGlobalCacheKey, @@ -39,19 +40,10 @@ export async function handleGlobalAdd ( 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 - // "./foo" would resolve against the temp install directory. - params = params.map((param) => resolveLocalParam(param, opts.dir)) const globalDir = opts.globalPkgDir! const globalBinDir = opts.bin! cleanOrphanedInstallDirs(globalDir) - // Install into a new directory first, then read the resolved aliases - // from the resulting package.json. This is more reliable than parsing - // aliases from CLI params (which may be tarballs, git URLs, etc.). - const installDir = createInstallDir(globalDir) - // Convert allowBuild array to allowBuilds Record (same conversion as add.handler) let allowBuilds = opts.allowBuilds ?? {} if (opts.allowBuild?.length) { @@ -61,6 +53,48 @@ export async function handleGlobalAdd ( } } + // Each space-separated CLI param becomes its own isolated install group. + // A param containing commas is split into multiple selectors that share a + // single group, so `pnpm add -g foo,bar qar` installs foo+bar together + // and qar separately. Local paths and URLs that legitimately contain + // commas are detected and kept whole. + const groups = params + .map((param) => splitCommaSeparated(param, opts.dir).map((token) => resolveLocalParam(token, opts.dir))) + .filter((group) => group.length > 0) + + for (const group of groups) { + // eslint-disable-next-line no-await-in-loop + await installGroup({ opts, globalDir, globalBinDir, allowBuilds, params: group }, commands) + } + + // The per-group `mutateModulesInSingleProject` calls run with + // `omitSummaryLog: true` so the default-reporter's summary block only + // appears once at the end, with every installed package listed under a + // single "global:" heading. Without this, the reporter would print + // group 1's summary and then ignore later groups, because its summary + // pipeline takes only the first `summary` log event. + summaryLogger.debug({ prefix: globalDir }) +} + +interface InstallGroupContext { + opts: GlobalAddOptions + globalDir: string + globalBinDir: string + allowBuilds: Record + params: string[] +} + +async function installGroup ( + ctx: InstallGroupContext, + commands: CommandHandlerMap +): Promise { + const { opts, globalDir, globalBinDir, allowBuilds, params } = ctx + + // Install into a new directory first, then read the resolved aliases + // from the resulting package.json. This is more reliable than parsing + // aliases from CLI params (which may be tarballs, git URLs, etc.). + const installDir = createInstallDir(globalDir) + const include = { dependencies: true, devDependencies: false, @@ -68,13 +102,13 @@ export async function handleGlobalAdd ( } const fetchFullMetadata = opts.supportedArchitectures?.libc != null && true - const makeInstallOpts = (dir: string, builds: Record) => ({ + const installOpts = { ...opts, global: false, - bin: path.join(dir, 'node_modules/.bin'), - dir, - lockfileDir: dir, - rootProjectManifestDir: dir, + bin: path.join(installDir, 'node_modules/.bin'), + dir: installDir, + lockfileDir: installDir, + rootProjectManifestDir: installDir, rootProjectManifest: undefined, saveProd: true, saveDev: false, @@ -86,10 +120,11 @@ export async function handleGlobalAdd ( fetchFullMetadata, include, includeDirect: include, - allowBuilds: builds, - }) + allowBuilds, + omitSummaryLog: true, + } - const ignoredBuilds = await installGlobalPackages(makeInstallOpts(installDir, allowBuilds), params) + const ignoredBuilds = await installGlobalPackages(installOpts, params) await promptApproveGlobalBuilds({ globalPkgDir: globalDir, @@ -134,6 +169,41 @@ export async function handleGlobalAdd ( await linkBinsOfPackages(pkgs, globalBinDir, { excludeBins: binsToSkip }) } +function splitCommaSeparated (param: string, baseDir: string): string[] { + if (!param.includes(',')) return [param] + // URLs may contain commas and are never a group of selectors. + if (param.includes('://')) return [param] + // For path-like specs (relative/absolute paths, file:, link:), the + // commas could either be part of a single path that legitimately + // contains commas, or be separators between multiple distinct paths. + // Resolve the ambiguity by checking whether the whole param actually + // refers to an existing local path on disk. + if (refersToExistingLocalPath(param, baseDir)) return [param] + return param.split(',').map((token) => token.trim()).filter(Boolean) +} + +function refersToExistingLocalPath (param: string, baseDir: string): boolean { + let pathPart: string + if (param.startsWith('file:')) { + pathPart = param.slice('file:'.length) + } else if (param.startsWith('link:')) { + pathPart = param.slice('link:'.length) + } else if (param[0] === '.' || param[0] === '/' || param[0] === '~') { + pathPart = param + } else if (/^[a-z]:[/\\]/i.test(param)) { + pathPart = param + } else { + return false + } + const resolved = path.isAbsolute(pathPart) ? pathPart : path.resolve(baseDir, pathPart) + try { + fs.statSync(resolved) + return true + } catch { + return false + } +} + function resolveLocalParam (param: string, baseDir: string): string { for (const prefix of ['file:', 'link:']) { if (param.startsWith(prefix)) { diff --git a/global/commands/src/installGlobalPackages.ts b/global/commands/src/installGlobalPackages.ts index ae223272aa..3636a9b1ca 100644 --- a/global/commands/src/installGlobalPackages.ts +++ b/global/commands/src/installGlobalPackages.ts @@ -13,6 +13,7 @@ export interface InstallGlobalPackagesOptions extends CreateStoreControllerOptio include: IncludedDependencies includeDirect?: IncludedDependencies fetchFullMetadata?: boolean + omitSummaryLog?: boolean rootProjectManifest?: unknown rootProjectManifestDir?: string saveDev?: boolean diff --git a/global/commands/tsconfig.json b/global/commands/tsconfig.json index b8b0d07a06..9ac68f03b7 100644 --- a/global/commands/tsconfig.json +++ b/global/commands/tsconfig.json @@ -30,6 +30,9 @@ { "path": "../../config/reader" }, + { + "path": "../../core/core-loggers" + }, { "path": "../../core/error" }, diff --git a/installing/deps-installer/src/install/extendInstallOptions.ts b/installing/deps-installer/src/install/extendInstallOptions.ts index 5450bc6286..aab529f0b6 100644 --- a/installing/deps-installer/src/install/extendInstallOptions.ts +++ b/installing/deps-installer/src/install/extendInstallOptions.ts @@ -180,6 +180,13 @@ export interface StrictInstallOptions { trustPolicyIgnoreAfter?: number packageVulnerabilityAudit?: PackageVulnerabilityAudit blockExoticSubdeps?: boolean + /** + * If true, `mutateModules` does not emit the per-install `summary` log + * event. Used by `pnpm add -g` when it runs multiple isolated installs + * inside a single command and wants to emit a single consolidated + * summary at the very end instead of one summary per install. + */ + omitSummaryLog: boolean /** URL of a pnpm agent server. See the pnpm-agent README. */ agent?: string } @@ -284,6 +291,7 @@ const defaults = (opts: InstallOptions): StrictInstallOptions => { virtualStoreDirMaxLength: 120, peersSuffixMaxLength: 1000, blockExoticSubdeps: false, + omitSummaryLog: false, } as StrictInstallOptions } diff --git a/installing/deps-installer/src/install/index.ts b/installing/deps-installer/src/install/index.ts index 6c3c458033..d1b94b23af 100644 --- a/installing/deps-installer/src/install/index.ts +++ b/installing/deps-installer/src/install/index.ts @@ -1676,7 +1676,9 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { rules: opts.peerDependencyRules, }) - summaryLogger.debug({ prefix: opts.lockfileDir }) + if (!opts.omitSummaryLog) { + summaryLogger.debug({ prefix: opts.lockfileDir }) + } // Similar to the sequencing for when the original wanted lockfile is // copied, the new lockfile passed here should be as close as possible to diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c39b4d2810..ecd0e538e7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4941,6 +4941,9 @@ importers: '@pnpm/config.reader': specifier: workspace:* version: link:../../config/reader + '@pnpm/core-loggers': + specifier: workspace:* + version: link:../../core/core-loggers '@pnpm/deps.inspection.list': specifier: workspace:* version: link:../../deps/inspection/list diff --git a/pnpm/src/main.ts b/pnpm/src/main.ts index 02457fc2e5..f38c9cca3d 100644 --- a/pnpm/src/main.ts +++ b/pnpm/src/main.ts @@ -183,9 +183,15 @@ export async function main (inputArgv: string[]): Promise { const printLogs = !config['parseable'] && !config['json'] if (printLogs) { + // `pnpm add -g` may install several isolated groups in one run, one + // per CLI param. When that happens, force the reporter to show the + // per-prefix progress/stats output so every group's stats line up + // with the right install dir instead of being silently dropped. + const multiGroupGlobalAdd = cmd === 'add' && cliOptions.global === true && cliParams.length > 1 initReporter(reporterType, { cmd, config: { ...config, ...context }, + hideProgressPrefix: multiGroupGlobalAdd ? false : undefined, }) global[REPORTER_INITIALIZED] = reporterType } diff --git a/pnpm/src/reporter/index.ts b/pnpm/src/reporter/index.ts index cc71fe44af..ef67e558cb 100644 --- a/pnpm/src/reporter/index.ts +++ b/pnpm/src/reporter/index.ts @@ -12,6 +12,7 @@ export function initReporter ( opts: { cmd: string | null config: Config & ConfigContext + hideProgressPrefix?: boolean } ): void { switch (reporterType) { @@ -29,6 +30,7 @@ export function initReporter ( throttleProgress: 200, hideAddedPkgsProgress: opts.config.lockfileOnly, hideLifecyclePrefix: opts.config.reporterHidePrefix, + hideProgressPrefix: opts.hideProgressPrefix, }, streamParser: streamParser as StreamParser, }) @@ -46,6 +48,7 @@ export function initReporter ( logLevel: opts.config.loglevel as LogLevel, throttleProgress: 1000, hideLifecyclePrefix: opts.config.reporterHidePrefix, + hideProgressPrefix: opts.hideProgressPrefix, }, streamParser: streamParser as StreamParser, }) diff --git a/pnpm/test/install/global.ts b/pnpm/test/install/global.ts index ef82b267c1..f956c59dd3 100644 --- a/pnpm/test/install/global.ts +++ b/pnpm/test/install/global.ts @@ -25,6 +25,10 @@ function globalPkgDir (pnpmHome: string): string { * and returns the path to the package's node_modules entry. */ function findGlobalPkg (globalDir: string, pkgName: string): string | null { + return findGlobalPkgInstall(globalDir, pkgName)?.pkgPath ?? null +} + +function findGlobalPkgInstall (globalDir: string, pkgName: string): { installDir: string, pkgPath: string } | null { let entries: fs.Dirent[] try { entries = fs.readdirSync(globalDir, { withFileTypes: true }) @@ -46,7 +50,7 @@ function findGlobalPkg (globalDir: string, pkgName: string): string | null { continue } if (pkgJson.dependencies?.[pkgName]) { - return path.join(installDir, 'node_modules', pkgName) + return { installDir, pkgPath: path.join(installDir, 'node_modules', pkgName) } } } return null @@ -605,8 +609,8 @@ test('global remove deletes install group and bin shims', async () => { })) fs.writeFileSync(path.join(pkgB, 'index.js'), '#!/usr/bin/env node\nconsole.log("b")\n') - // Install as a group - await execPnpm(['add', '-g', pkgA, pkgB], { env }) + // Install as a single bundled group via comma syntax + await execPnpm(['add', '-g', `${pkgA},${pkgB}`], { env }) expect(fs.existsSync(path.join(pnpmHome, 'bin', 'tool-a-bin'))).toBeTruthy() expect(fs.existsSync(path.join(pnpmHome, 'bin', 'tool-b-bin'))).toBeTruthy() @@ -617,3 +621,84 @@ test('global remove deletes install group and bin shims', async () => { expect(findGlobalPkg(globalPkgDir(pnpmHome), 'tool-a')).toBeNull() expect(findGlobalPkg(globalPkgDir(pnpmHome), 'tool-b')).toBeNull() }) + +test('global add installs each space-separated package into its own isolated group', async () => { + prepare() + const global = path.resolve('..', 'global') + const pnpmHome = path.join(global, 'pnpm') + fs.mkdirSync(pnpmHome, { recursive: true }) + + const env = { [PATH_NAME]: path.join(pnpmHome, 'bin'), PNPM_HOME: pnpmHome, XDG_DATA_HOME: global } + + await execPnpm(['add', '-g', 'is-positive@1.0.0', 'is-negative@1.0.0'], { env }) + + const positive = findGlobalPkgInstall(globalPkgDir(pnpmHome), 'is-positive') + const negative = findGlobalPkgInstall(globalPkgDir(pnpmHome), 'is-negative') + expect(positive).toBeTruthy() + expect(negative).toBeTruthy() + + // Each package lives in its own install dir (they are not bundled together). + expect(positive!.installDir).not.toBe(negative!.installDir) + + // Removing one only removes that one — the other remains. + await execPnpm(['remove', '-g', 'is-positive'], { env }) + expect(findGlobalPkg(globalPkgDir(pnpmHome), 'is-positive')).toBeNull() + expect(findGlobalPkg(globalPkgDir(pnpmHome), 'is-negative')).toBeTruthy() +}) + +test('global add bundles comma-separated packages into a single group', async () => { + prepare() + const global = path.resolve('..', 'global') + const pnpmHome = path.join(global, 'pnpm') + fs.mkdirSync(pnpmHome, { recursive: true }) + + const env = { [PATH_NAME]: path.join(pnpmHome, 'bin'), PNPM_HOME: pnpmHome, XDG_DATA_HOME: global } + + // `is-positive,is-negative` is one group; `@pnpm.e2e/peer-c@1` is its own group. + await execPnpm(['add', '-g', 'is-positive@1.0.0,is-negative@1.0.0', '@pnpm.e2e/peer-c@1'], { env }) + + const positive = findGlobalPkgInstall(globalPkgDir(pnpmHome), 'is-positive') + const negative = findGlobalPkgInstall(globalPkgDir(pnpmHome), 'is-negative') + const peerC = findGlobalPkgInstall(globalPkgDir(pnpmHome), '@pnpm.e2e/peer-c') + expect(positive).toBeTruthy() + expect(negative).toBeTruthy() + expect(peerC).toBeTruthy() + + // is-positive and is-negative share an install dir (same group). + expect(positive!.installDir).toBe(negative!.installDir) + // peer-c is in a different install dir. + expect(peerC!.installDir).not.toBe(positive!.installDir) +}) + +test('global add does not treat commas inside a local path selector as a group separator', () => { + prepare() + const global = path.resolve('..', 'global') + const pnpmHome = path.join(global, 'pnpm') + fs.mkdirSync(pnpmHome, { recursive: true }) + + // Create a local package whose directory name contains a comma. + const pkgDir = path.resolve('..', 'tool,with,comma') + fs.mkdirSync(pkgDir, { recursive: true }) + fs.writeFileSync(path.join(pkgDir, 'package.json'), JSON.stringify({ + name: 'tool-comma', + version: '1.0.0', + bin: { 'tool-comma-bin': './index.js' }, + })) + fs.writeFileSync(path.join(pkgDir, 'index.js'), '#!/usr/bin/env node\nconsole.log("ok")\n') + + const env = { + [PATH_NAME]: path.join(pnpmHome, 'bin'), + PNPM_HOME: pnpmHome, + XDG_DATA_HOME: global, + pnpm_config_store_dir: path.resolve('..', 'store'), + } + + // The path contains commas but must be treated as one selector, not split. + execPnpmSync(['add', '-g', pkgDir], { env, expectSuccess: true }) + expect(findGlobalPkg(globalPkgDir(pnpmHome), 'tool-comma')).toBeTruthy() + expect(fs.existsSync(path.join(pnpmHome, 'bin', 'tool-comma-bin'))).toBeTruthy() + + // Same path via file: protocol should also be preserved. + execPnpmSync(['add', '-g', `file:${pkgDir}`], { env, expectSuccess: true }) + expect(findGlobalPkg(globalPkgDir(pnpmHome), 'tool-comma')).toBeTruthy() +})