Merge 1040e2a into 390eaa4

.gitignore

@@ -2,6 +2,7 @@
 
 .DS_Store
 .project
+.vscode
 bower_components
 node_modules
 npm-debug.log*
@@ -15,3 +16,5 @@ build
 */build
 coverage
 Procfile
+stdout*.log
+stderr*.log

.travis.yml

@@ -23,9 +23,6 @@ script:
   - npm --version
   - npm run builder:check-ci
 
-  # Do some primitive bash functional tests.
-  - bash test/func/test.sh
-
   # Manually send coverage reports to coveralls.
   # - Aggregate client results
   # - Single server and func test results

HISTORY.md

@@ -1,6 +1,14 @@
 History
 =======
 
+## Unreleased MAJOR
+
+* **Breaking**: Add `pre|post` lifecycle commands.
+  [#68](https://github.com/FormidableLabs/builder/issues/68)
+* **Breaking**: Error out on invalid or conflicting command line flags passed
+  to `builder`.
+* Add more explicit behavior and tests for `--setup` flag.
+
 ## 3.2.3
 
 * Account for `PATH` vs. `Path` env var differences in MinGW vs. cmd on

LICENSE.txt

@@ -1,6 +1,6 @@
 The MIT License (MIT)
 
-Copyright (c) 2015-2016 Formidable Labs
+Copyright (c) 2015-2018 Formidable Labs
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

README.md

@@ -59,6 +59,12 @@ the rough goals and motivations behind the project.
       - [builder run](#builder-run)
       - [builder concurrent](#builder-concurrent)
       - [builder envs](#builder-envs)
+    - [Setup Task](#setup-task)
+    - [Task Lifecycle](#task-lifecycle)
+      - [The Basics](#the-basics)
+      - [Other Builder Actions](#other-builder-actions)
+      - [Builder Flags During Pre and Post](#builder-flags-during-pre-and-post)
+      - [Task Prefix Complexities](#task-prefix-complexities)
     - [Custom Flags](#custom-flags)
     - [Expanding the Archetype Path](#expanding-the-archetype-path)
 - [Tasks](#tasks)
@@ -297,7 +303,10 @@ $ builder concurrent <task1> <task2> <task3>
 Flags:
 
 * `--tries`: Number of times to attempt a task (default: `1`)
-* `--setup`: Single task to run for the entirety of `<action>`
+* `--setup`: Single task to run for the entirety of `<action>`.
+    * **Note**: The `--setup` task is run at the start of the first main task
+      to actually run. This may _not_ be the first specified task however,
+      as `pre` tasks could end up with main tasks starting out of order.
 * `--queue`: Number of concurrent processes to run (default: unlimited - `0|null`)
 * `--[no-]buffer`: Buffer output until process end (default: `false`)
 * `--[no-]bail`: End all processes after the first failure (default: `true`)
@@ -350,6 +359,10 @@ Flags:
 
 * `--tries`: Number of times to attempt a task (default: `1`)
 * `--setup`: Single task to run for the entirety of `<action>`
+    * **Note**: The `--setup` task is run at the start of the first main task
+      to actually run. This may _not_ be the first specified task however,
+      as `pre` tasks could end up with main tasks per environment object
+      starting out of order.
 * `--queue`: Number of concurrent processes to run (default: unlimited - `0|null`)
 * `--[no-]buffer`: Buffer output until process end (default: `false`)
 * `--[no-]bail`: End all processes after the first failure (default: `true`)
@@ -375,6 +388,186 @@ The environment variable `FOO` will have a value of `"ENVS"` with the single
 environment object array item given to `builder envs` overriding the `--env`
 flag value.
 
+#### Setup Task
+
+A task specified in `--setup <task>` will have the following flags apply to
+the setup task as apply to the main task:
+
+* `--env`
+* `--env-path`
+* `--quiet`
+* `--log-level`
+
+The following flags do _not_ apply to a setup task:
+
+* `--` custom flags
+* `--tries`
+* `--expand-archetype`
+* `--queue`
+* `--buffer`
+
+That said, if you need things like `--tries`, etc., these can be always coded
+into a wrapped task like:
+
+```js
+"scripts": {
+  "setup-alone": "while sleep 1; do echo SETUP; done",
+  "setup": "builder run --tries=5 setup-alone",
+  "test": "mocha",
+  "test-full": "builder run --setup=setup test"
+}
+```
+
+#### Task Lifecycle
+
+Builder executes `pre<task>` and `post<task>` tasks the same as `npm` does,
+with some perhaps not completely obvious corner cases.
+
+##### The Basics
+
+If you have:
+
+```js
+"scripts": {
+  "prefoo": "echo PRE",
+  "foo": "echo TEMP",
+  "postfoo": "echo POST"
+}
+```
+
+And run `builder run foo`, then just like `npm`, builder will run in order:
+
+1. `prefoo`
+2. `foo`
+3. `postfoo`
+
+assuming each task succeeds, otherwise execution is terminated.
+
+`pre` and `post` tasks can be provided in an archetype and overridden in a root
+`package.json` in the exact same manner as normal Builder tasks.
+
+##### Other Builder Actions
+
+`builder run` works essentially the same as `npm run`. Things get a little
+messy with Builder's other execution options:
+
+`builder envs` runs `pre|post` tasks exactly **once** regardless of how many
+concurrent executions of the underlying task (with different environment
+variables) occur.
+
+`builder concurrent` runs appropriate `pre|post` tasks for each independent
+task. So, for something like:
+
+```js
+"scripts": {
+  "prefoo": "echo PRE FOO",
+  "foo": "echo TEMP FOO",
+  "postfoo": "echo POST FOO",
+  "prebar": "echo PRE BAR",
+  "bar": "echo TEMP BAR",
+  "postbar": "echo POST BAR"
+}
+```
+
+running `builder concurrent foo bar` would run **all** of the above tasks at
+the appropriate lifecycle moment.
+
+Note that things like a `--queue=NUM` limit on a concurrent task will have
+*all* of the `pre`, main, and `post` task need to finish serial execution before
+the next spot is freed up.
+
+The `--bail` flag applies to all of a single tasks `pre`, main, and `post`
+group. So if any of those fail, it's as if the main task failed.
+
+##### Builder Flags During Pre and Post
+
+*Applicable Flags*
+
+When executing a `<task>` that has `pre<task>` and/or `post<task>` entries, the
+following execution flags **do** apply to the `pre|post` tasks.
+
+* `--env` TODO_TEST
+* `--env-path` TODO_TEST
+* `--quiet` TODO_TEST
+* `--log-level` TODO_TEST
+
+These flags have mixed application:
+
+* `--queue`: Applies for `concurrent`, but not `run` or `envs`. TODO_TEST
+* `--buffer`: Applies for `concurrent`, but not `run` or `envs`. TODO_TEST
+* `--bail`: Applies for `concurrent`, but not `run` or `envs`. A `pre<task>`,
+  `<task>`, and a `post<task>` are treated as a group, so a failure of any
+  short-circuits the rests and ends with failures. But with `--bail=false` a
+  failure doesn't stop execution of the _other_ groups. TODO_TEST
+
+The following flags do _not_ apply to pre/post tasks:
+
+* `--` custom flags TODO_TEST
+* `--tries` TODO_TEST
+* `--expand-archetype` TODO_TEST/DECIDE ? (Might be better to **apply**)
+* `--setup`: A task specified in `--setup <task>` will not have `pre|post`
+  tasks apply. TODO_TEST
+
+We will explain a few of these situations in a bit more depth:
+
+*Custom Flags*
+
+The special `--` flag with any subsequent custom flags to the underlying task
+are only passed to the the main `<task>` and not `pre<task>` or `post<task>`.
+The rationale here is that custom command line flags most likely just apply to
+a single shell command (the main one).
+
+So, for example
+
+```js
+"scripts": {
+  "prefoo": "echo PRE",
+  "foo": "echo TEMP",
+  "postfoo": "echo POST"
+}
+```
+
+running `builder run foo -- --hi` would produce:
+
+```
+PRE
+TEMP --hi
+POST
+```
+
+*Other Flags*
+
+By contrast, the various other Builder-specific flags that can be applied to a
+task like `--env`, etc., **will** apply to `pre|post` tasks, under the
+assumption that control flags + environment variables will most likely want to
+be used for the execution of all commands in the workflow.
+
+So, for example:
+
+```js
+"scripts": {
+  "prefoo": "echo PRE $VAR",
+  "foo": "echo TEMP $VAR",
+  "postfoo": "echo POST $VAR"
+}
+```
+
+running `builder run foo --env '{"VAR":"HI"}'` would produce:
+
+```
+PRE HI
+TEMP HI
+POST HI
+```
+
+##### Task Prefix Complexities
+
+For the above example, if you have a task named `preprefoo`, then running
+`foo` **or** even `prefoo` directly will **not** run `preprefoo`. Builder
+follows `npm`'s current implementation which is roughly "add `pre|post` tasks
+to current execution as long as the task itself is not prefixed with
+`pre|post`". (_Note_ that `yarn` does not follow this logic in task execution).
+
 #### Custom Flags
 
 Just like [`npm run <task> [-- <args>...]`](https://docs.npmjs.com/cli/run-script),

bin/builder-core.js

@@ -5,7 +5,6 @@ var chalk = require("chalk");
 var Config = require("../lib/config");
 var Environment = require("../lib/environment");
 var Task = require("../lib/task");
-var runner = require("../lib/runner");
 var log = require("../lib/log");
 
 /**
@@ -50,8 +49,7 @@ module.exports = function (opts, callback) {
   var task = new Task({
     config: config,
     env: env,
-    argv: opts.argv,
-    runner: runner
+    argv: opts.argv
   });
 
   // Run the task

bin/builder.js

@@ -16,7 +16,7 @@ var msgs = [];
 
 // Infer if we are global and there is a local version available.
 var builderPath = require.resolve("./builder-core");
-var localPath = path.resolve(process.cwd(), "node_modules/builder/bin/builder-core.js");
+var localPath = path.resolve("node_modules/builder/bin/builder-core.js");
 
 // Swap to local path if different.
 if (builderPath !== localPath) {
@@ -28,7 +28,7 @@ if (builderPath !== localPath) {
     });
   } catch (err) {
     msgs.push({
-      level: "warn", type: "local-detect",
+      level: "info", type: "local-detect",
       msg: "Error importing local builder: " + err.message
     });
     msgs.push({

lib/args.js

@@ -1,5 +1,5 @@
 "use strict";
-/*eslint max-statements:[2, 20]*/
+/*eslint max-statements:[2, 30]*/
 
 /**
  * Argv command flags.
@@ -115,6 +115,13 @@ var FLAGS = {
   }
 };
 
+// Get all possible flag fields.
+var FLAGS_ALL_FIELDS = _.chain(FLAGS) // Use `_.chain` not `_()` because of `reduce`
+  .values()
+  .reduce(function (memo, obj) { return memo.concat(_.keys(obj)); }, [])
+  .uniq()
+  .value();
+
 // Convert our bespoke flags object into `nopt` options.
 var getOpts = function (obj) {
   return _.mapValues(obj, function (val) {
@@ -146,11 +153,13 @@ var version = function () {
 };
 
 // Option parser.
-var createFn = function (flags) {
+var createFn = function (flags, isGeneral) {
   var opts = getOpts(flags);
+  var flagKeys = _.keys(flags);
 
   return function (argv) {
     argv = argv || process.argv;
+    var unparsedArgv = argv;
 
     // Capture any flags after `--` like `npm run <task> -- <args>` does.
     // See: https://docs.npmjs.com/cli/run-script#description
@@ -167,6 +176,9 @@ var createFn = function (flags) {
     // Parse.
     var parsedOpts = nopt(opts, {}, argv);
 
+    // Hack in unparsed for pristine version.
+    parsedOpts.argv.unparsed = unparsedArgv;
+
     // Stash if log-level was actually set.
     var logLevel = parsedOpts["log-level"];
 
@@ -196,6 +208,19 @@ var createFn = function (flags) {
       parsedOpts["log-level"] = "none";
     }
 
+    // Validate no invalid or ambiguous flags.
+
+    var parsedKeys = _(parsedOpts).keys().without("argv").value();
+    var extraKeys = _.difference(parsedKeys, flagKeys);
+    if (isGeneral) {
+      // If the general command, allow flags from *any* other action.
+      // We'll eventually hit the right action with tighter enforcement.
+      extraKeys = _.difference(extraKeys, FLAGS_ALL_FIELDS);
+    }
+    if (extraKeys.length) {
+      throw new Error("Found invalid/conflicting keys: " + extraKeys.join(", "));
+    }
+
     return _(parsedOpts)
       // Camel-case flags.
       .mapKeys(function (val, key) { return _.camelCase(key); })
@@ -209,7 +234,11 @@ module.exports = _.extend({
   FLAGS: FLAGS,
   help: help,
   version: version
-}, _.mapValues(FLAGS, function (flags) {
+}, _.mapValues(FLAGS, function (flags, key) {
+  // Merge in general flags to all other configs.
+  var isGeneral = key === "general";
+  flags = isGeneral ? flags : _.extend({}, FLAGS.general, flags);
+
   // Add in `KEY()` methods.
-  return createFn(flags);
+  return createFn(flags, isGeneral);
 }));

lib/config.js

@@ -144,7 +144,7 @@ Config.prototype._loadArchetypePkg = function (name) {
  */
 Config.prototype._loadPkgs = function (archetypes) {
   var self = this;
-  var CWD_PKG = this._lazyRequire(path.join(process.cwd(), "package.json")) || {};
+  var CWD_PKG = this._lazyRequire(path.resolve("package.json")) || {};
 
   // Load base packages.
   var pkgs = [_.extend({ name: "ROOT" }, CWD_PKG)].concat(_.chain(archetypes)