Merge pull request #574 from desktop/env-map

Handle env vars in a case-preserving, case-insensitive manner on Windows
desktop · Oct 21, 2024 · b4d51b8 · b4d51b8
2 parents dd1f524 + 256ec73
commit b4d51b8
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 66 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -4,5 +4,6 @@
     "**/.DS_Store": true,
     "build": true
   },
-  "editor.formatOnSave": true
+  "editor.formatOnSave": true,
+  "typescript.tsdk": "node_modules\\typescript\\lib"
 }
diff --git a/lib/env-map.ts b/lib/env-map.ts
@@ -0,0 +1,85 @@
+const normalizeKey = (key: string) =>
+  process.platform === 'win32' ? key.toUpperCase() : key
+
+const set = (
+  map: Map<string, [string, string | undefined]>,
+  key: string,
+  value: string | undefined
+) => {
+  const existingKey = map.get(normalizeKey(key))?.[0]
+  map.set(normalizeKey(key), [existingKey ?? key, value])
+}
+
+/**
+ * On Windows this behaves as a case-insensitive, case-preserving map.
+ * On other platforms this is analog to Map<string, string | undefined>
+ */
+export class EnvMap implements Map<string, string | undefined> {
+  private readonly map = new Map<string, [string, string | undefined]>()
+
+  public get size() {
+    return this.map.size
+  }
+
+  public constructor(
+    iterable?: Iterable<readonly [string, string | undefined]>
+  ) {
+    if (iterable) {
+      for (const [k, v] of iterable) {
+        set(this.map, k, v)
+      }
+    }
+  }
+
+  [Symbol.iterator]() {
+    return this.entries()
+  }
+
+  get [Symbol.toStringTag]() {
+    return 'EnvMap'
+  }
+
+  public entries() {
+    return this.map.values()
+  }
+
+  public *keys(): IterableIterator<string> {
+    for (const [k] of this.map.values()) {
+      yield k
+    }
+  }
+
+  public *values(): IterableIterator<string | undefined> {
+    for (const [, v] of this.map.values()) {
+      yield v
+    }
+  }
+
+  public get(key: string) {
+    return this.map.get(normalizeKey(key))?.[1]
+  }
+
+  public set(key: string, value: string | undefined) {
+    set(this.map, key, value)
+    return this
+  }
+
+  public has(key: string) {
+    return this.map.has(normalizeKey(key))
+  }
+
+  public clear() {
+    this.map.clear()
+  }
+
+  public forEach(
+    callbackFn: (value: string | undefined, key: string, map: EnvMap) => void,
+    thisArg?: any
+  ) {
+    this.map.forEach(([k, v]) => callbackFn.call(thisArg, v, k, this))
+  }
+
+  public delete(key: string) {
+    return this.map.delete(normalizeKey(key))
+  }
+}
diff --git a/lib/git-environment.ts b/lib/git-environment.ts
@@ -1,4 +1,5 @@
 import * as path from 'path'
+import { EnvMap } from './env-map'
 
 export function resolveEmbeddedGitDir(): string {
   if (
@@ -21,21 +22,19 @@ export function resolveEmbeddedGitDir(): string {
  *  If a custom Git directory path is defined as the `LOCAL_GIT_DIRECTORY` environment variable, then
  *  returns with it after resolving it as a path.
  */
-export function resolveGitDir(env: Record<string, string | undefined>): string {
-  if (env.LOCAL_GIT_DIRECTORY != null) {
-    return path.resolve(env.LOCAL_GIT_DIRECTORY)
-  } else {
-    return resolveEmbeddedGitDir()
-  }
+export function resolveGitDir(
+  localGitDir = process.env.LOCAL_GIT_DIRECTORY
+): string {
+  return localGitDir ? path.resolve(localGitDir) : resolveEmbeddedGitDir()
 }
 
 /**
  *  Find the path to the embedded Git binary.
  */
 export function resolveGitBinary(
-  env: Record<string, string | undefined>
+  localGitDir = process.env.LOCAL_GIT_DIRECTORY
 ): string {
-  const gitDir = resolveGitDir(env)
+  const gitDir = resolveGitDir(localGitDir)
   if (process.platform === 'win32') {
     return path.join(gitDir, 'cmd', 'git.exe')
   } else {
@@ -50,12 +49,13 @@ export function resolveGitBinary(
  * then it returns with it after resolving it as a path.
  */
 export function resolveGitExecPath(
-  env: Record<string, string | undefined>
+  localGitDir = process.env.LOCAL_GIT_DIRECTORY,
+  gitExecPath = process.env.GIT_EXEC_PATH
 ): string {
-  if (env.GIT_EXEC_PATH) {
-    return path.resolve(env.GIT_EXEC_PATH)
+  if (gitExecPath) {
+    return path.resolve(gitExecPath)
   }
-  const gitDir = resolveGitDir(env)
+  const gitDir = resolveGitDir(localGitDir)
   if (process.platform === 'win32') {
     if (process.arch === 'x64') {
       return path.join(gitDir, 'mingw64', 'libexec', 'git-core')
@@ -76,69 +76,68 @@ export function resolveGitExecPath(
  * @param additional options to include with the process
  */
 export function setupEnvironment(
-  environmentVariables: Record<string, string | undefined>
+  environmentVariables: Record<string, string | undefined>,
+  processEnv = process.env
 ): {
   env: Record<string, string | undefined>
   gitLocation: string
 } {
-  // This will get Path, pATh, PATH et all on Windows
-  const PATH = process.env.PATH
-
-  const env: Record<string, string | undefined> = {
-    // Merge all of process.env except Path, PATH, et all, we'll add that in just a sec
-    ...Object.fromEntries(
-      Object.entries(process.env).filter(([k]) => k.toUpperCase() !== 'PATH')
-    ),
-    // Ensure PATH is always set in upper case not process.env.Path like can
-    // be on case-insensitive Windows
-    ...(PATH ? { PATH } : {}),
-    ...environmentVariables,
-  }
+  const env = new EnvMap([
+    ...Object.entries(processEnv),
+    ...Object.entries(environmentVariables),
+  ])
 
-  const gitLocation = resolveGitBinary(env)
-  const gitDir = resolveGitDir(env)
+  const localGitDir = env.get('LOCAL_GIT_DIRECTORY')
+  const gitLocation = resolveGitBinary(localGitDir)
+  const gitDir = resolveGitDir(localGitDir)
 
   if (process.platform === 'win32') {
     const mingw = process.arch === 'x64' ? 'mingw64' : 'mingw32'
-    env.PATH = `${gitDir}\\${mingw}\\bin;${gitDir}\\${mingw}\\usr\\bin;${
-      env.PATH ?? ''
-    }`
+    env.set(
+      'PATH',
+      `${gitDir}\\${mingw}\\bin;${gitDir}\\${mingw}\\usr\\bin;${
+        env.get('PATH') ?? ''
+      }`
+    )
   }
 
-  env.GIT_EXEC_PATH = resolveGitExecPath(env)
+  env.set(
+    'GIT_EXEC_PATH',
+    resolveGitExecPath(localGitDir, env.get('GIT_EXEC_PATH'))
+  )
 
   // On Windows the contained Git environment (minGit) ships with a system level
-  // gitconfig that we can control but on macOS and Linux /etc/gitconfig is used
+  // gitconfig that we can control but on macOS and Linux /etc/gitconfig is used\
   // as the system-wide configuration file and we're unable to modify it.
   //
   // So in order to be able to provide our own sane defaults that can be overriden
   // by the user's global and local configuration we'll tell Git to use
   // dugite-native's custom gitconfig on those platforms.
-  if (process.platform !== 'win32' && !env.GIT_CONFIG_SYSTEM) {
-    env.GIT_CONFIG_SYSTEM = path.join(gitDir, 'etc', 'gitconfig')
+  if (process.platform !== 'win32' && !env.get('GIT_CONFIG_SYSTEM')) {
+    env.set('GIT_CONFIG_SYSTEM', path.join(gitDir, 'etc', 'gitconfig'))
   }
 
   if (process.platform === 'darwin' || process.platform === 'linux') {
     // templates are used to populate your .git folder
     // when a repository is initialized locally
     const templateDir = `${gitDir}/share/git-core/templates`
-    env.GIT_TEMPLATE_DIR = templateDir
+    env.set('GIT_TEMPLATE_DIR', templateDir)
   }
 
   if (process.platform === 'linux') {
     // when building Git for Linux and then running it from
     // an arbitrary location, you should set PREFIX for the
     // process to ensure that it knows how to resolve things
-    env.PREFIX = gitDir
+    env.set('PREFIX', gitDir)
 
-    if (!env.GIT_SSL_CAINFO && !env.LOCAL_GIT_DIRECTORY) {
+    if (!env.get('GIT_SSL_CAINFO') && !env.get('LOCAL_GIT_DIRECTORY')) {
       // use the SSL certificate bundle included in the distribution only
       // when using embedded Git and not providing your own bundle
       const distDir = resolveEmbeddedGitDir()
       const sslCABundle = `${distDir}/ssl/cacert.pem`
-      env.GIT_SSL_CAINFO = sslCABundle
+      env.set('GIT_SSL_CAINFO', sslCABundle)
     }
   }
 
-  return { env, gitLocation }
+  return { env: Object.fromEntries(env.entries()), gitLocation }
 }
diff --git a/package.json b/package.json
@@ -45,7 +45,7 @@
     "cross-env": "^5.2.0",
     "find-git-exec": "^0.0.4",
     "jest": "^28.1.3",
-    "prettier": "^2.7.1",
+    "prettier": "^3.3.1",
     "rimraf": "^2.5.4",
     "temp": "^0.9.0",
     "ts-jest": "^28.0.8",

diff --git a/test/fast/config-test.ts b/test/fast/config-test.ts
@@ -42,7 +42,7 @@ describe('config', () => {
       const originPath = origin.substring('file:'.length)
 
       expect(resolve(originPath)).toBe(
-        join(resolveGitDir(process.env), 'etc', 'gitconfig')
+        join(resolveGitDir(), 'etc', 'gitconfig')
       )
 
       expect(value).toBe('/etc/gitconfig')

diff --git a/test/fast/environment-test.ts b/test/fast/environment-test.ts
@@ -38,26 +38,22 @@ describe('environment variables', () => {
   })
 
   if (process.platform === 'win32') {
-    it('resulting PATH contains the original PATH', () => {
-      const originalPathKey = Object.keys(process.env).find(
-        k => k.toUpperCase() === 'PATH'
+    it('preserves case of path environment', () => {
+      const { env } = setupEnvironment(
+        { PATH: 'custom-path' },
+        { path: 'env-path' }
       )
-      expect(originalPathKey).not.toBeUndefined()
-
-      const originalPathValue = process.env.PATH
-
-      try {
-        delete process.env.PATH
-        process.env.Path = 'wow-such-case-insensitivity'
-        // This test will ensure that on platforms where env vars names are
-        // case-insensitive (like Windows) we don't end up with an invalid PATH
-        // and the original one lost in the process.
-        const { env } = setupEnvironment({})
-        expect(env.PATH).toContain('wow-such-case-insensitivity')
-      } finally {
-        delete process.env.Path
-        process.env[originalPathKey!] = originalPathValue
-      }
+      expect(env.PATH).toBeUndefined()
+      expect(env.path).toContain('custom-path')
+    })
+  } else {
+    it('treats environment variables as case-sensitive', () => {
+      const { env } = setupEnvironment(
+        { PATH: 'WOW_SUCH_CASE_SENSITIVITY' },
+        { path: 'wow-such-case-sensitivity' }
+      )
+      expect(env.PATH).toBe('WOW_SUCH_CASE_SENSITIVITY')
+      expect(env.path).toBe('wow-such-case-sensitivity')
     })
   }
 })
diff --git a/yarn.lock b/yarn.lock
@@ -2060,10 +2060,10 @@ pkg-dir@^4.2.0:
   dependencies:
     find-up "^4.0.0"
 
-prettier@^2.7.1:
-  version "2.7.1"
-  resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.7.1.tgz#e235806850d057f97bb08368a4f7d899f7760c64"
-  integrity sha512-ujppO+MkdPqoVINuDFDRLClm7D78qbDt0/NR+wp5FqEZOoTNAjPHWj17QRhu7geIHJfcNhRk1XVQmF8Bp3ye+g==
+prettier@^3.3.1:
+  version "3.3.1"
+  resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.3.1.tgz#e68935518dd90bb7ec4821ba970e68f8de16e1ac"
+  integrity sha512-7CAwy5dRsxs8PHXT3twixW9/OEll8MLE0VRPCJyl7CkS6VHGPSlsVaWTiASPTyGyYRyApxlaWTzwUxVNrhcwDg==
 
 pretty-format@^28.0.0, pretty-format@^28.1.3:
   version "28.1.3"