fix: fixing issues with global install

PR #2637
ref #1360

.changeset/long-otters-sell.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/config": major
+---
+
+`globalBin` is removed from the returned object.
+
+The value of `bin` is set by the `@pnpm/global-bin-dir` package when the `--global` option is used.

.changeset/seven-rocks-arrive.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/global-bin-dir": major
+---
+
+Finds a directory that is in PATH and we have permission to write to it.

.changeset/weak-feet-taste.md

@@ -0,0 +1,5 @@
+---
+"pnpm": patch
+---
+
+Find the proper directory for linking executables during global installation.

packages/config/package.json

@@ -33,6 +33,7 @@
   "dependencies": {
     "@pnpm/constants": "workspace:4.0.0",
     "@pnpm/error": "workspace:1.2.0",
+    "@pnpm/global-bin-dir": "workspace:^0.0.0",
     "@pnpm/types": "workspace:6.1.0",
     "@zkochan/npm-conf": "2.0.0",
     "camelcase": "6.0.0",

packages/config/src/Config.ts

@@ -19,7 +19,6 @@ export interface Config {
   filter: string[],
   rawLocalConfig: Record<string, any>, // tslint:disable-line
   rawConfig: Record<string, any>, // tslint:disable-line
-  globalBin: string,
   dryRun?: boolean, // This option might be not supported ever
   global?: boolean,
   globalDir: string,

packages/config/src/index.ts

@@ -1,5 +1,6 @@
 import { LAYOUT_VERSION } from '@pnpm/constants'
 import PnpmError from '@pnpm/error'
+import globalBinDir from '@pnpm/global-bin-dir'
 import loadNpmConf = require('@zkochan/npm-conf')
 import npmTypes = require('@zkochan/npm-conf/lib/types')
 import camelcase from 'camelcase'
@@ -195,9 +196,6 @@ export default async (
         ? npmConfig.globalPrefix
         : findBestGlobalPrefixOnWindows(npmConfig.globalPrefix, process.env)
     )
-  pnpmConfig.globalBin = process.platform === 'win32'
-    ? npmGlobalPrefix
-    : path.resolve(npmGlobalPrefix, 'bin')
   pnpmConfig.globalDir = pnpmConfig.globalDir ? npmGlobalPrefix : path.join(npmGlobalPrefix, 'pnpm-global')
   pnpmConfig.lockfileDir = pnpmConfig.lockfileDir ?? pnpmConfig.lockfileDirectory ?? pnpmConfig.shrinkwrapDirectory
   pnpmConfig.useLockfile = (() => {
@@ -221,7 +219,12 @@ export default async (
 
   if (cliOptions['global']) {
     pnpmConfig.dir = path.join(pnpmConfig.globalDir, LAYOUT_VERSION.toString())
-    pnpmConfig.bin = pnpmConfig.globalBin
+    pnpmConfig.bin = cliOptions['dir']
+      ? (
+        process.platform === 'win32'
+          ? cliOptions.dir : path.resolve(cliOptions.dir, 'bin')
+      )
+      : globalBinDir()
     pnpmConfig.allowNew = true
     pnpmConfig.ignoreCurrentPrefs = true
     pnpmConfig.saveProd = true

packages/config/tsconfig.json

@@ -15,6 +15,9 @@
     {
       "path": "../error"
     },
+    {
+      "path": "../global-bin-dir"
+    },
     {
       "path": "../types"
     }

packages/global-bin-dir/README.md

@@ -0,0 +1,15 @@
+# @pnpm/global-bin-dir
+
+> Finds a directory that is in PATH and we have permission to write to it
+
+[![npm version](https://img.shields.io/npm/v/@pnpm/global-bin-dir.svg)](https://www.npmjs.com/package/@pnpm/global-bin-dir)
+
+## Installation
+
+```sh
+<pnpm|npm|yarn> add @pnpm/global-bin-dir
+```
+
+## License
+
+MIT © [Zoltan Kochan](https://www.kochan.io/)

packages/global-bin-dir/package.json

@@ -0,0 +1,42 @@
+{
+  "name": "@pnpm/global-bin-dir",
+  "version": "0.0.0",
+  "description": "Finds a directory that is in PATH and we have permission to write to i",
+  "main": "lib/index.js",
+  "typings": "lib/index.d.ts",
+  "files": [
+    "lib",
+    "!*.map"
+  ],
+  "engines": {
+    "node": ">=10.13"
+  },
+  "scripts": {
+    "lint": "tslint -c ../../tslint.json src/**/*.ts test/**/*.ts",
+    "test": "pnpm run compile && pnpm run _test",
+    "prepublishOnly": "pnpm run compile",
+    "compile": "rimraf lib tsconfig.tsbuildinfo && tsc --build",
+    "_test": "cd ../.. && c8 --reporter lcov --reports-dir packages/global-bin-dir/coverage ts-node packages/global-bin-dir/test --type-check"
+  },
+  "repository": "https://github.com/pnpm/pnpm/blob/master/packages/global-bin-dir",
+  "keywords": [
+    "pnpm"
+  ],
+  "author": "Zoltan Kochan <z@kochan.io> (https://www.kochan.io/)",
+  "license": "MIT",
+  "bugs": {
+    "url": "https://github.com/pnpm/pnpm/issues"
+  },
+  "homepage": "https://github.com/pnpm/pnpm/blob/master/packages/global-bin-dir#readme",
+  "funding": "https://opencollective.com/pnpm",
+  "dependencies": {
+    "@pnpm/error": "workspace:^1.2.0",
+    "can-write-to-dir": "^1.0.0",
+    "path-name": "^1.0.0"
+  },
+  "devDependencies": {
+    "@pnpm/global-bin-dir": "link:",
+    "is-windows": "^1.0.2",
+    "proxyquire": "^2.1.3"
+  }
+}

packages/global-bin-dir/src/index.ts

@@ -0,0 +1,54 @@
+import PnpmError from '@pnpm/error'
+import { sync as canWriteToDir } from 'can-write-to-dir'
+import path = require('path')
+import PATH = require('path-name')
+
+export default function () {
+  if (!process.env[PATH]) {
+    throw new PnpmError('NO_PATH_ENV',
+      `Couldn't find a global directory for executables because the "${PATH}" environment variable is not set.`)
+  }
+  const dirs = process.env[PATH]?.split(path.delimiter) ?? []
+  return pickBestGlobalBinDir(dirs)
+}
+
+function pickBestGlobalBinDir (dirs: string[]) {
+  const nodeBinDir = path.dirname(process.execPath)
+  const noWriteAccessDirs = [] as string[]
+  for (const dir of dirs) {
+    if (
+      isUnderDir('node', dir) ||
+      isUnderDir('nodejs', dir) ||
+      isUnderDir('npm', dir) ||
+      isUnderDir('pnpm', dir) ||
+      nodeBinDir === dir
+    ) {
+      if (canWriteToDirAndExists(dir)) return dir
+      noWriteAccessDirs.push(dir)
+    }
+  }
+  if (noWriteAccessDirs.length === 0) {
+    throw new PnpmError('NO_GLOBAL_BIN_DIR', "Couldn't find a suitable global executables directory.", {
+      hint: `There should be a node, nodejs, npm, or pnpm directory in the "${PATH}" environment variable`,
+    })
+  }
+  throw new PnpmError('GLOBAL_BIN_DIR_PERMISSION', 'No write access to the found global executable directories', {
+    hint: `The found directories:
+${noWriteAccessDirs.join('\n')}`,
+  })
+}
+
+function isUnderDir (dir: string, target: string) {
+  target = target.endsWith(path.sep) ? target : `${target}${path.sep}`
+  return target.includes(`${path.sep}${dir}${path.sep}`) ||
+    target.includes(`${path.sep}.${dir}${path.sep}`)
+}
+
+function canWriteToDirAndExists (dir: string) {
+  try {
+    return canWriteToDir(dir)
+  } catch (err) {
+    if (err.code !== 'ENOENT') throw err
+    return false
+  }
+}

packages/global-bin-dir/test/index.ts

@@ -0,0 +1,108 @@
+import PnpmError from '@pnpm/error'
+import { sync as _canWriteToDir } from 'can-write-to-dir'
+import isWindows = require('is-windows')
+import path = require('path')
+import proxiquire = require('proxyquire')
+import test = require('tape')
+
+const makePath =
+  isWindows()
+    ? (...paths: string[]) => `C:\\${path.join(...paths)}`
+    : (...paths: string[]) => `/${path.join(...paths)}`
+
+let canWriteToDir!: typeof _canWriteToDir
+const FAKE_PATH = 'FAKE_PATH'
+
+const globalBinDir = proxiquire('../lib/index.js', {
+  'can-write-to-dir': {
+    sync: (dir: string) => canWriteToDir(dir),
+  },
+  'path-name': FAKE_PATH,
+}).default
+
+const userGlobalBin = makePath('usr', 'local', 'bin')
+const nodeGlobalBin = makePath('home', 'z', '.nvs', 'node', '12.0.0', 'x64', 'bin')
+const npmGlobalBin = makePath('home', 'z', '.npm')
+const pnpmGlobalBin = makePath('home', 'z', '.pnpm')
+const otherDir = makePath('some', 'dir')
+const currentExecDir = makePath('current', 'exec')
+process.env[FAKE_PATH] = [
+  userGlobalBin,
+  nodeGlobalBin,
+  npmGlobalBin,
+  pnpmGlobalBin,
+  otherDir,
+  currentExecDir,
+].join(path.delimiter)
+
+test('prefer a directory that has "nodejs", "npm", or "pnpm" in the path', (t) => {
+  canWriteToDir = () => true
+  t.equal(globalBinDir(), nodeGlobalBin)
+
+  canWriteToDir = (dir) => dir !== nodeGlobalBin
+  t.equal(globalBinDir(), npmGlobalBin)
+
+  canWriteToDir = (dir) => dir !== nodeGlobalBin && dir !== npmGlobalBin
+  t.equal(globalBinDir(), pnpmGlobalBin)
+
+  t.end()
+})
+
+test("ignore directories that don't exist", (t) => {
+  canWriteToDir = (dir) => {
+    if (dir === nodeGlobalBin) {
+      const err = new Error('Not exists')
+      err['code'] = 'ENOENT'
+      throw err
+    }
+    return true
+  }
+  t.equal(globalBinDir(), npmGlobalBin)
+  t.end()
+})
+
+test('prefer the directory of the currently executed nodejs command', (t) => {
+  const originalExecPath = process.execPath
+  process.execPath = path.join(currentExecDir, 'n')
+  canWriteToDir = (dir) => dir !== nodeGlobalBin && dir !== npmGlobalBin && dir !== pnpmGlobalBin
+  t.equal(globalBinDir(), currentExecDir)
+  process.execPath = originalExecPath
+  t.end()
+})
+
+test('when the process has no write access to any of the suitable directories, throw an error', (t) => {
+  canWriteToDir = (dir) => dir === otherDir
+  let err!: PnpmError
+  try {
+    globalBinDir()
+  } catch (_err) {
+    err = _err
+  }
+  t.ok(err)
+  t.equal(err.code, 'ERR_PNPM_GLOBAL_BIN_DIR_PERMISSION')
+  t.end()
+})
+
+test('throw an exception if non of the directories in the PATH are suitable', (t) => {
+  const pathEnv = process.env[FAKE_PATH]
+  process.env[FAKE_PATH] = [otherDir].join(path.delimiter)
+  canWriteToDir = () => true
+  let err!: PnpmError
+  try {
+    globalBinDir()
+  } catch (_err) {
+    err = _err
+  }
+  t.ok(err)
+  t.equal(err.code, 'ERR_PNPM_NO_GLOBAL_BIN_DIR')
+  process.env[FAKE_PATH] = pathEnv
+  t.end()
+})
+
+test('throw exception if PATH is not set', (t) => {
+  const pathEnv = process.env[FAKE_PATH]
+  delete process.env[FAKE_PATH]
+  t.throws(() => globalBinDir(), /Couldn't find a global directory/)
+  process.env[FAKE_PATH] = pathEnv
+  t.end()
+})

packages/global-bin-dir/tsconfig.json

@@ -0,0 +1,16 @@
+{
+  "extends": "@pnpm/tsconfig",
+  "compilerOptions": {
+    "outDir": "lib",
+    "rootDir": "src"
+  },
+  "include": [
+    "src/**/*.ts",
+    "../../typings/**/*.d.ts"
+  ],
+  "references": [
+    {
+      "path": "../error"
+    }
+  ]
+}

packages/plugin-commands-installation/package.json

@@ -63,6 +63,7 @@
     "@pnpm/filter-workspace-packages": "workspace:2.1.2",
     "@pnpm/find-workspace-dir": "workspace:1.0.0",
     "@pnpm/find-workspace-packages": "workspace:2.2.7",
+    "@pnpm/global-bin-dir": "workspace:^0.0.0",
     "@pnpm/manifest-utils": "workspace:1.0.2",
     "@pnpm/outdated": "workspace:7.1.1",
     "@pnpm/package-store": "workspace:9.0.9",

packages/plugin-commands-installation/src/link.ts

@@ -9,6 +9,7 @@ import { UNIVERSAL_OPTIONS } from '@pnpm/common-cli-options-help'
 import { Config, types as allTypes } from '@pnpm/config'
 import findWorkspaceDir from '@pnpm/find-workspace-dir'
 import findWorkspacePackages, { arrayOfWorkspacePackagesToMap } from '@pnpm/find-workspace-packages'
+import globalBinDir from '@pnpm/global-bin-dir'
 import { StoreController } from '@pnpm/package-store'
 import { createOrConnectStoreControllerCached, CreateStoreControllerOptions } from '@pnpm/store-connection-manager'
 import pLimit = require('p-limit')
@@ -77,7 +78,7 @@ export async function handler (
     | 'saveOptional'
     | 'saveProd'
     | 'workspaceDir'
-  > & Partial<Pick<Config, 'globalBin' | 'globalDir' | 'linkWorkspacePackages'>>,
+  > & Partial<Pick<Config, 'globalDir' | 'linkWorkspacePackages'>>,
   params: string[]
 ) {
   const cwd = opts?.dir ?? process.cwd()
@@ -106,7 +107,7 @@ export async function handler (
     const newManifest = await linkToGlobal(cwd, {
       ...linkOpts,
       // A temporary workaround. global bin/prefix are always defined when --global is set
-      globalBin: linkOpts.globalBin!,
+      globalBin: globalBinDir(),
       globalDir: linkOpts.globalDir!,
       manifest: manifest || {},
     })

packages/plugin-commands-installation/src/unlink.ts

@@ -44,6 +44,7 @@ export async function handler (
     Pick<Config,
       | 'allProjects'
       | 'bail'
+      | 'bin'
       | 'engineStrict'
       | 'linkWorkspacePackages'
       | 'selectedProjectsGraph'
@@ -62,6 +63,7 @@ export async function handler (
   }
   const store = await createOrConnectStoreController(opts)
   const unlinkOpts = Object.assign(opts, {
+    globalBin: opts.bin,
     storeController: store.ctrl,
     storeDir: store.dir,
   })

packages/plugin-commands-installation/tsconfig.json

@@ -36,6 +36,9 @@
     {
       "path": "../find-workspace-packages"
     },
+    {
+      "path": "../global-bin-dir"
+    },
     {
       "path": "../manifest-utils"
     },

packages/pnpm/test/link.ts

@@ -5,6 +5,7 @@ import isWindows = require('is-windows')
 import fs = require('mz/fs')
 import ncpCB = require('ncp')
 import path = require('path')
+import PATH = require('path-name')
 import readYamlFile from 'read-yaml-file'
 import tape = require('tape')
 import promisifyTape from 'tape-promise'
@@ -51,7 +52,12 @@ test('link global bin', async function (t: tape.Test) {
   process.chdir('..')
 
   const global = path.resolve('global')
-  const env = { NPM_CONFIG_PREFIX: global }
+  const globalBin = path.join(global, 'nodejs')
+  await fs.mkdir(globalBin, { recursive: true })
+  const env = {
+    NPM_CONFIG_PREFIX: global,
+    [PATH]: `${globalBin}${path.delimiter}${process.env[PATH]}`,
+  }
   if (process.env.APPDATA) env['APPDATA'] = global
 
   await writePkg('package-with-bin', { name: 'package-with-bin', version: '1.0.0', bin: 'bin.js' })
@@ -61,7 +67,6 @@ test('link global bin', async function (t: tape.Test) {
 
   await execPnpm(['link'], { env })
 
-  const globalBin = isWindows() ? global : path.join(global, 'bin')
   await isExecutable(t, path.join(globalBin, 'package-with-bin'))
 })
 

packages/pnpm/test/uninstall.ts

@@ -1,8 +1,10 @@
 import prepare from '@pnpm/prepare'
 import { fromDir as readPkgFromDir } from '@pnpm/read-package-json'
 import isWindows = require('is-windows')
+import fs = require('mz/fs')
 import path = require('path')
 import exists = require('path-exists')
+import PATH = require('path-name')
 import tape = require('tape')
 import promisifyTape from 'tape-promise'
 import { execPnpm } from './utils'
@@ -34,9 +36,13 @@ test('uninstall global package with its bin files', async (t: tape.Test) => {
   process.chdir('..')
 
   const global = path.resolve('global')
-  const globalBin = isWindows() ? path.join(global, 'npm') : path.join(global, 'bin')
+  const globalBin = path.join(global, 'nodejs')
+  await fs.mkdir(globalBin, { recursive: true })
 
-  const env = { NPM_CONFIG_PREFIX: global }
+  const env = {
+    NPM_CONFIG_PREFIX: global,
+    [PATH]: `${globalBin}${path.delimiter}${process.env[PATH]}`,
+  }
   if (process.env.APPDATA) env['APPDATA'] = global
 
   await execPnpm(['add', '-g', 'sh-hello-world@1.0.1'], { env })

pnpm-lock.yaml

@@ -184,6 +184,7 @@ importers:
     dependencies:
       '@pnpm/constants': 'link:../constants'
       '@pnpm/error': 'link:../error'
+      '@pnpm/global-bin-dir': 'link:../global-bin-dir'
       '@pnpm/types': 'link:../types'
       '@zkochan/npm-conf': 2.0.0
       camelcase: 6.0.0
@@ -201,6 +202,7 @@ importers:
       '@pnpm/config': 'link:'
       '@pnpm/constants': 'workspace:4.0.0'
       '@pnpm/error': 'workspace:1.2.0'
+      '@pnpm/global-bin-dir': 'workspace:^0.0.0'
       '@pnpm/types': 'workspace:6.1.0'
       '@types/mz': ^2.7.1
       '@types/ramda': ^0.27.6
@@ -596,6 +598,22 @@ importers:
       hosted-git-info: 3.0.4
       is-windows: 1.0.2
       semver: ^7.3.2
+  packages/global-bin-dir:
+    dependencies:
+      '@pnpm/error': 'link:../error'
+      can-write-to-dir: 1.0.0
+      path-name: 1.0.0
+    devDependencies:
+      '@pnpm/global-bin-dir': 'link:'
+      is-windows: 1.0.2
+      proxyquire: 2.1.3
+    specifiers:
+      '@pnpm/error': 'workspace:^1.2.0'
+      '@pnpm/global-bin-dir': 'link:'
+      can-write-to-dir: ^1.0.0
+      is-windows: ^1.0.2
+      path-name: ^1.0.0
+      proxyquire: ^2.1.3
   packages/headless:
     dependencies:
       '@pnpm/build-modules': 'link:../build-modules'
@@ -1482,6 +1500,7 @@ importers:
       '@pnpm/filter-workspace-packages': 'link:../filter-workspace-packages'
       '@pnpm/find-workspace-dir': 'link:../find-workspace-dir'
       '@pnpm/find-workspace-packages': 'link:../find-workspace-packages'
+      '@pnpm/global-bin-dir': 'link:../global-bin-dir'
       '@pnpm/manifest-utils': 'link:../manifest-utils'
       '@pnpm/outdated': 'link:../outdated'
       '@pnpm/package-store': 'link:../package-store'
@@ -1541,6 +1560,7 @@ importers:
       '@pnpm/filter-workspace-packages': 'workspace:2.1.2'
       '@pnpm/find-workspace-dir': 'workspace:1.0.0'
       '@pnpm/find-workspace-packages': 'workspace:2.2.7'
+      '@pnpm/global-bin-dir': 'workspace:^0.0.0'
       '@pnpm/lockfile-types': 'workspace:2.0.1'
       '@pnpm/logger': ^3.2.2
       '@pnpm/manifest-utils': 'workspace:1.0.2'
@@ -4969,6 +4989,15 @@ packages:
       node: '>=4'
     resolution:
       integrity: sha512-2zyqEs49pyv+Ne1rUsOYWo5kgaMJ62ztHV2agWHdkYSOcNLohOGB4rY8xjjCKnByWeBnOOvjBRAPdG+7LKUpcA==
+  /can-write-to-dir/1.0.0:
+    dependencies:
+      graceful-fs: 4.2.4
+      path-temp: 2.0.0
+    dev: false
+    engines:
+      node: '>=10.13'
+    resolution:
+      integrity: sha512-6BsSCtFRHLcigbgLfUW0kTB8Wze+e9oPWZzkd+4N5JL/sy90CVCBtU8JvXwbf5Z3lslnAAO+CdEjKbofot1VBw==
   /caseless/0.12.0:
     resolution:
       integrity: sha1-G2gcIf+EAzyCZUMJBolCDRhxUdw=