fix(setup): don't fail if PNPM_HOME is not yet in the system registry (#4760)

close #4757
2026-04-05 07:46:10 -04:00 · 2022-05-19 03:11:49 +03:00
parent 58d656b070
commit e6a9f157d6
3 changed files with 69 additions and 71 deletions
--- a/.changeset/dull-spies-eat.md
+++ b/.changeset/dull-spies-eat.md
@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-setup": patch
+"pnpm": patch
+---
+
+`pnpm setup` should not fail on Windows if `PNPM_HOME` is not yet in the system registry [#4757](https://github.com/pnpm/pnpm/issues/4757)
--- a/packages/plugin-commands-setup/src/setupOnWindows.ts
+++ b/packages/plugin-commands-setup/src/setupOnWindows.ts
@@ -25,9 +25,10 @@ export async function setupWindowsEnvironmentPath (pnpmHomeDir: string, opts: {
 }

 async function _setupWindowsEnvironmentPath (pnpmHomeDir: string, opts: { force: boolean }): Promise<string> {
+  const registryOutput = await getRegistryOutput()
  const logger: string[] = []
-  logger.push(logEnvUpdate(await updateEnvVariable('PNPM_HOME', pnpmHomeDir, opts), 'PNPM_HOME'))
-  logger.push(logEnvUpdate(await prependToPath('%PNPM_HOME%'), 'Path'))
+  logger.push(logEnvUpdate(await updateEnvVariable(registryOutput, 'PNPM_HOME', pnpmHomeDir, opts), 'PNPM_HOME'))
+  logger.push(logEnvUpdate(await prependToPath(registryOutput, '%PNPM_HOME%'), 'Path'))

  return logger.join('\n')
 }
@@ -40,8 +41,8 @@ function logEnvUpdate (envUpdateResult: 'skipped' | 'updated', envName: string):
  return ''
 }

-async function updateEnvVariable (name: string, value: string, opts: { force: boolean }) {
-  const currentValue = await getEnvValueFromRegistry(name)
+async function updateEnvVariable (registryOutput: string, name: string, value: string, opts: { force: boolean }) {
+  const currentValue = await getEnvValueFromRegistry(registryOutput, name)
  if (currentValue && !opts.force) {
    if (currentValue !== value) {
      throw new BadEnvVariableError({ envName: name, currentValue, wantedValue: value })
@@ -53,8 +54,8 @@ async function updateEnvVariable (name: string, value: string, opts: { force: bo
  }
 }

-async function prependToPath (prependDir: string) {
-  const pathData = await getEnvValueFromRegistry('Path')
+async function prependToPath (registryOutput: string, prependDir: string) {
+  const pathData = await getEnvValueFromRegistry(registryOutput, 'Path')
  if (pathData === undefined || pathData == null || pathData.trim() === '') {
    throw new PnpmError('NO_PATH', '"Path" environment variable is not found in the registry')
  } else if (pathData.split(path.delimiter).includes(prependDir)) {
@@ -66,27 +67,39 @@ async function prependToPath (prependDir: string) {
  }
 }

-async function getEnvValueFromRegistry (envVarName: string): Promise<string | undefined> {
-  const queryResult = await execa('reg', ['query', REG_KEY, '/v', envVarName], { windowsHide: false })
-  if (queryResult.failed) {
-    throw new PnpmError('REG_READ', 'Win32 registry environment values could not be retrieved')
+// `windowsHide` in `execa` is true by default, which will cause `chcp` to have no effect.
+const EXEC_OPTS = { windowsHide: false }
+
+/**
+ * We read all the registry values and then pick the keys that we need.
+ * This is done because if we would try to pick a key that is not in the registry, the command would fail.
+ * And it is hard to identify the real cause of the command failure.
+ */
+async function getRegistryOutput (): Promise<string> {
+  try {
+    const queryResult = await execa('reg', ['query', REG_KEY], EXEC_OPTS)
+    return queryResult.stdout
+  } catch (err: any) { // eslint-disable-line
+    throw new PnpmError('REG_READ', 'win32 registry environment values could not be retrieved')
  }
+}
+
+async function getEnvValueFromRegistry (registryOutput: string, envVarName: string): Promise<string | undefined> {
  const regexp = new RegExp(`^ {4}(?<name>${envVarName}) {4}(?<type>\\w+) {4}(?<data>.*)$`, 'gim')
-  const match = Array.from(queryResult.stdout.matchAll(regexp))[0] as IEnvironmentValueMatch
+  const match = Array.from(registryOutput.matchAll(regexp))[0] as IEnvironmentValueMatch
  return match?.groups.data
 }

 async function setEnvVarInRegistry (envVarName: string, envVarValue: string) {
-  // `windowsHide` in `execa` is true by default, which will cause `chcp` to have no effect.
-  const addResult = await execa('reg', ['add', REG_KEY, '/v', envVarName, '/t', 'REG_EXPAND_SZ', '/d', envVarValue, '/f'], { windowsHide: false })
-  if (addResult.failed) {
-    throw new PnpmError('FAILED_SET_ENV', `Failed to set "${envVarName}" to "${envVarValue}": ${addResult.stderr}`)
-  } else {
-    // When setting environment variables through the registry, they will not be recognized immediately.
-    // There is a workaround though, to set at least one environment variable with `setx`.
-    // We have some redundancy here because we run it for each env var.
-    // It would be enough also to run it only for the last changed env var.
-    // Read more at: https://bit.ly/39OlQnF
-    await execa('setx', [envVarName, envVarValue])
+  try {
+    await execa('reg', ['add', REG_KEY, '/v', envVarName, '/t', 'REG_EXPAND_SZ', '/d', envVarValue, '/f'], EXEC_OPTS)
+  } catch (err: any) { // eslint-disable-line
+    throw new PnpmError('FAILED_SET_ENV', `Failed to set "${envVarName}" to "${envVarValue}": ${err.stderr as string}`)
  }
+  // When setting environment variables through the registry, they will not be recognized immediately.
+  // There is a workaround though, to set at least one environment variable with `setx`.
+  // We have some redundancy here because we run it for each env var.
+  // It would be enough also to run it only for the last changed env var.
+  // Read more at: https://bit.ly/39OlQnF
+  await execa('setx', [envVarName, envVarValue], EXEC_OPTS)
 }
--- a/packages/plugin-commands-setup/test/setupOnWindows.test.ts
+++ b/packages/plugin-commands-setup/test/setupOnWindows.test.ts
@@ -31,6 +31,12 @@ afterAll(() => {

 const regKey = 'HKEY_CURRENT_USER\\Environment'

+function createExecaError ({ stderr }: { stderr: string}) {
+  const err = new Error('Command failed with exit code 1')
+  err['stderr'] = stderr
+  return err
+}
+
 test('win32 registry environment values could not be retrieved', async () => {
  execa['mockResolvedValueOnce']({
    failed: false,
@@ -38,12 +44,10 @@ test('win32 registry environment values could not be retrieved', async () => {
  }).mockResolvedValueOnce({
    failed: false,
    stdout: '',
-  }).mockResolvedValue({
-    failed: true,
-  })
+  }).mockRejectedValue(createExecaError({ stderr: 'ERROR: Some error' }))

  await expect(setup.handler({ pnpmHomeDir: tempDir(false) }))
-    .rejects.toThrow(/Win32 registry environment values could not be retrieved/)
+    .rejects.toThrow()
 })

 test('environment Path is not configured correctly', async () => {
@@ -64,7 +68,7 @@ test('environment Path is not configured correctly', async () => {
    })
  ).rejects.toThrow(/"Path" environment variable is not found/)

-  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey, '/v', 'PNPM_HOME'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
 })

 test('environment Path is empty', async () => {
@@ -91,7 +95,7 @@ HKEY_CURRENT_USER\\Environment
    })
  ).rejects.toThrow(/"Path" environment variable is not found/)

-  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey, '/v', 'PNPM_HOME'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
 })

 test('successful first time installation', async () => {
@@ -107,6 +111,7 @@ test('successful first time installation', async () => {
    failed: false,
    stdout: `
 HKEY_CURRENT_USER\\Environment
+    Path    REG_EXPAND_SZ    ${currentPathInRegistry}
 `,
  }).mockResolvedValueOnce({
    failed: false,
@@ -114,12 +119,6 @@ HKEY_CURRENT_USER\\Environment
  }).mockResolvedValueOnce({
    failed: false,
    stdout: 'setx PNPM_HOME',
-  }).mockResolvedValueOnce({
-    failed: false,
-    stdout: `
-HKEY_CURRENT_USER\\Environment
-    Path    REG_EXPAND_SZ    ${currentPathInRegistry}
-`,
  }).mockResolvedValueOnce({
    failed: false,
    stdout: 'setx PNPM_HOME',
@@ -132,12 +131,11 @@ HKEY_CURRENT_USER\\Environment
  const pnpmHomeDirNormalized = path.normalize(pnpmHomeDir)
  const output = await setup.handler({ pnpmHomeDir })

-  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey, '/v', 'PNPM_HOME'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_EXPAND_SZ', '/d', pnpmHomeDirNormalized, '/f'], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(5, 'setx', ['PNPM_HOME', pnpmHomeDirNormalized])
-  expect(execa).toHaveBeenNthCalledWith(6, 'reg', ['query', regKey, '/v', 'Path'], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(7, 'reg', ['add', regKey, '/v', 'Path', '/t', 'REG_EXPAND_SZ', '/d', `%PNPM_HOME%;${currentPathInRegistry}`, '/f'], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(8, 'setx', ['Path', `%PNPM_HOME%;${currentPathInRegistry}`])
+  expect(execa).toHaveBeenNthCalledWith(5, 'setx', ['PNPM_HOME', pnpmHomeDirNormalized], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(6, 'reg', ['add', regKey, '/v', 'Path', '/t', 'REG_EXPAND_SZ', '/d', `%PNPM_HOME%;${currentPathInRegistry}`, '/f'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(7, 'setx', ['Path', `%PNPM_HOME%;${currentPathInRegistry}`], { windowsHide: false })
  expect(output).toContain('Path was updated')
  expect(output).toContain('PNPM_HOME was updated')
 })
@@ -157,11 +155,6 @@ test('PNPM_HOME is already set, but Path is updated', async () => {
    stdout: `
 HKEY_CURRENT_USER\\Environment
    PNPM_HOME    REG_EXPAND_SZ    ${pnpmHomeDirNormalized}
-`,
-  }).mockResolvedValueOnce({
-    failed: false,
-    stdout: `
-HKEY_CURRENT_USER\\Environment
    Path    REG_EXPAND_SZ    ${currentPathInRegistry}
 `,
  }).mockResolvedValueOnce({
@@ -177,10 +170,9 @@ HKEY_CURRENT_USER\\Environment

  const output = await setup.handler({ pnpmHomeDir })

-  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey, '/v', 'PNPM_HOME'], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['query', regKey, '/v', 'Path'], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(5, 'reg', ['add', regKey, '/v', 'Path', '/t', 'REG_EXPAND_SZ', '/d', `%PNPM_HOME%;${currentPathInRegistry}`, '/f'], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(6, 'setx', ['Path', `%PNPM_HOME%;${currentPathInRegistry}`])
+  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'Path', '/t', 'REG_EXPAND_SZ', '/d', `%PNPM_HOME%;${currentPathInRegistry}`, '/f'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(5, 'setx', ['Path', `%PNPM_HOME%;${currentPathInRegistry}`], { windowsHide: false })
  expect(output).toContain('PNPM_HOME was already up-to-date')
  expect(output).toContain('Path was updated')
 })
@@ -223,19 +215,15 @@ test('setup overrides PNPM_HOME, when setup is forced', async () => {
    stdout: `
 HKEY_CURRENT_USER\\Environment
    PNPM_HOME    REG_EXPAND_SZ    .pnpm\\home
-`,
-  }).mockResolvedValueOnce({
-    failed: false,
-    stdout: '',
-  }).mockResolvedValueOnce({
-    failed: false,
-    stdout: '',
-  }).mockResolvedValueOnce({
-    failed: false,
-    stdout: `
-HKEY_CURRENT_USER\\Environment
    Path    REG_EXPAND_SZ    %USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps;%USERPROFILE%\\.config\\etc;.pnpm\\home;C:\\Windows;
 `,
+  }).mockResolvedValueOnce({
+    failed: false,
+    stdout: '',
+  }).mockResolvedValueOnce({
+    failed: false,
+    stdout: '',
+  }).mockResolvedValueOnce({
  }).mockResolvedValueOnce({
    failed: false,
    stdout: '',
@@ -254,7 +242,7 @@ HKEY_CURRENT_USER\\Environment
    pnpmHomeDir,
  })

-  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey, '/v', 'PNPM_HOME'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_EXPAND_SZ', '/d', pnpmHomeDirNormalized, '/f'], { windowsHide: false })
  expect(output).toContain('PNPM_HOME was updated')
 })
@@ -274,19 +262,10 @@ test('failure to install', async () => {
 HKEY_CURRENT_USER\\Environment
    Path    REG_EXPAND_SZ    ${currentPathInRegistry}
 `,
-  }).mockResolvedValueOnce({
-    failed: true,
-    stderr: 'FAILED TO SET PNPM_HOME',
-  }).mockResolvedValueOnce({
-    failed: true,
-    stderr: 'FAILED TO UPDATE PATH',
-  }).mockResolvedValue({
-    failed: true,
-    stderr: 'UNEXPECTED',
-  })
+  }).mockRejectedValue(new Error())

  const pnpmHomeDir = tempDir(false)
  await expect(
    setup.handler({ pnpmHomeDir })
-  ).rejects.toThrow(/Failed to set "PNPM_HOME"/)
+  ).rejects.toThrow()
 })