FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

child_process: simplify spawn argument parsing · nodejs/node@4f9cd27 · GitHub

/ node Public

Commit 4f9cd27

Browse files
cjihrig
committed
child_process: simplify spawn argument parsing
This commit simplifies the object returned by normalizeSpawnArguments(). This does impact monkey patching, as illustrated by the changes in tests. PR-URL: #27854 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 476c3ec commit 4f9cd27

5 files changed

Lines changed: 38 additions & 57 deletions

File tree

‎lib/child_process.js‎

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -460,16 +460,14 @@ function normalizeSpawnArguments(file, args, options) {
460460
}
461461

462462
// Validate windowsVerbatimArguments, if present.
463-
if (options.windowsVerbatimArguments != null &&
464-
typeof options.windowsVerbatimArguments !== 'boolean') {
463+
let { windowsVerbatimArguments } = options;
464+
if (windowsVerbatimArguments != null &&
465+
typeof windowsVerbatimArguments !== 'boolean') {
465466
throw new ERR_INVALID_ARG_TYPE('options.windowsVerbatimArguments',
466467
'boolean',
467-
options.windowsVerbatimArguments);
468+
windowsVerbatimArguments);
468469
}
469470

470-
// Make a shallow copy so we don't clobber the user's options object.
471-
options = { ...options };
472-
473471
if (options.shell) {
474472
const command = [file].concat(args).join(' ');
475473
// Set the shell, switches, and commands.
@@ -481,7 +479,7 @@ function normalizeSpawnArguments(file, args, options) {
481479
// '/d /s /c' is used only for cmd.exe.
482480
if (/^(?:.*\\)?cmd(?:\.exe)?$/i.test(file)) {
483481
args = ['/d', '/s', '/c', `"${command}"`];
484-
options.windowsVerbatimArguments = true;
482+
windowsVerbatimArguments = true;
485483
} else {
486484
args = ['-c', command];
487485
}
@@ -521,58 +519,42 @@ function normalizeSpawnArguments(file, args, options) {
521519
}
522520

523521
return {
524-
file: file,
525-
args: args,
526-
options: options,
527-
envPairs: envPairs
522+
// Make a shallow copy so we don't clobber the user's options object.
523+
...options,
524+
args,
525+
detached: !!options.detached,
526+
envPairs,
527+
file,
528+
windowsHide: !!options.windowsHide,
529+
windowsVerbatimArguments: !!windowsVerbatimArguments
528530
};
529531
}
530532

531533

532534
var spawn = exports.spawn = function spawn(file, args, options) {
533-
const opts = normalizeSpawnArguments(file, args, options);
534535
const child = new ChildProcess();
535536

536-
options = opts.options;
537-
debug('spawn', opts.args, options);
538-
539-
child.spawn({
540-
file: opts.file,
541-
args: opts.args,
542-
cwd: options.cwd,
543-
windowsHide: !!options.windowsHide,
544-
windowsVerbatimArguments: !!options.windowsVerbatimArguments,
545-
detached: !!options.detached,
546-
envPairs: opts.envPairs,
547-
stdio: options.stdio,
548-
uid: options.uid,
549-
gid: options.gid
550-
});
537+
options = normalizeSpawnArguments(file, args, options);
538+
debug('spawn', options);
539+
child.spawn(options);
551540

552541
return child;
553542
};
554543

555544
function spawnSync(file, args, options) {
556-
const opts = normalizeSpawnArguments(file, args, options);
557-
558-
const defaults = {
545+
options = {
559546
maxBuffer: MAX_BUFFER,
560-
...opts.options
547+
...normalizeSpawnArguments(file, args, options)
561548
};
562-
options = opts.options = defaults;
563549

564-
debug('spawnSync', opts.args, options);
550+
debug('spawnSync', options);
565551

566552
// Validate the timeout, if present.
567553
validateTimeout(options.timeout);
568554

569555
// Validate maxBuffer, if present.
570556
validateMaxBuffer(options.maxBuffer);
571557

572-
options.file = opts.file;
573-
options.args = opts.args;
574-
options.envPairs = opts.envPairs;
575-
576558
// Validate and translate the kill signal, if present.
577559
options.killSignal = sanitizeKillSignal(options.killSignal);
578560

@@ -603,7 +585,7 @@ function spawnSync(file, args, options) {
603585
}
604586
}
605587

606-
return child_process.spawnSync(opts);
588+
return child_process.spawnSync(options);
607589
}
608590
exports.spawnSync = spawnSync;
609591

@@ -628,15 +610,15 @@ function checkExecSyncError(ret, args, cmd) {
628610

629611

630612
function execFileSync(command, args, options) {
631-
const opts = normalizeSpawnArguments(command, args, options);
632-
const inheritStderr = !opts.options.stdio;
613+
options = normalizeSpawnArguments(command, args, options);
633614

634-
const ret = spawnSync(opts.file, opts.args.slice(1), opts.options);
615+
const inheritStderr = !options.stdio;
616+
const ret = spawnSync(options.file, options.args.slice(1), options);
635617

636618
if (inheritStderr && ret.stderr)
637619
process.stderr.write(ret.stderr);
638620

639-
const err = checkExecSyncError(ret, opts.args, undefined);
621+
const err = checkExecSyncError(ret, options.args, undefined);
640622

641623
if (err)
642624
throw err;

‎lib/internal/child_process.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,8 +1022,7 @@ function maybeClose(subprocess) {
10221022
}
10231023
}
10241024

1025-
function spawnSync(opts) {
1026-
const options = opts.options;
1025+
function spawnSync(options) {
10271026
const result = spawn_sync.spawn(options);
10281027

10291028
if (result.output && options.encoding && options.encoding !== 'buffer') {
@@ -1038,9 +1037,9 @@ function spawnSync(opts) {
10381037
result.stderr = result.output && result.output[2];
10391038

10401039
if (result.error) {
1041-
result.error = errnoException(result.error, 'spawnSync ' + opts.file);
1042-
result.error.path = opts.file;
1043-
result.error.spawnargs = opts.args.slice(1);
1040+
result.error = errnoException(result.error, 'spawnSync ' + options.file);
1041+
result.error.path = options.file;
1042+
result.error.spawnargs = options.args.slice(1);
10441043
}
10451044

10461045
return result;

‎test/parallel/test-child-process-spawnsync-kill-signal.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ if (process.argv[2] === 'child') {
3636
// Verify that the default kill signal is SIGTERM.
3737
{
3838
const child = spawn(undefined, (opts) => {
39-
assert.strictEqual(opts.options.killSignal, undefined);
39+
assert.strictEqual(opts.killSignal, undefined);
4040
});
4141

4242
assert.strictEqual(child.signal, 'SIGTERM');
@@ -45,7 +45,7 @@ if (process.argv[2] === 'child') {
4545
// Verify that a string signal name is handled properly.
4646
{
4747
const child = spawn('SIGKILL', (opts) => {
48-
assert.strictEqual(opts.options.killSignal, SIGKILL);
48+
assert.strictEqual(opts.killSignal, SIGKILL);
4949
});
5050

5151
assert.strictEqual(child.signal, 'SIGKILL');
@@ -56,7 +56,7 @@ if (process.argv[2] === 'child') {
5656
assert.strictEqual(typeof SIGKILL, 'number');
5757

5858
const child = spawn(SIGKILL, (opts) => {
59-
assert.strictEqual(opts.options.killSignal, SIGKILL);
59+
assert.strictEqual(opts.killSignal, SIGKILL);
6060
});
6161

6262
assert.strictEqual(child.signal, 'SIGKILL');

‎test/parallel/test-child-process-spawnsync-shell.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,12 @@ assert.strictEqual(env.stdout.toString().trim(), 'buzz');
6464
assert.strictEqual(opts.file, shellOutput);
6565
assert.deepStrictEqual(opts.args,
6666
[shellOutput, ...shellFlags, outputCmd]);
67-
assert.strictEqual(opts.options.shell, shell);
68-
assert.strictEqual(opts.options.file, opts.file);
69-
assert.deepStrictEqual(opts.options.args, opts.args);
70-
assert.strictEqual(opts.options.windowsHide, undefined);
71-
assert.strictEqual(opts.options.windowsVerbatimArguments,
72-
windowsVerbatim);
67+
assert.strictEqual(opts.shell, shell);
68+
assert.strictEqual(opts.file, opts.file);
69+
assert.deepStrictEqual(opts.args, opts.args);
70+
assert.strictEqual(opts.windowsHide, false);
71+
assert.strictEqual(opts.windowsVerbatimArguments,
72+
!!windowsVerbatim);
7373
});
7474
cp.spawnSync(cmd, { shell });
7575
internalCp.spawnSync = oldSpawnSync;

‎test/parallel/test-child-process-windows-hide.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internalCp.ChildProcess.prototype.spawn = common.mustCall(function(options) {
1919
}, 2);
2020

2121
internalCp.spawnSync = common.mustCall(function(options) {
22-
assert.strictEqual(options.options.windowsHide, true);
22+
assert.strictEqual(options.windowsHide, true);
2323
return originalSpawnSync.apply(this, arguments);
2424
});
2525

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL