diff --git a/.changeset/apply-bin-owner-overrides-link-bins.md b/.changeset/apply-bin-owner-overrides-link-bins.md new file mode 100644 index 0000000000..33c38d1731 --- /dev/null +++ b/.changeset/apply-bin-owner-overrides-link-bins.md @@ -0,0 +1,9 @@ +--- +"@pnpm/bins.resolver": patch +"@pnpm/bins.linker": patch +"pnpm": patch +--- + +Added `BIN_OWNER_OVERRIDES` and `pkgOwnsBin` to `@pnpm/bins.resolver`. Applied in bins.linker conflict resolution for consistent behavior between global conflict checking and actual bin linking, so packages like `npm` get priority for bins like `npx` even in non-global installs [#10850](https://github.com/pnpm/pnpm/issues/10850). + +Removed the redundant `ownName` field from `CommandInfo` since `pkgOwnsBin` already handles the `binName === pkgName` case. diff --git a/bins/linker/src/index.ts b/bins/linker/src/index.ts index 783a630f59..eb886603af 100644 --- a/bins/linker/src/index.ts +++ b/bins/linker/src/index.ts @@ -2,7 +2,7 @@ import { existsSync, promises as fs } from 'node:fs' import { createRequire } from 'node:module' import path from 'node:path' -import { type Command, getBinsFromPackageManifest } from '@pnpm/bins.resolver' +import { type Command, getBinsFromPackageManifest, pkgOwnsBin } from '@pnpm/bins.resolver' import { PnpmError } from '@pnpm/error' import { readModulesDir } from '@pnpm/fs.read-modules-dir' import { globalWarn, logger } from '@pnpm/logger' @@ -122,7 +122,6 @@ export async function linkBinsOfPackages ( } interface CommandInfo extends Command { - ownName: boolean pkgName: string pkgVersion: string makePowerShellShim: boolean @@ -169,8 +168,11 @@ function resolveCommandConflicts (group: CommandInfo[], binsDir: string): Comman } function compareCommandsInConflict (a: CommandInfo, b: CommandInfo): number { - if (a.ownName && !b.ownName) return 1 - if (!a.ownName && b.ownName) return -1 + // Check ownership: a package that owns the bin name gets priority + const aOwns = pkgOwnsBin(a.name, a.pkgName) + const bOwns = pkgOwnsBin(b.name, b.pkgName) + if (aOwns && !bOwns) return 1 + if (!aOwns && bOwns) return -1 if (a.pkgName !== b.pkgName) return a.pkgName.localeCompare(b.pkgName) // it's pointless to compare versions of 2 different package return semver.compare(a.pkgVersion, b.pkgVersion) } @@ -235,7 +237,6 @@ async function getPackageBinsFromManifest (manifest: DependencyManifest, pkgDir: } return cmds.map((cmd) => ({ ...cmd, - ownName: cmd.name === manifest.name, pkgName: manifest.name, pkgVersion: manifest.version, makePowerShellShim: POWER_SHELL_IS_SUPPORTED && manifest.name !== 'pnpm', diff --git a/bins/linker/test/fixtures/bin-owner-override/node/index.js b/bins/linker/test/fixtures/bin-owner-override/node/index.js new file mode 100644 index 0000000000..6c32a9cd1b --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/node/index.js @@ -0,0 +1 @@ +console.log('node') diff --git a/bins/linker/test/fixtures/bin-owner-override/node/npx.js b/bins/linker/test/fixtures/bin-owner-override/node/npx.js new file mode 100644 index 0000000000..9fa565183f --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/node/npx.js @@ -0,0 +1 @@ +console.log('npx from node') diff --git a/bins/linker/test/fixtures/bin-owner-override/node/package.json b/bins/linker/test/fixtures/bin-owner-override/node/package.json new file mode 100644 index 0000000000..2097a675b0 --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/node/package.json @@ -0,0 +1,8 @@ +{ + "name": "node", + "version": "1.0.0", + "bin": { + "node": "index.js", + "npx": "npx.js" + } +} diff --git a/bins/linker/test/fixtures/bin-owner-override/npm/bin/npm-cli.js b/bins/linker/test/fixtures/bin-owner-override/npm/bin/npm-cli.js new file mode 100644 index 0000000000..59cb236cde --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/npm/bin/npm-cli.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('npm cli') diff --git a/bins/linker/test/fixtures/bin-owner-override/npm/bin/npx-cli.js b/bins/linker/test/fixtures/bin-owner-override/npm/bin/npx-cli.js new file mode 100644 index 0000000000..b0a542404f --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/npm/bin/npx-cli.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('npx cli') diff --git a/bins/linker/test/fixtures/bin-owner-override/npm/package.json b/bins/linker/test/fixtures/bin-owner-override/npm/package.json new file mode 100644 index 0000000000..4fec99d931 --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/npm/package.json @@ -0,0 +1,8 @@ +{ + "name": "npm", + "version": "10.0.0", + "bin": { + "npm": "bin/npm-cli.js", + "npx": "bin/npx-cli.js" + } +} diff --git a/bins/linker/test/fixtures/bin-owner-override/other-pkg/index.js b/bins/linker/test/fixtures/bin-owner-override/other-pkg/index.js new file mode 100644 index 0000000000..65aafa49e8 --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/other-pkg/index.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('other-pkg npx') diff --git a/bins/linker/test/fixtures/bin-owner-override/other-pkg/package.json b/bins/linker/test/fixtures/bin-owner-override/other-pkg/package.json new file mode 100644 index 0000000000..065d2bb7af --- /dev/null +++ b/bins/linker/test/fixtures/bin-owner-override/other-pkg/package.json @@ -0,0 +1,7 @@ +{ + "name": "other-pkg", + "version": "1.0.0", + "bin": { + "npx": "index.js" + } +} diff --git a/bins/linker/test/index.ts b/bins/linker/test/index.ts index 6b1367eef9..d866242940 100644 --- a/bins/linker/test/index.ts +++ b/bins/linker/test/index.ts @@ -697,3 +697,30 @@ describe('node binary linking', () => { expect(fs.existsSync(path.join(binTarget, `node${CMD_EXTENSION}`))).toBe(false) }) }) + +test('linkBins() resolves conflicts using BIN_OWNER_OVERRIDES (npx owned by npm)', async () => { + const binTarget = temporaryDirectory() + const binOwnerOverrideFixture = f.prepare('bin-owner-override') + const warn = jest.fn() + + await linkBins(binOwnerOverrideFixture, binTarget, { warn }) + + // npx should be linked from npm package (owner override), not node or other-pkg + // BIN_OWNER_OVERRIDES says: npx is owned by npm + expect(binsConflictLogger.debug).toHaveBeenCalledWith( + expect.objectContaining({ + binaryName: 'npx', + binsDir: binTarget, + linkedPkgName: 'npm', + skippedPkgName: expect.any(String), + skippedPkgVersion: expect.any(String), + }) + ) + + const binLocation = path.join(binTarget, 'npx') + expect(fs.existsSync(binLocation)).toBe(true) + const content = fs.readFileSync(binLocation, 'utf8') + // npx should come from npm package, not node or other-pkg + // Use a regex that matches both forward and backslashes for Windows compatibility + expect(content).toMatch(/npm[/\\]bin[/\\]npx-cli\.js/) +}) diff --git a/bins/resolver/src/index.ts b/bins/resolver/src/index.ts index 69e15d1848..0b14652b56 100644 --- a/bins/resolver/src/index.ts +++ b/bins/resolver/src/index.ts @@ -9,6 +9,20 @@ export interface Command { path: string } +// Maps a bin name to all packages that are legitimate owners of it, beyond +// the default rule that a package named `X` owns the `X` bin. For example, +// `npx` ships inside the `npm` package, and `pnpx` ships inside both the +// `pnpm` package and the `@pnpm/exe` package. +export const BIN_OWNER_OVERRIDES: Record = { + npx: ['npm'], + pnpm: ['@pnpm/exe'], + pnpx: ['pnpm', '@pnpm/exe'], +} + +export function pkgOwnsBin (binName: string, pkgName: string): boolean { + return binName === pkgName || BIN_OWNER_OVERRIDES[binName]?.includes(pkgName) === true +} + export async function getBinsFromPackageManifest (manifest: DependencyManifest, pkgPath: string): Promise { if (manifest.bin) { return commandsFromBin(manifest.bin, manifest.name, pkgPath) diff --git a/global/commands/src/checkGlobalBinConflicts.ts b/global/commands/src/checkGlobalBinConflicts.ts index 2dbb75c8e5..afcc0e1647 100644 --- a/global/commands/src/checkGlobalBinConflicts.ts +++ b/global/commands/src/checkGlobalBinConflicts.ts @@ -1,7 +1,7 @@ import fs from 'node:fs' import path from 'node:path' -import { getBinsFromPackageManifest } from '@pnpm/bins.resolver' +import { getBinsFromPackageManifest, pkgOwnsBin } from '@pnpm/bins.resolver' import { PnpmError } from '@pnpm/error' import { type GlobalPackageInfo, @@ -10,20 +10,6 @@ import { import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader' import type { DependencyManifest } from '@pnpm/types' -// Maps a bin name to all packages that are legitimate owners of it, beyond -// the default rule that a package named `X` owns the `X` bin. For example, -// `npx` ships inside the `npm` package, and `pnpx` ships inside both the -// `pnpm` package and the `@pnpm/exe` package. -const BIN_OWNER_OVERRIDES: Record = { - npx: ['npm'], - pnpm: ['@pnpm/exe'], - pnpx: ['pnpm', '@pnpm/exe'], -} - -function pkgOwnsBin (binName: string, pkgName: string): boolean { - return binName === pkgName || BIN_OWNER_OVERRIDES[binName]?.includes(pkgName) === true -} - /** * Checks for bin name conflicts between new packages and existing global * packages. Returns a set of bin names that should be skipped during linking diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/.gitignore b/pkg-manager/link-bins/test/fixtures/bin-owner-override/.gitignore new file mode 100644 index 0000000000..5867a0493e --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/.gitignore @@ -0,0 +1,2 @@ +!**/node_modules/**/* +!/node_modules/ diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/index.js b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/index.js new file mode 100644 index 0000000000..6c32a9cd1b --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/index.js @@ -0,0 +1 @@ +console.log('node') diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/npx.js b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/npx.js new file mode 100644 index 0000000000..9fa565183f --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/npx.js @@ -0,0 +1 @@ +console.log('npx from node') diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/package.json b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/package.json new file mode 100644 index 0000000000..2097a675b0 --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/node/package.json @@ -0,0 +1,8 @@ +{ + "name": "node", + "version": "1.0.0", + "bin": { + "node": "index.js", + "npx": "npx.js" + } +} diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/bin/npm-cli.js b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/bin/npm-cli.js new file mode 100644 index 0000000000..59cb236cde --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/bin/npm-cli.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('npm cli') diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/bin/npx-cli.js b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/bin/npx-cli.js new file mode 100644 index 0000000000..b0a542404f --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/bin/npx-cli.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('npx cli') diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/package.json b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/package.json new file mode 100644 index 0000000000..4fec99d931 --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/npm/package.json @@ -0,0 +1,8 @@ +{ + "name": "npm", + "version": "10.0.0", + "bin": { + "npm": "bin/npm-cli.js", + "npx": "bin/npx-cli.js" + } +} diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/other-pkg/index.js b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/other-pkg/index.js new file mode 100644 index 0000000000..65aafa49e8 --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/other-pkg/index.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('other-pkg npx') diff --git a/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/other-pkg/package.json b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/other-pkg/package.json new file mode 100644 index 0000000000..065d2bb7af --- /dev/null +++ b/pkg-manager/link-bins/test/fixtures/bin-owner-override/node_modules/other-pkg/package.json @@ -0,0 +1,7 @@ +{ + "name": "other-pkg", + "version": "1.0.0", + "bin": { + "npx": "index.js" + } +}