Skip to content

Commit

Permalink
fix: fix paths being incorrectly normalized on unix
Browse files Browse the repository at this point in the history
This actually fixes another bug with `options.cwd` on Windows.

Closes #90.
  • Loading branch information
satazor committed Jan 31, 2018
1 parent 334d705 commit 06ee3c6
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 7 deletions.
6 changes: 5 additions & 1 deletion lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ function parseNonShell(parsed) {
// we need to double escape them
const needsDoubleEscapeMetaChars = isCmdShimRegExp.test(commandFile);

// Normalize posix paths into OS compatible paths (e.g.: foo/bar -> foo\bar)
// This is necessary otherwise it will always fail with ENOENT in those cases
parsed.command = path.normalize(parsed.command);

// Escape command & arguments
parsed.command = escape.command(parsed.command);
parsed.args = parsed.args.map((arg) => escape.argument(arg, needsDoubleEscapeMetaChars));
Expand Down Expand Up @@ -104,7 +108,7 @@ function parse(command, args, options) {

// Build our parsed object
const parsed = {
command: path.normalize(command),
command,
args,
options,
file: undefined,
Expand Down
35 changes: 30 additions & 5 deletions lib/util/resolveCommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,40 @@ const path = require('path');
const which = require('which');
const pathKey = require('path-key')();

function resolveCommandAttempt(parsed, withPathExt) {
withPathExt = !!withPathExt;
function resolveCommandAttempt(parsed, withoutPathExt) {
const cwd = process.cwd();
const hasCustomCwd = parsed.options.cwd != null;

// If a custom `cwd` was specified, we need to change the process cwd
// because `which` will do stat calls but does not support a custom cwd
if (hasCustomCwd) {
try {
process.chdir(parsed.options.cwd);
} catch (err) {
/* Empty */
}
}

let resolved;

try {
return which.sync(parsed.command, {
resolved = which.sync(parsed.command, {
path: (parsed.options.env || process.env)[pathKey],
pathExt: withPathExt && process.env.PATHEXT ? path.delimiter + process.env.PATHEXT : undefined,
pathExt: withoutPathExt ? path.delimiter : undefined,
});
} catch (e) { /* Empty */ }
} catch (e) {
/* Empty */
} finally {
process.chdir(cwd);
}

// If we successfully resolved, ensure that an absolute path is returned
// Note that when a custom `cwd` was used, we need to resolve to an absolute path based on it
if (resolved) {
resolved = path.resolve(hasCustomCwd ? parsed.options.cwd : '', resolved);
}

return resolved;
}

function resolveCommand(parsed) {
Expand Down
61 changes: 60 additions & 1 deletion test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ run.methods.forEach((method) => {
expect(stdout.trim()).toBe('foo');
});

it('should support shebang in executables with /usr/bin/env', async () => {
it('should support shebang in executables with `/usr/bin/env`', async () => {
const { stdout: stdout1 } = await run(method, `${__dirname}/fixtures/shebang`);

expect(stdout1).toBe('shebang works!');
Expand Down Expand Up @@ -219,6 +219,26 @@ run.methods.forEach((method) => {
expect(stdout3.trim()).toBe('foo');
});

it('should work with a relative posix path to a command with a custom `cwd`', async () => {
const relativeTestPath = path.relative(process.cwd(), __dirname).replace(/\\/, '/');

const { stdout: stdout1 } = await run(method, 'fixtures/say-foo', { cwd: relativeTestPath });

expect(stdout1.trim()).toBe('foo');

const { stdout: stdout2 } = await run(method, './fixtures/say-foo', { cwd: `./${relativeTestPath}` });

expect(stdout2.trim()).toBe('foo');

if (!isWin) {
return;
}

const { stdout: stdout3 } = await run(method, './fixtures/say-foo.bat', { cwd: `./${relativeTestPath}` });

expect(stdout3.trim()).toBe('foo');
});

{
const assertError = (err) => {
const syscall = run.isMethodSync(method) ? 'spawnSync' : 'spawn';
Expand Down Expand Up @@ -354,6 +374,45 @@ run.methods.forEach((method) => {
});
}

if (run.isMethodSync(method)) {
it('should fail with ENOENT a non-existing `cwd` was specified', () => {
expect.assertions(1);

try {
run(method, 'fixtures/say-foo', { cwd: 'somedirthatwillneverexist' });
} catch (err) {
expect(err.code).toBe('ENOENT');
}
});
} else {
it('should emit `error` and `close` if a non-existing `cwd` was specified', async () => {
expect.assertions(3);

await new Promise((resolve, reject) => {
const promise = run(method, 'somecommandthatwillneverexist', ['foo']);
const { cp } = promise;

promise.catch(() => {});

let timeout;

cp
.on('error', (err) => expect(err.code).toBe('ENOENT'))
.on('exit', () => {
cp.removeAllListeners();
clearTimeout(timeout);
reject(new Error('Should not emit exit'));
})
.on('close', (code, signal) => {
expect(code).not.toBe(0);
expect(signal).toBe(null);

timeout = setTimeout(resolve, 1000);
});
});
});
}

if (isWin) {
it('should use nodejs\' spawn when options.shell is specified (windows)', async () => {
const { stdout } = await run(method, 'echo', ['%RANDOM%'], { shell: true });
Expand Down

0 comments on commit 06ee3c6

Please sign in to comment.