Skip to content

Commit

Permalink
fix: use execFileSync when using a cmd with a path (#178)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ckerr authored Sep 11, 2020
1 parent 4e66e75 commit f696621
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 17 deletions.
14 changes: 6 additions & 8 deletions src/e-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
};
Expand Down
7 changes: 0 additions & 7 deletions src/utils/depot-tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -103,6 +97,5 @@ module.exports = {
path: DEPOT_TOOLS_DIR,
ensure: ensureDepotTools,
execFileSync: depotExecFileSync,
execSync: depotExecSync,
spawnSync: depotSpawnSync,
};
3 changes: 2 additions & 1 deletion src/utils/refresh-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit f696621

Please sign in to comment.