Skip to content

Commit

Permalink
Fix extendEnv option not working with shell option (#184)
Browse files Browse the repository at this point in the history
Fixes #183 

Options parsing was not correct for the `extendEnv` option which made it not work when combined with `shell: true` option.
  • Loading branch information
ehmicky authored and sindresorhus committed Mar 6, 2019
1 parent 1fd9db4 commit e897f44
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 24 deletions.
41 changes: 18 additions & 23 deletions index.js
Expand Up @@ -15,27 +15,31 @@ const stdio = require('./lib/stdio');
const TEN_MEGABYTES = 1000 * 1000 * 10;

function handleArgs(command, args, options) {
options = Object.assign({
extendEnv: true,
env: {}
}, options);

if (options.extendEnv) {
options.env = Object.assign({}, process.env, options.env);
}

const parsed = crossSpawn._parse(command, args, options);
command = parsed.command;
args = parsed.args;
options = parsed.options;

options = Object.assign({
maxBuffer: TEN_MEGABYTES,
buffer: true,
stripFinalNewline: true,
preferLocal: true,
localDir: parsed.options.cwd || process.cwd(),
localDir: options.cwd || process.cwd(),
encoding: 'utf8',
reject: true,
cleanup: true
}, parsed.options, {windowsHide: true});
}, options, {
windowsHide: true
});

if (options.extendEnv !== false) {
options.env = Object.assign({}, process.env, options.env);
}

if (options.preferLocal) {
options.env = npmRunPath.env(Object.assign({}, options, {cwd: options.localDir}));
}

// TODO: Remove in the next major release
if (options.stripEof === false) {
Expand All @@ -44,26 +48,17 @@ function handleArgs(command, args, options) {

options.stdio = stdio(options);

if (options.preferLocal) {
options.env = npmRunPath.env(Object.assign({}, options, {cwd: options.localDir}));
}

if (options.detached) {
// #115
options.cleanup = false;
}

if (process.platform === 'win32' && path.basename(parsed.command) === 'cmd.exe') {
if (process.platform === 'win32' && path.basename(command) === 'cmd.exe') {
// #116
parsed.args.unshift('/q');
args.unshift('/q');
}

return {
command: parsed.command,
args: parsed.args,
options,
parsed
};
return {command, args, options, parsed};
}

function handleInput(spawned, input) {
Expand Down
10 changes: 9 additions & 1 deletion test.js
Expand Up @@ -487,7 +487,7 @@ test('extend environment variables by default', async t => {
]);
});

test('do not extend environment with `extendEnv` option', async t => {
test('do not extend environment with `extendEnv: false`', async t => {
const result = await m.stdout('environment', [], {env: {BAR: 'bar', PATH: process.env.PATH}, extendEnv: false});

t.deepEqual(result.split('\n'), [
Expand All @@ -496,6 +496,14 @@ test('do not extend environment with `extendEnv` option', async t => {
]);
});

test('use extend environment with `extendEnv: true` and `shell: true`', async t => {
process.env.TEST = 'test';
const command = process.platform === 'win32' ? 'echo %TEST%' : 'echo $TEST';
const stdout = await m.stdout(command, {shell: true, env: {}, extendEnv: true});
t.is(stdout, 'test');
delete process.env.TEST;
});

test('do not buffer when streaming', async t => {
const result = await getStream(m('max-buffer', ['stdout', '21'], {maxBuffer: 10}).stdout);

Expand Down

0 comments on commit e897f44

Please sign in to comment.