Refactor setup (#4709)

* refactor: setup on Windows

* fix(setup): don't override existing PNPM_HOME by default

.changeset/five-planets-cover.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-setup": patch
+"pnpm": patch
+---
+
+`pnpm setup` should not override the PNPM_HOME env variable on Windows, unless `--force` is used.

packages/plugin-commands-setup/package.json

@@ -31,6 +31,7 @@
   "homepage": "https://github.com/pnpm/pnpm/blob/main/packages/plugin-commands-setup#readme",
   "dependencies": {
     "@pnpm/cli-utils": "workspace:0.7.4",
+    "@pnpm/error": "workspace:3.0.1",
     "execa": "npm:safe-execa@^0.1.1",
     "render-help": "^1.0.1"
   },

packages/plugin-commands-setup/src/setup.ts

@@ -8,7 +8,9 @@ import { setupWindowsEnvironmentPath } from './setupOnWindows'
 
 export const rcOptionsTypes = () => ({})
 
-export const cliOptionsTypes = () => ({})
+export const cliOptionsTypes = () => ({
+  force: Boolean,
+})
 
 export const shorthands = {}
 
@@ -18,6 +20,17 @@ export function help () {
   return renderHelp({
     description: 'Sets up pnpm',
     descriptionLists: [
+      {
+        title: 'Options',
+
+        list: [
+          {
+            description: 'Override the PNPM_HOME env variable in case it already exists',
+            name: '--force',
+            shortAlias: '-f',
+          },
+        ],
+      },
     ],
     url: docsUrl('setup'),
     usages: ['pnpm setup'],
@@ -47,6 +60,7 @@ function copyCli (currentLocation: string, targetDir: string) {
 
 export async function handler (
   opts: {
+    force?: boolean
     pnpmHomeDir: string
   }
 ) {
@@ -55,7 +69,7 @@ export async function handler (
   if (execPath.match(/\.[cm]?js$/) == null) {
     copyCli(execPath, opts.pnpmHomeDir)
   }
-  const updateOutput = await updateShell(currentShell, opts.pnpmHomeDir)
+  const updateOutput = await updateShell(currentShell, opts.pnpmHomeDir, { force: opts.force ?? false })
   return `${updateOutput}
 
 Setup complete. Open a new terminal to start using pnpm.`
@@ -68,7 +82,11 @@ function detectCurrentShell () {
   return typeof process.env.SHELL === 'string' ? path.basename(process.env.SHELL) : null
 }
 
-async function updateShell (currentShell: string | null, pnpmHomeDir: string): Promise<string> {
+async function updateShell (
+  currentShell: string | null,
+  pnpmHomeDir: string,
+  opts: { force: boolean }
+): Promise<string> {
   switch (currentShell) {
   case 'bash': {
     const configFile = path.join(os.homedir(), '.bashrc')
@@ -84,7 +102,7 @@ async function updateShell (currentShell: string | null, pnpmHomeDir: string): P
   }
 
   if (process.platform === 'win32') {
-    return setupWindowsEnvironmentPath(pnpmHomeDir)
+    return setupWindowsEnvironmentPath(pnpmHomeDir, opts)
   }
 
   return 'Could not infer shell type.'

packages/plugin-commands-setup/src/setupOnWindows.ts

@@ -1,35 +1,57 @@
+import PnpmError from '@pnpm/error'
 import { win32 as path } from 'path'
 import execa from 'execa'
 
 type IEnvironmentValueMatch = { groups: { name: string, type: string, data: string } } & RegExpMatchArray
 
-export async function setupWindowsEnvironmentPath (pnpmHomeDir: string): Promise<string> {
-  const pathRegex = /^ {4}(?<name>PATH) {4}(?<type>\w+) {4}(?<data>.*)$/gim
-  const pnpmHomeRegex = /^ {4}(?<name>PNPM_HOME) {4}(?<type>\w+) {4}(?<data>.*)$/gim
-  const regKey = 'HKEY_CURRENT_USER\\Environment'
+const REG_KEY = 'HKEY_CURRENT_USER\\Environment'
 
+function findEnvValuesInRegistry (regEntries: string, envVarName: string): IEnvironmentValueMatch[] {
+  const regexp = new RegExp(`^ {4}(?<name>${envVarName}) {4}(?<type>\\w+) {4}(?<data>.*)$`, 'gim')
+  return Array.from(regEntries.matchAll(regexp)) as IEnvironmentValueMatch[]
+}
+
+function setEnvVarInRegistry (envVarName: string, envVarValue: string) {
+  return execa('reg', ['add', REG_KEY, '/v', envVarName, '/t', 'REG_EXPAND_SZ', '/d', envVarValue, '/f'])
+}
+
+function pathIncludesDir (pathValue: string, dir: string): boolean {
+  const dirPath = path.parse(path.normalize(dir))
+  return pathValue
+    .split(path.delimiter)
+    .map(p => path.normalize(p))
+    .map(p => path.parse(p))
+    .map(p => `${p.dir}${path.sep}${p.base}`.toUpperCase())
+    .filter(p => p !== '')
+    .includes(`${dirPath.dir}${path.sep}${dirPath.base}`.toUpperCase())
+}
+
+export async function setupWindowsEnvironmentPath (pnpmHomeDir: string, opts: { force: boolean }): Promise<string> {
   // Use `chcp` to make `reg` use utf8 encoding for output.
   // Otherwise, the non-ascii characters in the environment variables will become garbled characters.
-  const queryResult = await execa(`chcp 65001>nul && reg query ${regKey}`, undefined, { shell: true })
+  const queryResult = await execa(`chcp 65001>nul && reg query ${REG_KEY}`, undefined, { shell: true })
 
   if (queryResult.failed) {
     return 'Win32 registry environment values could not be retrieved'
   }
 
   const queryOutput = queryResult.stdout
-  const pathValueMatch = [...queryOutput.matchAll(pathRegex)] as IEnvironmentValueMatch[]
-  const homeValueMatch = [...queryOutput.matchAll(pnpmHomeRegex)] as IEnvironmentValueMatch[]
+  const pathValueMatch = findEnvValuesInRegistry(queryOutput, 'PATH')
+  const homeValueMatch = findEnvValuesInRegistry(queryOutput, 'PNPM_HOME')
 
   let commitNeeded = false
-  let homeDir = pnpmHomeDir
   const logger = []
 
-  if (homeValueMatch.length === 1) {
-    homeDir = homeValueMatch[0].groups.data
-    logger.push(`Currently 'PNPM_HOME' is set to '${homeDir}'`)
+  if (homeValueMatch.length === 1 && !opts.force) {
+    const currentHomeDir = homeValueMatch[0].groups.data
+    if (currentHomeDir !== pnpmHomeDir) {
+      throw new PnpmError('DIFFERENT_HOME_DIR_IS_SET', `Currently 'PNPM_HOME' is set to '${currentHomeDir}'`, {
+        hint: 'If you want to override the existing PNPM_HOME env variable, use the --force option',
+      })
+    }
   } else {
-    logger.push(`Setting 'PNPM_HOME' to value '${homeDir}'`)
-    const addResult = await execa('reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_EXPAND_SZ', '/d', homeDir, '/f'])
+    logger.push(`Setting 'PNPM_HOME' to value '${pnpmHomeDir}'`)
+    const addResult = await setEnvVarInRegistry('PNPM_HOME', pnpmHomeDir)
     if (addResult.failed) {
       logger.push(`\t${addResult.stderr}`)
     } else {
@@ -43,19 +65,12 @@ export async function setupWindowsEnvironmentPath (pnpmHomeDir: string): Promise
     if (pathData == null || pathData.trim() === '') {
       logger.push('Current PATH is empty. No changes to this environment variable are applied')
     } else {
-      const homeDirPath = path.parse(path.normalize(homeDir))
-
-      if (pathData
-        .split(path.delimiter)
-        .map(p => path.normalize(p))
-        .map(p => path.parse(p))
-        .map(p => `${p.dir}${path.sep}${p.base}`.toUpperCase())
-        .filter(p => p !== '')
-        .includes(`${homeDirPath.dir}${path.sep}${homeDirPath.base}`.toUpperCase())) {
+      if (pathIncludesDir(pathData, pnpmHomeDir)) {
         logger.push('PATH already contains PNPM_HOME')
       } else {
         logger.push('Updating PATH')
-        const addResult = await execa('reg', ['add', regKey, '/v', pathValueMatch[0].groups.name, '/t', 'REG_EXPAND_SZ', '/d', `${homeDir}${path.delimiter}${pathData}`, '/f'])
+        const newPathValue = `${pnpmHomeDir}${path.delimiter}${pathData}`
+        const addResult = await setEnvVarInRegistry(pathValueMatch[0].groups.name, newPathValue)
         if (addResult.failed) {
           logger.push(`\t${addResult.stderr}`)
         } else {
@@ -69,7 +84,7 @@ export async function setupWindowsEnvironmentPath (pnpmHomeDir: string): Promise
   }
 
   if (commitNeeded) {
-    await execa('setx', ['PNPM_HOME', homeDir])
+    await execa('setx', ['PNPM_HOME', pnpmHomeDir])
   }
 
   return logger.join('\n')

packages/plugin-commands-setup/test/setupOnWindows/05.test.ts

@@ -27,11 +27,12 @@ const regKey = 'HKEY_CURRENT_USER\\Environment'
 
 test('PNPM_HOME is already set, but path is updated', async () => {
   const currentPathInRegistry = '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps;%USERPROFILE%\\.config\\etc;'
+  const pnpmHomeDir = '.pnpm\\home'
   execa['mockResolvedValueOnce']({
     failed: false,
     stdout: `
 HKEY_CURRENT_USER\\Environment
-    PNPM_HOME    REG_EXPAND_SZ    .pnpm\\home
+    PNPM_HOME    REG_EXPAND_SZ    ${pnpmHomeDir}
     Path    REG_EXPAND_SZ    ${currentPathInRegistry}
 `,
   }).mockResolvedValueOnce({
@@ -42,14 +43,11 @@ HKEY_CURRENT_USER\\Environment
     stderr: 'UNEXPECTED',
   })
 
-  const output = await setup.handler({
-    pnpmHomeDir: __dirname,
-  })
+  const output = await setup.handler({ pnpmHomeDir })
 
   expect(execa).toHaveBeenNthCalledWith(1, `chcp 65001>nul && reg query ${regKey}`, undefined, { shell: true })
   expect(execa).toHaveBeenNthCalledWith(2, 'reg', ['add', regKey, '/v', 'Path', '/t', 'REG_EXPAND_SZ', '/d', `${'.pnpm\\home'};${currentPathInRegistry}`, '/f'])
   expect(execa).toHaveBeenNthCalledWith(3, 'setx', ['PNPM_HOME', '.pnpm\\home'])
-  expect(output).toContain(`Currently 'PNPM_HOME' is set to '${'.pnpm\\home'}'`)
   expect(output).toContain('Updating PATH')
   expect(output).toContain('PATH UPDATED')
 })

packages/plugin-commands-setup/test/setupOnWindows/06.test.ts

@@ -25,7 +25,7 @@ afterAll(() => {
 
 const regKey = 'HKEY_CURRENT_USER\\Environment'
 
-test('Existing installation', async () => {
+test('setup throws an error if PNPM_HOME is already set to a different directory', async () => {
   execa['mockResolvedValueOnce']({
     failed: false,
     stdout: `
@@ -38,11 +38,32 @@ HKEY_CURRENT_USER\\Environment
     stderr: 'UNEXPECTED',
   })
 
+  await expect(
+    setup.handler({
+      pnpmHomeDir: __dirname,
+    })
+  ).rejects.toThrowError(/Currently 'PNPM_HOME' is set to '.pnpm\\home'/)
+})
+
+test('setup overrides PNPM_HOME, when setup is forced', async () => {
+  execa['mockResolvedValueOnce']({
+    failed: false,
+    stdout: `
+HKEY_CURRENT_USER\\Environment
+    PNPM_HOME    REG_EXPAND_SZ    .pnpm\\home
+    Path    REG_EXPAND_SZ    %USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps;%USERPROFILE%\\.config\\etc;.pnpm\\home;C:\\Windows;
+`,
+  }).mockResolvedValue({
+    failed: true,
+    stderr: 'UNEXPECTED',
+  })
+
+  const pnpmHomeDir = '.pnpm\\home'
   const output = await setup.handler({
-    pnpmHomeDir: __dirname,
+    force: true,
+    pnpmHomeDir,
   })
 
   expect(execa).toHaveBeenNthCalledWith(1, `chcp 65001>nul && reg query ${regKey}`, undefined, { shell: true })
-  expect(output).toContain(`Currently 'PNPM_HOME' is set to '${'.pnpm\\home'}'`)
-  expect(output).toContain('PATH already contains PNPM_HOME')
+  expect(output).toContain(`Setting 'PNPM_HOME' to value '${pnpmHomeDir}'`)
 })

packages/plugin-commands-setup/tsconfig.json

@@ -15,6 +15,9 @@
     },
     {
       "path": "../cli-utils"
+    },
+    {
+      "path": "../error"
     }
   ]
 }

pnpm-lock.yaml

@@ -2712,6 +2712,7 @@ importers:
   packages/plugin-commands-setup:
     specifiers:
       '@pnpm/cli-utils': workspace:0.7.4
+      '@pnpm/error': workspace:3.0.1
       '@pnpm/logger': ^4.0.0
       '@pnpm/plugin-commands-setup': workspace:2.0.4
       '@pnpm/prepare': workspace:*
@@ -2719,6 +2720,7 @@ importers:
       render-help: ^1.0.1
     dependencies:
       '@pnpm/cli-utils': link:../cli-utils
+      '@pnpm/error': link:../error
       execa: /safe-execa/0.1.1
       render-help: 1.0.2
     devDependencies: