fix: cli has no write access to the selected global dir (#3206)

close #1203

.changeset/neat-cats-search.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/config": major
+---
+
+Remove `pnpm-prefix` setting support.

.changeset/selfish-spies-whisper.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/config": major
+---
+
+`globalDir` is never set. Only the `dir` option is set with the global directory location when the `--global` is used. The pnpm CLI should have access to the global dir, otherwise an exception is thrown.

.changeset/small-flowers-trade.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/plugin-commands-installation": major
+---
+
+The --global option should be used when linking from/to the global modules directory.

packages/config/package.json

@@ -37,6 +37,7 @@
     "@pnpm/types": "workspace:6.4.0",
     "@zkochan/npm-conf": "2.0.2",
     "camelcase": "^6.2.0",
+    "can-write-to-dir": "^1.1.0",
     "is-subdir": "^1.1.1",
     "ramda": "^0.27.1",
     "realpath-missing": "^1.1.0",

packages/config/src/Config.ts

@@ -21,10 +21,8 @@ export interface Config {
   rawConfig: Record<string, any>, // eslint-disable-line
   dryRun?: boolean // This option might be not supported ever
   global?: boolean
-  globalDir: string
   dir: string
-  bin?: string
-  npmGlobalBinDir: string
+  bin: string
   ignoreScripts?: boolean
   save?: boolean
   saveProd?: boolean

packages/config/src/findBestGlobalPrefix.ts

@@ -5,6 +5,7 @@ export default function findBestGlobalPrefixOnWindows (
   defaultNpmGlobalPrefix: string,
   env: { [key: string]: string | undefined }
 ) {
+  if (process.platform !== 'win32') return defaultNpmGlobalPrefix
   if (
     (env.LOCALAPPDATA != null && isSubdir(env.LOCALAPPDATA, defaultNpmGlobalPrefix)) ||
     (env.APPDATA != null && isSubdir(env.APPDATA, defaultNpmGlobalPrefix))

packages/config/src/index.ts

@@ -5,11 +5,13 @@ import globalBinDir from '@pnpm/global-bin-dir'
 import camelcase from 'camelcase'
 import loadNpmConf from '@zkochan/npm-conf'
 import npmTypes from '@zkochan/npm-conf/lib/types'
+import { sync as canWriteToDir } from 'can-write-to-dir'
+import os from 'os'
 import * as R from 'ramda'
 import realpathMissing from 'realpath-missing'
 import whichcb from 'which'
 import getScopeRegistries, { normalizeRegistry } from './getScopeRegistries'
-import findBestGlobalPrefixOnWindows from './findBestGlobalPrefixOnWindows'
+import findBestGlobalPrefix from './findBestGlobalPrefix'
 import {
   Config,
   ConfigWithDeprecatedSettings,
@@ -221,16 +223,6 @@ export default async (
     default: normalizeRegistry(pnpmConfig.rawConfig.registry),
     ...getScopeRegistries(pnpmConfig.rawConfig),
   }
-  const npmGlobalPrefix: string = pnpmConfig.globalDir ?? pnpmConfig.rawConfig['pnpm-prefix'] ??
-    (
-      process.platform !== 'win32'
-        ? npmConfig.globalPrefix
-        : findBestGlobalPrefixOnWindows(npmConfig.globalPrefix, process.env)
-    )
-  pnpmConfig.npmGlobalBinDir = 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 = (() => {
     if (typeof pnpmConfig['lockfile'] === 'boolean') return pnpmConfig['lockfile']
@@ -252,14 +244,21 @@ export default async (
     : pnpmConfig['sharedWorkspaceLockfile']
 
   if (cliOptions['global']) {
-    pnpmConfig.save = true
-    pnpmConfig.dir = path.join(pnpmConfig.globalDir, LAYOUT_VERSION.toString())
+    const npmGlobalPrefix: string = findBestGlobalPrefix(npmConfig.globalPrefix, process.env)
+    const globalDirRoot = pnpmConfig['globalDir']
+      ? pnpmConfig['globalDir'] : path.join(firstWithWriteAccess([npmGlobalPrefix, os.homedir()]), 'pnpm-global')
+    pnpmConfig.dir = path.join(globalDirRoot, LAYOUT_VERSION.toString())
+
+    const npmGlobalBinDir = process.platform === 'win32'
+      ? npmGlobalPrefix
+      : path.resolve(npmGlobalPrefix, 'bin')
     pnpmConfig.bin = cliOptions.dir
       ? (
         process.platform === 'win32'
           ? cliOptions.dir : path.resolve(cliOptions.dir, 'bin')
       )
-      : globalBinDir([pnpmConfig.npmGlobalBinDir], { shouldAllowWrite: opts.globalDirShouldAllowWrite === true })
+      : globalBinDir([npmGlobalBinDir], { shouldAllowWrite: opts.globalDirShouldAllowWrite === true })
+    pnpmConfig.save = true
     pnpmConfig.allowNew = true
     pnpmConfig.ignoreCurrentPrefs = true
     pnpmConfig.saveProd = true
@@ -420,3 +419,11 @@ function getProcessEnv (env: string) {
     process.env[env.toUpperCase()] ??
     process.env[env.toLowerCase()]
 }
+
+function firstWithWriteAccess (dirs: string[]) {
+  const first = dirs.find((dir) => dir.includes('_npx') || canWriteToDir(dir))
+  if (first == null) {
+    throw new PnpmError('NO_SUITABLE_GLOBAL_DIR', `pnpm has no write access to global direcotry. Tried locations: ${dirs.join(', ')}`)
+  }
+  return first
+}

packages/config/test/findBestGlobalPrefix.test.ts

@@ -1,6 +1,6 @@
-import findBestGlobalPrefixOnWindows from '../lib/findBestGlobalPrefixOnWindows'
+import findBestGlobalPrefix from '../lib/findBestGlobalPrefix'
 
-test('findBestGlobalPrefixOnWindows()', () => {
+test('findBestGlobalPrefix()', () => {
   if (process.platform !== 'win32') {
     // skipping on non-windows
     return
@@ -13,24 +13,24 @@ test('findBestGlobalPrefixOnWindows()', () => {
 
   expect(
     // keep npm global prefix if is inside AppData\Local
-    findBestGlobalPrefixOnWindows('C:\\Users\\Imre\\AppData\\Local\\nvs\\default', env)).toEqual(
+    findBestGlobalPrefix('C:\\Users\\Imre\\AppData\\Local\\nvs\\default', env)).toEqual(
     'C:\\Users\\Imre\\AppData\\Local\\nvs\\default'
   )
 
   expect(
     // keep npm global prefix if is inside AppData\Roaming
-    findBestGlobalPrefixOnWindows('C:\\Users\\Imre\\AppData\\Roaming\\nvs\\default', env)).toEqual(
+    findBestGlobalPrefix('C:\\Users\\Imre\\AppData\\Roaming\\nvs\\default', env)).toEqual(
     'C:\\Users\\Imre\\AppData\\Roaming\\nvs\\default'
   )
 
   expect(
     // prefer location in AppData\Roaming
-    findBestGlobalPrefixOnWindows('C:\\foo', env)).toEqual(
+    findBestGlobalPrefix('C:\\foo', env)).toEqual(
     'C:\\Users\\Imre\\AppData\\Roaming\\npm'
   )
 
   expect(
-    findBestGlobalPrefixOnWindows('C:\\foo', {})).toEqual(
+    findBestGlobalPrefix('C:\\foo', {})).toEqual(
     'C:\\foo'
   )
 })

packages/config/test/index.ts

@@ -3,7 +3,6 @@ import getConfig from '@pnpm/config'
 import PnpmError from '@pnpm/error'
 import prepare, { prepareEmpty } from '@pnpm/prepare'
 
-import './findBestGlobalPrefixOnWindows'
 import { promises as fs } from 'fs'
 import path from 'path'
 import symlinkDir from 'symlink-dir'

packages/global-bin-dir/package.json

@@ -31,7 +31,7 @@
   "funding": "https://opencollective.com/pnpm",
   "dependencies": {
     "@pnpm/error": "workspace:1.4.0",
-    "can-write-to-dir": "^1.0.1",
+    "can-write-to-dir": "^1.1.0",
     "path-name": "^1.0.0"
   },
   "devDependencies": {

packages/plugin-commands-installation/package.json

@@ -64,7 +64,6 @@
     "@pnpm/filter-workspace-packages": "workspace:2.3.10",
     "@pnpm/find-workspace-dir": "workspace:2.0.0",
     "@pnpm/find-workspace-packages": "workspace:2.3.38",
-    "@pnpm/global-bin-dir": "workspace:1.2.6",
     "@pnpm/manifest-utils": "workspace:1.1.5",
     "@pnpm/outdated": "workspace:7.2.26",
     "@pnpm/package-store": "workspace:10.1.18",

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

@@ -10,7 +10,6 @@ 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 {
@@ -74,17 +73,17 @@ export function help () {
 
 export async function handler (
   opts: CreateStoreControllerOptions & Pick<Config,
+  | 'bin'
   | 'cliOptions'
   | 'engineStrict'
-  | 'npmGlobalBinDir'
   | 'saveDev'
   | 'saveOptional'
   | 'saveProd'
   | 'workspaceDir'
-  > & Partial<Pick<Config, 'globalDir' | 'linkWorkspacePackages'>>,
+  > & Partial<Pick<Config, 'linkWorkspacePackages'>>,
   params?: string[]
 ) {
-  const cwd = opts?.dir ?? process.cwd()
+  const cwd = process.cwd()
 
   const storeControllerCache = new Map<string, Promise<{dir: string, ctrl: StoreController}>>()
   let workspacePackagesArr
@@ -106,12 +105,12 @@ export async function handler (
 
   // pnpm link
   if (!params || !params.length) {
-    const { manifest, writeProjectManifest } = await tryReadProjectManifest(opts.globalDir!, opts)
+    const { manifest, writeProjectManifest } = await tryReadProjectManifest(opts.dir, opts)
     const newManifest = await linkToGlobal(cwd, {
       ...linkOpts,
-      // A temporary workaround. global bin/prefix are always defined when --global is set
-      globalBin: globalBinDir([linkOpts.npmGlobalBinDir]),
-      globalDir: linkOpts.globalDir!,
+      dir: cwd,
+      globalBin: linkOpts.bin,
+      globalDir: linkOpts.dir,
       manifest: manifest ?? {},
     })
     await writeProjectManifest(newManifest)
@@ -137,7 +136,7 @@ export async function handler (
     } else {
       globalPkgNames = pkgNames
     }
-    const globalPkgPath = pathAbsolute(opts.globalDir!)
+    const globalPkgPath = pathAbsolute(opts.dir)
     globalPkgNames.forEach((pkgName) => pkgPaths.push(path.join(globalPkgPath, 'node_modules', pkgName)))
   }
 

packages/plugin-commands-installation/test/add.ts

@@ -13,6 +13,7 @@ const DEFAULT_OPTIONS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   cliOptions: {},
   include: {
     dependencies: true,

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

@@ -24,12 +24,11 @@ test('linking multiple packages', async () => {
   console.log('linking linked-foo to global package')
   const linkOpts = {
     ...DEFAULT_OPTS,
-    npmGlobalBinDir: path.join(globalDir, 'bin'),
-    globalDir,
+    bin: path.join(globalDir, 'bin'),
+    dir: globalDir,
   }
   await link.handler({
     ...linkOpts,
-    dir: process.cwd(),
   })
 
   process.chdir('..')
@@ -37,7 +36,6 @@ test('linking multiple packages', async () => {
 
   await link.handler({
     ...linkOpts,
-    dir: process.cwd(),
   }, ['linked-foo', '../linked-bar'])
 
   await project.has('linked-foo')
@@ -64,9 +62,8 @@ test('link global bin', async function () {
 
   await link.handler({
     ...DEFAULT_OPTS,
-    dir: process.cwd(),
-    npmGlobalBinDir: globalBin,
-    globalDir,
+    bin: globalBin,
+    dir: globalDir,
   })
   process.env[PATH] = oldPath
 
@@ -87,7 +84,6 @@ test('relative link', async () => {
   await link.handler({
     ...DEFAULT_OPTS,
     dir: process.cwd(),
-    npmGlobalBinDir: '',
   }, [`../${linkedPkgName}`])
 
   await project.isExecutable('.bin/hello-world-js-bin')
@@ -119,7 +115,6 @@ test('absolute link', async () => {
   await link.handler({
     ...DEFAULT_OPTS,
     dir: process.cwd(),
-    npmGlobalBinDir: '',
   }, [linkedPkgPath])
 
   await project.isExecutable('.bin/hello-world-js-bin')
@@ -173,7 +168,6 @@ test('link --production', async () => {
     ...DEFAULT_OPTS,
     cliOptions: { production: true },
     dir: process.cwd(),
-    npmGlobalBinDir: '',
   }, ['../source'])
 
   await projects['source'].has('is-positive')

packages/plugin-commands-installation/test/peerDependencies.ts

@@ -10,6 +10,7 @@ const DEFAULT_OPTIONS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   cliOptions: {},
   include: {
     dependencies: true,

packages/plugin-commands-installation/test/prune.ts

@@ -11,6 +11,7 @@ const DEFAULT_OPTIONS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   cliOptions: {},
   include: {
     dependencies: true,
@@ -36,7 +37,6 @@ test('prune removes external link that is not in package.json', async () => {
   await link.handler({
     ...DEFAULT_OPTIONS,
     dir: process.cwd(),
-    npmGlobalBinDir: process.cwd(),
     storeDir,
   }, ['./local'])
 

packages/plugin-commands-installation/test/update/interactive.ts

@@ -24,6 +24,7 @@ const DEFAULT_OPTIONS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   cliOptions: {},
   include: {
     dependencies: true,

packages/plugin-commands-installation/test/utils.ts

@@ -8,6 +8,7 @@ export const DEFAULT_OPTS = {
     original: [],
   },
   bail: true,
+  bin: 'node_modules/.bin',
   ca: undefined,
   cert: undefined,
   cliOptions: {},

packages/plugin-commands-installation/tsconfig.json

@@ -45,9 +45,6 @@
     {
       "path": "../find-workspace-packages"
     },
-    {
-      "path": "../global-bin-dir"
-    },
     {
       "path": "../lockfile-types"
     },

packages/plugin-commands-listing/test/utils.ts

@@ -8,6 +8,7 @@ export const DEFAULT_OPTS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   ca: undefined,
   cert: undefined,
   cliOptions: {},

packages/plugin-commands-outdated/test/utils.ts

@@ -8,6 +8,7 @@ export const DEFAULT_OPTS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   ca: undefined,
   cert: undefined,
   cliOptions: {},

packages/plugin-commands-publishing/test/utils.ts

@@ -8,6 +8,7 @@ export const DEFAULT_OPTS = {
     original: [],
   },
   bail: false,
+  bin: 'node_modules/.bin',
   ca: undefined,
   cert: undefined,
   cliOptions: {},

packages/pnpm/test/install/global.ts

@@ -1,6 +1,7 @@
 import path from 'path'
 import { LAYOUT_VERSION } from '@pnpm/constants'
 import prepare from '@pnpm/prepare'
+import { promises as fs } from 'fs'
 import isWindows from 'is-windows'
 import exists from 'path-exists'
 import {
@@ -11,6 +12,7 @@ import {
 test('global installation', async () => {
   prepare()
   const global = path.resolve('..', 'global')
+  await fs.mkdir(global)
 
   const env = { NPM_CONFIG_PREFIX: global }
   if (process.env.APPDATA) env['APPDATA'] = global
@@ -44,6 +46,7 @@ test('always install latest when doing global installation without spec', async
   await addDistTag('peer-c', '2.0.0', 'latest')
 
   const global = path.resolve('..', 'global')
+  await fs.mkdir(global)
 
   const env = { NPM_CONFIG_PREFIX: global }
 
@@ -68,6 +71,7 @@ test('run lifecycle events of global packages in correct working directory', asy
 
   prepare()
   const global = path.resolve('..', 'global')
+  await fs.mkdir(global)
 
   const env = { NPM_CONFIG_PREFIX: global }
   if (process.env.APPDATA) env['APPDATA'] = global

packages/pnpm/test/install/selfUpdate.ts

@@ -1,5 +1,6 @@
 import path from 'path'
 import prepare from '@pnpm/prepare'
+import { promises as fs } from 'fs'
 import isWindows from 'is-windows'
 import pathExists from 'path-exists'
 import {
@@ -21,6 +22,7 @@ skipOnWindows('self-update stops the store server', async () => {
   expect(serverJson.connectionOptions).toBeTruthy()
 
   const global = path.resolve('global')
+  await fs.mkdir(global)
 
   const env = { NPM_CONFIG_PREFIX: global }
   if (process.env.APPDATA) env['APPDATA'] = global

packages/pnpm/test/root.ts

@@ -20,6 +20,7 @@ test('pnpm root -g', async () => {
   tempDir()
 
   const global = path.resolve('global')
+  await fs.mkdir(global)
 
   const env = { NPM_CONFIG_PREFIX: global }
   if (process.env.APPDATA) env['APPDATA'] = global

pnpm-lock.yaml

@@ -240,6 +240,7 @@ importers:
       '@pnpm/types': link:../types
       '@zkochan/npm-conf': 2.0.2
       camelcase: 6.2.0
+      can-write-to-dir: 1.1.0
       is-subdir: 1.2.0
       ramda: 0.27.1
       realpath-missing: 1.1.0
@@ -261,6 +262,7 @@ importers:
       '@types/which': ^2.0.0
       '@zkochan/npm-conf': 2.0.2
       camelcase: ^6.2.0
+      can-write-to-dir: ^1.1.0
       is-subdir: ^1.1.1
       ramda: ^0.27.1
       realpath-missing: ^1.1.0
@@ -703,7 +705,7 @@ importers:
     specifiers:
       '@pnpm/error': workspace:1.4.0
       '@pnpm/global-bin-dir': 'link:'
-      can-write-to-dir: ^1.0.1
+      can-write-to-dir: ^1.1.0
       is-windows: ^1.0.2
       path-name: ^1.0.0
 
@@ -1689,7 +1691,6 @@ 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
@@ -1751,7 +1752,6 @@ importers:
       '@pnpm/filter-workspace-packages': workspace:2.3.10
       '@pnpm/find-workspace-dir': workspace:2.0.0
       '@pnpm/find-workspace-packages': workspace:2.3.38
-      '@pnpm/global-bin-dir': workspace:1.2.6
       '@pnpm/lockfile-types': workspace:2.2.0
       '@pnpm/logger': ^3.2.3
       '@pnpm/manifest-utils': workspace:1.1.5
@@ -6271,6 +6271,7 @@ packages:
     resolution: {integrity: sha512-PYRx9toaBwM5kk+GTFuijayF5omWa/ToRCSlMUe2YA2Ki8+j0vOc+Hoe1bf7C5l1lQA2y3tE2t+PeQ4XiTlibg==}
     dependencies:
       path-temp: 2.0.0
+    dev: true
     engines:
       node: '>=10.13'