feat: allow global install to override bins owned by the new package#10828
Merged
Conversation
When a package name matches the bin name (e.g., `npm` package providing `npm` bin), the new package gets priority and can override an existing bin from another global package. The `npx` bin is also treated as owned by the `npm` package via a hardcoded override map. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…failing When installing a package globally (e.g. node) whose bins conflict with an already-installed package that owns those bins (e.g. npm owns npm/npx), the install now succeeds and skips linking the conflicting bins rather than aborting with GLOBAL_BIN_CONFLICT. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates pnpm’s global install/update flow to allow a newly installed global package to take precedence for binaries it “owns” (when the bin name matches the package name, plus a hardcoded ownership override for npx → npm), and to skip linking bins that are owned by an already-installed global package rather than failing the install.
Changes:
- Update
checkGlobalBinConflictsto support “bin ownership” priority rules and return aSetof bin names to skip during linking. - Pass the returned skip set through
globalAdd/globalUpdateintolinkBinsOfPackages, which now supports excluding specific bin names from linking. - Add Jest tests for the new conflict/ownership behavior and enable linting/testing for
global/commandstests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg-manager/link-bins/src/index.ts | Adds excludeBins option to filter out bins before linking. |
| global/commands/src/checkGlobalBinConflicts.ts | Implements bin ownership priority/skip logic and returns Set<string> of bins to skip. |
| global/commands/src/globalAdd.ts | Captures binsToSkip from conflict check and passes it into bin linking. |
| global/commands/src/globalUpdate.ts | Captures binsToSkip from conflict check and passes it into bin linking. |
| global/commands/test/tsconfig.json | Adds a TS config for tests in the global/commands package. |
| global/commands/test/checkGlobalBinConflicts.test.ts | Adds test coverage for ownership/override/skip scenarios. |
| global/commands/package.json | Runs lint on tests and adds Jest test script. |
| cspell.json | Adds fakehash to dictionary for test text. |
| .changeset/global-bin-ownership-priority.md | Declares a minor release for the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…i-provider bins Track all new bin providers (not just the last one) so ownership is correctly resolved when multiple new packages provide the same bin. Use manifest.name instead of the dependency alias when checking existing package ownership, fixing incorrect decisions for aliased installs. Use symlink-dir in tests for Windows compatibility and remove a non-null assertion in link-bins. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+12
to
+21
| // Bins that are logically owned by a package other than the one matching | ||
| // the bin name. For example, `npx` ships inside the `npm` package, so | ||
| // when `npm` is being installed globally it should be allowed to claim | ||
| // the `npx` bin as well. | ||
| const BIN_OWNER_OVERRIDES: Record<string, string> = { | ||
| npx: 'npm', | ||
| } | ||
|
|
||
| function pkgOwnsBin (binName: string, pkgName: string): boolean { | ||
| return binName === pkgName || BIN_OWNER_OVERRIDES[binName] === pkgName |
There was a problem hiding this comment.
BIN_OWNER_OVERRIDES / pkgOwnsBin() defines ownership rules (e.g. npx owned by npm), but the actual bin linking conflict resolution in @pnpm/link-bins only treats cmd.name === manifest.name as “owned” (see ownName in pkg-manager/link-bins/src/index.ts). If multiple new packages provide an overridden bin like npx, the linker may choose a non-owner, contradicting the ownership override semantics. Consider aligning the linker’s ownership check with the same override rules or ensuring checkGlobalBinConflicts enforces the intended winner when overrides apply.
Suggested change
| // Bins that are logically owned by a package other than the one matching | |
| // the bin name. For example, `npx` ships inside the `npm` package, so | |
| // when `npm` is being installed globally it should be allowed to claim | |
| // the `npx` bin as well. | |
| const BIN_OWNER_OVERRIDES: Record<string, string> = { | |
| npx: 'npm', | |
| } | |
| function pkgOwnsBin (binName: string, pkgName: string): boolean { | |
| return binName === pkgName || BIN_OWNER_OVERRIDES[binName] === pkgName | |
| function pkgOwnsBin (binName: string, pkgName: string): boolean { | |
| // A package is considered to "own" a bin only when the bin name matches | |
| // the package name. This mirrors the ownership rule used by | |
| // `@pnpm/link-bins` (see `ownName` in `pkg-manager/link-bins/src/index.ts`), | |
| // ensuring that conflict detection here is consistent with the actual | |
| // bin linking behavior. | |
| return binName === pkgName |
Convert the conflicting bins collection from an array to a Set for O(1) lookups. Use the dependency alias in the `pnpm remove -g` hint since that is what the user originally installed with, showing both alias and manifest name when they differ. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a package name matches the bin name (e.g.,
npmpackage providingnpmbin), the new package gets priority and can override an existing bin from another global package. Thenpxbin is also treated as owned by thenpmpackage via a hardcoded override map.