feat: prefer-symlinked-executables (#5048)

A new setting supported: `prefer-symlinked-executables` When `true`, on Posix systems pnpm will create symlinks to executables in `node_modules/.bin` instead of command shims. This setting is `true` by default when `node-linker` is set to `hoisted`. close #4782
pnpm · Jul 18, 2022 · 28f0005 · 28f0005
1 parent af79b61
commit 28f0005
Show file tree

Hide file tree

Showing 16 changed files with 137 additions and 16 deletions.
diff --git a/.changeset/dirty-mirrors-occur.md b/.changeset/dirty-mirrors-occur.md
@@ -0,0 +1,16 @@
+---
+"@pnpm/build-modules": minor
+"@pnpm/config": minor
+"@pnpm/core": minor
+"@pnpm/headless": minor
+"@pnpm/hoist": minor
+"@pnpm/link-bins": minor
+"pnpm": minor
+---
+
+A new setting supported: `prefer-symlinked-executables`. When `true`, pnpm will create symlinks to executables in
+`node_modules/.bin` instead of command shims (but on POSIX systems only).
+
+This setting is `true` by default when `node-linker` is set to `hoisted`.
+
+Related issue: [#4782](https://github.com/pnpm/pnpm/issues/4782).
diff --git a/packages/build-modules/src/index.ts b/packages/build-modules/src/index.ts
@@ -27,6 +27,7 @@ export default async (
     ignoreScripts?: boolean
     lockfileDir: string
     optional: boolean
+    preferSymlinkedExecutables?: boolean
     rawConfig: object
     unsafePerm: boolean
     userAgent: string
@@ -70,6 +71,7 @@ async function buildDependency (
     ignoreScripts?: boolean
     lockfileDir: string
     optional: boolean
+    preferSymlinkedExecutables?: boolean
     rawConfig: object
     rootModulesDir: string
     scriptsPrependNodePath?: boolean | 'warn-only'
@@ -168,6 +170,7 @@ export async function linkBinsOfDependencies (
   opts: {
     extraNodePaths?: string[]
     optional: boolean
+    preferSymlinkedExecutables?: boolean
     warn: (message: string) => void
   }
 ) {
@@ -204,11 +207,18 @@ export async function linkBinsOfDependencies (
     }))
   )
 
-  await linkBinsOfPackages(pkgs, binPath, { extraNodePaths: opts.extraNodePaths })
+  await linkBinsOfPackages(pkgs, binPath, {
+    extraNodePaths: opts.extraNodePaths,
+    preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+  })
 
   // link also the bundled dependencies` bins
   if (depNode.hasBundledDependencies) {
     const bundledModules = path.join(depNode.dir, 'node_modules')
-    await linkBins(bundledModules, binPath, { extraNodePaths: opts.extraNodePaths, warn: opts.warn })
+    await linkBins(bundledModules, binPath, {
+      extraNodePaths: opts.extraNodePaths,
+      preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+      warn: opts.warn,
+    })
   }
 }
diff --git a/packages/config/src/Config.ts b/packages/config/src/Config.ts
@@ -78,6 +78,7 @@ export interface Config {
   useNodeVersion?: string
   useStderr?: boolean
   nodeLinker?: 'hoisted' | 'isolated' | 'pnp'
+  preferSymlinkedExecutables?: boolean
 
   // proxy
   httpProxy?: string

diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts
@@ -83,6 +83,7 @@ export const types = Object.assign({
   pnpmfile: String,
   'prefer-frozen-lockfile': Boolean,
   'prefer-offline': Boolean,
+  'prefer-symlinked-executables': Boolean,
   'prefer-workspace-packages': Boolean,
   production: [null, true],
   'public-hoist-pattern': Array,
@@ -458,7 +459,16 @@ export default async (
   if (!pnpmConfig.noProxy) {
     pnpmConfig.noProxy = pnpmConfig['noproxy'] ?? getProcessEnv('no_proxy')
   }
-  pnpmConfig.enablePnp = pnpmConfig.nodeLinker === 'pnp'
+  switch (pnpmConfig.nodeLinker) {
+  case 'pnp':
+    pnpmConfig.enablePnp = pnpmConfig.nodeLinker === 'pnp'
+    break
+  case 'hoisted':
+    if (pnpmConfig.preferSymlinkedExecutables == null) {
+      pnpmConfig.preferSymlinkedExecutables = true
+    }
+    break
+  }
   if (!pnpmConfig.userConfig) {
     pnpmConfig.userConfig = npmConfig.sources.user?.data
   }

diff --git a/packages/config/test/index.ts b/packages/config/test/index.ts
@@ -897,3 +897,18 @@ test('getConfig() sets merge-git-branch-lockfiles when branch matches merge-git-
     expect(config.mergeGitBranchLockfiles).toBe(true)
   }
 })
+
+test('preferSymlinkedExecutables should be true when nodeLinker is hoisted', async () => {
+  prepareEmpty()
+
+  const { config } = await getConfig({
+    cliOptions: {
+      'node-linker': 'hoisted',
+    },
+    packageManager: {
+      name: 'pnpm',
+      version: '1.0.0',
+    },
+  })
+  expect(config.preferSymlinkedExecutables).toBeTruthy()
+})
diff --git a/packages/core/src/install/extendInstallOptions.ts b/packages/core/src/install/extendInstallOptions.ts
@@ -90,6 +90,7 @@ export interface StrictInstallOptions {
   modulesCacheMaxAge: number
   peerDependencyRules: PeerDependencyRules
   allowedDeprecatedVersions: AllowedDeprecatedVersions
+  preferSymlinkedExecutables: boolean
 
   publicHoistPattern: string[] | undefined
   hoistPattern: string[] | undefined

diff --git a/packages/core/src/install/index.ts b/packages/core/src/install/index.ts
@@ -937,6 +937,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
           ignoreScripts: opts.ignoreScripts,
           lockfileDir: ctx.lockfileDir,
           optional: opts.include.optionalDependencies,
+          preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
           rawConfig: opts.rawConfig,
           rootModulesDir: ctx.virtualStoreDir,
           scriptsPrependNodePath: opts.scriptsPrependNodePath,
@@ -972,6 +973,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
           }, {})
         linkedPackages = await linkBins(project.modulesDir, project.binsDir, {
           allowExoticManifests: true,
+          preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
           projectManifest: project.manifest,
           nodeExecPathByAlias,
           extraNodePaths: ctx.extraNodePaths,
@@ -1009,6 +1011,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
           project.binsDir,
           {
             extraNodePaths: ctx.extraNodePaths,
+            preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
           }
         )
       }
@@ -1170,6 +1173,7 @@ async function linkAllBins (
   depGraph: DependenciesGraph,
   opts: {
     extraNodePaths?: string[]
+    preferSymlinkedExecutables?: boolean
     optional: boolean
     warn: (message: string) => void
   }

diff --git a/packages/core/src/link/index.ts b/packages/core/src/link/index.ts
@@ -118,6 +118,7 @@ export default async function link (
   const linkToBin = maybeOpts?.linkToBin ?? path.join(destModules, '.bin')
   await linkBinsOfPackages(linkedPkgs.map((p) => ({ manifest: p.manifest, location: p.path })), linkToBin, {
     extraNodePaths: ctx.extraNodePaths,
+    preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
   })
 
   let newPkg!: ProjectManifest

diff --git a/packages/core/src/link/options.ts b/packages/core/src/link/options.ts
@@ -23,6 +23,7 @@ interface StrictLinkOptions {
   reporter: ReporterFunction
   targetDependenciesField?: DependenciesField
   dir: string
+  preferSymlinkedExecutables: boolean
 
   hoistPattern: string[] | undefined
   forceHoistPattern: boolean

diff --git a/packages/core/test/install/misc.ts b/packages/core/test/install/misc.ts
@@ -562,6 +562,20 @@ test('bin specified in the directories property linked to .bin folder', async ()
   await project.isExecutable('.bin/pkg-with-directories-bin')
 })
 
+test('bin specified in the directories property symlinked to .bin folder when prefer-symlinked-executables is true on POSIX', async () => {
+  const project = prepareEmpty()
+
+  const opts = await testDefaults({ fastUnpack: false, preferSymlinkedExecutables: true })
+  await addDependenciesToPackage({}, ['pkg-with-directories-bin'], opts)
+
+  await project.isExecutable('.bin/pkg-with-directories-bin')
+
+  if (!isWindows()) {
+    const link = await fs.readlink('node_modules/.bin/pkg-with-directories-bin')
+    expect(link).toBeTruthy()
+  }
+})
+
 testOnNonWindows('building native addons', async () => {
   const project = prepareEmpty()
 

diff --git a/packages/headless/src/index.ts b/packages/headless/src/index.ts
@@ -100,6 +100,7 @@ export interface HeadlessOptions {
   engineStrict: boolean
   extraBinPaths?: string[]
   extraNodePaths?: string[]
+  preferSymlinkedExecutables?: boolean
   hoistingLimits?: HoistingLimits
   ignoreScripts: boolean
   ignorePackageManifest?: boolean
@@ -304,6 +305,7 @@ export default async (opts: HeadlessOptions) => {
       force: opts.force,
       ignoreScripts: opts.ignoreScripts,
       lockfileDir: opts.lockfileDir,
+      preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
       sideEffectsCacheRead: opts.sideEffectsCacheRead,
     })
     stageLogger.debug({
@@ -355,6 +357,7 @@ export default async (opts: HeadlessOptions) => {
         extraNodePath: opts.extraNodePaths,
         lockfile: hoistLockfile,
         importerIds,
+        preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
         privateHoistedModulesDir: hoistedModulesDir,
         privateHoistPattern: opts.hoistPattern ?? [],
         publicHoistedModulesDir,
@@ -368,6 +371,7 @@ export default async (opts: HeadlessOptions) => {
     await linkAllBins(graph, {
       extraNodePaths: opts.extraNodePaths,
       optional: opts.include.optionalDependencies,
+      preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
       warn,
     })
 
@@ -433,6 +437,7 @@ export default async (opts: HeadlessOptions) => {
       ignoreScripts: opts.ignoreScripts,
       lockfileDir,
       optional: opts.include.optionalDependencies,
+      preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
       rawConfig: opts.rawConfig,
       rootModulesDir: virtualStoreDir,
       scriptsPrependNodePath: opts.scriptsPrependNodePath,
@@ -455,7 +460,10 @@ export default async (opts: HeadlessOptions) => {
     if (!opts.ignorePackageManifest) {
       await Promise.all(opts.projects.map(async (project) => {
         if (opts.publicHoistPattern?.length && path.relative(opts.lockfileDir, project.rootDir) === '') {
-          await linkBinsOfImporter(project, { extraNodePaths: opts.extraNodePaths })
+          await linkBinsOfImporter(project, {
+            extraNodePaths: opts.extraNodePaths,
+            preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+          })
         } else {
           const directPkgDirs = Object.values(directDependenciesByImporterId[project.id])
           await linkBinsOfPackages(
@@ -471,6 +479,7 @@ export default async (opts: HeadlessOptions) => {
             project.binsDir,
             {
               extraNodePaths: opts.extraNodePaths,
+              preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
             }
           )
         }
@@ -568,12 +577,13 @@ async function linkBinsOfImporter (
     modulesDir: string
     rootDir: string
   },
-  { extraNodePaths }: { extraNodePaths?: string[] } = {}
+  { extraNodePaths, preferSymlinkedExecutables }: { extraNodePaths?: string[], preferSymlinkedExecutables?: boolean } = {}
 ) {
   const warn = (message: string) => logger.info({ message, prefix: rootDir })
   return linkBins(modulesDir, binsDir, {
     extraNodePaths,
     allowExoticManifests: true,
+    preferSymlinkedExecutables,
     projectManifest: manifest,
     warn,
   })
@@ -725,6 +735,7 @@ async function linkAllBins (
   opts: {
     extraNodePaths?: string[]
     optional: boolean
+    preferSymlinkedExecutables?: boolean
     warn: (message: string) => void
   }
 ) {
@@ -745,7 +756,11 @@ async function linkAllBins (
         const pkgSnapshots = props<string, DependenciesGraphNode>(Object.values(childrenToLink), depGraph)
 
         if (pkgSnapshots.includes(undefined as any)) { // eslint-disable-line
-          await linkBins(depNode.modules, binPath, { extraNodePaths: opts.extraNodePaths, warn: opts.warn })
+          await linkBins(depNode.modules, binPath, {
+            extraNodePaths: opts.extraNodePaths,
+            preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+            warn: opts.warn,
+          })
         } else {
           const pkgs = await Promise.all(
             pkgSnapshots
@@ -756,13 +771,20 @@ async function linkAllBins (
               }))
           )
 
-          await linkBinsOfPackages(pkgs, binPath, { extraNodePaths: opts.extraNodePaths })
+          await linkBinsOfPackages(pkgs, binPath, {
+            extraNodePaths: opts.extraNodePaths,
+            preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+          })
         }
 
         // link also the bundled dependencies` bins
         if (depNode.hasBundledDependencies) {
           const bundledModules = path.join(depNode.dir, 'node_modules')
-          await linkBins(bundledModules, binPath, { extraNodePaths: opts.extraNodePaths, warn: opts.warn })
+          await linkBins(bundledModules, binPath, {
+            extraNodePaths: opts.extraNodePaths,
+            preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+            warn: opts.warn,
+          })
         }
       }))
   )

diff --git a/packages/headless/src/linkHoistedModules.ts b/packages/headless/src/linkHoistedModules.ts
@@ -29,6 +29,7 @@ export default async function linkHoistedModules (
     force: boolean
     ignoreScripts: boolean
     lockfileDir: string
+    preferSymlinkedExecutables?: boolean
     sideEffectsCacheRead: boolean
   }
 ): Promise<void> {
@@ -85,6 +86,7 @@ async function linkAllPkgsInOrder (
     force: boolean
     ignoreScripts: boolean
     lockfileDir: string
+    preferSymlinkedExecutables?: boolean
     sideEffectsCacheRead: boolean
     warn: (message: string) => void
   }
@@ -130,6 +132,7 @@ async function linkAllPkgsInOrder (
   const binsDir = path.join(modulesDir, '.bin')
   await linkBins(modulesDir, binsDir, {
     allowExoticManifests: true,
+    preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
     warn: opts.warn,
   })
 }
diff --git a/packages/hoist/src/index.ts b/packages/hoist/src/index.ts
@@ -17,6 +17,7 @@ const hoistLogger = logger('hoist')
 export default async function hoistByLockfile (
   opts: {
     extraNodePath?: string[]
+    preferSymlinkedExecutables?: boolean
     lockfile: Lockfile
     importerIds?: string[]
     privateHoistPattern: string[]
@@ -65,7 +66,10 @@ export default async function hoistByLockfile (
   // the bins of the project's direct dependencies.
   // This is possible because the publicly hoisted modules
   // are in the same directory as the regular dependencies.
-  await linkAllBins(opts.privateHoistedModulesDir, { extraNodePaths: opts.extraNodePath })
+  await linkAllBins(opts.privateHoistedModulesDir, {
+    extraNodePaths: opts.extraNodePath,
+    preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
+  })
 
   return hoistedDependencies
 }
@@ -85,7 +89,12 @@ function createGetAliasHoistType (
   }
 }
 
-async function linkAllBins (modulesDir: string, opts: { extraNodePaths?: string[] }) {
+interface LinkAllBinsOptions {
+  extraNodePaths?: string[]
+  preferSymlinkedExecutables?: boolean
+}
+
+async function linkAllBins (modulesDir: string, opts: LinkAllBinsOptions) {
   const bin = path.join(modulesDir, '.bin')
   const warn: WarnFunction = (message, code) => {
     if (code === 'BINARIES_CONFLICT') return
@@ -95,6 +104,7 @@ async function linkAllBins (modulesDir: string, opts: { extraNodePaths?: string[
     await linkBins(modulesDir, bin, {
       allowExoticManifests: true,
       extraNodePaths: opts.extraNodePaths,
+      preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
       warn,
     })
   } catch (err: any) { // eslint-disable-line

diff --git a/packages/link-bins/package.json b/packages/link-bins/package.json
@@ -44,7 +44,8 @@
     "is-windows": "^1.0.2",
     "normalize-path": "^3.0.0",
     "p-settle": "^4.1.1",
-    "ramda": "^0.28.0"
+    "ramda": "^0.28.0",
+    "symlink-dir": "^5.0.1"
   },
   "devDependencies": {
     "@pnpm/link-bins": "workspace:*",