From c27ea979deabd943066d369af44b73ff99673bdc Mon Sep 17 00:00:00 2001 From: ehmicky Date: Sun, 4 Feb 2024 13:46:37 +0000 Subject: [PATCH] Improve `cwd` option --- index.d.ts | 6 +- index.js | 23 +++----- lib/cwd.js | 49 +++++++++++++++++ lib/error.js | 7 ++- readme.md | 6 +- test/cwd.js | 103 +++++++++++++++++++++++++++++++++++ test/error.js | 22 ++------ test/helpers/fixtures-dir.js | 6 +- test/kill.js | 10 ++-- test/test.js | 42 +++++--------- 10 files changed, 199 insertions(+), 75 deletions(-) create mode 100644 lib/cwd.js create mode 100644 test/cwd.js diff --git a/index.d.ts b/index.d.ts index 840b1a44a7..da29b873f0 100644 --- a/index.d.ts +++ b/index.d.ts @@ -280,8 +280,6 @@ type CommonOptions = { /** Path to the Node.js executable to use in child processes. - This can be either an absolute path or a path relative to the `cwd` option. - Requires `preferLocal` to be `true`. For example, this can be used together with [`get-node`](https://github.com/ehmicky/get-node) to run a specific Node.js version in a child process. @@ -412,6 +410,8 @@ type CommonOptions = { /** Current working directory of the child process. + This is also used to resolve the `execPath` option when it is a relative path. + @default process.cwd() */ readonly cwd?: string | URL; @@ -711,7 +711,7 @@ type ExecaCommonReturnValue return env; }; -const normalizeFileUrl = file => file instanceof URL ? fileURLToPath(file) : file; - -const getFilePath = rawFile => { - const fileString = normalizeFileUrl(rawFile); - - if (typeof fileString !== 'string') { - throw new TypeError('First argument must be a string or a file URL.'); - } - - return fileString; -}; +const getFilePath = rawFile => safeNormalizeFileUrl(rawFile, 'First argument'); const handleArguments = (rawFile, rawArgs, rawOptions = {}) => { const filePath = getFilePath(rawFile); @@ -56,8 +46,9 @@ const handleArguments = (rawFile, rawArgs, rawOptions = {}) => { options.shell = normalizeFileUrl(options.shell); options.env = getEnv(options); options.forceKillAfterDelay = normalizeForceKillAfterDelay(options.forceKillAfterDelay); + options.cwd = normalizeCwd(options.cwd); - if (process.platform === 'win32' && path.basename(file, '.exe') === 'cmd') { + if (process.platform === 'win32' && basename(file, '.exe') === 'cmd') { // #116 args.unshift('/q'); } @@ -73,7 +64,7 @@ const addDefaultOptions = ({ stripFinalNewline = true, extendEnv = true, preferLocal = false, - cwd = process.cwd(), + cwd = getDefaultCwd(), localDir = cwd, execPath = process.execPath, encoding = 'utf8', @@ -210,6 +201,7 @@ const handlePromise = async ({spawned, options, stdioStreamsGroups, originalStre return { command, escapedCommand, + cwd: options.cwd, failed: false, timedOut: false, isCanceled: false, @@ -277,6 +269,7 @@ export function execaSync(rawFile, rawArgs, rawOptions) { return { command, escapedCommand, + cwd: options.cwd, failed: false, timedOut: false, isCanceled: false, diff --git a/lib/cwd.js b/lib/cwd.js new file mode 100644 index 0000000000..5d45356e3e --- /dev/null +++ b/lib/cwd.js @@ -0,0 +1,49 @@ +import {statSync} from 'node:fs'; +import {resolve} from 'node:path'; +import {fileURLToPath} from 'node:url'; +import process from 'node:process'; + +export const getDefaultCwd = () => { + try { + return process.cwd(); + } catch (error) { + error.message = `The current directory does not exist.\n${error.message}`; + throw error; + } +}; + +export const normalizeCwd = cwd => { + const cwdString = safeNormalizeFileUrl(cwd, 'The "cwd" option'); + return resolve(cwdString); +}; + +export const safeNormalizeFileUrl = (file, name) => { + const fileString = normalizeFileUrl(file); + + if (typeof fileString !== 'string') { + throw new TypeError(`${name} must be a string or a file URL: ${fileString}.`); + } + + return fileString; +}; + +export const normalizeFileUrl = file => file instanceof URL ? fileURLToPath(file) : file; + +export const fixCwdError = (originalMessage, cwd) => { + if (cwd === getDefaultCwd()) { + return originalMessage; + } + + let cwdStat; + try { + cwdStat = statSync(cwd); + } catch (error) { + return `The "cwd" option is invalid: ${cwd}.\n${error.message}\n${originalMessage}`; + } + + if (!cwdStat.isDirectory()) { + return `The "cwd" option is not a directory: ${cwd}.\n${originalMessage}`; + } + + return originalMessage; +}; diff --git a/lib/error.js b/lib/error.js index 7ff4446756..9eeb625c88 100644 --- a/lib/error.js +++ b/lib/error.js @@ -1,7 +1,7 @@ -import process from 'node:process'; import {signalsByName} from 'human-signals'; import stripFinalNewline from 'strip-final-newline'; import {isBinary, binaryToString} from './stdio/utils.js'; +import {fixCwdError} from './cwd.js'; const getErrorPrefix = ({timedOut, timeout, errorCode, signal, signalDescription, exitCode, isCanceled}) => { if (timedOut) { @@ -53,7 +53,7 @@ export const makeError = ({ escapedCommand, timedOut, isCanceled, - options: {timeoutDuration: timeout, cwd = process.cwd()}, + options: {timeoutDuration: timeout, cwd}, }) => { // `signal` and `exitCode` emitted on `spawned.on('exit')` event can be `null`. // We normalize them to `undefined` @@ -64,7 +64,8 @@ export const makeError = ({ const errorCode = error?.code; const prefix = getErrorPrefix({timedOut, timeout, errorCode, signal, signalDescription, exitCode, isCanceled}); const execaMessage = `Command ${prefix}: ${command}`; - const originalMessage = previousErrors.has(error) ? error.originalMessage : String(error?.message ?? error); + const originalErrorMessage = previousErrors.has(error) ? error.originalMessage : String(error?.message ?? error); + const originalMessage = fixCwdError(originalErrorMessage, cwd); const shortMessage = error === undefined ? execaMessage : `${execaMessage}\n${originalMessage}`; const messageStdio = all === undefined ? [stdio[2], stdio[1]] : [all]; const message = [shortMessage, ...messageStdio, ...stdio.slice(3)] diff --git a/readme.md b/readme.md index d60828e0ba..dd4d606e83 100644 --- a/readme.md +++ b/readme.md @@ -366,7 +366,7 @@ Since the escaping is fairly basic, this should not be executed directly as a pr Type: `string` -The `cwd` of the command if provided in the [command options](#cwd-1). Otherwise it is `process.cwd()`. +The [current directory](#cwd-1) in which the command was run. #### stdout @@ -510,6 +510,8 @@ Default: `process.cwd()` Current working directory of the child process. +This is also used to resolve the [`execPath`](#execpath) option when it is a relative path. + #### env Type: `object`\ @@ -549,8 +551,6 @@ Default: `process.execPath` (Current Node.js executable) Path to the Node.js executable to use in child processes. -This can be either an absolute path or a path relative to the [`cwd` option](#cwd). - Requires [`preferLocal`](#preferlocal) to be `true`. For example, this can be used together with [`get-node`](https://github.com/ehmicky/get-node) to run a specific Node.js version in a child process. diff --git a/test/cwd.js b/test/cwd.js new file mode 100644 index 0000000000..2ddb0ed4cb --- /dev/null +++ b/test/cwd.js @@ -0,0 +1,103 @@ +import {mkdir, rmdir} from 'node:fs/promises'; +import {relative, toNamespacedPath} from 'node:path'; +import process from 'node:process'; +import {pathToFileURL, fileURLToPath} from 'node:url'; +import tempfile from 'tempfile'; +import test from 'ava'; +import {execa, execaSync} from '../index.js'; +import {FIXTURES_DIR, setFixtureDir} from './helpers/fixtures-dir.js'; + +setFixtureDir(); + +const isWindows = process.platform === 'win32'; + +const testOptionCwdString = async (t, execaMethod) => { + const cwd = '/'; + const {stdout} = await execaMethod('node', ['-p', 'process.cwd()'], {cwd}); + t.is(toNamespacedPath(stdout), toNamespacedPath(cwd)); +}; + +test('The "cwd" option can be a string', testOptionCwdString, execa); +test('The "cwd" option can be a string - sync', testOptionCwdString, execaSync); + +const testOptionCwdUrl = async (t, execaMethod) => { + const cwd = '/'; + const cwdUrl = pathToFileURL(cwd); + const {stdout} = await execaMethod('node', ['-p', 'process.cwd()'], {cwd: cwdUrl}); + t.is(toNamespacedPath(stdout), toNamespacedPath(cwd)); +}; + +test('The "cwd" option can be a URL', testOptionCwdUrl, execa); +test('The "cwd" option can be a URL - sync', testOptionCwdUrl, execaSync); + +const testOptionCwdInvalid = (t, execaMethod) => { + t.throws(() => { + execaMethod('empty.js', {cwd: true}); + }, {message: /The "cwd" option must be a string or a file URL: true/}); +}; + +test('The "cwd" option cannot be an invalid type', testOptionCwdInvalid, execa); +test('The "cwd" option cannot be an invalid type - sync', testOptionCwdInvalid, execaSync); + +const testErrorCwdDefault = async (t, execaMethod) => { + const {cwd} = await execaMethod('empty.js'); + t.is(cwd, process.cwd()); +}; + +test('The "cwd" option defaults to process.cwd()', testErrorCwdDefault, execa); +test('The "cwd" option defaults to process.cwd() - sync', testErrorCwdDefault, execaSync); + +// Windows does not allow removing a directory used as `cwd` of a running process +if (!isWindows) { + const testCwdPreSpawn = async (t, execaMethod) => { + const currentCwd = process.cwd(); + const filePath = tempfile(); + await mkdir(filePath); + process.chdir(filePath); + await rmdir(filePath); + + try { + t.throws(() => { + execaMethod('empty.js'); + }, {message: /The current directory does not exist/}); + } finally { + process.chdir(currentCwd); + } + }; + + test.serial('The "cwd" option default fails if current cwd is missing', testCwdPreSpawn, execa); + test.serial('The "cwd" option default fails if current cwd is missing - sync', testCwdPreSpawn, execaSync); +} + +const cwdNotExisting = {cwd: 'does_not_exist', expectedCode: 'ENOENT', expectedMessage: 'The "cwd" option is invalid'}; +const cwdTooLong = {cwd: '.'.repeat(1e5), expectedCode: 'ENAMETOOLONG', expectedMessage: 'The "cwd" option is invalid'}; +const cwdNotDir = {cwd: fileURLToPath(import.meta.url), expectedCode: isWindows ? 'ENOENT' : 'ENOTDIR', expectedMessage: 'The "cwd" option is not a directory'}; + +const testCwdPostSpawn = async (t, {cwd, expectedCode, expectedMessage}, execaMethod) => { + const {failed, code, message} = await execaMethod('empty.js', {cwd, reject: false}); + t.true(failed); + t.is(code, expectedCode); + t.true(message.includes(expectedMessage)); + t.true(message.includes(cwd)); +}; + +test('The "cwd" option must be an existing file', testCwdPostSpawn, cwdNotExisting, execa); +test('The "cwd" option must be an existing file - sync', testCwdPostSpawn, cwdNotExisting, execaSync); +test('The "cwd" option must not be too long', testCwdPostSpawn, cwdTooLong, execa); +test('The "cwd" option must not be too long - sync', testCwdPostSpawn, cwdTooLong, execaSync); +test('The "cwd" option must be a directory', testCwdPostSpawn, cwdNotDir, execa); +test('The "cwd" option must be a directory - sync', testCwdPostSpawn, cwdNotDir, execaSync); + +const successProperties = {fixtureName: 'empty.js', expectedFailed: false}; +const errorProperties = {fixtureName: 'fail.js', expectedFailed: true}; + +const testErrorCwd = async (t, execaMethod, {fixtureName, expectedFailed}) => { + const {failed, cwd} = await execaMethod(fixtureName, {cwd: relative('.', FIXTURES_DIR), reject: false}); + t.is(failed, expectedFailed); + t.is(cwd, FIXTURES_DIR); +}; + +test('result.cwd is defined', testErrorCwd, execa, successProperties); +test('result.cwd is defined - sync', testErrorCwd, execaSync, successProperties); +test('error.cwd is defined', testErrorCwd, execa, errorProperties); +test('error.cwd is defined - sync', testErrorCwd, execaSync, errorProperties); diff --git a/test/error.js b/test/error.js index 63501dbdc0..50b84dc6f8 100644 --- a/test/error.js +++ b/test/error.js @@ -1,7 +1,7 @@ import process from 'node:process'; import test from 'ava'; import {execa, execaSync} from '../index.js'; -import {FIXTURES_DIR, setFixtureDir} from './helpers/fixtures-dir.js'; +import {setFixtureDir} from './helpers/fixtures-dir.js'; import {fullStdio, getStdio} from './helpers/stdio.js'; import {noopGenerator, outputObjectGenerator} from './helpers/generator.js'; import {foobarString} from './helpers/input.js'; @@ -17,6 +17,7 @@ test('Return value properties are not missing and are ordered', async t => { t.deepEqual(Reflect.ownKeys(result), [ 'command', 'escapedCommand', + 'cwd', 'failed', 'timedOut', 'isCanceled', @@ -185,8 +186,8 @@ test('error.message newlines are consistent - no newline', testErrorMessageConsi test('error.message newlines are consistent - newline', testErrorMessageConsistent, 'stdout\n'); test('Original error.message is kept', async t => { - const {originalMessage} = await t.throwsAsync(execa('noop.js', {cwd: 1})); - t.true(originalMessage.startsWith('The "options.cwd" property must be of type string or an instance of Buffer or URL. Received type number')); + const {originalMessage} = await t.throwsAsync(execa('noop.js', {uid: true})); + t.is(originalMessage, 'The "options.uid" property must be int32. Received type boolean (true)'); }); test('failed is false on success', async t => { @@ -320,21 +321,10 @@ test('error.code is undefined on success', async t => { }); test('error.code is defined on failure if applicable', async t => { - const {code} = await t.throwsAsync(execa('noop.js', {cwd: 1})); + const {code} = await t.throwsAsync(execa('noop.js', {uid: true})); t.is(code, 'ERR_INVALID_ARG_TYPE'); }); -test('error.cwd is defined on failure if applicable', async t => { - const {cwd} = await t.throwsAsync(execa('fail.js', [], {cwd: FIXTURES_DIR})); - t.is(cwd, FIXTURES_DIR); -}); - -test('error.cwd is undefined on failure if not passed as options', async t => { - const expectedCwd = process.cwd(); - const {cwd} = await t.throwsAsync(execa('fail.js')); - t.is(cwd, expectedCwd); -}); - const testUnusualError = async (t, error) => { const childProcess = execa('empty.js'); childProcess.emit('error', error); @@ -367,7 +357,7 @@ test('error instance can be a plain object', async t => { test('error instance can be shared', async t => { const originalMessage = foobarString; const error = new Error(originalMessage); - const fixtureName = 'noop.js'; + const fixtureName = 'empty.js'; const firstArgument = 'one'; const childProcess = execa(fixtureName, [firstArgument]); diff --git a/test/helpers/fixtures-dir.js b/test/helpers/fixtures-dir.js index 0942490829..e8d35a2a12 100644 --- a/test/helpers/fixtures-dir.js +++ b/test/helpers/fixtures-dir.js @@ -1,15 +1,15 @@ -import path from 'node:path'; +import {delimiter, resolve} from 'node:path'; import process from 'node:process'; import {fileURLToPath} from 'node:url'; import pathKey from 'path-key'; export const PATH_KEY = pathKey(); export const FIXTURES_DIR_URL = new URL('../fixtures/', import.meta.url); -export const FIXTURES_DIR = fileURLToPath(FIXTURES_DIR_URL); +export const FIXTURES_DIR = resolve(fileURLToPath(FIXTURES_DIR_URL)); // Add the fixtures directory to PATH so fixtures can be executed without adding // `node`. This is only meant to make writing tests simpler. export const setFixtureDir = () => { - process.env[PATH_KEY] = FIXTURES_DIR + path.delimiter + process.env[PATH_KEY]; + process.env[PATH_KEY] = FIXTURES_DIR + delimiter + process.env[PATH_KEY]; }; diff --git a/test/kill.js b/test/kill.js index 0260dbe74e..9990ad302a 100644 --- a/test/kill.js +++ b/test/kill.js @@ -10,6 +10,7 @@ import {setFixtureDir} from './helpers/fixtures-dir.js'; setFixtureDir(); +const isWindows = process.platform === 'win32'; const TIMEOUT_REGEXP = /timed out after/; const spawnNoKillable = async (forceKillAfterDelay, options) => { @@ -47,7 +48,7 @@ test('`forceKillAfterDelay` should not be negative', testInvalidForceKill, -1); // `SIGTERM` cannot be caught on Windows, and it always aborts the process (like `SIGKILL` on Unix). // Therefore, this feature and those tests must be different on Windows. -if (process.platform === 'win32') { +if (isWindows) { test('Can call `.kill()` with `forceKillAfterDelay` on Windows', async t => { const {subprocess} = await spawnNoKillable(1); subprocess.kill(); @@ -289,11 +290,10 @@ const pollForProcessExit = async pid => { // - on Linux subprocesses are never killed regardless of `options.detached` // With `options.cleanup`, subprocesses are always killed // - `options.cleanup` with SIGKILL is a noop, since it cannot be handled -const exitIfWindows = process.platform === 'win32'; -test('spawnAndKill SIGTERM', spawnAndKill, ['SIGTERM', false, false, exitIfWindows]); -test('spawnAndKill SIGKILL', spawnAndKill, ['SIGKILL', false, false, exitIfWindows]); +test('spawnAndKill SIGTERM', spawnAndKill, ['SIGTERM', false, false, isWindows]); +test('spawnAndKill SIGKILL', spawnAndKill, ['SIGKILL', false, false, isWindows]); test('spawnAndKill cleanup SIGTERM', spawnAndKill, ['SIGTERM', true, false, true]); -test('spawnAndKill cleanup SIGKILL', spawnAndKill, ['SIGKILL', true, false, exitIfWindows]); +test('spawnAndKill cleanup SIGKILL', spawnAndKill, ['SIGKILL', true, false, isWindows]); test('spawnAndKill detached SIGTERM', spawnAndKill, ['SIGTERM', false, true, false]); test('spawnAndKill detached SIGKILL', spawnAndKill, ['SIGKILL', false, true, false]); test('spawnAndKill cleanup detached SIGTERM', spawnAndKill, ['SIGTERM', true, true, false]); diff --git a/test/test.js b/test/test.js index ba47710fae..53dad33feb 100644 --- a/test/test.js +++ b/test/test.js @@ -1,4 +1,4 @@ -import path from 'node:path'; +import {delimiter, join, basename} from 'node:path'; import process from 'node:process'; import {fileURLToPath, pathToFileURL} from 'node:url'; import test from 'ava'; @@ -13,7 +13,8 @@ import {foobarString} from './helpers/input.js'; setFixtureDir(); process.env.FOO = 'foo'; -const ENOENT_REGEXP = process.platform === 'win32' ? /failed with exit code 1/ : /spawn.* ENOENT/; +const isWindows = process.platform === 'win32'; +const ENOENT_REGEXP = isWindows ? /failed with exit code 1/ : /spawn.* ENOENT/; const testOutput = async (t, index, execaMethod) => { const {stdout, stderr, stdio} = await execaMethod('noop-fd.js', [`${index}`, 'foobar'], fullStdio); @@ -46,7 +47,7 @@ test('cannot return input stdio[*]', async t => { t.is(stdio[3], undefined); }); -if (process.platform === 'win32') { +if (isWindows) { test('execa() - cmd file', async t => { const {stdout} = await execa('hello.cmd'); t.is(stdout, 'Hello World'); @@ -112,7 +113,7 @@ test('stripFinalNewline is not used in objectMode', async t => { }); const getPathWithoutLocalDir = () => { - const newPath = process.env[PATH_KEY].split(path.delimiter).filter(pathDir => !BIN_DIR_REGEXP.test(pathDir)).join(path.delimiter); + const newPath = process.env[PATH_KEY].split(delimiter).filter(pathDir => !BIN_DIR_REGEXP.test(pathDir)).join(delimiter); return {[PATH_KEY]: newPath}; }; @@ -139,9 +140,9 @@ test('preferLocal: undefined with $.sync', t => { }); test('localDir option', async t => { - const command = process.platform === 'win32' ? 'echo %PATH%' : 'echo $PATH'; + const command = isWindows ? 'echo %PATH%' : 'echo $PATH'; const {stdout} = await execa(command, {shell: true, preferLocal: true, localDir: '/test'}); - const envPaths = stdout.split(path.delimiter); + const envPaths = stdout.split(delimiter); t.true(envPaths.some(envPath => envPath.endsWith('.bin'))); }); @@ -194,12 +195,12 @@ test('do not try to consume streams twice', async t => { }); test('use relative path with \'..\' chars', async t => { - const pathViaParentDir = path.join('..', path.basename(fileURLToPath(new URL('..', import.meta.url))), 'test', 'fixtures', 'noop.js'); + const pathViaParentDir = join('..', basename(fileURLToPath(new URL('..', import.meta.url))), 'test', 'fixtures', 'noop.js'); const {stdout} = await execa(pathViaParentDir, ['foo']); t.is(stdout, 'foo'); }); -if (process.platform !== 'win32') { +if (!isWindows) { test('execa() rejects if running non-executable', async t => { await t.throwsAsync(execa('non-executable.js')); }); @@ -209,7 +210,7 @@ if (process.platform !== 'win32') { }); } -if (process.platform !== 'win32') { +if (!isWindows) { test('write to fast-exit process', async t => { // Try-catch here is necessary, because this test is not 100% accurate // Sometimes process can manage to accept input before exiting @@ -237,33 +238,20 @@ test('do not extend environment with `extendEnv: false`', async t => { t.deepEqual(stdout.split('\n'), ['undefined', 'bar']); }); -test('can use `options.cwd` as a string', async t => { - const cwd = '/'; - const {stdout} = await execa('node', ['-p', 'process.cwd()'], {cwd}); - t.is(path.toNamespacedPath(stdout), path.toNamespacedPath(cwd)); -}); - test('localDir option can be a URL', async t => { - const command = process.platform === 'win32' ? 'echo %PATH%' : 'echo $PATH'; + const command = isWindows ? 'echo %PATH%' : 'echo $PATH'; const {stdout} = await execa(command, {shell: true, preferLocal: true, localDir: pathToFileURL('/test')}); - const envPaths = stdout.split(path.delimiter); + const envPaths = stdout.split(delimiter); t.true(envPaths.some(envPath => envPath.endsWith('.bin'))); }); -test('can use `options.cwd` as a URL', async t => { - const cwd = '/'; - const cwdUrl = pathToFileURL(cwd); - const {stdout} = await execa('node', ['-p', 'process.cwd()'], {cwd: cwdUrl}); - t.is(path.toNamespacedPath(stdout), path.toNamespacedPath(cwd)); -}); - test('can use `options.shell: true`', async t => { const {stdout} = await execa('node test/fixtures/noop.js foo', {shell: true}); t.is(stdout, 'foo'); }); const testShellPath = async (t, mapPath) => { - const shellPath = process.platform === 'win32' ? 'cmd.exe' : 'bash'; + const shellPath = isWindows ? 'cmd.exe' : 'bash'; const shell = mapPath(await which(shellPath)); const {stdout} = await execa('node test/fixtures/noop.js foo', {shell}); t.is(stdout, 'foo'); @@ -274,7 +262,7 @@ test('can use `options.shell: file URL`', testShellPath, pathToFileURL); 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 command = isWindows ? 'echo %TEST%' : 'echo $TEST'; const {stdout} = await execa(command, {shell: true, env: {}, extendEnv: true}); t.is(stdout, 'test'); delete process.env.TEST; @@ -313,7 +301,7 @@ test('execaNode()\'s command argument cannot be a non-file URL', testInvalidFile const testInvalidCommand = async (t, execaMethod) => { t.throws(() => { execaMethod(['command', 'arg']); - }, {message: /must be a string or a file URL/}); + }, {message: /First argument must be a string or a file URL/}); }; test('execa()\'s command argument must be a string or file URL', testInvalidCommand, execa);