From e660b2d9d601b3dfa94ce41230360d4c84fbfeda Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 10 Nov 2014 17:31:54 -0800 Subject: [PATCH] ProjectContext: download missing packages Note that unlike the previous implementation, we show a 'downloading' progress bar iff we actually download any packages. (In 1.0 we show a 'downloading' message while just deciding if we need to download anything; on devel right now we never show 'downloading' since that was confusing.) --- tools/commands-packages.js | 7 +- tools/package-map.js | 9 +++ tools/project-context.js | 39 ++++++++-- tools/tropohouse.js | 156 +++++++++++++++++++++++++++---------- 4 files changed, 160 insertions(+), 51 deletions(-) diff --git a/tools/commands-packages.js b/tools/commands-packages.js index 303f8d331e..bf7a3844d7 100644 --- a/tools/commands-packages.js +++ b/tools/commands-packages.js @@ -2883,10 +2883,13 @@ main.registerCommand({ }, function (options) { console.log("I AM IN AN APP", options.appDir); var projectContextModule = require('./project-context.js'); - var projectContext = new projectContextModule.ProjectContext(options.appDir); + var projectContext = new projectContextModule.ProjectContext({ + projectDir: options.appDir, + tropohouse: tropohouse.default + }); var messages = buildmessage.capture(function () { - projectContext.resolveConstraints(); + projectContext.prepareProjectForBuild(); }); if (messages.hasMessages()) { Console.error("=> Errors while initializing project:"); diff --git a/tools/package-map.js b/tools/package-map.js index 86047b2fc9..746df7146f 100644 --- a/tools/package-map.js +++ b/tools/package-map.js @@ -15,3 +15,12 @@ exports.PackageMap = function (versions, cat) { } }); }; + +_.extend(exports.PackageMap.prototype, { + eachPackage: function (iterator) { + var self = this; + _.each(self._map, function (info, name) { + iterator(name, _.pick(info, 'kind', 'version')); + }); + } +}); diff --git a/tools/project-context.js b/tools/project-context.js index dd7da24ab0..30fe6acbd5 100644 --- a/tools/project-context.js +++ b/tools/project-context.js @@ -11,10 +11,15 @@ var isopackets = require('./isopackets.js'); var packageMap = require('./package-map.js'); var utils = require('./utils.js'); -exports.ProjectContext = function (appDir) { +exports.ProjectContext = function (options) { var self = this; + if (!options.projectDir) + throw Error("missing projectDir!"); + if (!options.tropohouse) + throw Error("missing tropohouse!"); - self.rootDir = appDir; + self.projectDir = options.projectDir; + self.tropohouse = options.tropohouse; self.packageMap = null; // XXX #3006: Things we're leaving off for now: @@ -26,7 +31,18 @@ exports.ProjectContext = function (appDir) { }; _.extend(exports.ProjectContext.prototype, { - resolveConstraints: function () { + prepareProjectForBuild: function () { + var self = this; + buildmessage.assertInCapture(); + + self._resolveConstraints(); + if (!self.packageMap) + return; // error is already in buildmessage + + self._downloadMissingPackages(); + }, + + _resolveConstraints: function () { var self = this; buildmessage.assertInCapture(); var cat = self._makeProjectCatalog(); @@ -62,7 +78,7 @@ _.extend(exports.ProjectContext.prototype, { _localPackageSearchDirs: function () { var self = this; - var searchDirs = [path.join(self.rootDir, 'packages')]; + var searchDirs = [path.join(self.projectDir, 'packages')]; if (process.env.PACKAGE_DIRS) { // User can provide additional package directories to search in @@ -121,7 +137,7 @@ _.extend(exports.ProjectContext.prototype, { buildmessage.assertInCapture(); buildmessage.enterJob({ title: "reading packages file" }, function () { - var packagesFileLines = files.getLinesOrEmpty(self._getConstraintFile()); + var packagesFileLines = files.getLinesOrEmpty(self._constraintFilename()); _.each(packagesFileLines, function (line) { line = files.trimLine(line); if (line === '') @@ -170,11 +186,20 @@ _.extend(exports.ProjectContext.prototype, { return resolver; }, + _downloadMissingPackages: function () { + var self = this; + buildmessage.assertInCapture(); + if (!self.packageMap) + throw Error("which packages to download?"); + // XXX #3006 This downloads archinfo.host packages. How about + // for deploy? + self.tropohouse.downloadPackagesMissingFromMap(self.packageMap); + }, // Returns the file path to the .meteor/packages file, containing the // constraints for this specific project. - _getConstraintFile : function () { + _constraintFilename : function () { var self = this; - return path.join(self.rootDir, '.meteor', 'packages'); + return path.join(self.projectDir, '.meteor', 'packages'); } }); diff --git a/tools/tropohouse.js b/tools/tropohouse.js index d6fb6fe161..caf66c66fa 100644 --- a/tools/tropohouse.js +++ b/tools/tropohouse.js @@ -182,6 +182,11 @@ _.extend(exports.Tropohouse.prototype, { // // XXX more precise error handling in offline case. maybe throw instead like // warehouse does. actually, generally deal with error handling. + // + // XXX This function is in transition. If the returnDownloadCallback option + // is passed, then it returns null if no download is needed and returns a + // callback that does the download if a download is needed. Otherwise it + // just downloads the package itself. maybeDownloadPackageForArchitectures: function (options) { var self = this; if (!options.packageName) @@ -199,8 +204,11 @@ _.extend(exports.Tropohouse.prototype, { // already have it) // (In the special case of springboarding, we avoid using self.catalog // here because it is catalog.complete and is not yet initialized.) + // XXX #3006 we also trigger this case any time we're coming from + // the downloadPackagesMissingFromMap case, but eventually this + // line should just vanish if (!options.definitelyNotLocal && self.catalog.isLocalPackage(packageName)) - return; + return null; // Figure out what arches (if any) we have loaded for this package version // already. @@ -238,11 +246,9 @@ _.extend(exports.Tropohouse.prototype, { // Have everything we need? Great. if (!archesToDownload.length) { Console.debug("Local package version is up-to-date:", packageName + "@" + version); - return; + return null; } - Console.debug("Downloading missing local versions of package", - packageName + "@" + version, ":", archesToDownload); // Since we are downloading from the server (and we've already done the // local package check), we can use the official catalog here. (This is @@ -257,47 +263,68 @@ _.extend(exports.Tropohouse.prototype, { throw e; } - buildmessage.enterJob({ - title: " Installing " + packageName + "@" + version + "..." - }, function() { - var buildTempDirs = []; - // If there's already a package in the tropohouse, start with it. - if (packageLinkTarget) { - buildTempDirs.push(path.resolve(path.dirname(packageLinkFile), - packageLinkTarget)); - } - // XXX how does concurrency work here? we could just get errors if we try - // to rename over the other thing? but that's the same as in warehouse? - _.each(buildsToDownload, function (build) { - buildTempDirs.push(self.downloadBuildToTempDir({packageName: packageName, version: version}, build)); - }); + var actuallyDownload = function (useBuildmessage) { + if (useBuildmessage) + buildmessage.assertInCapture(); - // We need to turn our builds into a single isopack. - var isopack = new Isopack; - _.each(buildTempDirs, function (buildTempDir, i) { - isopack._loadUnibuildsFromPath( - packageName, - buildTempDir, - {firstIsopack: i === 0}); - }); - // Note: wipeAllPackages depends on this filename structure, as does the - // part above which readlinks. - var newPackageLinkTarget = '.' + version + '.' - + utils.randomToken() + '++' + isopack.buildArchitectures(); - var combinedDirectory = self.packagePath(packageName, newPackageLinkTarget); - isopack.saveToPath(combinedDirectory, { - // We got this from the server, so we can't rebuild it. - elideBuildInfo: true - }); - files.symlinkOverSync(newPackageLinkTarget, packageLinkFile); + Console.debug("Downloading missing local versions of package", + packageName + "@" + version, ":", archesToDownload); - // Clean up old version. - if (packageLinkTarget) { - files.rm_recursive(self.packagePath(packageName, packageLinkTarget)); - } - }); + buildmessage.enterJob({ + title: "downloading " + packageName + "@" + version + "..." + }, function() { + var buildTempDirs = []; + // If there's already a package in the tropohouse, start with it. + if (packageLinkTarget) { + buildTempDirs.push(path.resolve(path.dirname(packageLinkFile), + packageLinkTarget)); + } + // XXX how does concurrency work here? we could just get errors if we + // try to rename over the other thing? but that's the same as in + // warehouse? + _.each(buildsToDownload, function (build) { + try { + buildTempDirs.push(self.downloadBuildToTempDir( + { packageName: packageName, version: version }, build)); + } catch (e) { + if (!useBuildmessage || !(e instanceof files.OfflineError)) + throw e; + buildmessage.exception(e); + } + }); + if (useBuildmessage && buildmessage.jobHasMessages()) + return; - return; + // We need to turn our builds into a single isopack. + var isopack = new Isopack; + _.each(buildTempDirs, function (buildTempDir, i) { + isopack._loadUnibuildsFromPath( + packageName, + buildTempDir, + {firstIsopack: i === 0}); + }); + // Note: wipeAllPackages depends on this filename structure, as does the + // part above which readlinks. + var newPackageLinkTarget = '.' + version + '.' + + utils.randomToken() + '++' + isopack.buildArchitectures(); + var combinedDirectory = self.packagePath( + packageName, newPackageLinkTarget); + isopack.saveToPath(combinedDirectory, { + // We got this from the server, so we can't rebuild it. + elideBuildInfo: true + }); + files.symlinkOverSync(newPackageLinkTarget, packageLinkFile); + + // Clean up old version. + if (packageLinkTarget) { + files.rm_recursive(self.packagePath(packageName, packageLinkTarget)); + } + }); + }; + + if (options.returnDownloadCallback) + return actuallyDownload; + actuallyDownload(); }, @@ -310,6 +337,8 @@ _.extend(exports.Tropohouse.prototype, { // XXX This function's error handling capabilities are poor. It's supposed to // return a data structure that its callers check, but most of its callers // don't check it. Bleah. Should rewrite this and all of its callers. + // + // XXX #3006 This is being replaced by downloadPackagesMissingFromMap downloadMissingPackages: function (versionMap, options) { var self = this; options = options || {}; @@ -347,6 +376,49 @@ _.extend(exports.Tropohouse.prototype, { return downloadedPackages; }, + // Takes in a PackageMap object. Downloads any versioned packages we don't + // already have. + // + // Reports errors via buildmessage. + downloadPackagesMissingFromMap: function (packageMap, options) { + var self = this; + buildmessage.assertInCapture(); + options = options || {}; + var serverArch = options.serverArch || archinfo.host(); + + var downloadCallbacks = {}; + buildmessage.enterJob('checking package versions', function () { + packageMap.eachPackage(function (packageName, info) { + if (info.kind !== 'versioned') + return; + try { + var downloadCallback = self.maybeDownloadPackageForArchitectures({ + returnDownloadCallback: true, + packageName: packageName, + version: info.version, + architectures: [serverArch], + // Don't let tropohouse talk to the catalog, since there's no point. + definitelyNotLocal: true + }); + } catch (e) { + if (!e.noCompatibleBuildError) + throw e; + buildmessage.exception(e); + return; + } + if (downloadCallback) + downloadCallbacks[packageName] = downloadCallback; + }); + }); + + buildmessage.forkJoin( + { title: 'downloading packages', parallel: true}, + downloadCallbacks, + function (cb, packageName) { + cb(true); + }); + }, + latestMeteorSymlink: function () { var self = this; var linkPath = path.join(self.root, 'meteor');