Merge 51c013d into 2a47ce7

.travis.yml

@@ -30,8 +30,8 @@ script:
   - npm --version
   - npm run builder:check-ci
 
-  # Also use builder itself to run the check-ci command
-  - node bin/builder.js run builder:check-ci
+  # Do some primitive bash functional tests.
+  - bash test/func/test.sh
 
   # Manually send coverage reports to coveralls.
   # - Aggregate client results

README.md

@@ -1,4 +1,5 @@
 [![Travis Status][trav_img]][trav_site]
+[![Appveyor Status][av_img]][av_site]
 [![Coverage Status][cov_img]][cov_site]
 
 Builder
@@ -86,12 +87,11 @@ the rough goals and motivations behind the project.
     - [Example `builder` Archetype Project Structure](#example-builder-archetype-project-structure)
 - [Tips, Tricks, & Notes](#tips-tricks-&-notes)
   - [PATH, NODE_PATH Resolution](#path-node_path-resolution)
+  - [Environment Variables](#environment-variables)
   - [Alternative to `npm link`](#alternative-to-npm-link)
   - [Project Root](#project-root)
   - [Avoid npm Lifecycle Commands](#avoid-npm-lifecycle-commands)
   - [Other Process Execution](#other-process-execution)
-  - [Terminal Color](#terminal-color)
-  - [Why Exec?](#why-exec)
   - [I Give Up. How Do I Abandon Builder?](#i-give-up-how-do-i-abandon-builder)
   - [Versions v1, v2, v3](#versions-v1-v2-v3)
 
@@ -1257,6 +1257,19 @@ The order of resolution doesn't often come up, but can sometimes be a factor
 in diagnosing archetype issues and script / file paths, especially when using
 `npm` v3.
 
+### Environment Variables
+
+Builder clones the entire environment object before mutating it for further
+execution of tasks. On Mac/Linux, this has no real change of behavior of how
+the execution environment works. However, on Windows, there are some subtle
+issues with the fact that Windows has a case-insensitive environment variable
+model wherein you can set `PATH` in a node process, but internally this is
+transformed to set `Path`. Builder specifically handles `PATH` correctly across
+platforms for it's own specific mutation.
+
+However, if your tasks rely on the Windows coercion of case-insensitivity of
+environment variables, you may run into some idiosyncratic problems with tasks.
+
 ### Alternative to `npm link`
 
 In some cases, `npm link` can interfere with the order of resolution. If you
@@ -1327,22 +1340,6 @@ of the environment enhancements it adds. So, for things that themselves exec
 or spawn processes, like `concurrently`, this can be a problem. Typically, you
 will need to have the actual command line processes invoked _by_ Builder.
 
-### Terminal Color
-
-Builder uses `exec` under the hood with piped `stdout` and `stderr`. Programs
-typically interpret the piped environment as "doesn't support color" and
-disable color. Consequently, you typically need to set a "**force color**"
-option on your executables in `scripts` commands if they exist.
-
-### Why Exec?
-
-So, why `exec` and not `spawn` or something similar that has a lot more process
-control and flexibility? The answer lies in the fact that most of what Builder
-consumes is shell strings to execute, like `script --foo --bar "Hi there"`.
-_Parsing_ these arguments into something easily consumable by `spawn` and always
-correct is quite challenging. `exec` works easily with straight strings, and
-since that is the target of `scripts` commands, that is what we use for Builder.
-
 ### I Give Up. How Do I Abandon Builder?
 
 Builder is designed to be as close to vanilla npm as possible. So, if for
@@ -1418,6 +1415,8 @@ things settle down in `v3.x`-on.
 [builder-react-component]: https://github.com/FormidableLabs/builder-react-component
 [trav_img]: https://api.travis-ci.org/FormidableLabs/builder.svg
 [trav_site]: https://travis-ci.org/FormidableLabs/builder
+[av_img]: https://ci.appveyor.com/api/projects/status/oq3m2hay1tl82tsj?svg=true
+[av_site]: https://ci.appveyor.com/project/FormidableLabs/builder
 [cov]: https://coveralls.io
 [cov_img]: https://img.shields.io/coveralls/FormidableLabs/builder.svg
 [cov_site]: https://coveralls.io/r/FormidableLabs/builder

appveyor.yml

@@ -0,0 +1,33 @@
+# Versions
+environment:
+  matrix:
+    - nodejs_version: 0.10
+    - nodejs_version: 0.12
+    - nodejs_version: 4
+    - nodejs_version: 5
+
+# Install scripts. (runs after repo cloning)
+install:
+  # Get the latest stable version of Node.js or io.js
+  - ps: Install-Product node $env:nodejs_version
+  # Install and use local, modern NPM
+  - npm install npm@next
+  # install modules
+  - node_modules\.bin\npm install
+
+# Post-install test scripts.
+test_script:
+  # Output useful info for debugging.
+  - node --version
+  - node_modules\.bin\npm --version
+  # run tests
+  - node_modules\.bin\npm run builder:check
+
+# Don't actually build.
+build: off
+
+matrix:
+  fast_finish: true
+
+cache:
+  - node_modules -> package.json   # local npm modules

lib/environment.js

@@ -8,9 +8,12 @@
  */
 var _ = require("lodash");
 var path = require("path");
+var clone = require("./utils/clone");
 
-// OS-agnostic path delimiter.
-var DELIM = process.platform.indexOf("win") === 0 ? ";" : ":";
+// OS-specific helpers
+var IS_WIN = process.platform.indexOf("win") === 0;
+var DELIM = IS_WIN ? ";" : ":";
+var ENV_PATH_NAME = IS_WIN ? "Path" : "PATH";
 
 // Node directories.
 var CWD_BIN = path.join(process.cwd(), "node_modules/.bin");
@@ -19,7 +22,7 @@ var CWD_NODE_PATH = path.join(process.cwd(), "node_modules");
 /**
  * Environment wrapper.
  *
- * **NOTE - Side Effects**: Mutates wrapped environment object.
+ * **NOTE**: Clones object if real `process.env` to avoid mutation
  *
  * @param {Object} opts         Options object
  * @param {Object} opts.config  Configuration object
@@ -29,7 +32,12 @@ var CWD_NODE_PATH = path.join(process.cwd(), "node_modules");
 var Environment = module.exports = function (opts) {
   opts = opts || {};
   this.config = opts.config;
+
+  // Clone if real process env to avoid direct mutation.
   this.env = opts.env || process.env;
+  if (this.env === process.env) {
+    this.env = clone(this.env);
+  }
 
   // Chalk: Force colors.
   this.env.FORCE_COLOR = "true";
@@ -40,7 +48,7 @@ var Environment = module.exports = function (opts) {
   }
 
   // Mutate environment paths.
-  this.env.PATH = this.updatePath(this.config.archetypePaths);
+  this.env[ENV_PATH_NAME] = this.updatePath(this.config.archetypePaths);
   this.env.NODE_PATH = this.updateNodePath(this.config.archetypeNodePaths);
 
   // Add in npm config environment variables.
@@ -59,7 +67,7 @@ var Environment = module.exports = function (opts) {
  * @returns {String}                  `PATH` environment variable
  */
 Environment.prototype.updatePath = function (archetypePaths) {
-  var basePath = (this.env.PATH || "")
+  var basePath = (this.env[ENV_PATH_NAME] || "")
     .split(DELIM)
     .filter(function (x) { return x; });
 
@@ -97,8 +105,6 @@ Environment.prototype.updateNodePath = function (archetypeNodePaths) {
 /**
  * Update environment with configuration variables from package.json
  *
- * **Note**: We only go _one level deep_ for process.env mutations.
- *
  * Resolution order:
  * 1. Existing environment
  * 2. `ROOT/package.json:config`

lib/log.js

@@ -2,6 +2,7 @@
 
 var chalk = require("chalk");
 var args = require("./args");
+var clone = require("./utils/clone");
 
 // Wrap "type".
 var wrapType = function (type) {
@@ -23,7 +24,7 @@ var log = module.exports = {
   /**
    * Set log level from command flags or environment.
    *
-   * **NOTE - Side Effects**: Mutates environment.
+   * **NOTE**: Clones object if real `process.env` to avoid mutation
    *
    * @param {Object} opts         Options object
    * @param {Object} opts.argv    Arguments array.
@@ -32,10 +33,15 @@ var log = module.exports = {
    */
   setLevel: function (opts) {
     opts = opts || {};
-    var env = opts.env && opts.env.env || process.env;
+
+    // Clone if real process env to avoid direct mutation.
+    log._env = opts.env && opts.env.env || log._env || process.env;
+    if (log._env === process.env) {
+      log._env = clone(log._env);
+    }
 
     // Try to determine log level from environment.
-    var level = env._BUILDER_ARGS_LOG_LEVEL;
+    var level = log._env._BUILDER_ARGS_LOG_LEVEL;
 
     // If not, determine log level from command line.
     if (!level) {
@@ -44,7 +50,7 @@ var log = module.exports = {
     }
 
     // Statefully set level.
-    env._BUILDER_ARGS_LOG_LEVEL = level;
+    log._env._BUILDER_ARGS_LOG_LEVEL = level;
     log._level = LEVELS[level];
     if (typeof log._level === "undefined") {
       throw new Error("Unknown log level: " + level);
@@ -56,7 +62,7 @@ var log = module.exports = {
 
   // Nuke everything for test runs.
   _unsetLevel: function () {
-    delete process.env._BUILDER_ARGS_LOG_LEVEL;
+    delete log._env._BUILDER_ARGS_LOG_LEVEL;
     delete log._level;
     delete log._queue;
   },

lib/runner.js

@@ -1,7 +1,6 @@
 "use strict";
 /*eslint max-params: [2, 4], max-statements: [2, 20] */
 
-var path = require("path");
 var _ = require("lodash");
 var async = require("async");
 var chalk = require("chalk");
@@ -11,6 +10,7 @@ var Tracker = require("./utils/tracker");
 var Config = require("./config");
 var Environment = require("./environment");
 var Task = require("./task");
+var clone = require("./utils/clone");
 var runner;
 
 // Helper for command strings for logging.
@@ -65,21 +65,28 @@ var cmdWithCustom = function (cmd, opts, env) {
  * - `['"]<token>`: Quotes before token.
  *
  * @param {String} str    String to parse
- * @param {String} token  Token to replace
+ * @param {String} tokens Tokens to replace (multiple for windows)
  * @param {String} sub    Replacement
- * @returns {String}      Mutated string.
+ * @returns {String}      Mutated string
  */
-var replaceToken = function (str, token, sub) {
-  var tokenRe = new RegExp("(^|\\s|\\'|\\\")(" + _.escapeRegExp(token) + ")", "g");
+var replaceToken = function (str, tokens, sub) {
+  var tokenRes = tokens.map(function (token) {
+    return {
+      token: token,
+      re: new RegExp("(^|\\s|\\'|\\\")(" + _.escapeRegExp(token) + ")", "g")
+    };
+  });
 
-  return str.replace(tokenRe, function (match, prelimMatch, tokenMatch/* offset, origStr*/) {
-    // Sanity check.
-    if (tokenMatch !== token) {
-      throw new Error("Bad match " + match + " for token " + token);
-    }
+  return tokenRes.reduce(function (memo, obj) {
+    return memo.replace(obj.re, function (match, prelimMatch, tokenMatch/* offset, origStr*/) {
+      // Sanity check.
+      if (tokenMatch !== obj.token) {
+        throw new Error("Bad match " + match + " for token " + obj.token);
+      }
 
-    return prelimMatch + sub;
-  });
+      return prelimMatch + sub;
+    });
+  }, str);
 };
 
 /**
@@ -128,9 +135,13 @@ var expandArchetype = function (cmd, opts, env) {
   }
 
   // Create final token for replacing.
-  var archetypeToken = path.join("node_modules", archetypeName);
+  var archetypeTokens = [["node_modules", archetypeName].join("/")]; // unix
+  if (process.platform.indexOf("win") === 0) {
+    // Add windows delimiter too.
+    archetypeTokens.push(["node_modules", archetypeName].join("\\"));
+  }
 
-  return replaceToken(cmd, archetypeToken, archetypePath);
+  return replaceToken(cmd, archetypeTokens, archetypePath);
 };
 
 /**
@@ -155,11 +166,11 @@ var run = function (cmd, shOpts, opts, callback) {
   shOpts = _.extend({
     env: {},
     stdio: buffer ? "pipe" : "inherit"
-  }, shOpts);
+  }, clone(shOpts)); // Clone deep to coerce env to plain object.
 
   // Copied from npm's lib/utils/lifecycle.js
   if (process.platform === "win32") {
-    sh = process.env.comspec || "cmd";
+    sh = shOpts.env.comspec || "cmd";
     shFlag = "/d /s /c";
     shOpts.windowsVerbatimArguments = true;
   }
@@ -420,15 +431,15 @@ runner = module.exports = {
     var finish = createFinish(shOpts, opts, tracker, callback);
 
     var queue = opts.queue;
-    var taskEnvs = opts._envs;
+    var taskEnvs = clone(opts._envs);
     var bail = opts.bail;
 
     log.info("envs", "Starting with queue size: " + chalk.magenta(queue || "unlimited"));
     async.mapLimit(taskEnvs, queue || Infinity, function (taskEnv, cb) {
       // Add each specific set of environment variables.
       // Clone `shOpts` to turn `env` into a plain object: in Node 4+
       // `process.env` is a special object which changes merge behavior.
-      var taskShOpts = _.merge(_.cloneDeep(shOpts), { env: taskEnv });
+      var taskShOpts = _.merge(clone(shOpts), { env: taskEnv });
       var taskOpts = _.extend({ tracker: tracker, taskEnv: taskEnv }, opts);
 
       log.info("envs", "Starting " + cmdStr(cmd, taskOpts));

lib/utils/clone.js

@@ -0,0 +1,17 @@
+"use strict";
+
+/**
+ * A super-simple clone.
+ *
+ * Rationale: Lodash's `_.clone(|Deep)` has problems with empty strings
+ * on certain Node versions with the ever terrifying `process.env` pseudo-
+ * object.
+ *
+ * Limitations: No functions, no circular references, etc.
+ *
+ * @param   {Object} obj  Object to clone
+ * @returns {Object}      Cloned object
+ */
+module.exports = function (obj) {
+  return JSON.parse(JSON.stringify(obj)); // Such hackery...
+};

test/func/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "builder",
+  "version": "0.0.1",
+  "config": {
+    "msg": "hi"
+  },
+  "scripts": {
+    "echo": "echo ECHO MSG: ${npm_package_config_msg}"
+  }
+}

test/func/test.sh

@@ -0,0 +1,19 @@
+#!/bin/bash
+
+# Change to test directory.
+cd "${0%/*}"
+
+# Variables
+BUILDER="${PWD}/../../bin/builder.js"
+
+echo -e "\nTEST: Basic config overrides"
+OUT=$(node ${BUILDER} run echo);
+if [[ $OUT != *"ECHO MSG: hi"*  ]]; then echo -e "\n\n=== FAILED OUTPUT ===\n${OUT}\n\n"; exit 1; fi
+
+echo -e "\nTEST: Silence log"
+OUT=$(node ${BUILDER} run echo --log-level=none)
+if [[ $OUT == *"builder-core:start"*  ]]; then echo -e "\n\n=== FAILED OUTPUT ===\n${OUT}\n\n"; exit 1; fi
+
+echo -e "\nTEST: Env overrides config"
+OUT=$(npm_package_config_msg="over" node ${BUILDER} run echo)
+if [[ $OUT != *"ECHO MSG: over"*  ]]; then echo -e "\n\n=== FAILED OUTPUT ===\n${OUT}\n\n"; exit 1; fi

test/server/spec/base.spec.js

@@ -50,6 +50,10 @@ afterEach(function (done) {
   // Remove logs, ignoring errors.
   async.parallel([
     function (cb) { fs.unlink("stdout.log", function () { cb(); }); },
+    function (cb) { fs.unlink("stdout-setup.log", function () { cb(); }); },
+    function (cb) { fs.unlink("stdout-1.log", function () { cb(); }); },
+    function (cb) { fs.unlink("stdout-2.log", function () { cb(); }); },
+    function (cb) { fs.unlink("stdout-3.log", function () { cb(); }); },
     function (cb) { fs.unlink("stderr.log", function () { cb(); }); }
   ], done);
 });