diff --git a/.changeset/skip-existing-bin-files.md b/.changeset/skip-existing-bin-files.md new file mode 100644 index 0000000000..6815ccd720 --- /dev/null +++ b/.changeset/skip-existing-bin-files.md @@ -0,0 +1,6 @@ +--- +"@pnpm/bins.linker": patch +"pnpm": patch +--- + +Skip linking bins that already reference the correct target. This avoids redundant I/O during repeated installs and prevents permission errors when the store is read-only (e.g. Docker layer caching, CI prewarm, NFS). Bins that reference a stale or incorrect target are still rewritten. diff --git a/bins/linker/src/index.ts b/bins/linker/src/index.ts index eb886603af..ab222afda1 100644 --- a/bins/linker/src/index.ts +++ b/bins/linker/src/index.ts @@ -10,7 +10,7 @@ import { readPackageJsonFromDir } from '@pnpm/pkg-manifest.reader' import { getAllDependenciesFromManifest } from '@pnpm/pkg-manifest.utils' import type { DependencyManifest, EngineDependency, ProjectManifest } from '@pnpm/types' import { safeReadProjectManifestOnly } from '@pnpm/workspace.project-manifest-reader' -import cmdShim from '@zkochan/cmd-shim' +import { cmdShim, isShimPointingAt } from '@zkochan/cmd-shim' import { rimraf } from '@zkochan/rimraf' import fixBin from 'bin-links/lib/fix-bin.js' import { isSubdir } from 'is-subdir' @@ -26,6 +26,8 @@ const binsConflictLogger = logger('bins-conflict') const IS_WINDOWS = isWindows() const EXECUTABLE_SHEBANG_SUPPORTED = !IS_WINDOWS const POWER_SHELL_IS_SUPPORTED = IS_WINDOWS +// A cmd-shim is a small shell script. Anything larger is a binary and should not be read. +const CMD_SHIM_MAX_SIZE = 4 * 1024 export type WarningCode = 'BINARIES_CONFLICT' | 'EMPTY_BIN' @@ -258,6 +260,24 @@ export interface LinkBinOptions { async function linkBin (cmd: CommandInfo, binsDir: string, opts?: LinkBinOptions): Promise { const externalBinPath = path.join(binsDir, cmd.name) + // Skip if the existing bin already references the correct target. + // This avoids redundant I/O on warm installs and EACCES on read-only stores. + // We verify the target path — not just existence — so that conflict resolution + // changes or provider swaps still get the bin rewritten. + try { + const stat = await fs.lstat(externalBinPath) + if (stat.isSymbolicLink()) { + const target = await fs.readlink(externalBinPath) + if (target === cmd.path || path.resolve(binsDir, target) === path.resolve(cmd.path)) { + return + } + } else if (stat.isFile() && stat.size < CMD_SHIM_MAX_SIZE) { + const content = await fs.readFile(externalBinPath, 'utf8') + if (isShimPointingAt(content, cmd.path)) { + return + } + } + } catch {} if (IS_WINDOWS) { const exePath = path.join(binsDir, `${cmd.name}${getExeExtension()}`) if (existsSync(exePath)) { diff --git a/bins/linker/test/index.ts b/bins/linker/test/index.ts index d866242940..6f23819f6b 100644 --- a/bins/linker/test/index.ts +++ b/bins/linker/test/index.ts @@ -76,6 +76,44 @@ test('linkBins()', async () => { } }) +test('linkBins() skips bins that already reference the correct target', async () => { + const binTarget = temporaryDirectory() + const warn = jest.fn() + const simpleFixture = f.prepare('simple-fixture') + + await linkBins(path.join(simpleFixture, 'node_modules'), binTarget, { warn }) + + const binLocation = path.join(binTarget, 'simple') + expect(fs.existsSync(binLocation)).toBe(true) + const originalContent = fs.readFileSync(binLocation, 'utf8') + // The bin contains a cmd-shim-target marker with the correct target path + const expectedTarget = normalizePath(path.join(simpleFixture, 'node_modules', 'simple', 'index.js')) + expect(originalContent).toContain(`# cmd-shim-target=${expectedTarget}\n`) + // Append a sentinel to the existing (correct) content to prove it is not rewritten + const sentinel = originalContent + '\n# sentinel' + fs.writeFileSync(binLocation, sentinel, 'utf8') + + await linkBins(path.join(simpleFixture, 'node_modules'), binTarget, { warn }) + + expect(fs.readFileSync(binLocation, 'utf8')).toBe(sentinel) +}) + +test('linkBins() rewrites bins that lack a target marker', async () => { + const binTarget = temporaryDirectory() + const warn = jest.fn() + const simpleFixture = f.prepare('simple-fixture') + + // Create a stale bin without a cmd-shim-target marker + fs.mkdirSync(binTarget, { recursive: true }) + const binLocation = path.join(binTarget, 'simple') + fs.writeFileSync(binLocation, '#!/bin/sh\n"$basedir/../wrong-pkg/index.js" "$@"', 'utf8') + + await linkBins(path.join(simpleFixture, 'node_modules'), binTarget, { warn }) + + const content = fs.readFileSync(binLocation, 'utf8') + expect(content).not.toContain('wrong-pkg') +}) + test('linkBins() never creates a PowerShell shim for the pnpm CLI', async () => { const binTarget = temporaryDirectory() const fixture = f.prepare('pnpm-cli') diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7501807757..72aea06179 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -425,8 +425,8 @@ catalogs: specifier: ^4.0.8 version: 4.1.3 '@zkochan/cmd-shim': - specifier: ^8.0.1 - version: 8.0.1 + specifier: ^9.0.0 + version: 9.0.0 '@zkochan/retry': specifier: ^0.2.0 version: 0.2.0 @@ -1436,7 +1436,7 @@ importers: version: link:../../workspace/project-manifest-reader '@zkochan/cmd-shim': specifier: 'catalog:' - version: 8.0.1 + version: 9.0.0 '@zkochan/rimraf': specifier: 'catalog:' version: 4.0.0 @@ -11562,8 +11562,8 @@ packages: resolution: {integrity: sha512-E5mgrRS8Kk80n19Xxmrx5qO9UG03FyZd8Me5gxYi++VPZsOv8+OsclA+0Fth4KTDCrQ/FkJryNFKJ6/642lo4g==} engines: {node: '>=18.12'} - '@zkochan/cmd-shim@8.0.1': - resolution: {integrity: sha512-fz72az47CZQE5vD+NWd51VBq8II6FAebp+SXRsS0b+sNHUXNFA3/F1dgBSkNQO6/BClomsV4WaRFqAkLdUe1zw==} + '@zkochan/cmd-shim@9.0.0': + resolution: {integrity: sha512-UH8qOvWZjZJr/mLk5xbrhGmBAf8iLbDkCZCyjnXri5cVFpvZgBUszZJmTmvDTIrzLAZ8lb++qvedOdsRokaQBg==} engines: {node: '>=22.13'} '@zkochan/diable@1.0.2': @@ -19868,11 +19868,9 @@ snapshots: graceful-fs: 4.2.11(patch_hash=68ebc232025360cb3dcd3081f4067f4e9fc022ab6b6f71a3230e86c7a5b337d1) is-windows: 1.0.2 - '@zkochan/cmd-shim@8.0.1': + '@zkochan/cmd-shim@9.0.0': dependencies: cmd-extension: 1.0.2 - graceful-fs: 4.2.11(patch_hash=68ebc232025360cb3dcd3081f4067f4e9fc022ab6b6f71a3230e86c7a5b337d1) - is-windows: 1.0.2 '@zkochan/diable@1.0.2': dependencies: diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index a378e7902b..b5b85d7590 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -148,7 +148,7 @@ catalog: '@yarnpkg/nm': 4.0.7 '@yarnpkg/parsers': 3.0.3 '@yarnpkg/pnp': ^4.0.8 - '@zkochan/cmd-shim': ^8.0.1 + '@zkochan/cmd-shim': ^9.0.0 '@zkochan/retry': ^0.2.0 '@zkochan/rimraf': ^4.0.0 '@zkochan/table': ^2.0.1 @@ -349,7 +349,7 @@ minimumReleaseAgeExclude: - '@pnpm/*' - '@rushstack/worker-pool@0.7.7' - '@zkochan/*' - - '@zkochan/cmd-shim@8.0.0 || 8.0.1' + - '@zkochan/cmd-shim@9.0.0' - better-path-resolve - body-parser@2.2.1 - can-link