diff --git a/package.json b/package.json index 8d712bac0..07f7d5130 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,7 @@ "deprecation-cop": "0.54.1", "dev-live-reload": "0.47.0", "encoding-selector": "0.22.0", - "exception-reporting": "0.38.1", + "exception-reporting": "0.38.2", "find-and-replace": "0.201.0", "fuzzy-finder": "1.3.0", "git-diff": "1.1.0", diff --git a/spec/environment-helpers-spec.js b/spec/environment-helpers-spec.js index 2309be802..5d2ca8250 100644 --- a/spec/environment-helpers-spec.js +++ b/spec/environment-helpers-spec.js @@ -1,159 +1,82 @@ -'use babel' +/** @babel */ /* eslint-env jasmine */ import child_process from 'child_process' -import environmentHelpers from '../src/environment-helpers' -import os from 'os' +import updateProcessEnv from '../src/update-process-env' +import dedent from 'dedent' -describe('Environment handling', () => { - let originalEnv - let options +describe('updateProcessEnv(launchEnv)', function () { + let originalProcessEnv, originalProcessPlatform - beforeEach(() => { - originalEnv = process.env - delete process._originalEnv - options = { - platform: process.platform, - env: Object.assign({}, process.env) - } + beforeEach(function () { + originalProcessEnv = process.env + originalProcessPlatform = process.platform + process.env = {} }) - afterEach(() => { - process.env = originalEnv - delete process._originalEnv + afterEach(function () { + process.env = originalProcessEnv + process.platform = originalProcessPlatform }) - describe('on macOS, when PWD is not set', () => { - beforeEach(() => { - options.platform = 'darwin' - }) + describe('when the launch environment appears to come from a shell', function () { + it('updates process.env to match the launch environment', function () { + process.env = { + WILL_BE_DELETED: 'hi', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', + } + const initialProcessEnv = process.env - describe('needsPatching', () => { - it('returns true if PWD is unset', () => { - delete options.env.PWD - expect(environmentHelpers.needsPatching(options)).toBe(true) - options.env.PWD = undefined - expect(environmentHelpers.needsPatching(options)).toBe(true) - options.env.PWD = null - expect(environmentHelpers.needsPatching(options)).toBe(true) - options.env.PWD = false - expect(environmentHelpers.needsPatching(options)).toBe(true) + updateProcessEnv({PWD: '/the/dir', KEY1: 'value1', KEY2: 'value2'}) + expect(process.env).toEqual({ + PWD: '/the/dir', + KEY1: 'value1', + KEY2: 'value2', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', }) - it('returns false if PWD is set', () => { - options.env.PWD = 'xterm' - expect(environmentHelpers.needsPatching(options)).toBe(false) - }) - }) - - describe('normalize', () => { - it('changes process.env if PWD is unset', () => { - if (process.platform === 'win32') { - return - } - delete options.env.PWD - environmentHelpers.normalize(options) - expect(process._originalEnv).toBeDefined() - expect(process._originalEnv).toBeTruthy() - expect(process.env).toBeDefined() - expect(process.env).toBeTruthy() - expect(process.env.PWD).toBeDefined() - expect(process.env.PWD).toBeTruthy() - expect(process.env.PATH).toBeDefined() - expect(process.env.PATH).toBeTruthy() - expect(process.env.ATOM_HOME).toBeDefined() - expect(process.env.ATOM_HOME).toBeTruthy() - }) + // See #11302. On Windows, `process.env` is a magic object that offers + // case-insensitive environment variable matching, so we cannot replace it + // with another object. + expect(process.env).toBe(initialProcessEnv) }) }) - describe('on a platform other than macOS', () => { - beforeEach(() => { - options.platform = 'penguin' - }) + describe('when the launch environment does not come from a shell', function () { + describe('on osx', function () { + it('updates process.env to match the environment in the user\'s login shell', function () { + process.platform = 'darwin' + process.env.SHELL = '/my/custom/bash' - describe('needsPatching', () => { - it('returns false if PWD is set or unset', () => { - delete options.env.PWD - expect(environmentHelpers.needsPatching(options)).toBe(false) - options.env.PWD = undefined - expect(environmentHelpers.needsPatching(options)).toBe(false) - options.env.PWD = null - expect(environmentHelpers.needsPatching(options)).toBe(false) - options.env.PWD = false - expect(environmentHelpers.needsPatching(options)).toBe(false) - options.env.PWD = '/' - expect(environmentHelpers.needsPatching(options)).toBe(false) - }) - - it('returns false for linux', () => { - options.platform = 'linux' - options.PWD = '/' - expect(environmentHelpers.needsPatching(options)).toBe(false) - }) - - it('returns false for windows', () => { - options.platform = 'win32' - options.PWD = 'c:\\' - expect(environmentHelpers.needsPatching(options)).toBe(false) - }) - }) - - describe('normalize', () => { - it('does not change the environment', () => { - if (process.platform === 'win32') { - return - } - delete options.env.PWD - environmentHelpers.normalize(options) - expect(process._originalEnv).toBeUndefined() - expect(process.env).toBeDefined() - expect(process.env).toBeTruthy() - expect(process.env.PATH).toBeDefined() - expect(process.env.PATH).toBeTruthy() - expect(process.env.PWD).toBeUndefined() - expect(process.env.PATH).toBe(originalEnv.PATH) - expect(process.env.ATOM_HOME).toBeDefined() - expect(process.env.ATOM_HOME).toBeTruthy() - }) - }) - }) - - describe('getFromShell', () => { - describe('when things are configured properly', () => { - beforeEach(() => { spyOn(child_process, 'spawnSync').andReturn({ - stdout: 'FOO=BAR' + os.EOL + 'TERM=xterm-something' + os.EOL + - 'PATH=/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path' + stdout: dedent` + FOO=BAR=BAZ=QUUX + TERM=xterm-something + PATH=/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path + ` + }) + + updateProcessEnv(process.env) + expect(child_process.spawnSync.mostRecentCall.args[0]).toBe('/my/custom/bash') + expect(process.env).toEqual({ + FOO: 'BAR=BAZ=QUUX', + TERM: 'xterm-something', + PATH: '/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path' }) }) - - it('returns an object containing the information from the user\'s shell environment', () => { - let env = environmentHelpers.getFromShell() - expect(env.FOO).toEqual('BAR') - expect(env.TERM).toEqual('xterm-something') - expect(env.PATH).toEqual('/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path') - }) }) - describe('when an error occurs launching the shell', () => { - beforeEach(() => { - spyOn(child_process, 'spawnSync').andReturn({ - error: new Error('testing when an error occurs') - }) - }) + describe('not on osx', function () { + it('does not update process.env', function () { + process.platform = 'win32' + spyOn(child_process, 'spawnSync') + process.env = {FOO: 'bar'} - it('returns undefined', () => { - expect(environmentHelpers.getFromShell()).toBeUndefined() - }) - - it('leaves the environment as-is when normalize() is called', () => { - options.platform = 'darwin' - delete options.env.PWD - expect(environmentHelpers.needsPatching(options)).toBe(true) - environmentHelpers.normalize(options) - expect(process.env).toBeDefined() - expect(process._originalEnv).toBeUndefined() + updateProcessEnv(process.env) + expect(child_process.spawnSync).not.toHaveBeenCalled() + expect(process.env).toEqual({FOO: 'bar'}) }) }) }) diff --git a/src/environment-helpers.js b/src/environment-helpers.js deleted file mode 100644 index f4224fbe4..000000000 --- a/src/environment-helpers.js +++ /dev/null @@ -1,116 +0,0 @@ -'use babel' - -import {spawnSync} from 'child_process' -import os from 'os' - -// Gets a dump of the user's configured shell environment. -// -// Returns the output of the `env` command or `undefined` if there was an error. -function getRawShellEnv () { - let shell = getUserShell() - - // The `-ilc` set of options was tested to work with the macOS v10.11 - // default-installed versions of bash, zsh, sh, and ksh. It *does not* - // work with csh or tcsh. - let results = spawnSync(shell, ['-ilc', 'env'], {encoding: 'utf8'}) - if (results.error || !results.stdout || results.stdout.length <= 0) { - return - } - - return results.stdout -} - -function getUserShell () { - if (process.env.SHELL) { - return process.env.SHELL - } - - return '/bin/bash' -} - -// Gets the user's configured shell environment. -// -// Returns a copy of the user's shell enviroment. -function getFromShell () { - let shellEnvText = getRawShellEnv() - if (!shellEnvText) { - return - } - - let env = {} - - for (let line of shellEnvText.split(os.EOL)) { - if (line.includes('=')) { - let components = line.split('=') - if (components.length === 2) { - env[components[0]] = components[1] - } else { - let k = components.shift() - let v = components.join('=') - env[k] = v - } - } - } - - return env -} - -function needsPatching (options = { platform: process.platform, env: process.env }) { - if (options.platform === 'darwin' && !options.env.PWD) { - let shell = getUserShell() - if (shell.endsWith('csh') || shell.endsWith('tcsh') || shell.endsWith('fish')) { - return false - } - return true - } - - return false -} - -// Fix for #11302 because `process.env` on Windows is a magic object that offers case-insensitive -// environment variable matching. By always cloning to `process.env` we prevent breaking the -// underlying functionality. -function clone (to, from) { - for (var key in to) { - // Don't erase NODE_ENV. Fixes #12024 - if (key !== 'NODE_ENV') { - delete to[key] - } - } - - Object.assign(to, from) -} - -function normalize (options = {}) { - if (options && options.env) { - clone(process.env, options.env) - } - - if (!options.env) { - options.env = process.env - } - - if (!options.platform) { - options.platform = process.platform - } - - if (needsPatching(options)) { - // Patch the `process.env` on startup to fix the problem first documented - // in #4126. Retain the original in case someone needs it. - let shellEnv = getFromShell() - if (shellEnv && shellEnv.PATH) { - process._originalEnv = Object.assign({}, process.env) - clone(process.env, shellEnv) - } - } -} - -function replace (env) { - if (!env || !env.PATH) { - return - } - - clone(process.env, env) -} - -export default { getFromShell, needsPatching, normalize, replace } diff --git a/src/initialize-application-window.coffee b/src/initialize-application-window.coffee index 463bdd48e..f85207153 100644 --- a/src/initialize-application-window.coffee +++ b/src/initialize-application-window.coffee @@ -1,15 +1,13 @@ # Like sands through the hourglass, so are the days of our lives. module.exports = ({blobStore}) -> - environmentHelpers = require('./environment-helpers') + updateProcessEnv = require('./update-process-env') path = require 'path' require './window' {getWindowLoadSettings} = require './window-load-settings-helpers' {ipcRenderer} = require 'electron' {resourcePath, isSpec, devMode, env} = getWindowLoadSettings() - # Set baseline environment - environmentHelpers.normalize({env: env}) - env = process.env + updateProcessEnv(env) # Add application-specific exports to module search path. exportsPath = path.join(resourcePath, 'exports') @@ -37,5 +35,5 @@ module.exports = ({blobStore}) -> setTimeout (-> document.querySelector('atom-workspace').focus()), 0 window.addEventListener('focus', windowFocused) ipcRenderer.on('environment', (event, env) -> - environmentHelpers.replace(env) + updateProcessEnv(env) ) diff --git a/src/update-process-env.js b/src/update-process-env.js new file mode 100644 index 000000000..f9a55083c --- /dev/null +++ b/src/update-process-env.js @@ -0,0 +1,49 @@ +/** @babel */ + +import {spawnSync} from 'child_process' + +const ENVIRONMENT_VARIABLES_TO_PRESERVE = new Set(['NODE_ENV', 'NODE_PATH']) + +export default function updateProcessEnv (launchEnv) { + let envToAssign + if (launchEnv.PWD) { + envToAssign = launchEnv + } else { + if (process.platform === 'darwin') { + envToAssign = getEnvFromShell() + } + } + + if (envToAssign) { + for (let key in process.env) { + if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) { + delete process.env[key] + } + } + + for (let key in envToAssign) { + if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) { + process.env[key] = envToAssign[key] + } + } + } +} + +function getEnvFromShell () { + let shell = process.env.SHELL + if (shell && (shell.endsWith('/bash') || shell.endsWith('/sh'))) { + let {stdout} = spawnSync(shell, ['-ilc', 'env'], {encoding: 'utf8'}) + if (stdout) { + let result = {} + for (let line of stdout.split('\n')) { + if (line.includes('=')) { + let components = line.split('=') + let key = components.shift() + let value = components.join('=') + result[key] = value + } + } + return result + } + } +}