Merge pull request #570 from desktop/fix-buiding-env-path

Make sure we are copying the path variable

lib/git-environment.ts

@@ -81,8 +81,17 @@ export function setupEnvironment(
   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> = {
-    ...process.env,
+    // 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,
   }
 
@@ -91,22 +100,13 @@ export function setupEnvironment(
 
   if (process.platform === 'win32') {
     const mingw = process.arch === 'x64' ? 'mingw64' : 'mingw32'
-    env.PATH = `${gitDir}\\${mingw}\\bin;${gitDir}\\${mingw}\\usr\\bin;${env.PATH}`
+    env.PATH = `${gitDir}\\${mingw}\\bin;${gitDir}\\${mingw}\\usr\\bin;${
+      env.PATH ?? ''
+    }`
   }
 
   env.GIT_EXEC_PATH = resolveGitExecPath(env)
 
-  if (process.platform === 'win32') {
-    // while reading the environment variable is case-insensitive
-    // you can create a hash with multiple values, which means the
-    // wrong value might be used when spawning the child process
-    //
-    // this ensures we only ever supply one value for PATH
-    if (env.Path) {
-      delete env.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
   // as the system-wide configuration file and we're unable to modify it.

test/fast/environment-test.ts

@@ -36,4 +36,28 @@ describe('environment variables', () => {
       delete process.env.GIT_EXEC_PATH
     }
   })
+
+  if (process.platform === 'win32') {
+    it('resulting PATH contains the original PATH', () => {
+      const originalPathKey = Object.keys(process.env).find(
+        k => k.toUpperCase() === '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
+      }
+    })
+  }
 })