Skip to content

Commit 06ee3c6

Browse files
committed
fix: fix paths being incorrectly normalized on unix
This actually fixes another bug with `options.cwd` on Windows. Closes #90.
1 parent 334d705 commit 06ee3c6

File tree

3 files changed

+95
-7
lines changed

3 files changed

+95
-7
lines changed

lib/parse.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ function parseNonShell(parsed) {
4949
// we need to double escape them
5050
const needsDoubleEscapeMetaChars = isCmdShimRegExp.test(commandFile);
5151

52+
// Normalize posix paths into OS compatible paths (e.g.: foo/bar -> foo\bar)
53+
// This is necessary otherwise it will always fail with ENOENT in those cases
54+
parsed.command = path.normalize(parsed.command);
55+
5256
// Escape command & arguments
5357
parsed.command = escape.command(parsed.command);
5458
parsed.args = parsed.args.map((arg) => escape.argument(arg, needsDoubleEscapeMetaChars));
@@ -104,7 +108,7 @@ function parse(command, args, options) {
104108

105109
// Build our parsed object
106110
const parsed = {
107-
command: path.normalize(command),
111+
command,
108112
args,
109113
options,
110114
file: undefined,

lib/util/resolveCommand.js

+30-5
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,40 @@ const path = require('path');
44
const which = require('which');
55
const pathKey = require('path-key')();
66

7-
function resolveCommandAttempt(parsed, withPathExt) {
8-
withPathExt = !!withPathExt;
7+
function resolveCommandAttempt(parsed, withoutPathExt) {
8+
const cwd = process.cwd();
9+
const hasCustomCwd = parsed.options.cwd != null;
10+
11+
// If a custom `cwd` was specified, we need to change the process cwd
12+
// because `which` will do stat calls but does not support a custom cwd
13+
if (hasCustomCwd) {
14+
try {
15+
process.chdir(parsed.options.cwd);
16+
} catch (err) {
17+
/* Empty */
18+
}
19+
}
20+
21+
let resolved;
922

1023
try {
11-
return which.sync(parsed.command, {
24+
resolved = which.sync(parsed.command, {
1225
path: (parsed.options.env || process.env)[pathKey],
13-
pathExt: withPathExt && process.env.PATHEXT ? path.delimiter + process.env.PATHEXT : undefined,
26+
pathExt: withoutPathExt ? path.delimiter : undefined,
1427
});
15-
} catch (e) { /* Empty */ }
28+
} catch (e) {
29+
/* Empty */
30+
} finally {
31+
process.chdir(cwd);
32+
}
33+
34+
// If we successfully resolved, ensure that an absolute path is returned
35+
// Note that when a custom `cwd` was used, we need to resolve to an absolute path based on it
36+
if (resolved) {
37+
resolved = path.resolve(hasCustomCwd ? parsed.options.cwd : '', resolved);
38+
}
39+
40+
return resolved;
1641
}
1742

1843
function resolveCommand(parsed) {

test/index.test.js

+60-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ run.methods.forEach((method) => {
3232
expect(stdout.trim()).toBe('foo');
3333
});
3434

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

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

222+
it('should work with a relative posix path to a command with a custom `cwd`', async () => {
223+
const relativeTestPath = path.relative(process.cwd(), __dirname).replace(/\\/, '/');
224+
225+
const { stdout: stdout1 } = await run(method, 'fixtures/say-foo', { cwd: relativeTestPath });
226+
227+
expect(stdout1.trim()).toBe('foo');
228+
229+
const { stdout: stdout2 } = await run(method, './fixtures/say-foo', { cwd: `./${relativeTestPath}` });
230+
231+
expect(stdout2.trim()).toBe('foo');
232+
233+
if (!isWin) {
234+
return;
235+
}
236+
237+
const { stdout: stdout3 } = await run(method, './fixtures/say-foo.bat', { cwd: `./${relativeTestPath}` });
238+
239+
expect(stdout3.trim()).toBe('foo');
240+
});
241+
222242
{
223243
const assertError = (err) => {
224244
const syscall = run.isMethodSync(method) ? 'spawnSync' : 'spawn';
@@ -354,6 +374,45 @@ run.methods.forEach((method) => {
354374
});
355375
}
356376

377+
if (run.isMethodSync(method)) {
378+
it('should fail with ENOENT a non-existing `cwd` was specified', () => {
379+
expect.assertions(1);
380+
381+
try {
382+
run(method, 'fixtures/say-foo', { cwd: 'somedirthatwillneverexist' });
383+
} catch (err) {
384+
expect(err.code).toBe('ENOENT');
385+
}
386+
});
387+
} else {
388+
it('should emit `error` and `close` if a non-existing `cwd` was specified', async () => {
389+
expect.assertions(3);
390+
391+
await new Promise((resolve, reject) => {
392+
const promise = run(method, 'somecommandthatwillneverexist', ['foo']);
393+
const { cp } = promise;
394+
395+
promise.catch(() => {});
396+
397+
let timeout;
398+
399+
cp
400+
.on('error', (err) => expect(err.code).toBe('ENOENT'))
401+
.on('exit', () => {
402+
cp.removeAllListeners();
403+
clearTimeout(timeout);
404+
reject(new Error('Should not emit exit'));
405+
})
406+
.on('close', (code, signal) => {
407+
expect(code).not.toBe(0);
408+
expect(signal).toBe(null);
409+
410+
timeout = setTimeout(resolve, 1000);
411+
});
412+
});
413+
});
414+
}
415+
357416
if (isWin) {
358417
it('should use nodejs\' spawn when options.shell is specified (windows)', async () => {
359418
const { stdout } = await run(method, 'echo', ['%RANDOM%'], { shell: true });

0 commit comments

Comments
 (0)