Skip to content

Commit d8d3c58

Browse files
elibarzilaymprobst
authored andcommitted
scripts/build/utils simplify exec
Using shell-based execution is always a bad idea; this thing didn't do that via an option, but instead did it manually by constructing a shell command so it suffers from the same diseases. Perhaps there was need for this at some point in the past, but things are pretty robust now, so there's no need to avoid running the command normally. The only thing that is needed is to add `which` which also handles windows executable suffixes. I tried this with a fresh clone on windows, where the tree and TS are installed in paths that have spaces, and everything works as it should.
1 parent 71a1416 commit d8d3c58

File tree

1 file changed

+3
-15
lines changed

1 file changed

+3
-15
lines changed

scripts/build/utils.js

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@ const del = require("del");
99
const File = require("vinyl");
1010
const ts = require("../../lib/typescript");
1111
const chalk = require("chalk");
12+
const which = require("which");
1213
const { spawn } = require("child_process");
1314
const { CancellationToken, CancelError, Deferred } = require("prex");
1415
const { Readable, Duplex } = require("stream");
1516

16-
const isWindows = /^win/.test(process.platform);
17-
1817
/**
1918
* Executes the provided command once with the supplied arguments.
2019
* @param {string} cmd
@@ -32,12 +31,8 @@ function exec(cmd, args, options = {}) {
3231
const { ignoreExitCode, cancelToken = CancellationToken.none, waitForExit = true } = options;
3332
cancelToken.throwIfCancellationRequested();
3433

35-
// TODO (weswig): Update child_process types to add windowsVerbatimArguments to the type definition
36-
const subshellFlag = isWindows ? "/c" : "-c";
37-
const command = isWindows ? [possiblyQuote(cmd), ...args] : [`${cmd} ${args.join(" ")}`];
38-
3934
if (!options.hidePrompt) log(`> ${chalk.green(cmd)} ${args.join(" ")}`);
40-
const proc = spawn(isWindows ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: waitForExit ? "inherit" : "ignore", windowsVerbatimArguments: true });
35+
const proc = spawn(which.sync(cmd), args, { stdio: waitForExit ? "inherit" : "ignore" });
4136
const registration = cancelToken.register(() => {
4237
log(`${chalk.red("killing")} '${chalk.green(cmd)} ${args.join(" ")}'...`);
4338
proc.kill("SIGINT");
@@ -68,13 +63,6 @@ function exec(cmd, args, options = {}) {
6863
}
6964
exports.exec = exec;
7065

71-
/**
72-
* @param {string} cmd
73-
*/
74-
function possiblyQuote(cmd) {
75-
return cmd.indexOf(" ") >= 0 ? `"${cmd}"` : cmd;
76-
}
77-
7866
/**
7967
* @param {ts.Diagnostic[]} diagnostics
8068
* @param {{ cwd?: string, pretty?: boolean }} [options]
@@ -440,4 +428,4 @@ class Debouncer {
440428
}
441429
}
442430
}
443-
exports.Debouncer = Debouncer;
431+
exports.Debouncer = Debouncer;

0 commit comments

Comments
 (0)