Refactor Staging/Production Alternate Host Implementation

lib/main.js

@@ -72,12 +72,6 @@ function run() {
     return;
   }
 
-  // set binary.host when appropriate. host determines the s3 target bucket.
-  const target = prog.setBinaryHostProperty(command.name);
-  if (target && ['install', 'publish', 'unpublish', 'info'].indexOf(command.name) >= 0) {
-    log.info('using binary.host: ' + prog.package_json.binary.host);
-  }
-
   prog.commands[command.name](command.args, function(err) {
     if (err) {
       log.error(command.name + ' error');

lib/mock/http.js

@@ -7,8 +7,7 @@ const path = require('path');
 const nock = require('nock');
 const os = require('os');
 
-const log = require('npmlog');
-log.disableProgress(); // disable the display of a progress bar
+const log = require('../util/log.js');
 log.heading = 'node-pre-gyp'; // differentiate node-pre-gyp's logs from npm's
 
 function http_mock() {
@@ -17,6 +16,7 @@ function http_mock() {
   const basePath = `${os.tmpdir()}/mock`;
 
   nock(new RegExp('([a-z0-9]+[.])*s3[.]us-east-1[.]amazonaws[.]com'))
+    .persist()
     .get(() => true) //a function that always returns true is a catch all for nock
     .reply(
       (uri) => {

lib/mock/s3.js

@@ -5,8 +5,7 @@ module.exports = exports = s3_mock;
 const AWSMock = require('mock-aws-s3');
 const os = require('os');
 
-const log = require('npmlog');
-log.disableProgress(); // disable the display of a progress bar
+const log = require('../util/log.js');
 log.heading = 'node-pre-gyp'; // differentiate node-pre-gyp's logs from npm's
 
 function s3_mock() {

lib/node-pre-gyp.js

@@ -73,11 +73,8 @@ function Run({ package_json_path = './package.json', argv }) {
   });
 
   this.parseArgv(argv);
-
-  // this is set to true after the binary.host property was set to
-  // either staging_host or production_host.
-  this.binaryHostSet = false;
 }
+
 inherits(Run, EE);
 exports.Run = Run;
 const proto = Run.prototype;
@@ -201,67 +198,6 @@ proto.parseArgv = function parseOpts(argv) {
   log.resume();
 };
 
-/**
- * allow the binary.host property to be set at execution time.
- *
- * for this to take effect requires all the following to be true.
- * - binary is a property in package.json
- * - binary.host is falsey
- * - binary.staging_host is not empty
- * - binary.production_host is not empty
- *
- * if any of the previous checks fail then the function returns an empty string
- * and makes no changes to package.json's binary property.
- *
- *
- * if command is "publish" then the default is set to "binary.staging_host"
- * if command is not "publish" the the default is set to "binary.production_host"
- *
- * if the command-line option '--s3_host' is set to "staging" or "production" then
- * "binary.host" is set to the specified "staging_host" or "production_host". if
- * '--s3_host' is any other value an exception is thrown.
- *
- * if '--s3_host' is not present then "binary.host" is set to the default as above.
- *
- * this strategy was chosen so that any command other than "publish" or "unpublish" uses "production"
- * as the default without requiring any command-line options but that "publish" and "unpublish" require
- * '--s3_host production_host' to be specified in order to *really* publish (or unpublish). publishing
- * to staging can be done freely without worrying about disturbing any production releases.
- */
-proto.setBinaryHostProperty = function(command) {
-  if (this.binaryHostSet) {
-    return this.package_json.binary.host;
-  }
-  const p = this.package_json;
-  // don't set anything if host is present. it must be left blank to trigger this.
-  if (!p || !p.binary || p.binary.host) {
-    return '';
-  }
-  // and both staging and production must be present. errors will be reported later.
-  if (!p.binary.staging_host || !p.binary.production_host) {
-    return '';
-  }
-  let target = 'production_host';
-  if (command === 'publish' || command === 'unpublish') {
-    target = 'staging_host';
-  }
-  // the environment variable has priority over the default or the command line. if
-  // either the env var or the command line option are invalid throw an error.
-  const npg_s3_host = process.env.node_pre_gyp_s3_host;
-  if (npg_s3_host === 'staging' || npg_s3_host === 'production') {
-    target = `${npg_s3_host}_host`;
-  } else if (this.opts['s3_host'] === 'staging' || this.opts['s3_host'] === 'production') {
-    target = `${this.opts['s3_host']}_host`;
-  } else if (this.opts['s3_host'] || npg_s3_host) {
-    throw new Error(`invalid s3_host ${this.opts['s3_host'] || npg_s3_host}`);
-  }
-
-  p.binary.host = p.binary[target];
-  this.binaryHostSet = true;
-
-  return p.binary.host;
-};
-
 /**
  * Returns the usage instructions for node-pre-gyp.
  */

lib/pre-binding.js

@@ -19,7 +19,6 @@ exports.find = function(package_json_path, opts) {
     throw new Error(package_json_path + 'does not exist');
   }
   const prog = new npg.Run({ package_json_path, argv: process.argv });
-  prog.setBinaryHostProperty();
   const package_json = prog.package_json;
 
   versioning.validate_config(package_json, opts);

lib/util/versioning.js

@@ -186,12 +186,6 @@ function get_runtime_abi(runtime, target_version) {
 }
 module.exports.get_runtime_abi = get_runtime_abi;
 
-const required_parameters = [
-  'module_name',
-  'module_path',
-  'host'
-];
-
 function validate_config(package_json, opts) {
   const msg = package_json.name + ' package.json is not node-pre-gyp ready:\n';
   const missing = [];
@@ -207,25 +201,33 @@ function validate_config(package_json, opts) {
   if (!package_json.binary) {
     missing.push('binary');
   }
-  const o = package_json.binary;
-  if (o) {
-    required_parameters.forEach((p) => {
-      if (!o[p] || typeof o[p] !== 'string') {
-        missing.push('binary.' + p);
-      }
-    });
+
+  if (package_json.binary) {
+    if (!package_json.binary.module_name) {
+      missing.push('binary.module_name');
+    }
+    if (!package_json.binary.module_path) {
+      missing.push('binary.module_path');
+    }
+    if (!package_json.binary.host && !package_json.binary.production_host) {
+      missing.push('binary.host');
+    }
   }
 
   if (missing.length >= 1) {
     throw new Error(msg + 'package.json must declare these properties: \n' + missing.join('\n'));
   }
-  if (o) {
-    // enforce https over http
-    const protocol = url.parse(o.host).protocol;
-    if (protocol === 'http:') {
-      throw new Error("'host' protocol (" + protocol + ") is invalid - only 'https:' is accepted");
-    }
+
+  if (package_json.binary) {
+    // for all possible host definitions - verify https usage
+    ['host', 'production_host', 'staging_host', 'development_host'].filter((item) => package_json.binary[item]).forEach((item) => {
+      const protocol = url.parse(package_json.binary[item]).protocol;
+      if (protocol === 'http:') {
+        throw new Error(msg + "'" + item + "' protocol (" + protocol + ") is invalid - only 'https:' is accepted");
+      }
+    });
   }
+
   napi.validate_package_json(package_json, opts);
 }
 
@@ -309,11 +311,46 @@ module.exports.evaluate = function(package_json, options, napi_build_version) {
     region: package_json.binary.region,
     s3ForcePathStyle: package_json.binary.s3ForcePathStyle || false
   };
-    // support host mirror with npm config `--{module_name}_binary_host_mirror`
-    // e.g.: https://github.com/node-inspector/v8-profiler/blob/master/package.json#L25
-    // > npm install v8-profiler --profiler_binary_host_mirror=https://registry.npmmirror.com/node-inspector/
+
+  // user can define a target host key to use (development_host, staging_host, production_host)
+  // by  setting the name of the host (development, staging, production)
+  // into an environment variable or via a command line option.
+  // the environment variable has priority over the the command line.
+  let targetHost = process.env.node_pre_gyp_s3_host || options.s3_host;
+
+  // if value is not one of the allowed or the matching key is not found in package.json
+  // silently ignore the option
+  if (['production', 'staging', 'development'].indexOf(targetHost) === -1 || !package_json.binary[`${targetHost}_host`]) {
+    targetHost = '';
+  }
+
+  // the production host is as specified in 'host' key (default)
+  // unless there is none and alias production_host is specified (backwards compatibility)
+  // note: package.json is verified in validate_config to include at least one of the two.
+  let host = package_json.binary.host ||  package_json.binary.production_host;
+
+  // when a valid target is specified by user, the host is from that target (or 'host')
+  if (targetHost === 'staging') {
+    host = package_json.binary.staging_host;
+  } else if (targetHost === 'development') {
+    host = package_json.binary.development_host;
+  } else if (!targetHost && (package_json.binary.development_host || package_json.binary.staging_host)) {
+    // when host not specifically set via command line or environment variable
+    // but staging and/or development host are present in package.json
+    // for any command (or command chain) that includes publish or unpublish
+    // default to lower host (development, and if not preset, staging).
+    if (options.argv && options.argv.remain.some((item) => (item === 'publish' || item === 'unpublish'))) {
+      host = package_json.binary.development_host || package_json.binary.staging_host;
+    }
+  }
+
+  // support host mirror with npm config `--{module_name}_binary_host_mirror`
+  // e.g.: https://github.com/node-inspector/v8-profiler/blob/master/package.json#L25
+  // > npm install v8-profiler --profiler_binary_host_mirror=https://registry.npmmirror.com/node-inspector/
   const validModuleName = opts.module_name.replace('-', '_');
-  const host = process.env['npm_config_' + validModuleName + '_binary_host_mirror'] || package_json.binary.host;
+  // explicitly set mirror overrides everything set above
+  host = process.env['npm_config_' + validModuleName + '_binary_host_mirror'] || host;
+
   opts.host = fix_slashes(eval_template(host, opts));
   opts.module_path = eval_template(package_json.binary.module_path, opts);
   // now we resolve the module_path to ensure it is absolute so that binding.gyp variables work predictably