From 1022d6b48352bd6da35c0d400463fdcac05c8c9c Mon Sep 17 00:00:00 2001 From: Allen Luce Date: Thu, 26 Apr 2018 11:38:37 -0700 Subject: [PATCH 1/4] Allow for single-level module_path when packing Prior to 0.7.1, a module_path of `./lib` would result in a package containing files like this (using `test/app8` as an example): lib/app8.node The switch to tar 3 in 0.7.1 causes the same package to end up like this: package.json app8.cc binding.gyp index.js README.md lib/app8.node which causes a problem during remote binary installation: node-pre-gyp ERR! Pre-built binaries not installable for PACKAGE@0.0.1 and node@8.3.0 (node-v57 ABI, glibc) (falling back to source compile with node-gyp) node-pre-gyp ERR! Hit error ENOTDIR: Cannot cd into '..dirs/package/lib' This commit fixes this problem and allows for single-level module_path values. --- lib/package.js | 7 ++++-- test/app8/.gitignore | 5 +++++ test/app8/README.md | 3 +++ test/app8/app8.cc | 38 +++++++++++++++++++++++++++++++++ test/app8/binding.gyp | 13 ++++++++++++ test/app8/index.js | 6 ++++++ test/app8/package.json | 21 ++++++++++++++++++ test/build.test.js | 48 ++++++++++++++++++++++++++++++++++++------ 8 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 test/app8/.gitignore create mode 100644 test/app8/README.md create mode 100644 test/app8/app8.cc create mode 100644 test/app8/binding.gyp create mode 100644 test/app8/index.js create mode 100644 test/app8/package.json diff --git a/lib/package.js b/lib/package.js index f31ae632..4959265f 100644 --- a/lib/package.js +++ b/lib/package.js @@ -33,15 +33,18 @@ function _package(gyp, argv, callback) { return true; }; mkdirp(path.dirname(tarball),function(err) { - from = path.dirname(from); if (err) return callback(err); packlist({ path: from }).then(function(files) { + var base = path.basename(from); + files = files.map(function(file) { + return path.join(base, file); + }); tar.create({ portable: true, gzip: true, onentry: filter_func, file: tarball, - cwd: from + cwd: path.dirname(from) }, files, function(err) { if (err) console.error('['+package_json.name+'] ' + err.message); else log.info('package','Binary staged at "' + tarball + '"'); diff --git a/test/app8/.gitignore b/test/app8/.gitignore new file mode 100644 index 00000000..1868e0ab --- /dev/null +++ b/test/app8/.gitignore @@ -0,0 +1,5 @@ +.DS_Store +build/ +lib/ +node_modules +npm-debug.log diff --git a/test/app8/README.md b/test/app8/README.md new file mode 100644 index 00000000..8e61d318 --- /dev/null +++ b/test/app8/README.md @@ -0,0 +1,3 @@ +# Test app + +Demonstrates a simpler configuration that uses node-pre-gyp. diff --git a/test/app8/app8.cc b/test/app8/app8.cc new file mode 100644 index 00000000..2da5911b --- /dev/null +++ b/test/app8/app8.cc @@ -0,0 +1,38 @@ +// v8 +#include + +// node.js +#include +#include + +#if (NODE_MODULE_VERSION > 0x000B) + + static void get_hello(const v8::FunctionCallbackInfo& args) + { + v8::HandleScope scope(v8::Isolate::GetCurrent()); + args.GetReturnValue().Set(v8::String::NewFromUtf8(v8::Isolate::GetCurrent(),"hello")); + } + +#else + + static v8::Handle get_hello(const v8::Arguments& args) + { + v8::HandleScope scope; + return scope.Close(v8::String::New("hello")); + } + +#endif + +extern "C" { + static void start(v8::Handle target) { +#if (NODE_MODULE_VERSION > 0x000B) + v8::HandleScope scope(v8::Isolate::GetCurrent()); +#else + v8::HandleScope scope; +#endif + NODE_SET_METHOD(target, "hello", get_hello); + } +} + +NODE_MODULE(app8, start) + diff --git a/test/app8/binding.gyp b/test/app8/binding.gyp new file mode 100644 index 00000000..ed6dd2c2 --- /dev/null +++ b/test/app8/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/app8/index.js b/test/app8/index.js new file mode 100644 index 00000000..084fc583 --- /dev/null +++ b/test/app8/index.js @@ -0,0 +1,6 @@ +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); + +require('assert').equal(binding.hello(),"hello"); \ No newline at end of file diff --git a/test/app8/package.json b/test/app8/package.json new file mode 100644 index 00000000..10f33c18 --- /dev/null +++ b/test/app8/package.json @@ -0,0 +1,21 @@ +{ + "name": "node-pre-gyp-test-app8", + "author": "Dane Springmeyer ", + "description": "node-pre-gyp test", + "repository": { + "type": "git", + "url": "git://github.com/mapbox/node-pre-gyp.git" + }, + "license": "BSD-3-Clause", + "version": "0.1.0", + "main": "./index.js", + "binary": { + "module_name": "app8", + "module_path": "./lib", + "host": "https://node-pre-gyp-tests.s3-us-west-1.amazonaws.com" + }, + "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 3da46231..001c3e9d 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -8,32 +8,43 @@ var rm = require('rimraf'); var path = require('path'); var getPrevious = require('./target_version.util.js'); var napi = require ('../lib/util/napi.js'); +var tar = require('tar'); // The list of different sample apps that we use to test var apps = [ { 'name': 'app1', - 'args': '' + 'args': '', + 'files': ['binding/app1.node'] }, { - 'name': 'app2', - 'args': '--custom_include_path=../include --debug' + 'name': 'app2', + 'args': '--custom_include_path=../include --debug', + 'files': ['node-pre-gyp-test-app2/app2.node'] }, { 'name': 'app2', - 'args': '--custom_include_path=../include --toolset=cpp11' + 'args': '--custom_include_path=../include --toolset=cpp11', + 'files': ['node-pre-gyp-test-app2/app2.node'] }, { 'name': 'app3', - 'args': '' + 'args': '', + 'files': ['node-v48-darwin-x64/app3.node'] }, { 'name': 'app4', - 'args': '' + 'args': '', + 'files': ['node-v48-darwin-x64/app4.node', 'node-v48-darwin-x64/lib.target/mylib.dylib'] }, { 'name': 'app7', 'args': '' + }, + { + 'name': 'app8', + 'args': '', + 'files': ['lib/app8.node'] } ]; @@ -211,7 +222,30 @@ apps.forEach(function(app) { test(app.name + ' packages ' + app.args, function(t) { run('node-pre-gyp', 'package', '', app, {}, function(err,stdout,stderr) { t.ifError(err); - t.end(); + // Make sure a tarball was created + run('node-pre-gyp', 'reveal', 'staged_tarball --silent', app, {}, function(err,stdout,stderr) { + t.ifError(err); + var staged_tarball = stdout.trim(); + if (staged_tarball.indexOf('\n') !== -1) { // take just the first line + staged_tarball = staged_tarball.substr(0,staged_tarball.indexOf('\n')); + } + var tarball_path = path.join(__dirname, app.name, staged_tarball); + t.ok(existsSync(tarball_path),'staged tarball is a valid file'); + if (!app.files) { + return t.end(); + } + // Make sure the package contains what we expect + var entries = []; + tar.t({ + file: tarball_path, + sync: true, + onentry: function (entry) { + entries.push(entry.path); + } + }); + t.same(entries.sort(), app.files.sort(), 'staged tarball contains the right files'); + t.end(); + }); }); }); From d399a1e9dab17e62a5b8f031dc713fa564df199d Mon Sep 17 00:00:00 2001 From: Allen Luce Date: Thu, 26 Apr 2018 14:09:54 -0700 Subject: [PATCH 2/4] Compare apples to apples But don't compare Linux to Apple. --- test/build.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/build.test.js b/test/build.test.js index 001c3e9d..efad5bb8 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -8,8 +8,10 @@ var rm = require('rimraf'); var path = require('path'); var getPrevious = require('./target_version.util.js'); var napi = require ('../lib/util/napi.js'); +var versioning = require('../lib/util/versioning.js'); var tar = require('tar'); +var localVer = [versioning.get_runtime_abi('node'), process.platform, process.arch].join('-'); // The list of different sample apps that we use to test var apps = [ { @@ -30,12 +32,12 @@ var apps = [ { 'name': 'app3', 'args': '', - 'files': ['node-v48-darwin-x64/app3.node'] + 'files': [path.join(localVer, 'app3.node')] }, { 'name': 'app4', 'args': '', - 'files': ['node-v48-darwin-x64/app4.node', 'node-v48-darwin-x64/lib.target/mylib.dylib'] + 'files': [path.join(localVer, 'app4.node'), path.join(localVer, 'lib.target', 'mylib.dylib')] }, { 'name': 'app7', From 76bb9b64d0ad3ce688676d152de5228dc13f3c17 Mon Sep 17 00:00:00 2001 From: Allen Luce Date: Thu, 26 Apr 2018 14:33:52 -0700 Subject: [PATCH 3/4] Use a platform-specific dynamic object extension This should work everywhere except AIX. --- test/build.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/build.test.js b/test/build.test.js index efad5bb8..d848643d 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -12,6 +12,8 @@ var versioning = require('../lib/util/versioning.js'); var tar = require('tar'); var localVer = [versioning.get_runtime_abi('node'), process.platform, process.arch].join('-'); +var SOEXT = process.platform === 'darwin' ? 'dylib': 'so'; + // The list of different sample apps that we use to test var apps = [ { @@ -37,7 +39,7 @@ var apps = [ { 'name': 'app4', 'args': '', - 'files': [path.join(localVer, 'app4.node'), path.join(localVer, 'lib.target', 'mylib.dylib')] + 'files': [path.join(localVer, 'app4.node'), path.join(localVer, 'lib.target', 'mylib.' + SOEXT)] }, { 'name': 'app7', From 6801a8ba68fd740bde92534f3208271537547b7b Mon Sep 17 00:00:00 2001 From: Allen Luce Date: Thu, 26 Apr 2018 15:56:38 -0700 Subject: [PATCH 4/4] Fix Win32 file expectations This is based on what showed up during an Appveyor run. Someone with more Win32 module building experience should verify that these are expected and correct. --- test/build.test.js | 112 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 104 insertions(+), 8 deletions(-) diff --git a/test/build.test.js b/test/build.test.js index d848643d..0a86b620 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -12,34 +12,100 @@ var versioning = require('../lib/util/versioning.js'); var tar = require('tar'); var localVer = [versioning.get_runtime_abi('node'), process.platform, process.arch].join('-'); -var SOEXT = process.platform === 'darwin' ? 'dylib': 'so'; +var SOEXT = {'darwin': 'dylib', 'linux': 'so', 'win32': 'dll'}[process.platform]; // The list of different sample apps that we use to test var apps = [ { 'name': 'app1', 'args': '', - 'files': ['binding/app1.node'] + 'files': { + 'base': ['binding/app1.node'], + 'win32': { + 'base': [ + 'binding/app1.exp', + 'binding/app1.lib', + 'binding/app1.map', + 'binding/app1.node' + ], + 'node-v59': [ + 'binding/app1.exp', + 'binding/app1.iobj', + 'binding/app1.ipdb', + 'binding/app1.lib', + 'binding/app1.map', + 'binding/app1.node' + ] + } + } }, { 'name': 'app2', 'args': '--custom_include_path=../include --debug', - 'files': ['node-pre-gyp-test-app2/app2.node'] + 'files': { + 'base': ['node-pre-gyp-test-app2/app2.node'], + 'win32': [ + 'node-pre-gyp-test-app2/app2.exp', + 'node-pre-gyp-test-app2/app2.ilk', + 'node-pre-gyp-test-app2/app2.lib', + 'node-pre-gyp-test-app2/app2.map', + 'node-pre-gyp-test-app2/app2.node' + ] + } }, { 'name': 'app2', 'args': '--custom_include_path=../include --toolset=cpp11', - 'files': ['node-pre-gyp-test-app2/app2.node'] + 'files': { + 'base': ['node-pre-gyp-test-app2/app2.node'], + 'win32': { + 'base': [ + 'node-pre-gyp-test-app2/app2.exp', + 'node-pre-gyp-test-app2/app2.lib', + 'node-pre-gyp-test-app2/app2.map', + 'node-pre-gyp-test-app2/app2.node' + ], + 'node-v59': [ + 'node-pre-gyp-test-app2/app2.exp', + 'node-pre-gyp-test-app2/app2.iobj', + 'node-pre-gyp-test-app2/app2.ipdb', + 'node-pre-gyp-test-app2/app2.lib', + 'node-pre-gyp-test-app2/app2.map', + 'node-pre-gyp-test-app2/app2.node' + ] + } + } }, { 'name': 'app3', 'args': '', - 'files': [path.join(localVer, 'app3.node')] + 'files': { + 'base': [[localVer, 'app3.node'].join('/')], + 'win32': { + 'base': [ + [localVer, 'app3.exp'].join('/'), + [localVer, 'app3.lib'].join('/'), + [localVer, 'app3.map'].join('/'), + [localVer, 'app3.node'].join('/') + ], + 'node-v59': [ + [localVer, 'app3.exp'].join('/'), + [localVer, 'app3.iobj'].join('/'), + [localVer, 'app3.ipdb'].join('/'), + [localVer, 'app3.lib'].join('/'), + [localVer, 'app3.map'].join('/'), + [localVer, 'app3.node'].join('/'), + ] + } + } }, { 'name': 'app4', 'args': '', - 'files': [path.join(localVer, 'app4.node'), path.join(localVer, 'lib.target', 'mylib.' + SOEXT)] + 'files': { + 'base': [[localVer, 'app4.node'].join('/'), [localVer, 'lib.target', 'mylib.' + SOEXT].join('/')], + 'win32': [[localVer, 'app4.node'].join('/'), [localVer, 'mylib.' + SOEXT].join('/')] + } }, { 'name': 'app7', @@ -48,7 +114,25 @@ var apps = [ { 'name': 'app8', 'args': '', - 'files': ['lib/app8.node'] + 'files': { + 'base': ['lib/app8.node'], + 'win32': { + 'base': [ + 'lib/app8.exp', + 'lib/app8.lib', + 'lib/app8.map', + 'lib/app8.node' + ], + 'node-v59': [ + 'lib/app8.exp', + 'lib/app8.iobj', + 'lib/app8.ipdb', + 'lib/app8.lib', + 'lib/app8.map', + 'lib/app8.node' + ] + } + } } ]; @@ -247,7 +331,19 @@ apps.forEach(function(app) { entries.push(entry.path); } }); - t.same(entries.sort(), app.files.sort(), 'staged tarball contains the right files'); + var files = app.files.base; + var nodever = versioning.get_runtime_abi('node'); + // Look for a more specific choice + if (app.files.hasOwnProperty(process.platform)) { + if (app.files[process.platform].hasOwnProperty(nodever)) { + files = app.files[process.platform][nodever]; + } else if (app.files[process.platform].hasOwnProperty('base')) { + files = app.files[process.platform].base; + } else { + files = app.files[process.platform]; + } + } + t.same(entries.sort(), files.sort(), 'staged tarball contains the right files'); t.end(); }); });