From c570e14420463e89877b9cbdb09e8b277801d125 Mon Sep 17 00:00:00 2001 From: Joe Fitzgerald Date: Tue, 30 Aug 2016 19:52:56 +0000 Subject: [PATCH 1/6] Get Environment From Shell On Linux --- spec/update-process-env-spec.js | 86 ++++++++++++++++++++++++++------- src/update-process-env.js | 44 +++++++++++------ 2 files changed, 98 insertions(+), 32 deletions(-) diff --git a/spec/update-process-env-spec.js b/spec/update-process-env-spec.js index 1de89beba..476a4df8a 100644 --- a/spec/update-process-env-spec.js +++ b/spec/update-process-env-spec.js @@ -31,9 +31,10 @@ describe('updateProcessEnv(launchEnv)', function () { } const initialProcessEnv = process.env - updateProcessEnv({PWD: '/the/dir', KEY1: 'value1', KEY2: 'value2'}) + updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'}) expect(process.env).toEqual({ PWD: '/the/dir', + TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2', NODE_ENV: 'the-node-env', @@ -57,26 +58,29 @@ describe('updateProcessEnv(launchEnv)', function () { ATOM_HOME: '/the/atom/home' } - updateProcessEnv({PWD: '/the/dir'}) + updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something'}) expect(process.env).toEqual({ PWD: '/the/dir', + TERM: 'xterm-something', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - updateProcessEnv({PWD: '/the/dir', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')}) + updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')}) expect(process.env).toEqual({ PWD: '/the/dir', + TERM: 'xterm-something', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - updateProcessEnv({PWD: '/the/dir', ATOM_HOME: newAtomHomePath}) + updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', ATOM_HOME: newAtomHomePath}) expect(process.env).toEqual({ PWD: '/the/dir', + TERM: 'xterm-something', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: newAtomHomePath @@ -111,6 +115,33 @@ describe('updateProcessEnv(launchEnv)', function () { }) }) + describe('on linux', function () { + it('updates process.env to match the environment in the user\'s login shell', function () { + process.platform = 'linux' + process.env.SHELL = '/my/custom/bash' + delete process.env.TERM + + spyOn(child_process, 'spawnSync').andReturn({ + 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' + }) + + // Doesn't error + updateProcessEnv(null) + }) + }) + describe('not on osx', function () { it('does not update process.env', function () { process.platform = 'win32' @@ -124,30 +155,49 @@ describe('updateProcessEnv(launchEnv)', function () { }) describe('shouldGetEnvFromShell()', function () { - it('returns the shell when the shell should be patched', function () { + it('indicates when the environment should be fetched from the shell', function () { process.platform = 'darwin' - expect(shouldGetEnvFromShell('/bin/sh')).toBe(true) - expect(shouldGetEnvFromShell('/usr/local/bin/sh')).toBe(true) - expect(shouldGetEnvFromShell('/bin/bash')).toBe(true) - expect(shouldGetEnvFromShell('/usr/local/bin/bash')).toBe(true) - expect(shouldGetEnvFromShell('/bin/zsh')).toBe(true) - expect(shouldGetEnvFromShell('/usr/local/bin/zsh')).toBe(true) - expect(shouldGetEnvFromShell('/bin/fish')).toBe(true) - expect(shouldGetEnvFromShell('/usr/local/bin/fish')).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) + process.platform = 'linux' + expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) }) it('returns false when the shell should not be patched', function () { process.platform = 'darwin' - expect(shouldGetEnvFromShell('/bin/unsupported')).toBe(false) - expect(shouldGetEnvFromShell('/bin/shh')).toBe(false) - expect(shouldGetEnvFromShell('/bin/tcsh')).toBe(false) - expect(shouldGetEnvFromShell('/usr/csh')).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/bin/unsupported'})).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/bin/shh'})).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/bin/tcsh'})).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/usr/csh'})).toBe(false) + + process.platform = 'linux' + expect(shouldGetEnvFromShell({SHELL: '/bin/unsupported'})).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/bin/shh'})).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/bin/tcsh'})).toBe(false) + expect(shouldGetEnvFromShell({SHELL: '/usr/csh'})).toBe(false) }) it('returns false when the shell is undefined or empty', function () { process.platform = 'darwin' expect(shouldGetEnvFromShell(undefined)).toBe(false) - expect(shouldGetEnvFromShell('')).toBe(false) + expect(shouldGetEnvFromShell({})).toBe(false) + + process.platform = 'linux' + expect(shouldGetEnvFromShell(undefined)).toBe(false) + expect(shouldGetEnvFromShell({})).toBe(false) }) }) }) diff --git a/src/update-process-env.js b/src/update-process-env.js index c65e8c4fd..1b4bda58d 100644 --- a/src/update-process-env.js +++ b/src/update-process-env.js @@ -9,21 +9,25 @@ const ENVIRONMENT_VARIABLES_TO_PRESERVE = new Set([ 'ATOM_HOME' ]) -const OSX_SHELLS = new Set([ +const SHELLS_KNOWN_TO_WORK = new Set([ '/sh', '/bash', '/zsh', '/fish' ]) +const PLATFORMS_KNOWN_TO_WORK = new Set([ + 'darwin', + 'linux' +]) + function updateProcessEnv (launchEnv) { let envToAssign - if (launchEnv && launchEnv.PWD) { + + if (launchEnv && shouldGetEnvFromShell(launchEnv)) { + envToAssign = getEnvFromShell(launchEnv) + } else if (launchEnv && launchEnv.PWD) { // Launched from shell envToAssign = launchEnv - } else { - if (process.platform === 'darwin') { - envToAssign = getEnvFromShell() - } } if (envToAssign) { @@ -45,12 +49,25 @@ function updateProcessEnv (launchEnv) { } } -function shouldGetEnvFromShell (shell) { - if (!shell || shell.trim() === '') { +function shouldGetEnvFromShell (env) { + if (!PLATFORMS_KNOWN_TO_WORK.has(process.platform)) { // Untested platforms return false } - for (let s of OSX_SHELLS) { - if (shell.endsWith(s)) { + + if (!env || !env.SHELL || env.SHELL.trim() === '') { // Nothing to launch + return false + } + + if (process.platform === 'linux' && env.TERM) { // Launched from shell + return false + } + + if (process.platform === 'darwin' && env.PWD) { // Launched from shell + return false + } + + for (let s of SHELLS_KNOWN_TO_WORK) { + if (env.SHELL.endsWith(s)) { return true } } @@ -58,13 +75,12 @@ function shouldGetEnvFromShell (shell) { return false } -function getEnvFromShell () { - let shell = process.env.SHELL - if (!shouldGetEnvFromShell(shell)) { +function getEnvFromShell (env) { + if (!shouldGetEnvFromShell(env)) { return } - let {stdout} = spawnSync(shell, ['-ilc', 'command env'], {encoding: 'utf8'}) + let {stdout} = spawnSync(env.SHELL, ['-ilc', 'command env'], {encoding: 'utf8'}) if (stdout) { let result = {} for (let line of stdout.split('\n')) { From 10270609d80e183b048b4a48d7a969908334d5a9 Mon Sep 17 00:00:00 2001 From: Joe Fitzgerald Date: Wed, 31 Aug 2016 06:07:15 +0000 Subject: [PATCH 2/6] Use ATOM_SUPPRESS_ENV_PATCHING Environment Variable - Stop using shell whitelist --- atom.sh | 2 + spec/update-process-env-spec.js | 74 ++++++++++++++++++--------------- src/update-process-env.js | 35 +++++----------- 3 files changed, 54 insertions(+), 57 deletions(-) diff --git a/atom.sh b/atom.sh index 0a452c7f9..3c1b65e9b 100755 --- a/atom.sh +++ b/atom.sh @@ -15,6 +15,8 @@ else BETA_VERSION= fi +export ATOM_SUPPRESS_ENV_PATCHING=true + while getopts ":wtfvh-:" opt; do case "$opt" in -) diff --git a/spec/update-process-env-spec.js b/spec/update-process-env-spec.js index 476a4df8a..ddc6f4ade 100644 --- a/spec/update-process-env-spec.js +++ b/spec/update-process-env-spec.js @@ -29,10 +29,12 @@ describe('updateProcessEnv(launchEnv)', function () { NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' } + const initialProcessEnv = process.env - updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'}) + updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'}) expect(process.env).toEqual({ + ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', @@ -58,34 +60,62 @@ describe('updateProcessEnv(launchEnv)', function () { ATOM_HOME: '/the/atom/home' } - updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something'}) + updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir'}) expect(process.env).toEqual({ PWD: '/the/dir', - TERM: 'xterm-something', + ATOM_SUPPRESS_ENV_PATCHING: 'true', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')}) + updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')}) expect(process.env).toEqual({ + ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', - TERM: 'xterm-something', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - - updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', ATOM_HOME: newAtomHomePath}) + updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', ATOM_HOME: newAtomHomePath}) expect(process.env).toEqual({ + ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', - TERM: 'xterm-something', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: newAtomHomePath }) }) + + it('allows ATOM_SUPPRESS_ENV_PATCHING to be preserved if set', function () { + newAtomHomePath = temp.mkdirSync('atom-home') + + process.env = { + WILL_BE_DELETED: 'hi', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', + ATOM_HOME: '/the/atom/home' + } + + updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir'}) + expect(process.env).toEqual({ + PWD: '/the/dir', + ATOM_SUPPRESS_ENV_PATCHING: 'true', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', + ATOM_HOME: '/the/atom/home' + }) + + updateProcessEnv({PWD: '/the/dir'}) + expect(process.env).toEqual({ + ATOM_SUPPRESS_ENV_PATCHING: 'true', + PWD: '/the/dir', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', + ATOM_HOME: '/the/atom/home' + }) + }) }) describe('when the launch environment does not come from a shell', function () { @@ -119,7 +149,6 @@ describe('updateProcessEnv(launchEnv)', function () { it('updates process.env to match the environment in the user\'s login shell', function () { process.platform = 'linux' process.env.SHELL = '/my/custom/bash' - delete process.env.TERM spyOn(child_process, 'spawnSync').andReturn({ stdout: dedent` @@ -142,7 +171,7 @@ describe('updateProcessEnv(launchEnv)', function () { }) }) - describe('not on osx', function () { + describe('on windows', function () { it('does not update process.env', function () { process.platform = 'win32' spyOn(child_process, 'spawnSync') @@ -158,36 +187,15 @@ describe('updateProcessEnv(launchEnv)', function () { it('indicates when the environment should be fetched from the shell', function () { process.platform = 'darwin' expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) process.platform = 'linux' expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true) - expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) }) it('returns false when the shell should not be patched', function () { process.platform = 'darwin' - expect(shouldGetEnvFromShell({SHELL: '/bin/unsupported'})).toBe(false) - expect(shouldGetEnvFromShell({SHELL: '/bin/shh'})).toBe(false) - expect(shouldGetEnvFromShell({SHELL: '/bin/tcsh'})).toBe(false) - expect(shouldGetEnvFromShell({SHELL: '/usr/csh'})).toBe(false) - + expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false) process.platform = 'linux' - expect(shouldGetEnvFromShell({SHELL: '/bin/unsupported'})).toBe(false) - expect(shouldGetEnvFromShell({SHELL: '/bin/shh'})).toBe(false) - expect(shouldGetEnvFromShell({SHELL: '/bin/tcsh'})).toBe(false) - expect(shouldGetEnvFromShell({SHELL: '/usr/csh'})).toBe(false) + expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false) }) it('returns false when the shell is undefined or empty', function () { diff --git a/src/update-process-env.js b/src/update-process-env.js index 1b4bda58d..fbae1c983 100644 --- a/src/update-process-env.js +++ b/src/update-process-env.js @@ -6,14 +6,8 @@ import {spawnSync} from 'child_process' const ENVIRONMENT_VARIABLES_TO_PRESERVE = new Set([ 'NODE_ENV', 'NODE_PATH', - 'ATOM_HOME' -]) - -const SHELLS_KNOWN_TO_WORK = new Set([ - '/sh', - '/bash', - '/zsh', - '/fish' + 'ATOM_HOME', + 'ATOM_SUPPRESS_ENV_PATCHING' ]) const PLATFORMS_KNOWN_TO_WORK = new Set([ @@ -23,10 +17,9 @@ const PLATFORMS_KNOWN_TO_WORK = new Set([ function updateProcessEnv (launchEnv) { let envToAssign - if (launchEnv && shouldGetEnvFromShell(launchEnv)) { envToAssign = getEnvFromShell(launchEnv) - } else if (launchEnv && launchEnv.PWD) { // Launched from shell + } else if (launchEnv && launchEnv.PWD) { envToAssign = launchEnv } @@ -40,6 +33,10 @@ function updateProcessEnv (launchEnv) { for (let key in envToAssign) { if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) { process.env[key] = envToAssign[key] + } else { + if (!process.env[key] && envToAssign[key]) { + process.env[key] = envToAssign[key] + } } } @@ -50,29 +47,19 @@ function updateProcessEnv (launchEnv) { } function shouldGetEnvFromShell (env) { - if (!PLATFORMS_KNOWN_TO_WORK.has(process.platform)) { // Untested platforms + if (!PLATFORMS_KNOWN_TO_WORK.has(process.platform)) { return false } - if (!env || !env.SHELL || env.SHELL.trim() === '') { // Nothing to launch + if (!env || !env.SHELL || env.SHELL.trim() === '') { return false } - if (process.platform === 'linux' && env.TERM) { // Launched from shell + if (env.ATOM_SUPPRESS_ENV_PATCHING || process.env.ATOM_SUPPRESS_ENV_PATCHING) { return false } - if (process.platform === 'darwin' && env.PWD) { // Launched from shell - return false - } - - for (let s of SHELLS_KNOWN_TO_WORK) { - if (env.SHELL.endsWith(s)) { - return true - } - } - - return false + return true } function getEnvFromShell (env) { From b321ab6a38660780a235247972f523cc5a7cf0e3 Mon Sep 17 00:00:00 2001 From: Joe Fitzgerald Date: Wed, 31 Aug 2016 01:02:49 -0600 Subject: [PATCH 3/6] Prevent Regression For Common Shells - Fix lint warnings --- spec/update-process-env-spec.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/spec/update-process-env-spec.js b/spec/update-process-env-spec.js index ddc6f4ade..05c49f791 100644 --- a/spec/update-process-env-spec.js +++ b/spec/update-process-env-spec.js @@ -51,7 +51,7 @@ describe('updateProcessEnv(launchEnv)', function () { }) it('allows ATOM_HOME to be overwritten only if the new value is a valid path', function () { - newAtomHomePath = temp.mkdirSync('atom-home') + let newAtomHomePath = temp.mkdirSync('atom-home') process.env = { WILL_BE_DELETED: 'hi', @@ -89,8 +89,6 @@ describe('updateProcessEnv(launchEnv)', function () { }) it('allows ATOM_SUPPRESS_ENV_PATCHING to be preserved if set', function () { - newAtomHomePath = temp.mkdirSync('atom-home') - process.env = { WILL_BE_DELETED: 'hi', NODE_ENV: 'the-node-env', @@ -187,8 +185,22 @@ describe('updateProcessEnv(launchEnv)', function () { it('indicates when the environment should be fetched from the shell', function () { process.platform = 'darwin' expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) process.platform = 'linux' expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true) + expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) }) it('returns false when the shell should not be patched', function () { From 3c7a89ec933b3ff8d6f464c4c09a4d6d59464f28 Mon Sep 17 00:00:00 2001 From: Joe Fitzgerald Date: Thu, 8 Sep 2016 11:17:42 -0600 Subject: [PATCH 4/6] Add Spec To Validate Updating An Existing Env Var - :art: Update spec description to more accurately reflect the intent --- spec/update-process-env-spec.js | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/spec/update-process-env-spec.js b/spec/update-process-env-spec.js index 05c49f791..deb46ba4b 100644 --- a/spec/update-process-env-spec.js +++ b/spec/update-process-env-spec.js @@ -96,16 +96,16 @@ describe('updateProcessEnv(launchEnv)', function () { ATOM_HOME: '/the/atom/home' } - updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir'}) + updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home'}) expect(process.env).toEqual({ - PWD: '/the/dir', ATOM_SUPPRESS_ENV_PATCHING: 'true', + PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - updateProcessEnv({PWD: '/the/dir'}) + updateProcessEnv({PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home'}) expect(process.env).toEqual({ ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', @@ -114,6 +114,30 @@ describe('updateProcessEnv(launchEnv)', function () { ATOM_HOME: '/the/atom/home' }) }) + + it('allows an existing env variable to be updated', function () { + process.env = { + WILL_BE_UPDATED: 'old-value', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', + ATOM_HOME: '/the/atom/home' + } + + updateProcessEnv(process.env) + expect(process.env).toEqual(process.env) + + let updatedEnv = { + ATOM_SUPPRESS_ENV_PATCHING: 'true', + WILL_BE_UPDATED: 'new-value', + NODE_ENV: 'the-node-env', + NODE_PATH: '/the/node/path', + ATOM_HOME: '/the/atom/home', + PWD: '/the/dir' + } + + updateProcessEnv(updatedEnv) + expect(process.env).toEqual(updatedEnv) + }) }) describe('when the launch environment does not come from a shell', function () { @@ -203,7 +227,7 @@ describe('updateProcessEnv(launchEnv)', function () { expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true) }) - it('returns false when the shell should not be patched', function () { + it('returns false when the environment indicates that Atom was launched from a shell', function () { process.platform = 'darwin' expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false) process.platform = 'linux' From 36291f6a8f7431d11743d91f71e968ca587d3698 Mon Sep 17 00:00:00 2001 From: Joe Fitzgerald Date: Thu, 8 Sep 2016 11:18:28 -0600 Subject: [PATCH 5/6] Combine Check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The additional (!process.env[key] && envToAssign[key]) check allows “preserved” variables to be set for the first time if they are currently unset --- src/update-process-env.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/update-process-env.js b/src/update-process-env.js index fbae1c983..a2f1715f3 100644 --- a/src/update-process-env.js +++ b/src/update-process-env.js @@ -31,12 +31,8 @@ function updateProcessEnv (launchEnv) { } for (let key in envToAssign) { - if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) { + if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key) || (!process.env[key] && envToAssign[key])) { process.env[key] = envToAssign[key] - } else { - if (!process.env[key] && envToAssign[key]) { - process.env[key] = envToAssign[key] - } } } From d4d05244b313358c20717a7db07ec2085b5c39e3 Mon Sep 17 00:00:00 2001 From: Joe Fitzgerald Date: Thu, 8 Sep 2016 12:24:33 -0600 Subject: [PATCH 6/6] ATOM_SUPPRESS_ENV_PATCHING > ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT --- atom.sh | 2 +- spec/update-process-env-spec.js | 30 +++++++++++++++--------------- src/update-process-env.js | 4 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/atom.sh b/atom.sh index 3c1b65e9b..0655e343e 100755 --- a/atom.sh +++ b/atom.sh @@ -15,7 +15,7 @@ else BETA_VERSION= fi -export ATOM_SUPPRESS_ENV_PATCHING=true +export ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT=true while getopts ":wtfvh-:" opt; do case "$opt" in diff --git a/spec/update-process-env-spec.js b/spec/update-process-env-spec.js index deb46ba4b..8c9db2b16 100644 --- a/spec/update-process-env-spec.js +++ b/spec/update-process-env-spec.js @@ -32,9 +32,9 @@ describe('updateProcessEnv(launchEnv)', function () { const initialProcessEnv = process.env - updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'}) + updateProcessEnv({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'}) expect(process.env).toEqual({ - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', @@ -60,27 +60,27 @@ describe('updateProcessEnv(launchEnv)', function () { ATOM_HOME: '/the/atom/home' } - updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir'}) + updateProcessEnv({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir'}) expect(process.env).toEqual({ PWD: '/the/dir', - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')}) + updateProcessEnv({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')}) expect(process.env).toEqual({ - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home' }) - updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', ATOM_HOME: newAtomHomePath}) + updateProcessEnv({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', ATOM_HOME: newAtomHomePath}) expect(process.env).toEqual({ - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', @@ -88,7 +88,7 @@ describe('updateProcessEnv(launchEnv)', function () { }) }) - it('allows ATOM_SUPPRESS_ENV_PATCHING to be preserved if set', function () { + it('allows ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT to be preserved if set', function () { process.env = { WILL_BE_DELETED: 'hi', NODE_ENV: 'the-node-env', @@ -96,9 +96,9 @@ describe('updateProcessEnv(launchEnv)', function () { ATOM_HOME: '/the/atom/home' } - updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home'}) + updateProcessEnv({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home'}) expect(process.env).toEqual({ - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', @@ -107,7 +107,7 @@ describe('updateProcessEnv(launchEnv)', function () { updateProcessEnv({PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', ATOM_HOME: '/the/atom/home'}) expect(process.env).toEqual({ - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', PWD: '/the/dir', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', @@ -127,7 +127,7 @@ describe('updateProcessEnv(launchEnv)', function () { expect(process.env).toEqual(process.env) let updatedEnv = { - ATOM_SUPPRESS_ENV_PATCHING: 'true', + ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', WILL_BE_UPDATED: 'new-value', NODE_ENV: 'the-node-env', NODE_PATH: '/the/node/path', @@ -229,9 +229,9 @@ describe('updateProcessEnv(launchEnv)', function () { it('returns false when the environment indicates that Atom was launched from a shell', function () { process.platform = 'darwin' - expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false) + expect(shouldGetEnvFromShell({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', SHELL: '/bin/sh'})).toBe(false) process.platform = 'linux' - expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false) + expect(shouldGetEnvFromShell({ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT: 'true', SHELL: '/bin/sh'})).toBe(false) }) it('returns false when the shell is undefined or empty', function () { diff --git a/src/update-process-env.js b/src/update-process-env.js index a2f1715f3..6544a6612 100644 --- a/src/update-process-env.js +++ b/src/update-process-env.js @@ -7,7 +7,7 @@ const ENVIRONMENT_VARIABLES_TO_PRESERVE = new Set([ 'NODE_ENV', 'NODE_PATH', 'ATOM_HOME', - 'ATOM_SUPPRESS_ENV_PATCHING' + 'ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT' ]) const PLATFORMS_KNOWN_TO_WORK = new Set([ @@ -51,7 +51,7 @@ function shouldGetEnvFromShell (env) { return false } - if (env.ATOM_SUPPRESS_ENV_PATCHING || process.env.ATOM_SUPPRESS_ENV_PATCHING) { + if (env.ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT || process.env.ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT) { return false }