From 8bc03dc36ebc63e14a3a70cc9b49f9b7b0066ad6 Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Wed, 31 Jan 2018 18:19:23 -0800 Subject: [PATCH 01/10] Initial commit. --- lib/build.js | 7 ++ lib/clean.js | 6 +- lib/configure.js | 4 + lib/install.js | 4 +- lib/node-pre-gyp.js | 9 +++ lib/package.js | 4 +- lib/publish.js | 4 +- lib/rebuild.js | 16 +++- lib/reinstall.js | 9 ++- lib/reveal.js | 4 +- lib/testbinary.js | 4 +- lib/testpackage.js | 4 +- lib/unpublish.js | 4 +- lib/util/handle_gyp_opts.js | 9 ++- lib/util/napi.js | 156 ++++++++++++++++++++++++++++++++++++ lib/util/versioning.js | 7 +- 16 files changed, 234 insertions(+), 17 deletions(-) create mode 100644 lib/util/napi.js diff --git a/lib/build.js b/lib/build.js index 1074fa3b..63bceb8b 100644 --- a/lib/build.js +++ b/lib/build.js @@ -4,6 +4,7 @@ module.exports = exports = build; exports.usage = 'Attempts to compile the module by dispatching to node-gyp or nw-gyp'; +var napi = require('./util/napi.js'); var compile = require('./util/compile.js'); var handle_gyp_opts = require('./util/handle_gyp_opts.js'); var configure = require('./configure.js'); @@ -16,7 +17,13 @@ function do_build(gyp,argv,callback) { concat(['--']). concat(result.unparsed); } + if (result.opts.napi_build_version) { + napi.swap_build_dir_in(result.opts.napi_build_version); + } compile.run_gyp(final_args,result.opts,function(err) { + if (result.opts.napi_build_version) { + napi.swap_build_dir_out(result.opts.napi_build_version); + } return callback(err); }); }); diff --git a/lib/clean.js b/lib/clean.js index a1289f68..0001c94a 100644 --- a/lib/clean.js +++ b/lib/clean.js @@ -8,11 +8,13 @@ var fs = require('fs'); var rm = require('rimraf'); var exists = require('fs').exists || require('path').exists; var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); function clean (gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); - var to_delete = opts.module_path; + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); + var to_delete = napi_build_version ? napi.get_build_dir(napi_build_version) : opts.module_path; exists(to_delete, function(found) { if (found) { if (!gyp.opts.silent_clean) console.log('['+package_json.name+'] Removing "%s"', to_delete); diff --git a/lib/configure.js b/lib/configure.js index 1ea56642..c64b292e 100644 --- a/lib/configure.js +++ b/lib/configure.js @@ -4,6 +4,7 @@ module.exports = exports = configure; exports.usage = 'Attempts to configure node-gyp or nw-gyp build'; +var napi = require('./util/napi.js'); var compile = require('./util/compile.js'); var handle_gyp_opts = require('./util/handle_gyp_opts.js'); @@ -41,6 +42,9 @@ function configure(gyp, argv, callback) { concat(result.unparsed); } compile.run_gyp(['configure'].concat(final_args),result.opts,function(err) { + if (result.opts.napi_build_version) { + napi.swap_build_dir_out(result.opts.napi_build_version); + } return callback(err); }); } diff --git a/lib/install.js b/lib/install.js index 009c357a..fae9d1dd 100644 --- a/lib/install.js +++ b/lib/install.js @@ -10,6 +10,7 @@ var zlib = require('zlib'); var log = require('npmlog'); var existsAsync = fs.exists || path.exists; var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); var npgVersion = 'unknown'; try { @@ -156,6 +157,7 @@ function print_fallback_error(err,opts,package_json) { function install(gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var source_build = gyp.opts['build-from-source'] || gyp.opts.build_from_source; var update_binary = gyp.opts['update-binary'] || gyp.opts.update_binary; var should_do_source_build = source_build === package_json.name || (source_build === true || source_build === 'true'); @@ -176,7 +178,7 @@ function install(gyp, argv, callback) { } var opts; try { - opts = versioning.evaluate(package_json, gyp.opts); + opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); } catch (err) { return callback(err); } diff --git a/lib/node-pre-gyp.js b/lib/node-pre-gyp.js index feb963fa..3cf04c3f 100644 --- a/lib/node-pre-gyp.js +++ b/lib/node-pre-gyp.js @@ -10,10 +10,12 @@ module.exports = exports; * Module dependencies. */ +var fs = require('fs'); var path = require('path'); var nopt = require('nopt'); var log = require('npmlog'); log.disableProgress(); +var napi = require('./util/napi.js'); var EE = require('events').EventEmitter; var inherits = require('util').inherits; @@ -128,6 +130,13 @@ proto.parseArgv = function parseOpts (argv) { commands[commands.length - 1].args = argv.splice(0); } + // expand commands entries for multiple napi builds + var dir = this.opts.directory; + 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); + // support for inheriting config env variables from npm var npm_config_prefix = 'npm_config_'; Object.keys(process.env).forEach(function (name) { diff --git a/lib/package.js b/lib/package.js index 3a35f653..5c9b514c 100644 --- a/lib/package.js +++ b/lib/package.js @@ -8,6 +8,7 @@ var fs = require('fs'); var path = require('path'); var log = require('npmlog'); var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); var write = require('fs').createWriteStream; var existsAsync = fs.exists || path.exists; var mkdirp = require('mkdirp'); @@ -15,7 +16,8 @@ var mkdirp = require('mkdirp'); function _package(gyp, argv, callback) { var pack = require('tar-pack').pack; var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var from = opts.module_path; var binary_module = path.join(from,opts.module_name + '.node'); existsAsync(binary_module,function(found) { diff --git a/lib/publish.js b/lib/publish.js index d666b013..4829dfea 100644 --- a/lib/publish.js +++ b/lib/publish.js @@ -8,6 +8,7 @@ var fs = require('fs'); var path = require('path'); var log = require('npmlog'); var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); var s3_setup = require('./util/s3_setup.js'); var existsAsync = fs.exists || path.exists; var url = require('url'); @@ -16,7 +17,8 @@ var config = require('rc')("node_pre_gyp",{acl:"public-read"}); function publish(gyp, argv, callback) { var AWS = require("aws-sdk"); var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var tarball = opts.staged_tarball; existsAsync(tarball,function(found) { if (!found) { diff --git a/lib/rebuild.js b/lib/rebuild.js index 48a7b74b..214c15e5 100644 --- a/lib/rebuild.js +++ b/lib/rebuild.js @@ -4,10 +4,18 @@ module.exports = exports = rebuild; exports.usage = 'Runs "clean" and "build" at once'; +var fs = require('fs'); +var napi = require('./util/napi.js'); + function rebuild (gyp, argv, callback) { - gyp.todo.unshift( - { name: 'clean', args: [] }, - { name: 'build', args: ['rebuild'] } - ); + var package_json = JSON.parse(fs.readFileSync('./package.json')); + var commands = [ + { name: 'clean', args: [] }, + { name: 'build', args: ['rebuild'] } + ]; + commands = napi.expand_commands(package_json, commands) + for (var i = commands.length; i !== 0; i--) { + gyp.todo.unshift(commands[i-1]); + } process.nextTick(callback); } diff --git a/lib/reinstall.js b/lib/reinstall.js index ed65d54e..52e17aea 100644 --- a/lib/reinstall.js +++ b/lib/reinstall.js @@ -4,10 +4,17 @@ module.exports = exports = rebuild; exports.usage = 'Runs "clean" and "install" at once'; +var fs = require('fs'); +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_version(package_json); + if (napi_build_version != null) installArgs = [ napi.get_command_arg (napi_build_version) ]; gyp.todo.unshift( { name: 'clean', args: [] }, - { name: 'install', args: [] } + { name: 'install', args: installArgs } ); process.nextTick(callback); } diff --git a/lib/reveal.js b/lib/reveal.js index e6d00eb3..11e26fd1 100644 --- a/lib/reveal.js +++ b/lib/reveal.js @@ -6,6 +6,7 @@ exports.usage = 'Reveals data on the versioned binary'; var fs = require('fs'); var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); function unix_paths(key, val) { return val && val.replace ? val.replace(/\\/g, '/') : val; @@ -13,7 +14,8 @@ function unix_paths(key, val) { function reveal(gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var hit = false; // if a second arg is passed look to see // if it is a known option diff --git a/lib/testbinary.js b/lib/testbinary.js index ff9542e5..bf2ead57 100644 --- a/lib/testbinary.js +++ b/lib/testbinary.js @@ -9,6 +9,7 @@ var path = require('path'); var log = require('npmlog'); var cp = require('child_process'); var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); var path = require('path'); function testbinary(gyp, argv, callback) { @@ -16,7 +17,8 @@ function testbinary(gyp, argv, callback) { var options = {}; var shell_cmd = process.execPath; var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); // skip validation for runtimes we don't explicitly support (like electron) if (opts.runtime && opts.runtime !== 'node-webkit' && diff --git a/lib/testpackage.js b/lib/testpackage.js index 1d4cc607..fddbd44e 100644 --- a/lib/testpackage.js +++ b/lib/testpackage.js @@ -9,13 +9,15 @@ var path = require('path'); var log = require('npmlog'); var existsAsync = fs.exists || path.exists; var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); var testbinary = require('./testbinary.js'); var read = require('fs').createReadStream; var zlib = require('zlib'); function testpackage(gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var tarball = opts.staged_tarball; existsAsync(tarball, function(found) { if (!found) { diff --git a/lib/unpublish.js b/lib/unpublish.js index 43f8e66d..36652937 100644 --- a/lib/unpublish.js +++ b/lib/unpublish.js @@ -7,6 +7,7 @@ exports.usage = 'Unpublishes pre-built binary (requires aws-sdk)'; var fs = require('fs'); var log = require('npmlog'); var versioning = require('./util/versioning.js'); +var napi = require('./util/napi.js'); var s3_setup = require('./util/s3_setup.js'); var url = require('url'); var config = require('rc')("node_pre_gyp",{acl:"public-read"}); @@ -14,7 +15,8 @@ var config = require('rc')("node_pre_gyp",{acl:"public-read"}); function unpublish(gyp, argv, callback) { var AWS = require("aws-sdk"); var package_json = JSON.parse(fs.readFileSync('./package.json')); - var opts = versioning.evaluate(package_json, gyp.opts); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); s3_setup.detect(opts.hosted_path,config); AWS.config.update(config); var key_name = url.resolve(config.prefix,opts.package_name); diff --git a/lib/util/handle_gyp_opts.js b/lib/util/handle_gyp_opts.js index 39fe1a28..3f0acb89 100644 --- a/lib/util/handle_gyp_opts.js +++ b/lib/util/handle_gyp_opts.js @@ -4,6 +4,7 @@ module.exports = exports = handle_gyp_opts; var fs = require('fs'); var versioning = require('./versioning.js'); +var napi = require('./napi.js'); /* @@ -44,6 +45,9 @@ var share_with_node_gyp = [ 'module', 'module_name', 'module_path', + 'napi_version', + `node_abi_napi`, + 'napi_build_version' ]; function handle_gyp_opts(gyp, argv, callback) { @@ -51,8 +55,9 @@ function handle_gyp_opts(gyp, argv, callback) { // Collect node-pre-gyp specific variables to pass to node-gyp var node_pre_gyp_options = []; // generate custom node-pre-gyp versioning info - var opts = versioning.evaluate(JSON.parse(fs.readFileSync('./package.json')), gyp.opts); - share_with_node_gyp.forEach(function(key) { + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var opts = versioning.evaluate(JSON.parse(fs.readFileSync('./package.json')), gyp.opts, napi_build_version); + share_with_node_gyp.forEach(function(key) { var val = opts[key]; if (val) { node_pre_gyp_options.push('--' + key + '=' + val); diff --git a/lib/util/napi.js b/lib/util/napi.js new file mode 100644 index 00000000..b6fa0443 --- /dev/null +++ b/lib/util/napi.js @@ -0,0 +1,156 @@ +"use strict"; + +var fs = require('fs'); +var rm = require('rimraf'); + +module.exports = exports; + +var versionArray = process.version + .substr(1) + .replace(/-.*$/, '') + .split('.') + .map(function(item) { + return +item; + }); + +var napi_multiple_commands = [ + 'build', + 'clean', + 'configure', + 'package', + 'publish', + 'reveal', + 'testbinary', + 'testpackage', + 'unpublish' +]; + +var napi_build_version_tag = 'napi_build_version='; + +module.exports.get_napi_version = function() { + // returns the non-zero numeric napi version or undefined if napi is not supported. + var version = process.versions.napi; // can be undefined + if (!version) { + if (versionArray[0] === 9 && versionArray[1] >= 3) version = 2; // 9.3.0+ + else if (versionArray[0] === 8) version = 1; // 8.0.0+ + } + return version; +}; + +module.exports.get_napi_version_as_string = function() { + // returns the napi version as a string or an empty string if napi is not supported. + var version = module.exports.get_napi_version(); + return version ? ''+version : ''; +}; + +module.exports.validate_package_json = function(package_json) { // return err + var binary = package_json.binary; + var module_path_ok = binary.module_path && binary.module_path.includes('{napi_build_version}'); + var remote_path_ok = binary.remote_path && binary.remote_path.includes('{napi_build_version}'); + var package_name_ok = binary.package_name && binary.package_name.includes('{napi_build_version}'); + var napi_build_versions = get_napi_build_versions(package_json); + + if (napi_build_versions) { + napi_build_versions.forEach(function(napi_build_version){ + if (!((parseInt(napi_build_version) === napi_build_version) && (napi_build_version > 0))) { + throw new Error("All values specified in napi_versions must be positive integers."); + } + }); + } + + if (napi_build_versions && (!module_path_ok || (!remote_path_ok && !package_name_ok))) { + throw new Error("When napi_versions is specified; module_path and either remote_path or " + + "package_name must contain the substitution string '{napi_build_version}`."); + } + + if ((module_path_ok || remote_path_ok || package_name_ok) && !napi_build_versions) { + 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)) { + throw new Error( + 'The N-API version of this Node instance is ' + module.exports.get_napi_version() + '. ' + + 'This module supports N-API version(s) ' + get_napi_build_versions(package_json) + '. ' + + 'This Node instance cannot run this module.'); + } + +}; + +module.exports.expand_commands = function(package_json, commands) { + var expanded_commands = []; + var napi_build_versions = get_napi_build_versions(package_json); + 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 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)) { + napi_build_versions.forEach(function(napi_build_version){ + var args = command.args.slice(); + args.push (napi_build_version_tag+napi_build_version); + expanded_commands.push ({ name: command.name, args: args }); + }); + } else { + expanded_commands.push (command); + } + }); + return expanded_commands; +}; + +function get_napi_build_versions (package_json) { + var napi_build_versions = []; + if (package_json.binary.napi_versions) { // remove duplicates + package_json.binary.napi_versions.forEach(function(napi_version) { + if (!napi_build_versions.includes(napi_version)) napi_build_versions.push(napi_version); + }); + } + return napi_build_versions.length ? napi_build_versions : undefined; +}; + +module.exports.get_command_arg = function(napi_build_version) { + return napi_build_version_tag + napi_build_version; +}; + +module.exports.get_napi_build_version_from_command_args = function(command_args) { + for (var i = 0; i < command_args.length; i++) { + var arg = command_args[i]; + if (arg.indexOf(napi_build_version_tag) === 0) { + return parseInt(arg.substr(napi_build_version_tag.length)); + } + } + return undefined; +}; + +module.exports.swap_build_dir_out = function(napi_build_version) { + if (napi_build_version) { + rm.sync(module.exports.get_build_dir(napi_build_version)); + fs.renameSync('build', module.exports.get_build_dir(napi_build_version)); + } +}; + +module.exports.swap_build_dir_in = function(napi_build_version) { + if (napi_build_version) { + rm.sync('build'); + fs.renameSync(module.exports.get_build_dir(napi_build_version), 'build'); + } +}; + +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) { + var best_napi_build_version = 0; + var napi_build_versions = get_napi_build_versions (package_json); + if (napi_build_versions) { + var our_napi_version = module.exports.get_napi_version(); + napi_build_versions.forEach(function(napi_build_version){ + if (napi_build_version > best_napi_build_version && + napi_build_version <= our_napi_version) { + best_napi_build_version = napi_build_version; + } + }); + } + return best_napi_build_version === 0 ? undefined : best_napi_build_version; +}; diff --git a/lib/util/versioning.js b/lib/util/versioning.js index 0fd57031..caf81482 100644 --- a/lib/util/versioning.js +++ b/lib/util/versioning.js @@ -6,6 +6,7 @@ var path = require('path'); var semver = require('semver'); var url = require('url'); var detect_libc = require('detect-libc'); +var napi = require('./napi.js'); var abi_crosswalk; @@ -225,6 +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); } module.exports.validate_config = validate_config; @@ -272,7 +274,7 @@ module.exports.get_process_runtime = get_process_runtime; var default_package_name = '{module_name}-v{version}-{node_abi}-{platform}-{arch}.tar.gz'; var default_remote_path = ''; -module.exports.evaluate = function(package_json,options) { +module.exports.evaluate = function(package_json,options,napi_build_version) { options = options || {}; validate_config(package_json); var v = package_json.version; @@ -291,6 +293,9 @@ module.exports.evaluate = function(package_json,options) { 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 target: options.target || '', platform: options.target_platform || process.platform, target_platform: options.target_platform || process.platform, From 35c20f755de76a10408a95f74878c5a4a75dea62 Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Thu, 1 Feb 2018 17:56:28 -0800 Subject: [PATCH 02/10] Installation fixes, app7 test app, README.md --- README.md | 75 ++++++++++++++++++++++++++++++++++++++++++ lib/build.js | 1 + lib/install.js | 3 +- lib/pre-binding.js | 7 +++- lib/util/napi.js | 10 +++--- test/app7/.gitignore | 5 +++ test/app7/README.md | 3 ++ test/app7/app7.cc | 24 ++++++++++++++ test/app7/binding.gyp | 13 ++++++++ test/app7/index.js | 6 ++++ test/app7/package.json | 27 +++++++++++++++ test/build.test.js | 12 ++++--- 12 files changed, 175 insertions(+), 11 deletions(-) create mode 100644 test/app7/.gitignore create mode 100644 test/app7/README.md create mode 100644 test/app7/app7.cc create mode 100644 test/app7/binding.gyp create mode 100644 test/app7/index.js create mode 100644 test/app7/package.json diff --git a/README.md b/README.md index 0b2b0f39..0eba3a33 100644 --- a/README.md +++ b/README.md @@ -285,6 +285,81 @@ What will happen is this: If a a binary was not available for a given platform and `--fallback-to-build` was used then `node-gyp rebuild` will be called to try to source compile the module. +## N-API Considerations + +[N-API](https://nodejs.org/api/n-api.html#n_api_n_api) is an ABI-stable alternative to previous technologies such as [nan](https://github.com/nodejs/nan) which are tied to a specific Node runtime engine. N-API is Node runtime engine agnostic and guarantees modules created today will continue to run, without changes, into the future. + +Using `node-pre-gyp` with N-API projects requires a handful of additional congiguration values and imposes some additional requirements. + +The most significant difference is that an N-API module can be coded to target multiple N-API versions. Therefore, an N-API module must declare in its `package.json` file which N-API versions the module is designed to run against. In addition, since multiple builds may be required for a single module, path and file names must be specified in way that avoids naming conflicts. + +### The `napi_versions` array property + +An N-API modules must declare in its `package.json` file, the N-API versions the module is intended to support. This is accomplished by including an `napi-versions` array property in the `binary` object. For example: + +```js +"binary": { + "module_name": "your_module", + "module_path": "your_module_path", + "host": "https://your_bucket.s3-us-west-1.amazonaws.com", + "napi_versions": [1,3] + } +``` + +If the `napi_versions` array property is *not* present, `node-pre-gyp` operates as it always has. Including the `napi_versions` array property instructs `node-pre-gyp` that this is a N-API module build. + +When the `napi_versions` array property is present, `node-pre-gyp` fires off multiple operations, one for each of the N-API versions in the array. In the example above, two operations are initiated, one for N-API version 1 and second for N-API version 3. How this version number is communicated is described next. + +### The `napi_build_version` value + +For each of the N-API module operations `node-pre-gyp` initiates, it insures that the `napi_build_version` is set appropriately. + +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 + +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)", +] +``` + +This insures that `NAPI_BUILD_VERSION`, an integer value, is declared appropriately to the C/C++ code for each build. + +### 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.) + +Here's an example: + +```js +"binary": { + "module_name": "your_module", + "module_path": "./lib/binding/napi-v{napi_build_version}", + "remote_path": "./{module_name}/v{version}/{configuration}/", + "package_name": "{platform}-{arch}-napi-v{napi_build_version}.tar.gz", + "host": "https://your_bucket.s3-us-west-1.amazonaws.com", + "napi_versions": [1,3] + } +``` + +### Two additional configuration values + +For those who need them in legacy projects, two additional configuration values are available for all builds. + +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. + +2. `node_abi_napi` If the value returned for `napi_version` is non empty, this value is `'napi'`. If the value returned for `napi_version` is empty, this value is the value returned for `node_abi`. + +These values are present for use in the `binding.gyp` file and may be used as `{napi_version}` and `{node_abi_napi}` for text substituion in the `package.json` file. + ## S3 Hosting You can host wherever you choose but S3 is cheap, `node-pre-gyp publish` expects it, and S3 can be integrated well with [Travis.ci](http://travis-ci.org) to automate builds for OS X and Ubuntu, and with [Appveyor](http://appveyor.com) to automate builds for Windows. Here is an approach to do this: diff --git a/lib/build.js b/lib/build.js index 63bceb8b..028d4d23 100644 --- a/lib/build.js +++ b/lib/build.js @@ -35,6 +35,7 @@ function build(gyp, argv, callback) { // We map `node-pre-gyp build` to `node-gyp configure build` so that we do not // trigger a clean and therefore do not pay the penalty of a full recompile if (argv.length && (argv.indexOf('rebuild') > -1)) { + argv.shift(); // remove `rebuild` // here we map `node-pre-gyp rebuild` to `node-gyp rebuild` which internally means // "clean + configure + build" and triggers a full recompile compile.run_gyp(['clean'],{},function(err) { diff --git a/lib/install.js b/lib/install.js index fae9d1dd..20599ce5 100644 --- a/lib/install.js +++ b/lib/install.js @@ -127,7 +127,8 @@ function place_binary(from,to,opts,callback) { } function do_build(gyp,argv,callback) { - gyp.todo.push( { name: 'build', args: ['rebuild'] } ); + var args = ['rebuild'].concat(argv); + gyp.todo.push( { name: 'build', args: args } ); process.nextTick(callback); } diff --git a/lib/pre-binding.js b/lib/pre-binding.js index 326a486c..bc69d5ed 100644 --- a/lib/pre-binding.js +++ b/lib/pre-binding.js @@ -1,6 +1,7 @@ "use strict"; var versioning = require('../lib/util/versioning.js'); +var napi = require('../lib/util/napi.js'); var existsSync = require('fs').existsSync || require('path').existsSync; var path = require('path'); @@ -18,8 +19,12 @@ exports.find = function(package_json_path,opts) { } var package_json = require(package_json_path); versioning.validate_config(package_json); + var napi_build_version; + if (napi.get_napi_build_versions (package_json)) { + napi_build_version = napi.get_best_napi_build_version(package_json); + } opts = opts || {}; if (!opts.module_root) opts.module_root = path.dirname(package_json_path); - var meta = versioning.evaluate(package_json,opts); + var meta = versioning.evaluate(package_json,opts,napi_build_version); return meta.module; }; diff --git a/lib/util/napi.js b/lib/util/napi.js index b6fa0443..f41aa096 100644 --- a/lib/util/napi.js +++ b/lib/util/napi.js @@ -48,7 +48,7 @@ module.exports.validate_package_json = function(package_json) { // return err var module_path_ok = binary.module_path && binary.module_path.includes('{napi_build_version}'); var remote_path_ok = binary.remote_path && binary.remote_path.includes('{napi_build_version}'); var package_name_ok = binary.package_name && binary.package_name.includes('{napi_build_version}'); - var napi_build_versions = get_napi_build_versions(package_json); + var napi_build_versions = module.exports.get_napi_build_versions(package_json); if (napi_build_versions) { napi_build_versions.forEach(function(napi_build_version){ @@ -71,7 +71,7 @@ module.exports.validate_package_json = function(package_json) { // return err if (napi_build_versions && !module.exports.get_best_napi_build_version(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) ' + get_napi_build_versions(package_json) + '. ' + + 'This module supports N-API version(s) ' + module.exports.get_napi_build_versions(package_json) + '. ' + 'This Node instance cannot run this module.'); } @@ -79,7 +79,7 @@ module.exports.validate_package_json = function(package_json) { // return err module.exports.expand_commands = function(package_json, commands) { var expanded_commands = []; - var napi_build_versions = get_napi_build_versions(package_json); + var napi_build_versions = module.exports.get_napi_build_versions(package_json); commands.forEach(function(command){ if (napi_build_versions && command.name === 'install') { var napi_build_version = module.exports.get_best_napi_build_version(package_json); @@ -98,7 +98,7 @@ module.exports.expand_commands = function(package_json, commands) { return expanded_commands; }; -function get_napi_build_versions (package_json) { +module.exports.get_napi_build_versions = function(package_json) { var napi_build_versions = []; if (package_json.binary.napi_versions) { // remove duplicates package_json.binary.napi_versions.forEach(function(napi_version) { @@ -142,7 +142,7 @@ module.exports.get_build_dir = function(napi_build_version) { module.exports.get_best_napi_build_version = function(package_json) { var best_napi_build_version = 0; - var napi_build_versions = get_napi_build_versions (package_json); + var napi_build_versions = module.exports.get_napi_build_versions (package_json); if (napi_build_versions) { var our_napi_version = module.exports.get_napi_version(); napi_build_versions.forEach(function(napi_build_version){ diff --git a/test/app7/.gitignore b/test/app7/.gitignore new file mode 100644 index 00000000..f6a05031 --- /dev/null +++ b/test/app7/.gitignore @@ -0,0 +1,5 @@ +.DS_Store +build/ +lib/binding/ +node_modules +npm-debug.log \ No newline at end of file diff --git a/test/app7/README.md b/test/app7/README.md new file mode 100644 index 00000000..f47278a3 --- /dev/null +++ b/test/app7/README.md @@ -0,0 +1,3 @@ +# Test app + +Demonstrates using N-API to return a JavaScript string object. diff --git a/test/app7/app7.cc b/test/app7/app7.cc new file mode 100644 index 00000000..6e6510f7 --- /dev/null +++ b/test/app7/app7.cc @@ -0,0 +1,24 @@ +// https://github.com/nodejs/abi-stable-node-addon-examples/blob/master/1_hello_world/napi/hello.cc +#include +#include + +napi_value Method(napi_env env, napi_callback_info info) { + napi_status status; + napi_value world; + status = napi_create_string_utf8(env, "world", 5, &world); + assert(status == napi_ok); + return world; +} + +#define DECLARE_NAPI_METHOD(name, func) \ + { name, 0, func, 0, 0, 0, napi_default, 0 } + +napi_value Init(napi_env env, napi_value exports) { + napi_status status; + napi_property_descriptor desc = DECLARE_NAPI_METHOD("hello", Method); + status = napi_define_properties(env, exports, 1, &desc); + assert(status == napi_ok); + return exports; +} + +NAPI_MODULE(hello, Init) diff --git a/test/app7/binding.gyp b/test/app7/binding.gyp new file mode 100644 index 00000000..1ef2bc57 --- /dev/null +++ b/test/app7/binding.gyp @@ -0,0 +1,13 @@ +{ + "targets": [ + { + "target_name": "<(module_name)", + "sources": [ "<(module_name).cc" ], + 'product_dir': '<(module_path)', + "xcode_settings": { + "MACOSX_DEPLOYMENT_TARGET":"10.9", + "CLANG_CXX_LIBRARY": "libc++" + } + } + ] +} diff --git a/test/app7/index.js b/test/app7/index.js new file mode 100644 index 00000000..dccd8a0a --- /dev/null +++ b/test/app7/index.js @@ -0,0 +1,6 @@ +var binary = require('@inspiredware/node-pre-gyp'); +var path = require('path') +var binding_path = binary.find(path.resolve(path.join(__dirname,'./package.json'))); +var binding = require(binding_path); + +require('assert').equal(binding.hello(),"world"); \ No newline at end of file diff --git a/test/app7/package.json b/test/app7/package.json new file mode 100644 index 00000000..0f7a25c8 --- /dev/null +++ b/test/app7/package.json @@ -0,0 +1,27 @@ +{ + "name": "node-pre-gyp-test-app7", + "author": "Jim Schlight ", + "description": "node-pre-gyp napi test", + "repository": { + "type": "git", + "url": "git://github.com/mapbox/node-pre-gyp.git" + }, + "license": "BSD-3-Clause", + "version": "0.1.0", + "main": "./index.js", + "dependencies": { + "node-pre-gyp": "0.6.x" + }, + "binary": { + "module_name": "app7", + "module_path": "./lib/binding/napi-v{napi_build_version}", + "remote_path": "./{module_name}/v{version}/{configuration}/", + "package_name": "{module_name}-v{version}-{platform}-{arch}-napi-v{napi_build_version}.tar.gz", + "host": "https://node-pre-gyp-tests.s3-us-west-1.amazonaws.com", + "napi_versions": [1,2] + }, + "scripts": { + "install": "node-pre-gyp install --fallback-to-build", + "test": "node index.js" + } +} diff --git a/test/build.test.js b/test/build.test.js index 2352544e..09d443bf 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -26,10 +26,14 @@ var apps = [ 'name': 'app3', 'args': '' }, - { - 'name': 'app4', - 'args': '' - } + { + 'name': 'app4', + 'args': '' + }, + { + 'name': 'app7', + 'args': '' + } ]; From 411f5be3542e60ae8756c9c65b823bcd672df8d6 Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Fri, 2 Feb 2018 13:34:00 -0800 Subject: [PATCH 03/10] Windows fixes --- lib/build.js | 4 ++-- lib/configure.js | 2 +- lib/util/handle_gyp_opts.js | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/build.js b/lib/build.js index 028d4d23..36b6e5dd 100644 --- a/lib/build.js +++ b/lib/build.js @@ -17,11 +17,11 @@ function do_build(gyp,argv,callback) { concat(['--']). concat(result.unparsed); } - if (result.opts.napi_build_version) { + if (!err && result.opts.napi_build_version) { napi.swap_build_dir_in(result.opts.napi_build_version); } compile.run_gyp(final_args,result.opts,function(err) { - if (result.opts.napi_build_version) { + if (!err && result.opts.napi_build_version) { napi.swap_build_dir_out(result.opts.napi_build_version); } return callback(err); diff --git a/lib/configure.js b/lib/configure.js index c64b292e..a6e34382 100644 --- a/lib/configure.js +++ b/lib/configure.js @@ -42,7 +42,7 @@ function configure(gyp, argv, callback) { concat(result.unparsed); } compile.run_gyp(['configure'].concat(final_args),result.opts,function(err) { - if (result.opts.napi_build_version) { + if (!err && result.opts.napi_build_version) { napi.swap_build_dir_out(result.opts.napi_build_version); } return callback(err); diff --git a/lib/util/handle_gyp_opts.js b/lib/util/handle_gyp_opts.js index 3f0acb89..bdd5c75f 100644 --- a/lib/util/handle_gyp_opts.js +++ b/lib/util/handle_gyp_opts.js @@ -62,7 +62,8 @@ function handle_gyp_opts(gyp, argv, callback) { if (val) { node_pre_gyp_options.push('--' + key + '=' + val); } else { - return callback(new Error("Option " + key + " required but not found by node-pre-gyp")); + if (key !== 'napi_build_version') + return callback(new Error("Option " + key + " required but not found by node-pre-gyp")); } }); From 1122fdb620f5e05a7cee62d9e1a4cd1511a1bfb3 Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Fri, 2 Feb 2018 17:12:52 -0800 Subject: [PATCH 04/10] Fixes clean and app7 for automated testing --- lib/clean.js | 2 +- test/app7/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/clean.js b/lib/clean.js index 0001c94a..26caf133 100644 --- a/lib/clean.js +++ b/lib/clean.js @@ -14,7 +14,7 @@ function clean (gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); - var to_delete = napi_build_version ? napi.get_build_dir(napi_build_version) : opts.module_path; + var to_delete = opts.module_path; exists(to_delete, function(found) { if (found) { if (!gyp.opts.silent_clean) console.log('['+package_json.name+'] Removing "%s"', to_delete); diff --git a/test/app7/index.js b/test/app7/index.js index dccd8a0a..24d008da 100644 --- a/test/app7/index.js +++ b/test/app7/index.js @@ -1,4 +1,4 @@ -var binary = require('@inspiredware/node-pre-gyp'); +var binary = require('node-pre-gyp'); var path = require('path') var binding_path = binary.find(path.resolve(path.join(__dirname,'./package.json'))); var binding = require(binding_path); From f0719bdf8bf3c6549fa2fe04a25872eae0ac991f Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Mon, 5 Feb 2018 10:19:54 -0800 Subject: [PATCH 05/10] Fix for reveal command --- lib/reveal.js | 2 +- lib/util/napi.js | 2 +- test/app7/package.json | 3 +-- test/build.test.js | 1 + 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/reveal.js b/lib/reveal.js index 11e26fd1..cd387bb4 100644 --- a/lib/reveal.js +++ b/lib/reveal.js @@ -20,7 +20,7 @@ function reveal(gyp, argv, callback) { // if a second arg is passed look to see // if it is a known option //console.log(JSON.stringify(gyp.opts,null,1)) - var remain = gyp.opts.argv.remain.pop(); + var remain = gyp.opts.argv.remain[gyp.opts.argv.remain.length-1]; if (remain && opts.hasOwnProperty(remain)) { console.log(opts[remain].replace(/\\/g, '/')); hit = true; diff --git a/lib/util/napi.js b/lib/util/napi.js index f41aa096..fc1605a7 100644 --- a/lib/util/napi.js +++ b/lib/util/napi.js @@ -30,7 +30,7 @@ var napi_build_version_tag = 'napi_build_version='; module.exports.get_napi_version = function() { // returns the non-zero numeric napi version or undefined if napi is not supported. var version = process.versions.napi; // can be undefined - if (!version) { + if (!version) { // this code should never need to be updated if (versionArray[0] === 9 && versionArray[1] >= 3) version = 2; // 9.3.0+ else if (versionArray[0] === 8) version = 1; // 8.0.0+ } diff --git a/test/app7/package.json b/test/app7/package.json index 0f7a25c8..fa6d0bbf 100644 --- a/test/app7/package.json +++ b/test/app7/package.json @@ -21,7 +21,6 @@ "napi_versions": [1,2] }, "scripts": { - "install": "node-pre-gyp install --fallback-to-build", - "test": "node index.js" + "install": "node-pre-gyp install --fallback-to-build" } } diff --git a/test/build.test.js b/test/build.test.js index 09d443bf..b3ba1e24 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -176,6 +176,7 @@ apps.forEach(function(app) { run('node-pre-gyp', 'reveal', 'module_path --silent', app, {}, function(err,stdout,stderr) { t.ifError(err); var module_path = stdout.trim(); + if (module_path.includes('\n')) module_path = module_path.substr(0,module_path.indexOf('\n')); t.stringContains(module_path,app.name); t.ok(existsSync(module_path),'is valid path to existing binary: '+ module_path); var module_binary = path.join(module_path,app.name+'.node'); From 82a641e9de590bfb8b91140bbf1a2ac755aa7091 Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Wed, 7 Feb 2018 14:53:54 -0800 Subject: [PATCH 06/10] Code cleanup. --- lib/build.js | 12 ++++++------ lib/clean.js | 2 +- lib/publish.js | 2 +- lib/rebuild.js | 2 +- lib/reveal.js | 2 +- lib/testbinary.js | 2 +- lib/testpackage.js | 2 +- lib/unpublish.js | 2 +- lib/util/handle_gyp_opts.js | 4 ++-- lib/util/napi.js | 14 +++++++------- 10 files changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/build.js b/lib/build.js index 36b6e5dd..be97d66d 100644 --- a/lib/build.js +++ b/lib/build.js @@ -17,13 +17,13 @@ function do_build(gyp,argv,callback) { concat(['--']). concat(result.unparsed); } - if (!err && result.opts.napi_build_version) { - napi.swap_build_dir_in(result.opts.napi_build_version); - } + if (!err && result.opts.napi_build_version) { + 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) { - napi.swap_build_dir_out(result.opts.napi_build_version); - } + if (!err && result.opts.napi_build_version) { + napi.swap_build_dir_out(result.opts.napi_build_version); + } return callback(err); }); }); diff --git a/lib/clean.js b/lib/clean.js index 26caf133..d8436de5 100644 --- a/lib/clean.js +++ b/lib/clean.js @@ -12,7 +12,7 @@ var napi = require('./util/napi.js'); function clean (gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); - var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var to_delete = opts.module_path; exists(to_delete, function(found) { diff --git a/lib/publish.js b/lib/publish.js index 4829dfea..376e3984 100644 --- a/lib/publish.js +++ b/lib/publish.js @@ -17,7 +17,7 @@ var config = require('rc')("node_pre_gyp",{acl:"public-read"}); function publish(gyp, argv, callback) { var AWS = require("aws-sdk"); var package_json = JSON.parse(fs.readFileSync('./package.json')); - var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var tarball = opts.staged_tarball; existsAsync(tarball,function(found) { diff --git a/lib/rebuild.js b/lib/rebuild.js index 214c15e5..1e3a885c 100644 --- a/lib/rebuild.js +++ b/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, commands); for (var i = commands.length; i !== 0; i--) { gyp.todo.unshift(commands[i-1]); } diff --git a/lib/reveal.js b/lib/reveal.js index cd387bb4..13d2f725 100644 --- a/lib/reveal.js +++ b/lib/reveal.js @@ -14,7 +14,7 @@ function unix_paths(key, val) { function reveal(gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); - var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var hit = false; // if a second arg is passed look to see diff --git a/lib/testbinary.js b/lib/testbinary.js index bf2ead57..453987c3 100644 --- a/lib/testbinary.js +++ b/lib/testbinary.js @@ -17,7 +17,7 @@ function testbinary(gyp, argv, callback) { var options = {}; var shell_cmd = process.execPath; var package_json = JSON.parse(fs.readFileSync('./package.json')); - var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); // skip validation for runtimes we don't explicitly support (like electron) if (opts.runtime && diff --git a/lib/testpackage.js b/lib/testpackage.js index fddbd44e..2e1c2fea 100644 --- a/lib/testpackage.js +++ b/lib/testpackage.js @@ -16,7 +16,7 @@ var zlib = require('zlib'); function testpackage(gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); - var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); var tarball = opts.staged_tarball; existsAsync(tarball, function(found) { diff --git a/lib/unpublish.js b/lib/unpublish.js index 36652937..94c93dd8 100644 --- a/lib/unpublish.js +++ b/lib/unpublish.js @@ -15,7 +15,7 @@ var config = require('rc')("node_pre_gyp",{acl:"public-read"}); function unpublish(gyp, argv, callback) { var AWS = require("aws-sdk"); var package_json = JSON.parse(fs.readFileSync('./package.json')); - var napi_build_version = napi.get_napi_build_version_from_command_args(argv); + var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(package_json, gyp.opts, napi_build_version); s3_setup.detect(opts.hosted_path,config); AWS.config.update(config); diff --git a/lib/util/handle_gyp_opts.js b/lib/util/handle_gyp_opts.js index bdd5c75f..01d6df48 100644 --- a/lib/util/handle_gyp_opts.js +++ b/lib/util/handle_gyp_opts.js @@ -46,7 +46,7 @@ var share_with_node_gyp = [ 'module_name', 'module_path', 'napi_version', - `node_abi_napi`, + 'node_abi_napi', 'napi_build_version' ]; @@ -57,7 +57,7 @@ function handle_gyp_opts(gyp, argv, callback) { // generate custom node-pre-gyp versioning info var napi_build_version = napi.get_napi_build_version_from_command_args(argv); var opts = versioning.evaluate(JSON.parse(fs.readFileSync('./package.json')), gyp.opts, napi_build_version); - share_with_node_gyp.forEach(function(key) { + share_with_node_gyp.forEach(function(key) { var val = opts[key]; if (val) { node_pre_gyp_options.push('--' + key + '=' + val); diff --git a/lib/util/napi.js b/lib/util/napi.js index fc1605a7..9abeec5a 100644 --- a/lib/util/napi.js +++ b/lib/util/napi.js @@ -45,14 +45,14 @@ module.exports.get_napi_version_as_string = function() { module.exports.validate_package_json = function(package_json) { // return err var binary = package_json.binary; - var module_path_ok = binary.module_path && binary.module_path.includes('{napi_build_version}'); - var remote_path_ok = binary.remote_path && binary.remote_path.includes('{napi_build_version}'); - var package_name_ok = binary.package_name && binary.package_name.includes('{napi_build_version}'); + 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); if (napi_build_versions) { napi_build_versions.forEach(function(napi_build_version){ - if (!((parseInt(napi_build_version) === napi_build_version) && (napi_build_version > 0))) { + if (!(parseInt(napi_build_version,10) === napi_build_version && napi_build_version > 0)) { throw new Error("All values specified in napi_versions must be positive integers."); } }); @@ -85,7 +85,7 @@ module.exports.expand_commands = function(package_json, commands) { var napi_build_version = module.exports.get_best_napi_build_version(package_json); 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); @@ -102,7 +102,7 @@ module.exports.get_napi_build_versions = function(package_json) { var napi_build_versions = []; if (package_json.binary.napi_versions) { // remove duplicates 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; @@ -116,7 +116,7 @@ module.exports.get_napi_build_version_from_command_args = function(command_args) for (var i = 0; i < command_args.length; i++) { var arg = command_args[i]; if (arg.indexOf(napi_build_version_tag) === 0) { - return parseInt(arg.substr(napi_build_version_tag.length)); + return parseInt(arg.substr(napi_build_version_tag.length),10); } } return undefined; From 488ac7bb2ee0cfe4be109bb449895fc68497ca1c Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Wed, 7 Feb 2018 15:13:46 -0800 Subject: [PATCH 07/10] Fix for code cleanup --- lib/util/napi.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util/napi.js b/lib/util/napi.js index 9abeec5a..b74b94f4 100644 --- a/lib/util/napi.js +++ b/lib/util/napi.js @@ -85,7 +85,7 @@ module.exports.expand_commands = function(package_json, commands) { var napi_build_version = module.exports.get_best_napi_build_version(package_json); 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.indexOf(command.name) !== -1) { + } else if (napi_build_versions && napi_multiple_commands.includes(command.name)) { napi_build_versions.forEach(function(napi_build_version){ var args = command.args.slice(); args.push (napi_build_version_tag+napi_build_version); @@ -102,7 +102,7 @@ module.exports.get_napi_build_versions = function(package_json) { var napi_build_versions = []; if (package_json.binary.napi_versions) { // remove duplicates package_json.binary.napi_versions.forEach(function(napi_version) { - if (!napi_build_versions.indexOf(napi_version) !== -1) napi_build_versions.push(napi_version); + if (!napi_build_versions.includes(napi_version)) napi_build_versions.push(napi_version); }); } return napi_build_versions.length ? napi_build_versions : undefined; From 8f7c4974dad1e6ad74cf5696ab150a805516e2fa Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Thu, 8 Feb 2018 09:56:28 -0800 Subject: [PATCH 08/10] CI tweaks --- test/build.test.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/build.test.js b/test/build.test.js index b3ba1e24..888e3dc8 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -7,6 +7,7 @@ var fs = require('fs'); var rm = require('rimraf'); var path = require('path'); var getPrevious = require('./target_version.util.js'); +var napi = require ('../lib/util/napi.js'); // The list of different sample apps that we use to test var apps = [ @@ -26,14 +27,14 @@ var apps = [ 'name': 'app3', 'args': '' }, - { - 'name': 'app4', - 'args': '' - }, - { - 'name': 'app7', - 'args': '' - } + { + 'name': 'app4', + 'args': '' + }, + { + 'name': 'app7', + 'args': '' + } ]; @@ -110,6 +111,8 @@ test(app.name + ' passes --dist-url down to node-gyp via npm ' + app.args, funct apps.forEach(function(app) { + if (app.name === 'app7' && !napi.get_napi_version()) return; + // clear out entire binding directory // to ensure no stale builds. This is needed // because "node-pre-gyp clean" only removes @@ -176,7 +179,8 @@ apps.forEach(function(app) { run('node-pre-gyp', 'reveal', 'module_path --silent', app, {}, function(err,stdout,stderr) { t.ifError(err); var module_path = stdout.trim(); - if (module_path.includes('\n')) module_path = module_path.substr(0,module_path.indexOf('\n')); + if (module_path.indexOf('\r') !== -1) module_path = module_path.substr(0,module_path.indexOf('\r')); // win32 + else if (module_path.indexOf('\n') !== -1) module_path = module_path.substr(0,module_path.indexOf('\n')); t.stringContains(module_path,app.name); t.ok(existsSync(module_path),'is valid path to existing binary: '+ module_path); var module_binary = path.join(module_path,app.name+'.node'); From 98704913de2eee840b9805b890020efd60571eee Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Mon, 12 Feb 2018 13:09:13 -0800 Subject: [PATCH 09/10] Addresses CI build errors --- lib/util/handle_gyp_opts.js | 2 +- test/build.test.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/util/handle_gyp_opts.js b/lib/util/handle_gyp_opts.js index 01d6df48..afe7279d 100644 --- a/lib/util/handle_gyp_opts.js +++ b/lib/util/handle_gyp_opts.js @@ -62,7 +62,7 @@ function handle_gyp_opts(gyp, argv, callback) { if (val) { node_pre_gyp_options.push('--' + key + '=' + val); } else { - if (key !== 'napi_build_version') + if (key !== 'napi_version' && key !== 'node_abi_napi' && key !== 'napi_build_version') return callback(new Error("Option " + key + " required but not found by node-pre-gyp")); } }); diff --git a/test/build.test.js b/test/build.test.js index 888e3dc8..f451a675 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -179,8 +179,11 @@ apps.forEach(function(app) { run('node-pre-gyp', 'reveal', 'module_path --silent', app, {}, function(err,stdout,stderr) { t.ifError(err); var module_path = stdout.trim(); - if (module_path.indexOf('\r') !== -1) module_path = module_path.substr(0,module_path.indexOf('\r')); // win32 - else if (module_path.indexOf('\n') !== -1) module_path = module_path.substr(0,module_path.indexOf('\n')); + if (module_path.indexOf('\n') !== -1) { // take just the first line + module_path = module_path.substr(0,module_path.indexOf('\n')); + } else if (module_path.indexOf('\\n') !== -1) { // win32 "anomaly" + module_path = module_path.substr(0,module_path.indexOf('\\n')); + } t.stringContains(module_path,app.name); t.ok(existsSync(module_path),'is valid path to existing binary: '+ module_path); var module_binary = path.join(module_path,app.name+'.node'); From 9684ef6a65671c1e70423590ac9604ed9141e955 Mon Sep 17 00:00:00 2001 From: Jim Schlight Date: Mon, 12 Feb 2018 17:53:43 -0800 Subject: [PATCH 10/10] Another CI build tweak --- test/build.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/build.test.js b/test/build.test.js index f451a675..3da46231 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -181,8 +181,6 @@ apps.forEach(function(app) { var module_path = stdout.trim(); if (module_path.indexOf('\n') !== -1) { // take just the first line module_path = module_path.substr(0,module_path.indexOf('\n')); - } else if (module_path.indexOf('\\n') !== -1) { // win32 "anomaly" - module_path = module_path.substr(0,module_path.indexOf('\\n')); } t.stringContains(module_path,app.name); t.ok(existsSync(module_path),'is valid path to existing binary: '+ module_path); @@ -238,6 +236,9 @@ apps.forEach(function(app) { run('node-pre-gyp', 'reveal', 'package_name', app, {}, function(err,stdout,stderr) { t.ifError(err); var package_name = stdout.trim(); + if (package_name.indexOf('\n') !== -1) { // take just the first line + package_name = package_name.substr(0,package_name.indexOf('\n')); + } run('node-pre-gyp', 'info', '', app, {}, function(err,stdout,stderr) { t.ifError(err); t.stringContains(stdout,package_name);