From b97f7c5a768eca41ea9a3dcb392e44cd5d91747e Mon Sep 17 00:00:00 2001 From: ekatek Date: Tue, 27 May 2014 16:07:43 -0700 Subject: [PATCH] renamed some catalog variables, more cleanup around catalog --- tools/catalog-base.js | 40 +++++++++++++++++++++++++----- tools/catalog.js | 13 +++------- tools/commands.js | 55 ++++++++++++++++++++++++----------------- tools/compiler.js | 6 ++--- tools/main.js | 35 ++++++++++++++------------ tools/package-loader.js | 8 +++--- tools/package-source.js | 6 ++--- tools/project.js | 2 +- tools/release.js | 15 +++++------ tools/run-app.js | 2 +- tools/tropohouse.js | 2 +- tools/unipackage.js | 2 +- 12 files changed, 109 insertions(+), 77 deletions(-) diff --git a/tools/catalog-base.js b/tools/catalog-base.js index db00043548..6be5e9f9ab 100644 --- a/tools/catalog-base.js +++ b/tools/catalog-base.js @@ -40,6 +40,9 @@ baseCatalog.BaseCatalog = function () { }; +// XXX: We have a pattern on retrieval of data, where we try, fail, then try to +// refresh. Do we want to keep that? I think so. Come back to it. + _.extend(baseCatalog.BaseCatalog.prototype, { // Set all the collections to their initial values, which are mostly // blank. This does not set self.initialized -- do that manually in the child @@ -97,13 +100,23 @@ _.extend(baseCatalog.BaseCatalog.prototype, { var self = this; self._requireInitialized(); - var versionRecord = _.findWhere(self.releaseVersions, - { track: track, version: version }); + var retrieveRecord = function () { + return _.findWhere(self.releaseVersions, + { track: track, version: version }); + }; + var versionRecord = retrieveRecord(); + // The first time, we try to refresh and try again. If we don't have the + // information after that, tough luck. if (!versionRecord) { - return null; + self.refresh(); + versionRecord = retrieveRecord(); + } + if (!versionRecord) { + return null; } return versionRecord; + }, // Return an array with the names of all of the release tracks that we know @@ -175,10 +188,20 @@ _.extend(baseCatalog.BaseCatalog.prototype, { version = self._getLocalVersion(version); } - var versionRecord = _.findWhere(self.versions, { packageName: name, - version: version }); + var retrieveRecord = function () { + return _.findWhere(self.versions, { packageName: name, + version: version }); + }; + var versionRecord = retrieveRecord(); + + // The first time, we try to refresh and try again. If we don't have the + // information after that, tough luck. if (!versionRecord) { - return null; + self.refresh(); + versionRecord = retrieveRecord(); + } + if (!versionRecord) { + return null; } return versionRecord; }, @@ -312,5 +335,10 @@ _.extend(baseCatalog.BaseCatalog.prototype, { return packageDir; } return null; + }, + + // Reload catalog data to account for new information if needed. + refresh: function () { + throw new Error("no such thing as a base refresh"); } }); diff --git a/tools/catalog.js b/tools/catalog.js index 21f189d17e..c478a07c61 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -729,16 +729,11 @@ _.extend(CompleteCatalog.prototype, { // This is the catalog that's used to answer the specific question of "so what's // on the server?". It does not contain any local catalogs. Typically, we call -// catalog.serverCatalog.refresh(true) to update data.json. -catalog.serverCatalog = new ServerCatalog(); +// catalog.official.refresh(true) to update data.json. +catalog.official = new ServerCatalog(); // This is the catalog that's used to actually drive the constraint solver: it // contains local packages, and since local packages always beat server // packages, it doesn't contain any information about the server version of -// local packages. Typically, we call catalog.catalog.refresh() after doing a -// sync-refresh of serverCatalog; since only serverCatalog does the sync -// request, the two catalogs are not fighting over the data files on disk. -// -// XXX we haven't finished this refactoring yet so there are plenty of -// catalog.catalog.refresh(true) calls -catalog.catalog = new CompleteCatalog(); +// local packages. +catalog.complete = new CompleteCatalog(); diff --git a/tools/commands.js b/tools/commands.js index 87bf84001e..91fdee127e 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -22,11 +22,17 @@ var packageCache = require('./package-cache.js'); var PackageLoader = require('./package-loader.js'); var PackageSource = require('./package-source.js'); var compiler = require('./compiler.js'); -var catalog = require('./catalog.js').catalog; -var serverCatalog = require('./catalog.js').serverCatalog; +var catalogs = require('./catalog.js'); var stats = require('./stats.js'); var unipackage = require('./unipackage.js'); +// Reminder: we have two catalogs. The complete catalog contains local packages, +// and uses them to override the server data. The official catalog only knows +// about server packages. All query operations on package server contents should +// refer to the /official/ catalog. +var catalog = catalogs.complete; +var officialCatalog = catalogs.official; + // Given a site name passed on the command line (eg, 'mysite'), return // a fully-qualified hostname ('mysite.meteor.com'). // @@ -410,7 +416,7 @@ main.registerCommand({ var couldNotContactServer = false; // Refresh the catalog, cacheing the remote package data on the server. - serverCatalog.refresh(true); + officialCatalog.refresh(true); // refuse to update if we're in a git checkout. if (! files.usesWarehouse()) { @@ -638,7 +644,7 @@ main.registerCommand({ var failed = false; // Refresh the catalog, cacheing the remote package data on the server. - serverCatalog.refresh(true); + officialCatalog.refresh(); // Read in existing package dependencies. var packages = project.getConstraints(); @@ -816,7 +822,7 @@ main.registerCommand({ // server. Technically, we don't need to do this, since it is unlikely that // new data will change our constraint solver decisions. But as a user, I // would expect this command to update the local catalog. - serverCatalog.refresh(true); + officialCatalog.refresh(true); // Read in existing package dependencies. var packages = project.getConstraints(); @@ -882,7 +888,7 @@ main.registerCommand({ // are only calling 'using', this is not nessessary, but, once again, as a // user, I would not be surprised to see this contact the server. In the // future, we should move this call to sync somewhere in the background. - serverCatalog.refresh(true); + officialCatalog.refresh(true); if (options.releases && options.using) { console.log("XXX: The contents of your release file."); @@ -1590,13 +1596,16 @@ main.registerCommand({ var releaseNameAndVersion = options.args[0]; var outputDirectory = options.args[1]; - serverCatalog.refresh(true); + // In this function, we want to use the official catalog everywhere, because + // we assume that all packages have been published (along with the release + // obviously) and we want to be sure to only bundle the published versions. + officialCatalog.refresh(); var parsed = utils.splitConstraint(releaseNameAndVersion); if (!parsed.constraint) throw new main.ShowUsage; - var release = serverCatalog.getReleaseVersion(parsed.package, + var release = officialCatalog.getReleaseVersion(parsed.package, parsed.constraint); if (!release) { // XXX this could also mean package unknown. @@ -1607,7 +1616,7 @@ main.registerCommand({ var toolPkg = release.tool && utils.splitConstraint(release.tool); if (! (toolPkg && toolPkg.constraint)) throw new Error("bad tool in release: " + toolPkg); - var toolPkgBuilds = serverCatalog.getAllBuilds( + var toolPkgBuilds = officialCatalog.getAllBuilds( toolPkg.package, toolPkg.constraint); if (!toolPkgBuilds) { // XXX this could also mean package unknown. @@ -1641,7 +1650,7 @@ main.registerCommand({ // need for the OSes that the tool is built for. _.each(osArches, function (osArch) { _.each(release.packages, function (pkgVersion, pkgName) { - if (!serverCatalog.getBuildsForArches(pkgName, pkgVersion, [osArch])) { + if (!officialCatalog.getBuildsForArches(pkgName, pkgVersion, [osArch])) { throw Error("missing build of " + pkgName + "@" + pkgVersion + " for " + osArch); } @@ -1653,11 +1662,11 @@ main.registerCommand({ _.each(osArches, function (osArch) { var tmpdir = files.mkdtemp(); // We're going to build and tar up a tropohouse in a temporary directory; we - // don't want to use any of our local packages, so we use serverCatalog + // don't want to use any of our local packages, so we use officialCatalog // instead of catalog. // XXX update to '.meteor' when we combine houses var tmpTropo = new tropohouse.Tropohouse( - path.join(tmpdir, '.meteor0'), serverCatalog); + path.join(tmpdir, '.meteor0'), officialCatalog); tmpTropo.maybeDownloadPackageForArchitectures( {packageName: toolPkg.package, version: toolPkg.constraint}, [osArch]); // XXX 'browser' too? @@ -1774,7 +1783,7 @@ main.registerCommand({ // Refresh the catalog, caching the remote package data on the server. We can // optimize the workflow by using this data to weed out obviously incorrect // submissions before they ever hit the wire. - serverCatalog.refresh(true); + officialCatalog.refresh(true); try { var conn = packageClient.loggedInPackagesConnection(); @@ -1823,7 +1832,8 @@ main.registerCommand({ }*/ // We have initialized everything, so perform the publish oepration. - var ec = packageClient.publishPackage(packageSource, compileResult, conn, { new: options.create }); + var ec = packageClient.publishPackage( + packageSource, compileResult, conn, { new: options.create }); // We are only publishing one package, so we should close the connection, and // then exit with the previous error code. @@ -1842,13 +1852,13 @@ main.registerCommand({ }, function (options) { // Refresh the catalog, cacheing the remote package data on the server. - serverCatalog.refresh(true); + officialCatalog.refresh(true); if (! catalog.getPackage(options.name)) { process.stderr.write('No package named ' + options.name); return 1; } - var pkgVersion = catalog.getVersion(options.name, options.versionString); + var pkgVersion = officialCatalog.getVersion(options.name, options.versionString); if (! pkgVersion) { process.stderr.write('There is no version ' + options.versionString + ' for package ' + @@ -1915,7 +1925,7 @@ main.registerCommand({ }, function (options) { // Refresh the catalog, cacheing the remote package data on the server. - serverCatalog.refresh(true); + officialCatalog.refresh(true); try { var conn = packageClient.loggedInPackagesConnection(); @@ -2060,7 +2070,8 @@ main.registerCommand({ // Let's get the server version that this local package is // overwriting. If such a version exists, we will need to make sure // that the contents are the same. - var oldVersion = serverCatalog.getVersion(item, packageSource.version); + var oldVersion = officialCatalog.getVersion + (item, packageSource.version); // Include this package in our release. myPackages[item] = packageSource.version; @@ -2086,7 +2097,7 @@ main.registerCommand({ compileResult: compileResult}; return; } else { - var existingBuild = serverCatalog.getBuildWithArchesString( + var existingBuild = officialCatalog.getBuildWithArchesString( oldVersion, compileResult.unipackage.architecturesString()); @@ -2138,7 +2149,7 @@ main.registerCommand({ _.each(toPublish, function(prebuilt, name) { var opts = { - new: !serverCatalog.getPackage(name) + new: !officialCatalog.getPackage(name) }; process.stdout.write("Publishing package: " + name + "\n"); @@ -2169,7 +2180,7 @@ main.registerCommand({ // Check if the release track exists. If it doesn't, need the create flag. if (!options['create-track']) { - var trackRecord = serverCatalog.getReleaseTrack(relConf.track); + var trackRecord = officialCatalog.getReleaseTrack(relConf.track); if (!trackRecord) { process.stderr.write('There is no release track named ' + relConf.track + '. If you are creating a new track, use the --create-track flag. \n'); @@ -2196,7 +2207,7 @@ main.registerCommand({ }); // Get it back. - serverCatalog.refresh(true); + officialCatalog.refresh(true); process.stdout.write("Done! \n"); return 0; diff --git a/tools/compiler.js b/tools/compiler.js index dca95890d6..e212ba0161 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -162,7 +162,7 @@ var determineBuildTimeDependencies = function (packageSource) { }); var versions = packageSource.dependencyVersions.dependencies || {}; - ret.packageDependencies = catalog.catalog.resolveConstraints(constraints, + ret.packageDependencies = catalog.complete.resolveConstraints(constraints, { previousSolution: versions }); // We care about differentiating between all dependencies (which we save in @@ -192,7 +192,7 @@ var determineBuildTimeDependencies = function (packageSource) { var pluginVersion = pluginVersions[info.name] || {}; ret.pluginDependencies[info.name] = - catalog.catalog.resolveConstraints( + catalog.complete.resolveConstraints( constraints, { previousSolution: pluginVersion }); }); @@ -824,7 +824,7 @@ var getPluginProviders = function (versions) { _.each(versions, function (version, name) { // Direct dependencies only create a build-order constraint if // they contain a plugin. - var catalogVersion = catalog.catalog.getVersion(name, version); + var catalogVersion = catalog.complete.getVersion(name, version); if (catalogVersion && catalogVersion.containsPlugins) { result[name] = version; } diff --git a/tools/main.js b/tools/main.js index da4b7a7db7..8ff6787500 100644 --- a/tools/main.js +++ b/tools/main.js @@ -595,24 +595,27 @@ Fiber(function () { files.getCurrentToolsDir(), 'packages')); } - // Initialize the singleton Catalog. Only after this point is the - // Catalog (and therefore uniload) usable. - // - // If the --offline-catalog option is set, the catalog will be offline and - // will never attempt to contact the server for more recent data. Otherwise, - // the catalog will attempt to synchronize with the remote package server. - catalog.catalog.initialize({ - localPackageDirs: localPackageDirs, - offline: _.has(rawOptions, '--offline-catalog') + // Initialize the complete Catalog, which we use to retrieve packages. Only + // after this point is the Catalog (and therefore uniload) usable. + catalog.complete.initialize({ + localPackageDirs: localPackageDirs }); - // XXX maybe only do this for commands that need it - catalog.serverCatalog.initialize({ - offline: _.has(rawOptions, '--offline-catalog') - }); - // We need to delete the option or we will throw an error. - // XXX: This seems like a hack? - delete rawOptions['--offline-catalog']; + // Initialize the server catalog. We don't load data into the server catalog + // until refresh is called, so this probably doesn't take up too much + // memory. We could move this initialization to commands.js or something, but + // this makes it easier to not bother propagating the offline-catalog option + // through every command that might care about it. + // + // If the --offline-catalog option is set, the + // catalog will be offline and will never attempt to contact the server for + // more recent data. Otherwise, the catalog will attempt to synchronize with + // the remote package server. + catalog.official.initialize({ + offline: _.has(rawOptions, '--offline-catalog') + }); + // Delete the offline option. + delete rawOptions['--offline-catalog']; // Now before we do anything else, figure out the release to use, // and if that release goes with a different version of the tools, diff --git a/tools/package-loader.js b/tools/package-loader.js index 250263c571..ee5d5e92d4 100644 --- a/tools/package-loader.js +++ b/tools/package-loader.js @@ -2,7 +2,7 @@ var fs = require('fs'); var path = require('path'); var _ = require('underscore'); var packageCache = require('./package-cache.js'); -var catalog = require('./catalog.js'); +var catalogs = require('./catalog.js'); var utils = require('./utils.js'); var buildmessage = require('./buildmessage.js'); var unipackage = require('./unipackage.js'); @@ -66,9 +66,9 @@ _.extend(PackageLoader.prototype, { var versionRecord; if (self.versions === null) { - versionRecord = catalog.catalog.getLatestVersion(name); + versionRecord = catalogs.complete.getLatestVersion(name); } else if (_.has(self.versions, name)) { - versionRecord = catalog.catalog.getVersion(name, self.versions[name]); + versionRecord = catalogs.complete.getVersion(name, self.versions[name]); } else { throw new Error("no version specified for package " + name); } @@ -106,7 +106,7 @@ _.extend(PackageLoader.prototype, { version = null; } - return catalog.catalog.getLoadPathForPackage(name, version); + return catalogs.complete.getLoadPathForPackage(name, version); }, // Given a package name like "ddp" and an architecture, get the build of that diff --git a/tools/package-source.js b/tools/package-source.js index a6b083cbe7..66495b0bd3 100644 --- a/tools/package-source.js +++ b/tools/package-source.js @@ -768,7 +768,6 @@ _.extend(PackageSource.prototype, { }); }, - // Use this release to resolve unclear dependencies for this package. If // you don't fill in dependencies for some of your implies/uses, we will // look at the packages listed in the release to figure that out. @@ -777,14 +776,13 @@ _.extend(PackageSource.prototype, { // XXX: Error handling if (relInf.length !== 2) throw new Error("Incorrect release spec"); - var catalog = require('./catalog.js'); - releaseRecord = catalog.catalog.getReleaseVersion(relInf[0], relInf[1]); + var catalog = require('./catalog.js').complete; + releaseRecord = catalog.getReleaseVersion(relInf[0], relInf[1]); if (!releaseRecord) { throw new Error("Unknown release"); } }, - // Export symbols from this package. // // @param symbols String (eg "Foo") or array of String diff --git a/tools/project.js b/tools/project.js index d3747861ad..dbc9f8e666 100644 --- a/tools/project.js +++ b/tools/project.js @@ -171,7 +171,7 @@ _.extend(Project.prototype, { // solution. Remember to check 'ignoreProjectDeps', otherwise it will just // try to look up the solution in our own dependencies and it will be a // disaster. - var newVersions = catalog.catalog.resolveConstraints( + var newVersions = catalog.complete.resolveConstraints( self.combinedConstraints, { previousSolution: self.dependencies }, { ignoreProjectDeps: true } diff --git a/tools/release.js b/tools/release.js index 50f4571e2c..6969dd49dc 100644 --- a/tools/release.js +++ b/tools/release.js @@ -89,7 +89,9 @@ _.extend(Release.prototype, { // the dependencyVersions, we have already run the constraint solver, so the // catalog has been initialized. var catalog = require('./catalog.js'); - var catversion = catalog.catalog.getLatestVersion("meteor-tool").version; + // We call this on the complete catalog, because it is possible for us to + // have a local version of the tool. + var catversion = catalog.complete.getLatestVersion("meteor-tool").version; // The catalog version is going to have a +local at the end. We will never // be able to springboard to that, so we should skip it. var eqVersion = catversion.split("+")[0]; @@ -166,7 +168,7 @@ release.latestDownloaded = function () { if (process.env.METEOR_TEST_LATEST_RELEASE) return process.env.METEOR_TEST_LATEST_RELEASE; - var defaultRelease = catalog.serverCatalog.getDefaultReleaseVersion(); + var defaultRelease = catalog.official.getDefaultReleaseVersion(); if (!defaultRelease) { throw new Error("no latest release available?"); } @@ -208,17 +210,12 @@ release.load = function (name, options) { track = parts[0]; version = parts[1]; } else { - track = catalog.DEFAULT_TRACK; + track = catalog.official.DEFAULT_TRACK; version = parts[0]; name = track + '@' + version; } - var releaseVersion = catalog.catalog.getReleaseVersion(track, version); - if (releaseVersion === null) { - // XXX maybe a better pattern for this? - catalog.serverCatalog.refresh(true); - releaseVersion = catalog.catalog.getReleaseVersion(track, version); - } + var releaseVersion = catalog.official.getReleaseVersion(track, version); if (releaseVersion === null) { // XXX check the warehouse too, or maybe before refresh // XXX Pre090 better error, probably something like diff --git a/tools/run-app.js b/tools/run-app.js index 3e51e9459e..dbf8806558 100644 --- a/tools/run-app.js +++ b/tools/run-app.js @@ -449,7 +449,7 @@ _.extend(AppRunner.prototype, { // HACK: Also make sure we notice when somebody adds a package to // the app packages dir that may override a catalog package. - catalog.catalog.watchLocalPackageDirs(watchSet); + catalog.complete.watchLocalPackageDirs(watchSet); // Were there errors? if (bundleResult.errors) { diff --git a/tools/tropohouse.js b/tools/tropohouse.js index 24e5fe3fe8..efc73cb74c 100644 --- a/tools/tropohouse.js +++ b/tools/tropohouse.js @@ -39,7 +39,7 @@ var defaultWarehouseDir = function () { // download local packages; you can make your own Tropohouse to override these // things. exports.default = new exports.Tropohouse( - defaultWarehouseDir(), catalog.catalog); + defaultWarehouseDir(), catalog.complete); _.extend(exports.Tropohouse.prototype, { // Return the directory within the warehouse that would contain downloaded diff --git a/tools/unipackage.js b/tools/unipackage.js index ca4b49bcd5..d1bcd8b39d 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -965,7 +965,7 @@ _.extend(Unipackage.prototype, { self.buildTimeDirectDependencies, function (packageName, version) { // filter if (packageName !== self.name) { - var catalogVersion = catalog.catalog.getVersion(packageName, + var catalogVersion = catalog.complete.getVersion(packageName, version); // XXX This could throw if we call it on a freshly-built // unipackage (as opposed to one read from disk that has real