From e61b7b2e4661f7c95780009114a0f20573d19f55 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Fri, 11 Sep 2020 13:36:13 -0500
Subject: [PATCH] fix: use execFileSync when using a cmd with a path

Previously execSync was used, taking the full command line as a single
string. This would be misparsed if the path had spaces in it. For example
"C:\Users\My Name\e\third_party\depot_tools\gn" would see "C:\Users\My"
as the command.

NB: the new code in e-build.js is cleaner, but it wasn't clear (to me,
anyway) that the changed handling of escapes would work. FWIW I tested
this change on both Win and POSIX.
---
 src/e-build.js            | 14 ++++++--------
 src/utils/arm.js          |  2 +-
 src/utils/depot-tools.js  |  7 -------
 src/utils/refresh-path.js |  3 ++-
 4 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/e-build.js b/src/e-build.js
index 7539c7a7..1254201d 100644
--- a/src/e-build.js
+++ b/src/e-build.js
@@ -12,14 +12,12 @@ const goma = require('./utils/goma');
 
 function runGNGen(config) {
   depot.ensure();
-
-  const gnExec = os.platform() === 'win32' ? 'gn.bat' : 'gn';
-  const gn_args = config.gen.args.join(' ').replace(/\"/g, '\\"'); // gn parses this part -- inner quotes must be escaped
-  const exec = `${path.resolve(depot.path, gnExec)} gen "out/${
-    config.gen.out
-  }" --args="${gn_args}"`;
-  const opts = { cwd: path.resolve(config.root, 'src') };
-  depot.execSync(config, exec, opts);
+  const gnBasename = os.platform() === 'win32' ? 'gn.bat' : 'gn';
+  const gnPath = path.resolve(depot.path, gnBasename);
+  const gnArgs = config.gen.args.join(' ');
+  const execArgs = ['gen', `out/${config.gen.out}`, `--args=${gnArgs}`];
+  const execOpts = { cwd: path.resolve(config.root, 'src') };
+  depot.execFileSync(config, gnPath, execArgs, execOpts);
 }
 
 function ensureGNGen(config) {
diff --git a/src/utils/arm.js b/src/utils/arm.js
index dcd0966e..1e5c0318 100644
--- a/src/utils/arm.js
+++ b/src/utils/arm.js
@@ -2,7 +2,7 @@ const cp = require('child_process');
 const path = require('path');
 
 const getIsArm = () => {
-  const output = cp.execSync(`uname -m`);
+  const output = cp.execSync('uname -m');
 
   return output.includes('arm');
 };
diff --git a/src/utils/depot-tools.js b/src/utils/depot-tools.js
index e9d7505b..8b0ec398 100644
--- a/src/utils/depot-tools.js
+++ b/src/utils/depot-tools.js
@@ -74,12 +74,6 @@ function depotOpts(config, opts = {}) {
   return opts;
 }
 
-function depotExecSync(config, cmd, opts_in) {
-  const opts = depotOpts(config, opts_in);
-  console.log(color.childExec(cmd, null, opts));
-  childProcess.execSync(cmd, opts);
-}
-
 function depotSpawnSync(config, cmd, args, opts_in) {
   const opts = depotOpts(config, opts_in);
   if (opts_in.msg) {
@@ -103,6 +97,5 @@ module.exports = {
   path: DEPOT_TOOLS_DIR,
   ensure: ensureDepotTools,
   execFileSync: depotExecFileSync,
-  execSync: depotExecSync,
   spawnSync: depotSpawnSync,
 };
diff --git a/src/utils/refresh-path.js b/src/utils/refresh-path.js
index 8b0e184d..a5cb6b5c 100644
--- a/src/utils/refresh-path.js
+++ b/src/utils/refresh-path.js
@@ -3,7 +3,8 @@ const path = require('path');
 
 const refreshPathVariable = () => {
   if (process.platform === 'win32') {
-    const output = cp.execSync(path.resolve(__dirname, 'get-path.bat'));
+    const file = path.resolve(__dirname, 'get-path.bat');
+    const output = cp.execFileSync(file);
     const pathOut = output.toString();
     process.env.PATH = pathOut;
   }