Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use execa for child_process calls #759

Merged
merged 24 commits into from
Apr 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4396d93
deps: add strong-log-transformer
Apr 11, 2017
4b15f49
deps: add p-finally (explicit)
Apr 12, 2017
55393c0
ChildProcessUtilities: use execa and strong-log-transformer
Apr 11, 2017
9bbc044
ConventionalCommitUtilities: execa compat
Apr 11, 2017
5a26e2b
FileSystemUtilities: execa compat, passing --no-glob and trailing sla…
Apr 11, 2017
81ee012
GitUtilities: execa compat
Apr 11, 2017
ce2cd47
NpmUtilities: execa compat
Apr 11, 2017
3f2a138
DiffCommand: execa compat
Apr 11, 2017
b9a9005
ExecCommand: execa compat (with getOpts refactoring)
Apr 11, 2017
51a34db
ImportCommand: execa compat
Apr 12, 2017
7680a8c
RunCommand: don't log stdout when an error was caught
Apr 12, 2017
3893a02
Command: --sort arg is boolean, --concurrency is already coerced in g…
Apr 12, 2017
9116534
bin/lerna: workaround non-interactive bug with yargs.terminalWidth()
Apr 12, 2017
a6183e1
bin/lerna: add -h/--help, -v/--version to global options
Apr 12, 2017
8e6f529
bin/lerna: move setLogLevel closer to yargs config
Apr 12, 2017
b751f7f
bin/lerna: remove signal-exit unload()
Apr 12, 2017
698e96b
ExecCommand: fix args, integration tests
Apr 12, 2017
71ad4a1
test/integration/lerna-bootstrap: update snapshots
Apr 12, 2017
a530af4
test/integration/lerna-import: tweak indentation
Apr 12, 2017
1c4ab4b
test/integration/lerna-run: update snapshots
Apr 12, 2017
d9bc9d4
test/integration/lerna-run: replace npm script with test --ignore
Apr 12, 2017
34febec
test/integration: unwrap some chained assertions
Apr 12, 2017
0ceb520
test/integration/lerna-run: ensure consistent order with --concurrency=1
Apr 13, 2017
069daca
test/integration/lerna-run: don't let npm emit 'Error: no test specif…
evocateur Apr 13, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions bin/lerna.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,35 @@ const logger = require("../lib/logger");
const yargs = require("yargs");
const path = require("path");

// the options grouped under "Global Options:" header
const globalKeys = Object.keys(globalOptions).concat([
"loglevel",
"help",
"version",
]);

// workaround non-interactive yargs.terminalWidth() error
// until https://github.com/yargs/yargs/pull/837 is released
function terminalWidth() {
return typeof process.stdout.columns !== "undefined" ? process.stdout.columns : null;
}

logger.setLogLevel(yargs.argv.loglevel);

yargs
.epilogue("For more information, find our manual at https://github.com/lerna/lerna")
.usage("$ lerna [command] [flags]")
.wrap(yargs.terminalWidth())
.usage("Usage: $0 <command> [options]")
.wrap(terminalWidth())
.option("loglevel", {
default: "info",
describe: "What level of logs to report. On failure, all logs are written to lerna-debug.log in the"
+ "current working directory.",
type: "string",
global: true
})
.options(globalOptions).group(Object.keys(globalOptions), "Global Options:")
.options(globalOptions).group(globalKeys, "Global Options:")
.commandDir(path.join(__dirname, "..", "lib", "commands"))
.demandCommand()
.help()
.help("h").alias("h", "help")
.version().alias("v", "version")
.argv;

require("signal-exit").unload();
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"load-json-file": "^2.0.0",
"lodash": "^4.17.4",
"minimatch": "^3.0.0",
"p-finally": "^1.0.0",
"path-exists": "^3.0.0",
"progress": "^2.0.0",
"read-cmd-shim": "^1.0.1",
Expand All @@ -56,6 +57,7 @@
"safe-buffer": "^5.0.1",
"semver": "^5.1.0",
"signal-exit": "^3.0.2",
"strong-log-transformer": "^1.0.6",
"temp-write": "^3.2.0",
"write-json-file": "^2.0.0",
"write-pkg": "^2.1.0",
Expand Down
154 changes: 49 additions & 105 deletions src/ChildProcessUtilities.js
Original file line number Diff line number Diff line change
@@ -1,113 +1,46 @@
import child from "child_process";
import spawn from "cross-spawn";
import { EventEmitter } from "events";
import execa from "execa";
import pFinally from "p-finally";
import logTransformer from "strong-log-transformer";

// Keep track of how many live children we have.
let children = 0;

// maxBuffer value for running exec
const MAX_BUFFER = 500 * 1024;

// This is used to alert listeners when all children have exited.
const emitter = new EventEmitter;
const emitter = new EventEmitter();

export default class ChildProcessUtilities {
static exec(command, opts, callback) {
const mergedOpts = Object.assign({
maxBuffer: MAX_BUFFER
}, opts);
return ChildProcessUtilities.registerChild(
child.exec(command, mergedOpts, (err, stdout, stderr) => {
if (err != null) {

// If the error from `child.exec` is just that the child process
// emitted too much on stderr, then that stderr output is likely to
// be useful.
if (/^stderr maxBuffer exceeded/.test(err.message)) {
err = `Error: ${err.message}. Partial output follows:\n\n${stderr}`;
}

callback(err || stderr, stdout);
} else {
callback(null, stdout);
}
})
);
}
static exec(command, args, opts, callback) {
const options = Object.assign({}, opts);
options.stdio = "pipe"; // node default

static execSync(command, opts) {
const mergedOpts = Object.assign({
encoding: "utf8",
maxBuffer: MAX_BUFFER
}, opts);

let stdout = child.execSync(command, mergedOpts);
if (stdout) {
// stdout is undefined when stdio[1] is anything other than "pipe"
// and there's no point trimming an empty string (no piped stdout)
stdout = stdout.trim();
}
return _spawn(command, args, options, callback);
}

return stdout;
static execSync(command, args, opts) {
return execa.sync(command, args, opts).stdout;
}

static spawn(command, args, opts, callback) {
let output = "";

const childProcess = _spawn(command, args, opts, (err) => callback(err, output));
const options = Object.assign({}, opts);
options.stdio = "inherit";

// By default stderr, stdout are inherited from us (just sent to _our_ output).
// If the caller overrode that to "pipe", then we'll gather that up and
// call back with it in case of failure.
if (childProcess.stderr) {
childProcess.stderr.setEncoding("utf8");
childProcess.stderr.on("data", (chunk) => output += chunk);
}

if (childProcess.stdout) {
childProcess.stdout.setEncoding("utf8");
childProcess.stdout.on("data", (chunk) => output += chunk);
}
return _spawn(command, args, options, callback);
}

static spawnStreaming(command, args, opts, prefix, callback) {
opts = Object.assign({}, opts, {
stdio: ["ignore", "pipe", "pipe"],
});

const childProcess = _spawn(command, args, opts, callback);

["stdout", "stderr"].forEach((stream) => {
let partialLine = "";
childProcess[stream].setEncoding("utf8")
.on("data", (chunk) => {
const lines = chunk.split("\n");
lines[0] = partialLine + lines[0];
partialLine = lines.pop();
lines.forEach((line) => process[stream].write(prefix + line + "\n"));
})
.on("end", () => {
if (partialLine) {

// If the child process ended its output with no final newline we
// need to flush that out. We'll add a newline ourselves so we
// don't end up with output from multiple children on the same
// line.
process[stream].write(prefix + partialLine + "\n");
}
});
});
}
const options = Object.assign({}, opts);
options.stdio = ["ignore", "pipe", "pipe"];

const spawned = _spawn(command, args, options, callback);

const prefixedStdout = logTransformer({ tag: `${prefix}:` });
const prefixedStderr = logTransformer({ tag: `${prefix} ERROR`, mergeMultiline: true });

static registerChild(child) {
children++;
child.on("exit", () => {
children--;
if (children === 0) {
emitter.emit("empty");
}
});
return child;
spawned.stdout.pipe(prefixedStdout).pipe(process.stdout);
spawned.stderr.pipe(prefixedStderr).pipe(process.stderr);

return spawned;
}

static getChildProcessCount() {
Expand All @@ -119,18 +52,29 @@ export default class ChildProcessUtilities {
}
}

function registerChild(child) {
children++;

pFinally(child, () => {
children--;

if (children === 0) {
emitter.emit("empty");
}
});
}

function _spawn(command, args, opts, callback) {
return ChildProcessUtilities.registerChild(
spawn(command, args, Object.assign({
stdio: "inherit"
}, opts))
.on("error", () => {})
.on("close", (code) => {
if (code) {
callback(`Command exited with status ${code}: ${command} ${args.join(" ")}`);
} else {
callback(null);
}
})
);
const child = execa(command, args, opts);

registerChild(child);

if (callback) {
child.then(
(result) => callback(null, result.stdout),
(err) => callback(err)
);
}

return child;
}
2 changes: 1 addition & 1 deletion src/Command.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export const builder = {
type: "number",
requiresArg: true,
default: DEFAULT_CONCURRENCY,
coerce: (val) => Math.max(1, val)
},
"ignore": {
describe: "Ignores packages with names matching the given glob (Works only in combination with the "
Expand All @@ -41,6 +40,7 @@ export const builder = {
},
"sort": {
describe: "Sort packages topologically",
type: "boolean",
default: true
}
};
Expand Down
19 changes: 6 additions & 13 deletions src/ConventionalCommitUtilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,21 @@ const CHANGELOG_HEADER = dedent(`# Change Log
All notable changes to this project will be documented in this file.
See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.`);

const RECOMMEND_CLI = require.resolve("conventional-recommended-bump/cli.js");
const CHANGELOG_CLI = require.resolve("conventional-changelog-cli/cli.js");

export default class ConventionalCommitUtilities {
@logger.logifySync()
static recommendVersion(pkg, opts) {
const name = pkg.name;
const version = pkg.version;
const pkgLocation = pkg.location;

const recommendedBump = ChildProcessUtilities.execSync(
`${RECOMMEND_CLI} -l ${name} --commit-path=${pkgLocation} -p angular`,
"conventional-recommended-bump",
["-l", pkg.name, "--commit-path", pkg.location, "-p", "angular"],
opts
);

return semver.inc(version, recommendedBump);
return semver.inc(pkg.version, recommendedBump);
}

@logger.logifySync()
static updateChangelog(pkg, opts) {
const name = pkg.name;
const pkgLocation = pkg.location;
const pkgJsonLocation = path.join(pkgLocation, "package.json");
const pkgJsonLocation = path.join(pkg.location, "package.json");
const changelogLocation = ConventionalCommitUtilities.changelogLocation(pkg);

let changelogContents = "";
Expand All @@ -44,7 +36,8 @@ export default class ConventionalCommitUtilities {
// run conventional-changelog-cli to generate the markdown
// for the upcoming release.
const newEntry = ChildProcessUtilities.execSync(
`${CHANGELOG_CLI} -l ${name} --commit-path=${pkgLocation} --pkg=${pkgJsonLocation} -p angular`,
"conventional-changelog",
["-l", pkg.name, "--commit-path", pkg.location, "--pkg", pkgJsonLocation, "-p", "angular"],
opts
);

Expand Down
15 changes: 9 additions & 6 deletions src/FileSystemUtilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ function ensureEndsWithNewLine(string) {
return ENDS_WITH_NEW_LINE.test(string) ? string : string + "\n";
}

// globs only return directories with a trailing slash
function trailingSlash(filePath) {
return path.normalize(`${filePath}/`);
}

export default class FileSystemUtilities {
@logger.logifyAsync()
static mkdirp(filePath, callback) {
Expand Down Expand Up @@ -59,16 +64,14 @@ export default class FileSystemUtilities {
}

@logger.logifyAsync()
static rimraf(filePath, callback) {

static rimraf(dirPath, callback) {
// Shelling out to a child process for a noop is expensive.
// Checking if `filePath` exists to be removed is cheap.
// Checking if `dirPath` exists to be removed is cheap.
// This lets us short-circuit if we don't have anything to do.
pathExists(filePath).then((exists) => {
pathExists(dirPath).then((exists) => {
if (!exists) return callback();

// Note: if rimraf moves the location of its executable, this will need to be updated
ChildProcessUtilities.spawn(require.resolve("rimraf/bin"), [filePath], {}, callback);
ChildProcessUtilities.spawn("rimraf", ["--no-glob", trailingSlash(dirPath)], {}, callback);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lost the require.resolve, so it doesn't seem to work if rimraf is not in your path or a top-level dependency.

});
}

Expand Down
Loading