From bb3bdaed56d6540bdd9519f2dbcfdd27d99ed22e Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 2 Jun 2017 14:54:22 -0700 Subject: [PATCH 1/7] Upgrade to tar version 3 This brings node-pre-gyp into a more modern age of tar parsing and creation. Node-tar v3 is faster and more reliable, and removes several race conditions that were inherent in the fstream utility. This also removes the need for some workarounds to force directories to be readable. It also uses the npm-packlist module to decide what to put into a package, which will be used by npm itself relatively soon. Tar version 3 is only supported by node v4 and above, as it uses modern ES class and fat-arrow syntax. Thus, this is a breaking change, worthy of a semver major version bump, but it only leaves less than 10% of our users behind. (And folks who don't update Node.js out of unmaintained versions are unlikely to upgrade node-pre-gyp anyway.) The only change I'm somewhat unsure about is the use of the "portable" flag in tar.create here. This excludes system-specific items like username, uid, ctime, etc., resulting in a tarball that is 100% dependent on the contents and mtime of the items it includes. Of course, mtime will change if the package is rebuilt, but this means that multiple calls to `node-pre-gyp package` will not result in different bits on disk. --- appveyor.yml | 1 - lib/install.js | 46 ++++++++++++++++++++++-------------------- lib/package.js | 30 +++++++++++++++------------ lib/testpackage.js | 34 +++++++++++++++++-------------- package.json | 6 +++--- test/app1/package.json | 14 ++++++------- 6 files changed, 70 insertions(+), 61 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index e14e0724..1815c0fa 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,7 +2,6 @@ os: Visual Studio 2015 environment: matrix: - - nodejs_version: 0.10 - nodejs_version: 4 - nodejs_version: 6 - nodejs_version: 8 diff --git a/lib/install.js b/lib/install.js index 42d65cd8..aacbd9b6 100644 --- a/lib/install.js +++ b/lib/install.js @@ -6,10 +6,10 @@ exports.usage = 'Attempts to install pre-built binary for module'; var fs = require('fs'); var path = require('path'); -var zlib = require('zlib'); var log = require('npmlog'); var existsAsync = fs.exists || path.exists; var versioning = require('./util/versioning.js'); +var mkdirp = require('mkdirp'); var npgVersion = 'unknown'; try { @@ -75,8 +75,7 @@ function place_binary(from,to,opts,callback) { if (!req) return callback(new Error("empty req")); var badDownload = false; var extractCount = 0; - var gunzip = zlib.createGunzip(); - var extracter = require('tar').Extract({ path: to, strip: 1}); + var tar = require('tar'); function afterTarball(err) { if (err) return callback(err); @@ -89,18 +88,10 @@ function place_binary(from,to,opts,callback) { } function filter_func(entry) { - // ensure directories are +x - // https://github.com/mapnik/node-mapnik/issues/262 - entry.props.mode |= (entry.props.mode >>> 2) & parseInt('0111',8); log.info('install','unpacking ' + entry.path); extractCount++; } - gunzip.on('error', callback); - extracter.on('entry', filter_func); - extracter.on('error', callback); - extracter.on('end', afterTarball); - req.on('error', function(err) { badDownload = true; return callback(err); @@ -120,7 +111,11 @@ function place_binary(from,to,opts,callback) { return callback(err); } // start unzipping and untaring - req.pipe(gunzip).pipe(extracter); + req.pipe(tar.extract({ + cwd: to, + strip: 1, + onentry: filter_func + }).on('close', afterTarball).on('error', callback)); }); }); } @@ -173,25 +168,32 @@ function install(gyp, argv, callback) { var from = opts.hosted_tarball; var to = opts.module_path; var binary_module = path.join(to,opts.module_name + '.node'); - if (existsAsync(binary_module,function(found) { + existsAsync(binary_module,function(found) { if (found && !update_binary) { console.log('['+package_json.name+'] Success: "' + binary_module + '" already installed'); console.log('Pass --update-binary to reinstall or --build-from-source to recompile'); return callback(); } else { if (!update_binary) log.info('check','checked for "' + binary_module + '" (not found)'); - place_binary(from,to,opts,function(err) { - if (err && should_do_fallback_build) { - print_fallback_error(err,opts,package_json); - return do_build(gyp,argv,callback); - } else if (err) { - return callback(err); + mkdirp(to,function(err) { + if (err) { + after_place(err); } else { - console.log('['+package_json.name+'] Success: "' + binary_module + '" is installed via remote'); - return callback(); + place_binary(from,to,opts,after_place); } }); } - })); + function after_place(err) { + if (err && should_do_fallback_build) { + print_fallback_error(err,opts,package_json); + return do_build(gyp,argv,callback); + } else if (err) { + return callback(err); + } else { + console.log('['+package_json.name+'] Success: "' + binary_module + '" is installed via remote'); + return callback(); + } + } + }); } } diff --git a/lib/package.js b/lib/package.js index 3a35f653..7880394f 100644 --- a/lib/package.js +++ b/lib/package.js @@ -8,12 +8,12 @@ var fs = require('fs'); var path = require('path'); var log = require('npmlog'); var versioning = require('./util/versioning.js'); -var write = require('fs').createWriteStream; var existsAsync = fs.exists || path.exists; var mkdirp = require('mkdirp'); +var tar = require('tar'); function _package(gyp, argv, callback) { - var pack = require('tar-pack').pack; + var packlist = require('npm-packlist'); var package_json = JSON.parse(fs.readFileSync('./package.json')); var opts = versioning.evaluate(package_json, gyp.opts); var from = opts.module_path; @@ -30,17 +30,21 @@ function _package(gyp, argv, callback) { return true; }; mkdirp(path.dirname(tarball),function(err) { - if (err) throw err; - pack(from, { filter: filter_func }) - .pipe(write(tarball)) - .on('error', function(err) { - if (err) console.error('['+package_json.name+'] ' + err.message); - return callback(err); - }) - .on('close', function() { - log.info('package','Binary staged at "' + tarball + '"'); - return callback(); - }); + from = path.dirname(from); + if (err) return callback(err); + packlist({ path: from }).then(function(files) { + tar.create({ + portable: true, + gzip: true, + onentry: filter_func, + file: tarball, + cwd: from + }, files, function(err) { + if (err) console.error('['+package_json.name+'] ' + err.message); + else log.info('package','Binary staged at "' + tarball + '"'); + return callback(err); + }); + }, callback); }); }); } diff --git a/lib/testpackage.js b/lib/testpackage.js index 1d4cc607..9e5d0e02 100644 --- a/lib/testpackage.js +++ b/lib/testpackage.js @@ -10,8 +10,8 @@ var log = require('npmlog'); var existsAsync = fs.exists || path.exists; var versioning = require('./util/versioning.js'); var testbinary = require('./testbinary.js'); -var read = require('fs').createReadStream; -var zlib = require('zlib'); +var tar = require('tar'); +var mkdirp = require('mkdirp'); function testpackage(gyp, argv, callback) { var package_json = JSON.parse(fs.readFileSync('./package.json')); @@ -22,19 +22,24 @@ function testpackage(gyp, argv, callback) { return callback(new Error("Cannot test package because " + tarball + " missing: run `node-pre-gyp package` first")); } var to = opts.module_path; - var gunzip = zlib.createGunzip(); - var extracter = require('tar').Extract({ path: to, strip: 1 }); function filter_func(entry) { - // ensure directories are +x - // https://github.com/mapnik/node-mapnik/issues/262 - entry.props.mode |= (entry.props.mode >>> 2) & parseInt('0111',8); - log.info('install','unpacking ' + entry.path); + log.info('install','unpacking [' + entry.path + ']'); } - gunzip.on('error', callback); - extracter.on('error', callback); - extracter.on('entry', filter_func); - extracter.on('end', function(err) { - if (err) return callback(err); + + mkdirp(to, function(err) { + if (err) { + return callback(err); + } else { + tar.extract({ + file: tarball, + cwd: to, + strip: 1, + onentry: filter_func + }).then(after_extract, callback); + } + }); + + function after_extract() { testbinary(gyp,argv,function(err) { if (err) { return callback(err); @@ -43,7 +48,6 @@ function testpackage(gyp, argv, callback) { return callback(); } }); - }); - read(tarball).pipe(gunzip).pipe(extracter); + } }); } diff --git a/package.json b/package.json index b45d9ad8..401d1659 100644 --- a/package.json +++ b/package.json @@ -22,19 +22,19 @@ "dependencies": { "mkdirp": "^0.5.1", "nopt": "^4.0.1", + "npm-packlist": "^1.1.6", "npmlog": "^4.0.2", "rc": "^1.1.7", "request": "^2.81.0", "rimraf": "^2.6.1", "semver": "^5.3.0", - "tape": "^4.6.3", - "tar": "^2.2.1", - "tar-pack": "^3.4.0" + "tar": "^4" }, "devDependencies": { "aws-sdk": "^2.28.0", "retire": "^1.2.12", "jshint": "^2.9.4" + "tape": "^4.6.3", }, "jshintConfig": { "node": true, diff --git a/test/app1/package.json b/test/app1/package.json index 9818ffc3..272f6a3a 100644 --- a/test/app1/package.json +++ b/test/app1/package.json @@ -1,10 +1,10 @@ { "name": "node-pre-gyp-test-app1", "author": "Dane Springmeyer ", - "description":"node-pre-gyp test", - "repository" : { - "type" : "git", - "url" : "git://github.com/mapbox/node-pre-gyp.git" + "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", @@ -12,10 +12,10 @@ "binary": { "module_name": "app1", "module_path": "./lib/binding/", - "host":"https://node-pre-gyp-tests.s3-us-west-1.amazonaws.com" + "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" + "install": "node-pre-gyp install --fallback-to-build", + "test": "node index.js" } } From f2c5a5428035e60ebd1430c3f9be2922a0b4dca1 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 3 Jun 2017 09:43:01 -0700 Subject: [PATCH 2/7] Debugging appveyor, run npm ls after install --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index 1815c0fa..abd99ed9 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -36,6 +36,7 @@ install: - IF /I "%PLATFORM%" == "x64" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64 - IF /I "%PLATFORM%" == "x86" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86 - npm install + - npm ls - npm test build: off From c063e651721eedd543bc39419707097cf04b1176 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 3 Jun 2017 09:49:47 -0700 Subject: [PATCH 3/7] more appveyor debugging --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index abd99ed9..35018f9b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -37,6 +37,7 @@ install: - IF /I "%PLATFORM%" == "x86" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86 - npm install - npm ls + - npm install node-gyp - npm test build: off From 5ff88962e31b76e2b41efaae8df80eed0b484a03 Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 7 Sep 2017 15:35:33 -0700 Subject: [PATCH 4/7] package.json typo --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 401d1659..e75653ec 100644 --- a/package.json +++ b/package.json @@ -33,8 +33,8 @@ "devDependencies": { "aws-sdk": "^2.28.0", "retire": "^1.2.12", - "jshint": "^2.9.4" - "tape": "^4.6.3", + "jshint": "^2.9.4", + "tape": "^4.6.3" }, "jshintConfig": { "node": true, From ef7289f97a858fda2b94a9e2e9de646ec57dda04 Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 7 Sep 2017 21:04:48 -0700 Subject: [PATCH 5/7] appveyor: don't crash on extraneous deps --- appveyor.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 35018f9b..56528765 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -36,7 +36,6 @@ install: - IF /I "%PLATFORM%" == "x64" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64 - IF /I "%PLATFORM%" == "x86" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86 - npm install - - npm ls - npm install node-gyp - npm test From de160e03b4e3df29888cfb97d4c9c4e50f89efd5 Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 7 Sep 2017 21:09:25 -0700 Subject: [PATCH 6/7] appveyor: Use npm5, don't blow away rollback installs of node-gyp --- appveyor.yml | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 56528765..531f7364 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -17,17 +17,13 @@ install: - ps: Set-ExecutionPolicy Unrestricted -Scope CurrentUser -Force - npm config get # upgrade node-gyp to dodge 2013 compile issue present in the node gyp bundled with node v0.10 - # https://github.com/nodejs/node-gyp/issues/972#issuecomment-231055109 + - npm install npm@5 -g + - npm install node-gyp - IF "%nodejs_version:~0,1%"=="0" npm install node-gyp@3.x # upgrade node-gyp to dodge https://github.com/mapbox/node-pre-gyp/issues/209#issuecomment-307641388 # and allow make node 4.x x86 builds work # https://github.com/mapbox/node-pre-gyp/issues/209#issuecomment-217690537 - IF "%nodejs_version:~0,1%"=="4" npm install node-gyp@3.x - # downgrade npm to avoid multiple npm bugs: - # for node v6 this dodges npm 3.10.10 bug whereby --nodedir/--dist-url is not passed to node-gyp (https://github.com/mapbox/node-pre-gyp/issues/300) - # for all node x86 versions this dodges a mysterious ELIFECYCLE error: https://ci.appveyor.com/project/Mapbox/node-pre-gyp/build/1.0.582/job/b8q2nud6vkj0s6qo#L233 - # for node v8 this dodges https://github.com/mapbox/node-pre-gyp/issues/302 - - npm install npm@2.x -g - node --version - npm --version - node -e "console.log(process.arch);" @@ -36,7 +32,7 @@ install: - IF /I "%PLATFORM%" == "x64" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64 - IF /I "%PLATFORM%" == "x86" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86 - npm install - - npm install node-gyp + - npm install npm@2 -g - npm test build: off From ae2daac39753cef56d13bf170071014dde48468c Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 8 Sep 2017 11:32:34 -0700 Subject: [PATCH 7/7] use batch compare ops - https://stackoverflow.com/a/18499854 --- appveyor.yml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 531f7364..8b866e9f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,14 +16,9 @@ install: - ps: Install-Product node $env:nodejs_version $env:Platform - ps: Set-ExecutionPolicy Unrestricted -Scope CurrentUser -Force - npm config get - # upgrade node-gyp to dodge 2013 compile issue present in the node gyp bundled with node v0.10 - - npm install npm@5 -g - - npm install node-gyp - - IF "%nodejs_version:~0,1%"=="0" npm install node-gyp@3.x # upgrade node-gyp to dodge https://github.com/mapbox/node-pre-gyp/issues/209#issuecomment-307641388 # and allow make node 4.x x86 builds work - # https://github.com/mapbox/node-pre-gyp/issues/209#issuecomment-217690537 - - IF "%nodejs_version:~0,1%"=="4" npm install node-gyp@3.x + - IF "%nodejs_version:~0,1%" EQU "4" npm install node-gyp@3.x - node --version - npm --version - node -e "console.log(process.arch);" @@ -32,9 +27,10 @@ install: - IF /I "%PLATFORM%" == "x64" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64 - IF /I "%PLATFORM%" == "x86" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86 - npm install - - npm install npm@2 -g + # workaround https://github.com/mapbox/node-pre-gyp/issues/300#issuecomment-328179994 + - IF "%nodejs_version:~0,1%" GEQ "6" npm install npm@2 -g - npm test build: off test: off -deploy: off +deploy: off \ No newline at end of file