From d714d1f79211f984e10582eb7a5d8a544a37e694 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Thu, 21 Mar 2013 00:27:06 -0700 Subject: [PATCH] support github tarballs as dependencies --- tools/meteor_npm.js | 31 ++++++++++------ tools/tests/test_bundler_npm.js | 63 ++++++++++++++++++++++++--------- 2 files changed, 66 insertions(+), 28 deletions(-) diff --git a/tools/meteor_npm.js b/tools/meteor_npm.js index 7aabe34e3c..fc74ff55ad 100644 --- a/tools/meteor_npm.js +++ b/tools/meteor_npm.js @@ -25,9 +25,18 @@ cleanup.onExit(function () { var meteorNpm = module.exports = { _tmpDirs: [], + _isGitHubTarball: function (x) { + return /^https:\/\/github.com\/.*\/tarball\/[0-9a-f]{40}/.test(x); + }, + ensureOnlyExactVersions: function(npmDependencies) { + var self = this; _.each(npmDependencies, function(version, name) { - if (!semver.valid(version)) + // We want a given version of a smart package (package.js + + // .npm/npm-shrinkwrap.json) to pin down its dependencies precisely, so we + // don't want anything too vague. For now, we support semvers and github + // tarballs pointing at an exact commit. + if (!semver.valid(version) && !self._isGitHubTarball(version)) throw new Error( "Must declare exact version of npm package dependency: " + name + '@' + version); }); @@ -121,13 +130,7 @@ var meteorNpm = module.exports = { } }); - // if we had no installed dependencies to begin with, *DON'T* - // shrinkwrap. this is important so that we can pin versions of - // deep dependencies to tarballs, e.g. - // https://github.com/meteor/js-bson/tarball/master - if (!_.isEmpty(installedDependencies)) { - self._shrinkwrap(newPackageNpmDir); - } + self._shrinkwrap(newPackageNpmDir); self._createReadme(newPackageNpmDir); self._renameAlmostAtomically(newPackageNpmDir, packageNpmDir); } @@ -242,24 +245,30 @@ var meteorNpm = module.exports = { // map the structure returned from `npm ls` into the structure of // npmDependencies (e.g. {gcd: '0.0.0'}), so that they can be - // diffed. + // diffed. This only returns top-level dependencies. _installedDependencies: function(dir) { var self = this; return _.object( _.map( self._installedDependenciesTree(dir).dependencies, function(properties, name) { - return [name, properties.version]; + if (self._isGitHubTarball(properties.from)) + return [name, properties.from]; + else + return [name, properties.version]; })); }, _installNpmModule: function(name, version, dir) { this._ensureConnected(); + var installArg = this._isGitHubTarball(version) + ? version : (name + "@" + version); + // We don't use npm.commands.install since we couldn't // figure out how to silence all output (specifically the // installed tree which is printed out with `console.log`) this._execFileSync(path.join(files.get_dev_bundle(), "bin", "npm"), - ["install", name + "@" + version], + ["install", installArg], {cwd: dir}); }, diff --git a/tools/tests/test_bundler_npm.js b/tools/tests/test_bundler_npm.js index bcf6fead76..67f0a13d6f 100644 --- a/tools/tests/test_bundler_npm.js +++ b/tools/tests/test_bundler_npm.js @@ -40,15 +40,20 @@ var _assertCorrectPackageNpmDir = function(deps) { // to write these tests, so just transplant that information var actualMeteorNpmShrinkwrapDependencies = JSON.parse(fs.readFileSync(path.join(testPackageDir, ".npm", "npm-shrinkwrap.json"), 'utf8')).dependencies; var expectedMeteorNpmShrinkwrapDependencies = _.object(_.map(deps, function(version, name) { - var val = {version: version}; + var expected = {}; + if (/tarball/.test(version)) { + expected.from = version; + } else { + expected.version = version; + } // copy fields with values generated by shrinkwrap that can't be known to the - // test author. - // - // 'from' and 'resolved' were added when we upraded - // to Node 0.8.18 (5a54a5c79f71b7449c58d7ee809086bc0a542a15) - _.each(['dependencies', 'from', 'resolved'], function(key) { - if (actualMeteorNpmShrinkwrapDependencies[name][key]) + // test author. We set keys on val always in this order so that comparison works well. + var val = {}; + _.each(['version', 'from', 'resolved', 'dependencies'], function(key) { + if (expected[key]) + val[key] = expected[key]; + else if (actualMeteorNpmShrinkwrapDependencies[name][key]) val[key] = actualMeteorNpmShrinkwrapDependencies[name][key]; }); @@ -70,14 +75,13 @@ var _assertCorrectPackageNpmDir = function(deps) { // all expected dependencies are installed correctly, with the correct version _.each(deps, function(version, name) { - // presumably if this file is here we have correctly installed the package - assert(fs.existsSync(path.join(nodeModulesDir, name, 'LICENSE'))); + assert(looksInstalled(nodeModulesDir, name)); assert.equal(JSON.parse( fs.readFileSync( path.join(nodeModulesDir, name, "package.json"), 'utf8')).version, - version); + expectedMeteorNpmShrinkwrapDependencies[name].version); }); // all installed dependencies were expected to be found there, @@ -99,16 +103,24 @@ var _assertCorrectBundleNpmContents = function(bundleDir, deps) { // bundle actually has the npm modules _.each(deps, function(version, name) { - // presumably if this file is here we have correctly installed the package - assert(fs.existsSync(path.join(bundledPackageNodeModulesDir, name, 'LICENSE'))); + assert(looksInstalled(bundledPackageNodeModulesDir, name)); - assert.equal(JSON.parse( - fs.readFileSync(path.join(bundledPackageNodeModulesDir, name, 'package.json'), 'utf8')) - .version, - version); + if (!/tarball/.test(version)) { + assert.equal(JSON.parse( + fs.readFileSync(path.join(bundledPackageNodeModulesDir, name, 'package.json'), 'utf8')) + .version, + version); + } }); }; +var looksInstalled = function (nodeModulesDir, name) { + // All of the packages in this test have one of these two files, so presumably + // if one of these files is here we have correctly installed the package. + return fs.existsSync(path.join(nodeModulesDir, name, 'README.md')) || + fs.existsSync(path.join(nodeModulesDir, name, 'LICENSE')); +}; + /// /// TESTS /// @@ -219,13 +231,15 @@ assert.doesNotThrow(function () { _assertCorrectBundleNpmContents(tmpOutputDir, {gcd: '0.0.0', mime: '1.2.7'}); }); - console.log("bundle multiple apps in parallel using a meteor package dependent on an npm package"); // this fails if we don't manage the package .npm directory correctly // against parallel bundling. this happens if you are running more // than one app at once using a certain package and that package is // updated. +// XXX This doesn't pass if it has to remove a module! eg, move the gzippo test +// that is below to above this one. It will fail! assert.doesNotThrow(function () { + updateTestPackage({gcd: '0.0.0', mime: '1.2.7'}); // rm -rf .npm/node_modules, to make sure installing modules takes some time var nodeModulesDir = path.join(testPackageDir, ".npm", "node_modules"); assert(fs.existsSync(path.join(nodeModulesDir))); @@ -265,6 +279,21 @@ assert.doesNotThrow(function () { }); +console.log("app that uses gcd - install gzippo via tarball"); +assert.doesNotThrow(function () { + var deps = {gzippo: 'https://github.com/meteor/gzippo/tarball/1e4b955439abc643879ae264b28a761521818f3b'}; + updateTestPackage(deps); + var tmpOutputDir = tmpDir(); + var errors = bundler.bundle(appWithPackageDir, tmpOutputDir, {nodeModulesMode: 'skip', releaseStamp: 'none'}); + assert.strictEqual(errors, undefined, errors && errors[0]); + _assertCorrectPackageNpmDir(deps); + _assertCorrectBundleNpmContents(tmpOutputDir, deps); + // Check that a string introduced by our fork is in the source. + assert(/clientMaxAge = 604800000/.test( + fs.readFileSync( + path.join(testPackageDir, ".npm", "node_modules", "gzippo", "lib", "staticGzip.js"), "utf8"))); +}); + ///