Skip to content

Commit

Permalink
Remove codeFile parameter (#791)
Browse files Browse the repository at this point in the history
This parameter isn't needed, we can easily rely on exit code status for this.

Eliminating the parameter reduces file IO, code complexity, and removes a busy
loop.

This also removes some legacy code related to streams.

Issue #782
  • Loading branch information
nfischer committed Oct 31, 2017
1 parent 8451fce commit 6189d7f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 31 deletions.
14 changes: 5 additions & 9 deletions src/exec-child.js
Expand Up @@ -15,29 +15,25 @@ var execOptions = params.execOptions;
var pipe = params.pipe;
var stdoutFile = params.stdoutFile;
var stderrFile = params.stderrFile;
var codeFile = params.codeFile;

var c = childProcess.exec(cmd, execOptions, function (err) {
if (!err) {
fs.writeFileSync(codeFile, '0');
process.exitCode = 0;
} else if (err.code === undefined) {
fs.writeFileSync(codeFile, '1');
process.exitCode = 1;
} else {
fs.writeFileSync(codeFile, err.code.toString());
process.exitCode = err.code;
}
});

var stdoutStream = fs.createWriteStream(stdoutFile);
var stderrStream = fs.createWriteStream(stderrFile);

c.stdout.pipe(stdoutStream, { end: false });
c.stderr.pipe(stderrStream, { end: false });
c.stdout.pipe(stdoutStream);
c.stderr.pipe(stderrStream);
c.stdout.pipe(process.stdout);
c.stderr.pipe(process.stderr);

if (pipe) {
c.stdin.end(pipe);
}

c.stdout.on('end', stdoutStream.end);
c.stderr.on('end', stderrStream.end);
28 changes: 6 additions & 22 deletions src/exec.js
Expand Up @@ -13,18 +13,14 @@ common.register('exec', _exec, {
wrapOutput: false,
});

// Hack to run child_process.exec() synchronously (sync avoids callback hell)
// Uses a custom wait loop that checks for a flag file, created when the child process is done.
// (Can't do a wait loop that checks for internal Node variables/messages as
// Node is single-threaded; callbacks and other internal state changes are done in the
// event loop).
// We use this function to run exec synchronously while also providing realtime
// output.
function execSync(cmd, opts, pipe) {
if (!common.config.execPath) {
common.error('Unable to find a path to the node binary. Please manually set config.execPath');
}

var tempDir = _tempDir();
var codeFile = path.resolve(tempDir + '/' + common.randomFileName());
var paramsFile = path.resolve(tempDir + '/' + common.randomFileName());
var stderrFile = path.resolve(tempDir + '/' + common.randomFileName());
var stdoutFile = path.resolve(tempDir + '/' + common.randomFileName());
Expand All @@ -37,7 +33,6 @@ function execSync(cmd, opts, pipe) {
encoding: 'utf8',
}, opts);

if (fs.existsSync(codeFile)) common.unlinkSync(codeFile);
if (fs.existsSync(paramsFile)) common.unlinkSync(paramsFile);
if (fs.existsSync(stderrFile)) common.unlinkSync(stderrFile);
if (fs.existsSync(stdoutFile)) common.unlinkSync(stdoutFile);
Expand All @@ -50,7 +45,6 @@ function execSync(cmd, opts, pipe) {
pipe: pipe,
stdoutFile: stdoutFile,
stderrFile: stderrFile,
codeFile: codeFile,
};

fs.writeFileSync(paramsFile, JSON.stringify(paramsToSerialize), 'utf8');
Expand All @@ -67,6 +61,8 @@ function execSync(cmd, opts, pipe) {
opts.stdio = [0, 1, 2];
}

var code = 0;

// Welcome to the future
try {
// Bad things if we pass in a `shell` option to child_process.execFileSync,
Expand All @@ -75,19 +71,8 @@ function execSync(cmd, opts, pipe) {

child.execFileSync(common.config.execPath, execArgs, opts);
} catch (e) {
// Clean up immediately if we have an exception
try { common.unlinkSync(codeFile); } catch (e2) {}
try { common.unlinkSync(paramsFile); } catch (e2) {}
try { common.unlinkSync(stderrFile); } catch (e2) {}
try { common.unlinkSync(stdoutFile); } catch (e2) {}
throw e;
}

// At this point codeFile exists, but it's not necessarily flushed yet.
// Keep reading it until it is.
var code = parseInt('', 10);
while (isNaN(code)) {
code = parseInt(fs.readFileSync(codeFile, 'utf8'), 10);
// Commands with non-zero exit code raise an exception.
code = e.status;
}

// fs.readFileSync uses buffer encoding by default, so call
Expand All @@ -103,7 +88,6 @@ function execSync(cmd, opts, pipe) {
}

// No biggie if we can't erase the files now -- they're in a temp dir anyway
try { common.unlinkSync(codeFile); } catch (e) {}
try { common.unlinkSync(paramsFile); } catch (e) {}
try { common.unlinkSync(stderrFile); } catch (e) {}
try { common.unlinkSync(stdoutFile); } catch (e) {}
Expand Down

0 comments on commit 6189d7f

Please sign in to comment.