Merge pull request mapbox#405 from jschlight/napi-improvements

N-API Improvements

README.md

@@ -316,23 +316,25 @@ This value is of importance in two areas:
 1. The C/C++ code which needs to know against which N-API version it should compile.
 2. `node-pre-gyp` itself which must assign appropriate path and file names to avoid collisions.
 
-### Defining `NAPI_BUILD_VERSION` for the C/C++ code
+### Defining `NAPI_VERSION` for the C/C++ code
 
 The `napi_build_version` value is communicated to the C/C++ code by adding this code to the `binding.gyp` file:
 
 ```
 "defines": [
-    "NAPI_BUILD_VERSION=<(napi_build_version)",
+    "NAPI_VERSION=<(napi_build_version)",
 ]
 ```
 
-This ensures that `NAPI_BUILD_VERSION`, an integer value, is declared appropriately to the C/C++ code for each build.
+This ensures that `NAPI_VERSION`, an integer value, is declared appropriately to the C/C++ code for each build.
+
+> Note that earlier versions of this document recommended defining the symbol `NAPI_BUILD_VERSION`. `NAPI_VERSION` is prefered because it used by the N-API C/C++ headers to configure the specific N-API veriosn being requested. 
 
 ### Path and file naming requirements in `package.json`
 
 Since `node-pre-gyp` fires off multiple operations for each request, it is essential that path and file names be created in such a way as to avoid collisions. This is accomplished by imposing additional path and file naming requirements.
 
-Specifically, when performing N-API builds, the `{napi_build_version}` text substitution string  *must* be present in the `module_path` property. In addition, the `{napi_build_version}` text substitution string  *must* be present in either the `remote_path` or `package_name` property. (No problem if it's in both.)
+Specifically, when performing N-API builds, the `{napi_build_version}` text configuration value  *must* be present in the `module_path` property. In addition, the `{napi_build_version}` text configuration value  *must* be present in either the `remote_path` or `package_name` property. (No problem if it's in both.)
 
 Here's an example:
 
@@ -347,9 +349,42 @@ Here's an example:
   }
 ```
 
+## Supporting both N-API and NAN builds
+
+You may have a legacy native add-on that you wish to continue supporting for those versions of Node that do not support N-API, as you add N-API support for later Node versions. This can be accomplished by specifying the `node_napi_label` configuration value in the package.json `binary.package_name` property. 
+
+Placing the configuration value `node_napi_label` in the package.json `binary.package_name` property instructs `node-pre-gyp` to build all viable N-API binaries supported by the current Node instance. If the current Node instance does not support N-API, `node-pre-gyp` will request a traditional, non-N-API build. 
+
+The configuration value `node_napi_label` is set by `node-pre-gyp` to the type of build created, `napi` or `node`, and the version number. For N-API builds, the string contains the N-API version nad has values like `napi-v3`. For traditional, non-N-API builds, the string contains the ABI version with values like `node-v46`.
+
+Here's how the `binary` configuration above might be changed to support both N-API and NAN builds:
+
+```js
+"binary": {
+    "module_name": "your_module",
+    "module_path": "./lib/binding/{node_napi_label}",
+    "remote_path": "./{module_name}/v{version}/{configuration}/",
+    "package_name": "{platform}-{arch}-{node_napi_label}.tar.gz",
+    "host": "https://your_bucket.s3-us-west-1.amazonaws.com",
+    "napi_versions": [1,3]
+  }
+```
+
+The C/C++ symbol `NAPI_VERSION` can be used to distinguish N-API and non-N-API builds. The value of `NAPI_VERSION` is set to the integer N-API version for N-API builds and is set to `0` for non-N-API builds. 
+
+For example:
+
+```C
+#if NAPI_VERSION
+// N-API code goes here
+#else
+// NAN code goes here
+#endif
+```
+
 ### Two additional configuration values
 
-For those who need them in legacy projects, two additional configuration values are available for all builds.
+The following two configuration values, which were implemented in previous versions of `node-pre-gyp`, continue to exist, but have been replaced by the `node_napi_label` configuration value described above.
 
 1. `napi_version` If N-API is supported by the currently executing Node instance, this value is the N-API version number supported by Node. If N-API is not supported, this value is an empty string.
 

lib/build.js

@@ -21,7 +21,7 @@ function do_build(gyp,argv,callback) {
             napi.swap_build_dir_in(result.opts.napi_build_version);
         }
         compile.run_gyp(final_args,result.opts,function(err) {
-            if (!err && result.opts.napi_build_version) {
+            if (result.opts.napi_build_version) {
                 napi.swap_build_dir_out(result.opts.napi_build_version);
             }
             return callback(err);

lib/node-pre-gyp.js

@@ -135,7 +135,7 @@ proto.parseArgv = function parseOpts (argv) {
   if (dir == null) dir = process.cwd();
   var package_json = JSON.parse(fs.readFileSync(path.join(dir,'package.json')));
 
-  this.todo = napi.expand_commands (package_json, commands);
+  this.todo = napi.expand_commands (package_json, this.opts, commands);
 
   // support for inheriting config env variables from npm
   var npm_config_prefix = 'npm_config_';

lib/pre-binding.js

@@ -9,19 +9,19 @@ module.exports = exports;
 
 exports.usage = 'Finds the require path for the node-pre-gyp installed module';
 
-exports.validate = function(package_json) {
-    versioning.validate_config(package_json);
+exports.validate = function(package_json,opts) {
+    versioning.validate_config(package_json,opts);
 };
 
 exports.find = function(package_json_path,opts) {
    if (!existsSync(package_json_path)) {
         throw new Error("package.json does not exist at " + package_json_path);
    }
    var package_json = require(package_json_path);
-   versioning.validate_config(package_json);
+   versioning.validate_config(package_json,opts);
    var napi_build_version;
-   if (napi.get_napi_build_versions (package_json)) {
-       napi_build_version = napi.get_best_napi_build_version(package_json);
+   if (napi.get_napi_build_versions (package_json, opts)) {
+       napi_build_version = napi.get_best_napi_build_version(package_json, opts);
    }
    opts = opts || {};
    if (!opts.module_root) opts.module_root = path.dirname(package_json_path);

lib/rebuild.js

@@ -13,7 +13,7 @@ function rebuild (gyp, argv, callback) {
     { name: 'clean', args: [] },
     { name: 'build', args: ['rebuild'] }
     ];
-  commands = napi.expand_commands(package_json, commands);
+  commands = napi.expand_commands(package_json, gyp.opts, commands);
   for (var i = commands.length; i !== 0; i--) {
     gyp.todo.unshift(commands[i-1]);
   }

lib/reinstall.js

@@ -10,7 +10,7 @@ var napi = require('./util/napi.js');
 function rebuild (gyp, argv, callback) {
   var package_json = JSON.parse(fs.readFileSync('./package.json'));
   var installArgs = [];
-  var napi_build_version = napi.get_best_napi_build_version(package_json);
+  var napi_build_version = napi.get_best_napi_build_version(package_json, gyp.opts);
   if (napi_build_version != null) installArgs = [ napi.get_command_arg (napi_build_version) ];
   gyp.todo.unshift(
       { name: 'clean', args: [] },

lib/util/handle_gyp_opts.js

@@ -47,7 +47,8 @@ var share_with_node_gyp = [
   'module_path',
   'napi_version',
   'node_abi_napi',
-  'napi_build_version'
+  'napi_build_version',
+  'node_napi_label'
 ];
 
 function handle_gyp_opts(gyp, argv, callback) {
@@ -61,8 +62,10 @@ function handle_gyp_opts(gyp, argv, callback) {
         var val = opts[key];
         if (val) {
             node_pre_gyp_options.push('--' + key + '=' + val);
+        } else if (key === 'napi_build_version') {
+            node_pre_gyp_options.push('--' + key + '=0');
         } else {
-            if (key !== 'napi_version' && key !== 'node_abi_napi' && key !== 'napi_build_version')
+            if (key !== 'napi_version' && key !== 'node_abi_napi')
                 return callback(new Error("Option " + key + " required but not found by node-pre-gyp"));
         }
     });

lib/util/napi.js

@@ -2,6 +2,7 @@
 
 var fs = require('fs');
 var rm = require('rimraf');
+var log = require('npmlog');
 
 module.exports = exports;
 
@@ -27,8 +28,9 @@ var napi_multiple_commands = [
 
 var napi_build_version_tag = 'napi_build_version=';
 
-module.exports.get_napi_version = function() {
+module.exports.get_napi_version = function(target) { // target may be undefined
 	// returns the non-zero numeric napi version or undefined if napi is not supported.
+	// correctly supporting target requires an updated cross-walk
 	var version = process.versions.napi; // can be undefined
 	if (!version) { // this code should never need to be updated
 		if (versionArray[0] === 9 && versionArray[1] >= 3) version = 2; // 9.3.0+
@@ -37,18 +39,20 @@ module.exports.get_napi_version = function() {
 	return version;
 };
 
-module.exports.get_napi_version_as_string = function() {
+module.exports.get_napi_version_as_string = function(target) {
 	// returns the napi version as a string or an empty string if napi is not supported.
-	var version = module.exports.get_napi_version();
+	var version = module.exports.get_napi_version(target);
 	return version ? ''+version : '';
 };
 
-module.exports.validate_package_json = function(package_json) { // return err
+module.exports.validate_package_json = function(package_json, opts) { // throws Error
+
 	var binary = package_json.binary;
-	var module_path_ok = binary.module_path && binary.module_path.indexOf('{napi_build_version}') !== -1;
-	var remote_path_ok = binary.remote_path && binary.remote_path.indexOf('{napi_build_version}') !== -1;
-	var package_name_ok = binary.package_name && binary.package_name.indexOf('{napi_build_version}') !== -1;
-	var napi_build_versions = module.exports.get_napi_build_versions(package_json);
+	var module_path_ok = pathOK(binary.module_path);
+	var remote_path_ok = pathOK(binary.remote_path);
+	var package_name_ok = pathOK(binary.package_name);
+	var napi_build_versions = module.exports.get_napi_build_versions(package_json,opts,true);
+	var napi_build_versions_raw = module.exports.get_napi_build_versions_raw(package_json);
 
 	if (napi_build_versions) {
 		napi_build_versions.forEach(function(napi_build_version){
@@ -63,29 +67,41 @@ module.exports.validate_package_json = function(package_json) { // return err
 			"package_name must contain the substitution string '{napi_build_version}`.");
 	}
 
-	if ((module_path_ok || remote_path_ok || package_name_ok) && !napi_build_versions) {
+	if ((module_path_ok || remote_path_ok || package_name_ok) && !napi_build_versions_raw) {
 		throw new Error("When the substitution string '{napi_build_version}` is specified in " +
 			"module_path, remote_path, or package_name; napi_versions must also be specified.");
 	}
 
-	if (napi_build_versions && !module.exports.get_best_napi_build_version(package_json)) {
+	if (napi_build_versions && !module.exports.get_best_napi_build_version(package_json, opts) && 
+	module.exports.build_napi_only(package_json)) {
+		throw new Error(
+			'The N-API version of this Node instance is ' + module.exports.get_napi_version(opts ? opts.target : undefined) + '. ' +
+			'This module supports N-API version(s) ' + module.exports.get_napi_build_versions_raw(package_json) + '. ' +
+			'This Node instance cannot run this module.');
+	}
+
+	if (napi_build_versions_raw && !napi_build_versions && module.exports.build_napi_only(package_json)) {
 		throw new Error(
-			'The N-API version of this Node instance is ' + module.exports.get_napi_version() + '. ' +
-			'This module supports N-API version(s) ' + module.exports.get_napi_build_versions(package_json) + '. ' +
+			'The N-API version of this Node instance is ' + module.exports.get_napi_version(opts ? opts.target : undefined) + '. ' +
+			'This module supports N-API version(s) ' + module.exports.get_napi_build_versions_raw(package_json) + '. ' +
 			'This Node instance cannot run this module.');
 	}
 
 };
 
-module.exports.expand_commands = function(package_json, commands) {
+function pathOK (path) {
+	return path && (path.indexOf('{napi_build_version}') !== -1 || path.indexOf('{node_napi_label}') !== -1);
+}
+
+module.exports.expand_commands = function(package_json, opts, commands) {
 	var expanded_commands = [];
-	var napi_build_versions = module.exports.get_napi_build_versions(package_json);
+	var napi_build_versions = module.exports.get_napi_build_versions(package_json, opts);
 	commands.forEach(function(command){
 		if (napi_build_versions && command.name === 'install') {
-			var napi_build_version = module.exports.get_best_napi_build_version(package_json);
+			var napi_build_version = module.exports.get_best_napi_build_version(package_json, opts);
 			var args = napi_build_version ? [ napi_build_version_tag+napi_build_version ] : [ ];
 			expanded_commands.push ({ name: command.name, args: args });
-		} else if (napi_build_versions && napi_multiple_commands.includes(command.name)) {
+		} else if (napi_build_versions && napi_multiple_commands.indexOf(command.name) !== -1) {
 			napi_build_versions.forEach(function(napi_build_version){
 				var args = command.args.slice();
 				args.push (napi_build_version_tag+napi_build_version);
@@ -98,11 +114,38 @@ module.exports.expand_commands = function(package_json, commands) {
 	return expanded_commands;
 };
 
-module.exports.get_napi_build_versions = function(package_json) {
+module.exports.get_napi_build_versions = function(package_json, opts, warnings) { // opts may be undefined
+	var napi_build_versions = [];
+	var supported_napi_version = module.exports.get_napi_version(opts ? opts.target : undefined);
+	// remove duplicates, verify each napi version can actaully be built
+	if (package_json.binary && package_json.binary.napi_versions) {
+		package_json.binary.napi_versions.forEach(function(napi_version) {
+			var duplicated = napi_build_versions.indexOf(napi_version) !== -1;
+			if (!duplicated && supported_napi_version && napi_version <= supported_napi_version) {
+				napi_build_versions.push(napi_version);
+			} else if (warnings && !duplicated && supported_napi_version) {
+				log.info('This Node instance does not support builds for N-API version', napi_version);
+			}
+		});
+	}
+	if (opts && opts["build-latest-napi-version-only"]) {
+		var latest_version = 0;
+		napi_build_versions.forEach(function(napi_version) {
+			if (napi_version > latest_version) latest_version = napi_version;
+		});
+		napi_build_versions = latest_version ? [ latest_version ] : [];
+	}
+	return napi_build_versions.length ? napi_build_versions : undefined;
+};
+
+module.exports.get_napi_build_versions_raw = function(package_json) {
 	var napi_build_versions = [];
-	if (package_json.binary && package_json.binary.napi_versions) { // remove duplicates
+	// remove duplicates
+	if (package_json.binary && package_json.binary.napi_versions) {
 		package_json.binary.napi_versions.forEach(function(napi_version) {
-			if (!napi_build_versions.includes(napi_version)) napi_build_versions.push(napi_version);
+			if (napi_build_versions.indexOf(napi_version) === -1) {
+				napi_build_versions.push(napi_version);
+			}
 		});
 	}
 	return napi_build_versions.length ? napi_build_versions : undefined;
@@ -140,11 +183,11 @@ module.exports.get_build_dir = function(napi_build_version) {
 	return 'build-tmp-napi-v'+napi_build_version;
 };
 
-module.exports.get_best_napi_build_version = function(package_json) {
+module.exports.get_best_napi_build_version = function(package_json, opts) {
 	var best_napi_build_version = 0;
-	var napi_build_versions = module.exports.get_napi_build_versions (package_json);
+	var napi_build_versions = module.exports.get_napi_build_versions (package_json, opts);
 	if (napi_build_versions) {
-		var our_napi_version = module.exports.get_napi_version();
+		var our_napi_version = module.exports.get_napi_version(opts ? opts.target : undefined);
 		napi_build_versions.forEach(function(napi_build_version){
 			if (napi_build_version > best_napi_build_version &&
 				napi_build_version <= our_napi_version) {
@@ -154,3 +197,8 @@ module.exports.get_best_napi_build_version = function(package_json) {
 	}
 	return best_napi_build_version === 0 ? undefined : best_napi_build_version;
 };
+
+module.exports.build_napi_only = function(package_json) {
+	return package_json.binary && package_json.binary.package_name && 
+	package_json.binary.package_name.indexOf('{node_napi_label}') === -1;
+};

lib/util/versioning.js

@@ -192,7 +192,7 @@ var required_parameters = [
     'host'
 ];
 
-function validate_config(package_json) {
+function validate_config(package_json,opts) {
     var msg = package_json.name + ' package.json is not node-pre-gyp ready:\n';
     var missing = [];
     if (!package_json.main) {
@@ -226,7 +226,7 @@ function validate_config(package_json) {
             throw new Error("'host' protocol ("+protocol+") is invalid - only 'https:' is accepted");
         }
     }
-    napi.validate_package_json(package_json);
+    napi.validate_package_json(package_json,opts);
 }
 
 module.exports.validate_config = validate_config;
@@ -276,7 +276,7 @@ var default_remote_path = '';
 
 module.exports.evaluate = function(package_json,options,napi_build_version) {
     options = options || {};
-    validate_config(package_json);
+    validate_config(package_json,options); // options is a suitable substitute for opts in this case
     var v = package_json.version;
     var module_version = semver.parse(v);
     var runtime = options.runtime || get_process_runtime(process.versions);
@@ -293,9 +293,10 @@ module.exports.evaluate = function(package_json,options,napi_build_version) {
         patch: module_version.patch,
         runtime: runtime,
         node_abi: get_runtime_abi(runtime,options.target),
-        node_abi_napi: napi.get_napi_version() ? 'napi' : get_runtime_abi(runtime,options.target),
-        napi_version: napi.get_napi_version(), // non-zero numeric, undefined if unsupported
-        napi_build_version: napi_build_version, // undefined if not specified
+        node_abi_napi: napi.get_napi_version(options.target) ? 'napi' : get_runtime_abi(runtime,options.target),
+        napi_version: napi.get_napi_version(options.target), // non-zero numeric, undefined if unsupported
+        napi_build_version: napi_build_version || '',
+        node_napi_label: napi_build_version ? 'napi-v' + napi_build_version : get_runtime_abi(runtime,options.target),
         target: options.target || '',
         platform: options.target_platform || process.platform,
         target_platform: options.target_platform || process.platform,