From 7c3f607032d0317d4809f56783b9801646dddd16 Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 16 Oct 2021 02:33:59 -0400 Subject: [PATCH] `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. --- scripts/build/utils.js | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/scripts/build/utils.js b/scripts/build/utils.js index 6664a2fa67a..cfb7924d623 100644 --- a/scripts/build/utils.js +++ b/scripts/build/utils.js @@ -9,12 +9,11 @@ const del = require("del"); const File = require("vinyl"); const ts = require("../../lib/typescript"); const chalk = require("chalk"); +const which = require("which"); const { spawn } = require("child_process"); const { CancellationToken, CancelError, Deferred } = require("prex"); const { Readable, Duplex } = require("stream"); -const isWindows = /^win/.test(process.platform); - /** * Executes the provided command once with the supplied arguments. * @param {string} cmd @@ -32,12 +31,8 @@ function exec(cmd, args, options = {}) { const { ignoreExitCode, cancelToken = CancellationToken.none, waitForExit = true } = options; cancelToken.throwIfCancellationRequested(); - // TODO (weswig): Update child_process types to add windowsVerbatimArguments to the type definition - const subshellFlag = isWindows ? "/c" : "-c"; - const command = isWindows ? [possiblyQuote(cmd), ...args] : [`${cmd} ${args.join(" ")}`]; - if (!options.hidePrompt) log(`> ${chalk.green(cmd)} ${args.join(" ")}`); - const proc = spawn(isWindows ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: waitForExit ? "inherit" : "ignore", windowsVerbatimArguments: true }); + const proc = spawn(which.sync(cmd), args, { stdio: waitForExit ? "inherit" : "ignore" }); const registration = cancelToken.register(() => { log(`${chalk.red("killing")} '${chalk.green(cmd)} ${args.join(" ")}'...`); proc.kill("SIGINT"); @@ -68,13 +63,6 @@ function exec(cmd, args, options = {}) { } exports.exec = exec; -/** - * @param {string} cmd - */ -function possiblyQuote(cmd) { - return cmd.indexOf(" ") >= 0 ? `"${cmd}"` : cmd; -} - /** * @param {ts.Diagnostic[]} diagnostics * @param {{ cwd?: string, pretty?: boolean }} [options] @@ -440,4 +428,4 @@ class Debouncer { } } } -exports.Debouncer = Debouncer; \ No newline at end of file +exports.Debouncer = Debouncer;