Skip to content

Commit

Permalink
Revert "refactor(exec): remove paramsFile (#807)" (#819)
Browse files Browse the repository at this point in the history
This reverts commit 64d5899.

Reason for revert: If stdin is large, then the param object can become
an extremely long string, exceeding the maximum OS size limit on
commandline parameters.

Original change's description:
> refactor(exec): remove paramsFile (#807)
>
> The `paramsFile` is obsolete now that we use `execFileSync()` for our
> internal implementation. Instead, we pass parameters to the child
> process directly as a single commandline parameter to reduce file I/O.
>
> Issue #782

Fixes #818
  • Loading branch information
nfischer committed Jan 20, 2018
1 parent 902e49c commit cb9cf27
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/exec-child.js
Expand Up @@ -5,7 +5,10 @@ if (require.main !== module) {
var childProcess = require('child_process');
var fs = require('fs');

var params = JSON.parse(process.argv[2]);
var paramFilePath = process.argv[2];

var serializedParams = fs.readFileSync(paramFilePath, 'utf8');
var params = JSON.parse(serializedParams);

var cmd = params.command;
var execOptions = params.execOptions;
Expand Down
7 changes: 6 additions & 1 deletion src/exec.js
Expand Up @@ -21,6 +21,7 @@ function execSync(cmd, opts, pipe) {
}

var tempDir = _tempDir();
var paramsFile = path.resolve(tempDir + '/' + common.randomFileName());
var stderrFile = path.resolve(tempDir + '/' + common.randomFileName());
var stdoutFile = path.resolve(tempDir + '/' + common.randomFileName());

Expand All @@ -32,6 +33,7 @@ function execSync(cmd, opts, pipe) {
encoding: 'utf8',
}, opts);

if (fs.existsSync(paramsFile)) common.unlinkSync(paramsFile);
if (fs.existsSync(stderrFile)) common.unlinkSync(stderrFile);
if (fs.existsSync(stdoutFile)) common.unlinkSync(stdoutFile);

Expand All @@ -45,9 +47,11 @@ function execSync(cmd, opts, pipe) {
stderrFile: stderrFile,
};

fs.writeFileSync(paramsFile, JSON.stringify(paramsToSerialize), 'utf8');

var execArgs = [
path.join(__dirname, 'exec-child.js'),
JSON.stringify(paramsToSerialize),
paramsFile,
];

/* istanbul ignore else */
Expand Down Expand Up @@ -84,6 +88,7 @@ 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(paramsFile); } catch (e) {}
try { common.unlinkSync(stderrFile); } catch (e) {}
try { common.unlinkSync(stdoutFile); } catch (e) {}

Expand Down

0 comments on commit cb9cf27

Please sign in to comment.