fix: always prefer the pnpm home directory when search for global bin (#3723)

ref #3713

.changeset/serious-pets-brush.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/global-bin-dir": major
+---
+
+Always prefer the pnpm home directory, when searching for global bin location.

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

@@ -29,13 +29,17 @@ function pickBestGlobalBinDir (
   shouldAllowWrite: boolean
 ) {
   const noWriteAccessDirs = [] as string[]
+  const pnpmDir = dirs.find((dir) => isUnderDir('pnpm', dir.toLowerCase()))
+  if (pnpmDir != null) {
+    if (canWriteToDirAndExists(pnpmDir)) return pnpmDir
+    throw new PnpmError('PNPM_DIR_NOT_WRITABLE', `The CLI has no write access to the pnpm home directory at ${pnpmDir}`)
+  }
   for (const dir of dirs) {
     const lowCaseDir = dir.toLowerCase()
     if ((
       isUnderDir('node', lowCaseDir) ||
       isUnderDir('nodejs', lowCaseDir) ||
       isUnderDir('npm', lowCaseDir) ||
-      isUnderDir('pnpm', lowCaseDir) ||
       knownCandidates.some((candidate) => areDirsEqual(candidate, dir)) ||
       dirHasNodeRelatedCommand(dir)
     ) && !isUnderDir('_npx', lowCaseDir)) {

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

@@ -35,6 +35,16 @@ jest.mock('fs', () => {
   }
 })
 
+let originalPath: string | undefined
+
+beforeEach(() => {
+  originalPath = process.env[FAKE_PATH]
+})
+
+afterEach(() => {
+  process.env[FAKE_PATH] = originalPath
+})
+
 jest.mock('path-name', () => 'FAKE_PATH')
 
 const userGlobalBin = makePath('usr', 'local', 'bin')
@@ -45,34 +55,55 @@ const npxGlobalBin = makePath('home', 'z', '.npm', '_npx', '123')
 const otherDir = makePath('some', 'dir')
 const currentExecDir = makePath('current', 'exec')
 const dirWithTrailingSlash = `${makePath('current', 'slash')}${path.sep}`
-process.env[FAKE_PATH] = [
+const BIG_PATH = [
   npxGlobalBin,
   userGlobalBin,
   nodeGlobalBin,
   npmGlobalBin,
-  pnpmGlobalBin,
   otherDir,
   currentExecDir,
   dirWithTrailingSlash,
 ].join(path.delimiter)
 
-test('prefer a directory that has "nodejs", "npm", or "pnpm" in the path', () => {
+test('prefer the pnpm home directory', () => {
+  process.env[FAKE_PATH] = [
+    npmGlobalBin,
+    currentExecDir,
+    nodeGlobalBin,
+    pnpmGlobalBin,
+  ].join(path.delimiter)
+  canWriteToDir = () => true
+  expect(globalBinDir()).toStrictEqual(pnpmGlobalBin)
+})
+
+test('fail if there is no write access to the pnpm home directory', () => {
+  process.env[FAKE_PATH] = [
+    npmGlobalBin,
+    currentExecDir,
+    nodeGlobalBin,
+    pnpmGlobalBin,
+  ].join(path.delimiter)
+  canWriteToDir = (dir) => dir !== pnpmGlobalBin
+  expect(() => globalBinDir()).toThrow(`The CLI has no write access to the pnpm home directory at ${pnpmGlobalBin}`)
+})
+
+test('prefer a directory that has "nodejs" or "npm" in the path', () => {
+  process.env[FAKE_PATH] = BIG_PATH
   canWriteToDir = () => true
   expect(globalBinDir()).toStrictEqual(nodeGlobalBin)
 
   canWriteToDir = (dir) => dir !== nodeGlobalBin
   expect(globalBinDir()).toStrictEqual(npmGlobalBin)
-
-  canWriteToDir = (dir) => dir !== nodeGlobalBin && dir !== npmGlobalBin
-  expect(globalBinDir()).toStrictEqual(pnpmGlobalBin)
 })
 
 test('prefer directory that is passed in as a known suitable location', () => {
+  process.env[FAKE_PATH] = BIG_PATH
   canWriteToDir = () => true
   expect(globalBinDir([userGlobalBin])).toStrictEqual(userGlobalBin)
 })
 
 test("ignore directories that don't exist", () => {
+  process.env[FAKE_PATH] = BIG_PATH
   canWriteToDir = (dir) => {
     if (dir === nodeGlobalBin) {
       const err = new Error('Not exists')
@@ -85,6 +116,7 @@ test("ignore directories that don't exist", () => {
 })
 
 test('prefer the directory of the currently executed nodejs command', () => {
+  process.env[FAKE_PATH] = BIG_PATH
   const originalExecPath = process.execPath
   process.execPath = path.join(currentExecDir, 'n')
   canWriteToDir = (dir) => dir !== nodeGlobalBin && dir !== npmGlobalBin && dir !== pnpmGlobalBin
@@ -97,6 +129,7 @@ test('prefer the directory of the currently executed nodejs command', () => {
 })
 
 test('when the process has no write access to any of the suitable directories, throw an error', () => {
+  process.env[FAKE_PATH] = BIG_PATH
   canWriteToDir = (dir) => dir === otherDir
   let err!: PnpmError
   try {
@@ -109,12 +142,12 @@ test('when the process has no write access to any of the suitable directories, t
 })
 
 test('when the process has no write access to any of the suitable directories, but opts.shouldAllowWrite is false, return the first match', () => {
+  process.env[FAKE_PATH] = BIG_PATH
   canWriteToDir = (dir) => dir === otherDir
   expect(globalBinDir([], { shouldAllowWrite: false })).toEqual(nodeGlobalBin)
 })
 
 test('throw an exception if non of the directories in the PATH are suitable', () => {
-  const pathEnv = process.env[FAKE_PATH]
   process.env[FAKE_PATH] = [otherDir].join(path.delimiter)
   canWriteToDir = () => true
   let err!: PnpmError
@@ -125,31 +158,24 @@ test('throw an exception if non of the directories in the PATH are suitable', ()
   }
   expect(err).toBeDefined()
   expect(err.code).toEqual('ERR_PNPM_NO_GLOBAL_BIN_DIR')
-  process.env[FAKE_PATH] = pathEnv
 })
 
 test('throw exception if PATH is not set', () => {
-  const pathEnv = process.env[FAKE_PATH]
   delete process.env[FAKE_PATH]
   expect(() => globalBinDir()).toThrow(/Couldn't find a global directory/)
-  process.env[FAKE_PATH] = pathEnv
 })
 
 test('prefer a directory that has "Node" in the path', () => {
   const capitalizedNodeGlobalBin = makePath('home', 'z', '.nvs', 'Node', '12.0.0', 'x64', 'bin')
-  const pathEnv = process.env[FAKE_PATH]
   process.env[FAKE_PATH] = capitalizedNodeGlobalBin
 
   canWriteToDir = () => true
   expect(globalBinDir()).toEqual(capitalizedNodeGlobalBin)
-
-  process.env[FAKE_PATH] = pathEnv
 })
 
 test('select a directory that has a node command in it', () => {
   const dir1 = makePath('foo')
   const dir2 = makePath('bar')
-  const pathEnv = process.env[FAKE_PATH]
   process.env[FAKE_PATH] = [
     dir1,
     dir2,
@@ -158,14 +184,11 @@ test('select a directory that has a node command in it', () => {
   canWriteToDir = () => true
   readdirSync = (dir) => dir === dir2 ? [makeFileEntry('node')] : []
   expect(globalBinDir()).toEqual(dir2)
-
-  process.env[FAKE_PATH] = pathEnv
 })
 
 test('do not select a directory that has a node directory in it', () => {
   const dir1 = makePath('foo')
   const dir2 = makePath('bar')
-  const pathEnv = process.env[FAKE_PATH]
   process.env[FAKE_PATH] = [
     dir1,
     dir2,
@@ -175,14 +198,11 @@ test('do not select a directory that has a node directory in it', () => {
   readdirSync = (dir) => dir === dir2 ? [makeDirEntry('node')] : []
 
   expect(() => globalBinDir()).toThrow(/Couldn't find a suitable/)
-
-  process.env[FAKE_PATH] = pathEnv
 })
 
 test('select a directory that has a node.bat command in it', () => {
   const dir1 = makePath('foo')
   const dir2 = makePath('bar')
-  const pathEnv = process.env[FAKE_PATH]
   process.env[FAKE_PATH] = [
     dir1,
     dir2,
@@ -191,6 +211,4 @@ test('select a directory that has a node.bat command in it', () => {
   canWriteToDir = () => true
   readdirSync = (dir) => dir === dir2 ? [makeFileEntry('node.bat')] : []
   expect(globalBinDir()).toEqual(dir2)
-
-  process.env[FAKE_PATH] = pathEnv
 })