From b5d2cce30887290ddb883af87fa2fe9fd5d032ad Mon Sep 17 00:00:00 2001 From: David Glasser Date: Thu, 30 Oct 2014 22:43:13 -0700 Subject: [PATCH] Refactor LocalCatalog - Simplify internal data structure to just have one map of name -> various data instead of a whole bunch of different unsynchronized data structures (most of which were used with frequent O(n) operations). - Move the getLoadPathForPackage logic which combines local and remote packages into LayeredCatalog from LocalCatalog, and delete the unnecessary copy in BootstrapCatalogCheckout. - Rename a bunch of fields to make it explicit which ones contain package objects, which ones contain package directories, and which ones contain package *search* directories - Replace random version IDs and a long comment about why random is safe with sequential. (I don't think these version IDs are ever used anyway.) - Drop unused "initializing" option to refresh - Drop redundant call to _recomputeEffectiveLocalPackages in addLocalPackage (it is called immediately below by refresh). --- tools/catalog-bootstrap-checkout.js | 53 +-- tools/catalog-local.js | 465 ++++++++++-------------- tools/catalog.js | 14 +- tools/commands-packages.js | 2 +- tools/commands.js | 8 +- tools/main.js | 13 +- tools/tests/old/test-bundler-assets.js | 4 +- tools/tests/old/test-bundler-npm.js | 6 +- tools/tests/old/test-bundler-options.js | 4 +- 9 files changed, 235 insertions(+), 334 deletions(-) diff --git a/tools/catalog-bootstrap-checkout.js b/tools/catalog-bootstrap-checkout.js index d0d044e545..f3d0eb5247 100644 --- a/tools/catalog-bootstrap-checkout.js +++ b/tools/catalog-bootstrap-checkout.js @@ -44,61 +44,14 @@ _.extend(BootstrapCatalogCheckout.prototype, { constraint.constraints[0].type !== 'any-reasonable') { throw Error("Surprising constraint: " + JSON.stringify(constraint)); } - if (!_.has(self.versions, constraint.name)) { + if (!_.has(self.packages, constraint.name)) { throw Error("Trying to resolve unknown package: " + constraint.name); } - if (_.isEmpty(self.versions[constraint.name])) { - throw Error("Trying to resolve versionless package: " + constraint.name); - } - if (_.size(self.versions[constraint.name]) > 1) { - throw Error("Too many versions for package: " + constraint.name); - } - ret[constraint.name] = _.keys(self.versions[constraint.name])[0]; + ret[constraint.name] = + self.packages[constraint.name].versionRecord.version; }); return ret; - }, - - - // Given a name and a version of a package, return a path on disk - // from which we can load it. If we don't have it on disk (we - // haven't downloaded it, or it just plain doesn't exist in the - // catalog) return null. - // - // Doesn't download packages. Downloading should be done at the time - // that .meteor/versions is updated. - // - // HACK: Version can be null if you are certain that the package is to be - // loaded from local packages. In the future, version should always be - // required and we should confirm that the version on disk is the version that - // we asked for. This is to support isopack loader not having a version - // manifest. - getLoadPathForPackage: function (name, version, constraintSolverOpts) { - var self = this; - self._requireInitialized(); - buildmessage.assertInCapture(); - constraintSolverOpts = constraintSolverOpts || {}; - - // Check local packages first. - if (_.has(self.packageSources, name)) { - - // If we don't have a build of this package, we need to rebuild it. - self._build(name, {}, constraintSolverOpts); - - // Return the path. - return self.packageSources[name].sourceRoot; - } - - if (! version) { - throw new Error(name + " not a local package, and no version specified?"); - } - - var packageDir = tropohouse.default.packagePath(name, version); - if (fs.existsSync(packageDir)) { - return packageDir; - } - return null; } - }); exports.BootstrapCatalogCheckout = BootstrapCatalogCheckout; diff --git a/tools/catalog-local.js b/tools/catalog-local.js index 149be54bdb..36a61bae08 100644 --- a/tools/catalog-local.js +++ b/tools/catalog-local.js @@ -20,10 +20,9 @@ var VersionParser = require('./package-version-parser.js'); var LocalCatalog = function (options) { var self = this; - // Package server data. Mostly arrays of objects. - self.packages = null; - self.versions = null; // package name -> version -> object - self.builds = null; + // Package server data. Maps package name to a {packageSource, packageRecord, + // versionRecord, buildRecord} object. + self.packages = {}; // We use the initialization design pattern because it makes it easier to use // both of our catalogs as singletons. @@ -31,62 +30,63 @@ var LocalCatalog = function (options) { self.containingCatalog = options ? options.containingCatalog : self; // Local directories to search for package source trees - self.localPackageDirs = null; + self.localPackageSearchDirs = null; // Packagedirs specified by addLocalPackage: added explicitly through a - // directory. We mainly use this to allow the user to run test-packages against a - // package in a specific directory. - self.localPackages = []; + // directory. We mainly use this to allow the user to run test-packages + // against a package in a specific directory. + self.explicitlyAddedLocalPackageDirs = []; - // All packages found either by localPackageDirs or localPackages. There is a - // hierarghy of packages, as detailed below and there can only be one local - // version of a package at a time. This refers to the package by the specific - // package directory that we need to process. - self.effectiveLocalPackages = []; + // All packages found either by localPackageSearchDirs or + // explicitlyAddedLocalPackageDirs. There is a hierarchy of packages, as + // detailed below and there can only be one local version of a package at a + // time. This refers to the package by the specific package directory that we + // need to process. + self.effectiveLocalPackageDirs = []; // Each catalog needs its own package cache. self.packageCache = new packageCache.PackageCache(self.containingCatalog); - self.packageSources = null; - self.built = null; + self.buildRequested = null; + + self.nextVersionId = 1; }; _.extend(LocalCatalog.prototype, { toString: function () { var self = this; - return "LocalCatalog [localPackageDirs=" + self.localPackageDirs + "]"; + return "LocalCatalog [localPackageSearchDirs=" + + self.localPackageSearchDirs + "]"; }, // Initialize the Catalog. This must be called before any other // Catalog function. // options: - // - localPackageDirs: an array of paths on local disk, that - // contain subdirectories, that each contain a source tree for a - // package that should override the packages on the package - // server. For example, if there is a package 'foo' that we find - // through localPackageDirs, then we will ignore all versions of - // 'foo' that we find through the package server. Directories - // that don't exist (or paths that aren't directories) will be - // silently ignored. + // - localPackageSearchDirs: an array of paths on local disk, that contain + // subdirectories, that each contain a source tree for a package that + // should override the packages on the package server. For example, if + // there is a package 'foo' that we find through localPackageSearchDirs, + // then we will ignore all versions of 'foo' that we find through the + // package server. Directories that don't exist (or paths that aren't + // directories) will be silently ignored. initialize: function (options) { var self = this; buildmessage.assertInCapture(); options = options || {}; - // initializing this here to make it clear that this exists and we have - // access to it -- a map of names of local packages to their package - // sources. We call upon this when we compile the package. - self.packageSources = {}; - // At this point, effectiveLocalPackageDirs is just the local package // directories, since we haven't had a chance to add any other local // packages. Nonetheless, let's set those. - self.localPackageDirs = - _.filter(options.localPackageDirs || [], utils.isDirectory); + self.localPackageSearchDirs = []; + _.each(options.localPackageSearchDirs, function (dir) { + dir = path.resolve(dir); + if (utils.isDirectory(dir)) + self.localPackageSearchDirs.push(dir); + }); - self.refresh({initializing: true}); + self.refresh(); }, // Set all the collections to their initial values, which are mostly @@ -96,12 +96,9 @@ _.extend(LocalCatalog.prototype, { var self = this; // Initialize everything to its default version. - self.packages = []; - self.versions = {}; - self.builds = []; + self.packages = {}; - self.packageSources = {}; - self.built = {}; + self.buildRequested = {}; }, // Throw if the catalog's self.initialized value has not been set to true. @@ -118,37 +115,30 @@ _.extend(LocalCatalog.prototype, { var self = this; self._requireInitialized(); - return _.pluck(self.packages, 'name'); + return _.keys(self.packages); }, // Returns general (non-version-specific) information about a // package, or null if there is no such package. getPackage: function (name, options) { var self = this; - buildmessage.assertInCapture(); self._requireInitialized(); options = options || {}; - return _.findWhere(self.packages, { name: name }); + if (!_.has(self.packages, name)) + return null; + return self.packages[name].packageRecord; }, - // Given a package, returns an array of the versions available for - // this package (for any architecture), sorted from oldest to newest - // (according to the version string, not according to their - // publication date). Returns the empty array if the package doesn't - // exist or doesn't have any versions. - // - // (XXX: If local catalog is just the local overrides, wouldn't this always - // just return one record?) + // Given a package, returns an array of the versions available (ie, the one + // version we have, or an empty array). getSortedVersions: function (name) { var self = this; self._requireInitialized(); - if (!_.has(self.versions, name)) { + + if (!_.has(self.packages, name)) return []; - } - var ret = _.keys(self.versions[name]); - ret.sort(VersionParser.compare); - return ret; + return [self.packages[name].versionRecord.version]; }, // Return information about a particular version of a package, or @@ -157,28 +147,17 @@ _.extend(LocalCatalog.prototype, { var self = this; self._requireInitialized(); - var lookupVersion = function () { - return _.has(self.versions, name) && - _.has(self.versions[name], version) && - self.versions[name][version]; - }; - + if (!_.has(self.packages, name)) + return null; + var versionRecord = self.packages[name].versionRecord; // The catalog doesn't understand buildID versions and doesn't know about // them. Depending on when we build them, we can refer to local packages as // 1.0.0+local or 1.0.0+[buildId]. Luckily, we know which packages are // local, so just look those up by their local version instead. - // XXX ideally we'd only have isLocalPackage in the complete catalog and - // have CompleteCatalog override getVersion, but other things want - // to call isLocalPackage, eg maybeDownloadPackageForArchitectures - // which has the official package when running make-bootstrap-tarballs - if (self.isLocalPackage(name)) { - version = self._getLocalVersion(version); - // No need to refresh here: if we can't find the local version, refreshing - // isn't going to help! - return lookupVersion() || null; - } - - return lookupVersion() || null; + version = self._changeBuildIdToLocal(version); + if (versionRecord.version !== version) + return null; + return versionRecord; }, // As getVersion, but returns info on the latest version of the @@ -186,9 +165,9 @@ _.extend(LocalCatalog.prototype, { getLatestVersion: function (name) { var self = this; - var versions = self.getSortedVersions(name); - versions.reverse(); - return self.getVersion(name, versions[0]); + if (!_.has(self.packages, name)) + return null; + return self.packages[name].versionRecord; }, // Refresh the packages in the catalog, by re-scanning local packages. @@ -203,32 +182,29 @@ _.extend(LocalCatalog.prototype, { self.reset(); self._recomputeEffectiveLocalPackages(); - var allOK = self._loadLocalPackages({ watchSet: options.watchSet }); + self._loadLocalPackages({ watchSet: options.watchSet }); self.initialized = true; - // Rebuild the resolver, since packages may have changed. - self.resolver = null; - }, - // Compute self.effectiveLocalPackages from self.localPackageDirs - // and self.localPackages. + // Compute self.effectiveLocalPackageDirs from self.localPackageSearchDirs and + // self.explicitlyAddedLocalPackageDirs. _recomputeEffectiveLocalPackages: function () { var self = this; + self.effectiveLocalPackageDirs = _.clone( + self.explicitlyAddedLocalPackageDirs); - self.effectiveLocalPackages = _.clone(self.localPackages); - - _.each(self.localPackageDirs, function (localPackageDir) { - if (! utils.isDirectory(localPackageDir)) + _.each(self.localPackageSearchDirs, function (searchDir) { + if (! utils.isDirectory(searchDir)) return; - var contents = fs.readdirSync(localPackageDir); + var contents = fs.readdirSync(searchDir); _.each(contents, function (item) { - var packageDir = path.resolve(path.join(localPackageDir, item)); + var packageDir = path.join(searchDir, item); if (! utils.isDirectory(packageDir)) return; - // Consider a directory to be a package source tree if it - // contains 'package.js'. (We used to support isopacks in - // localPackageDirs, but no longer.) + // Consider a directory to be a package source tree if it contains + // 'package.js'. (We used to support isopacks in localPackageSearchDirs, + // but no longer.) if (fs.existsSync(path.join(packageDir, 'package.js'))) { // Let earlier package directories override later package // directories. @@ -236,7 +212,7 @@ _.extend(LocalCatalog.prototype, { // We don't know the name of the package, so we can't deal with // duplicates yet. We are going to have to rely on the fact that we // are putting these in in order, to be processed in order. - self.effectiveLocalPackages.push(packageDir); + self.effectiveLocalPackageDirs.push(packageDir); } }); }); @@ -247,21 +223,18 @@ _.extend(LocalCatalog.prototype, { options = options || {}; buildmessage.assertInCapture(); - var allOK = true; - // Load the package source from a directory. We don't know the names of our // local packages until we do this. // - // THIS MUST BE RUN IN LOAD ORDER. Let's say that we have two directories for - // mongo-livedata. The first one processed by this function will be canonical. - // The second one will be ignored. - // XXX: EEP. - // (note: this is the behavior that we want for overriding things in checkout. - // It is not clear that you get good UX if you have two packages with the same - // name in your app. We don't check that.) - var initSourceFromDir = function (packageDir, definiteName) { + // THIS MUST BE RUN IN LOAD ORDER. Let's say that we have two directories + // for mongo-livedata. The first one processed by this function will be + // canonical. The second one will be ignored. + // + // (note: this is the behavior that we want for overriding things in + // checkout. It is not clear that you get good UX if you have two packages + // with the same name in your app. We don't check that.) + var initSourceFromDir = function (packageDir, definiteName) { var packageSource = new PackageSource(self.containingCatalog); - var broken = false; buildmessage.enterJob({ title: "reading package from `" + packageDir + "`", rootPath: packageDir @@ -281,140 +254,116 @@ _.extend(LocalCatalog.prototype, { opts["name"] = definiteName; } packageSource.initFromPackageDir(packageDir, opts); - if (buildmessage.jobHasMessages()) { - broken = true; - allOK = false; + + if (options.watchSet) { + options.watchSet.merge(packageSource.pluginWatchSet); + _.each(packageSource.architectures, function (sourceArch) { + options.watchSet.merge(sourceArch.watchSet); + }); } - }); - if (options.watchSet) { - options.watchSet.merge(packageSource.pluginWatchSet); - _.each(packageSource.architectures, function (sourceArch) { - options.watchSet.merge(sourceArch.watchSet); - }); - } + // Recover by ignoring, but not until after we've augmented the watchSet + // (since we want the watchSet to include files with problems that the + // user may fix!) + if (buildmessage.jobHasMessages()) { + return; + } - // Recover by ignoring, but not until after we've augmented the watchSet - // (since we want the watchSet to include files with problems that the - // user may fix!) - if (broken) - return; + // Now that we have initialized the package from package.js, we know its + // name. + var name = packageSource.name; - // Now that we have initialized the package from package.js, we know its - // name. - var name = packageSource.name; + // We should only have one package dir for each name; in this case, we + // are going to take the first one we get (since we preserved the order + // in which we loaded local package dirs when running this function.) + if (_.has(self.packages, name)) + return; - // We should only have one package dir for each name; in this case, we are - // going to take the first one we get (since we preserved the order in - // which we loaded local package dirs when running this function.) - if (!self.packageSources[name]) { - self.packageSources[name] = packageSource; + // Accurate version numbers are of supreme importance, because we use + // version numbers (of build-time dependencies such as the coffeescript + // plugin), together with source file hashes and the notion of a + // repeatable build, to decide when a package build is out of date and + // trigger a rebuild of the package. + // + // The package we have just loaded may declare its version to be 1.2.3, + // but that doesn't mean it's really the official version 1.2.3 of the + // package. It only gets that version number officially when it's + // published to the package server. So what we'd like to do here is give + // it a version number like '1.2.3+', where is a hash + // of everything that's necessary to repeat the build exactly: all of + // the package's source files, all of the package's build-time + // dependencies, and the version of the Meteor build tool used to build + // it. + // + // Unfortunately we can't actually compute such a buildid yet since it + // depends on knowing the build-time dependencies of the package, which + // requires that we run the constraint solver, which can only be done + // once we've populated the catalog, which is what we're trying to do + // right now. + // + // So we have a workaround. For local packages we will fake the version + // in the catalog by setting the buildid to 'local', as in + // '1.2.3+local'. This is enough for the constraint solver to run, but + // any code that actually relies on accurate versions (for example, code + // that checks if a build is up to date) needs to be careful to get the + // versions not from the catalog but from the actual built Isopack + // objects, which will have accurate versions (with precise buildids) + // even for local packages. + var version = packageSource.version; + if (version.indexOf('+') !== -1) + throw new Error("version already has a buildid?"); + version = version + "+local"; - // If this is NOT a test package AND it has tests (tests will be marked - // as test packages by package source, so we will not recurse + self.packages[name] = { + packageSource: packageSource, + packageRecord: { + name: name, + maintainers: null, + lastUpdated: null + }, + versionRecord: { + _id: "VID" + self.nextVersionId++, + packageName: name, + testName: packageSource.testName, + version: version, + publishedBy: null, + earliestCompatibleVersion: packageSource.earliestCompatibleVersion, + description: packageSource.metadata.summary, + dependencies: packageSource.getDependencyMetadata(), + source: null, + lastUpdated: null, + published: null, + isTest: packageSource.isTest, + debugOnly: packageSource.debugOnly, + containsPlugins: packageSource.containsPlugins() + }, + buildRecord: null + }; + + // If this is NOT a test package AND it has tests (tests will be + // marked as test packages by package source, so we will not recurse // infinitely), then process that too. if (!packageSource.isTest && packageSource.testName) { initSourceFromDir(packageSource.sourceRoot, packageSource.testName); } - } - }; - - // Given a package-source, create its catalog record. - var initCatalogRecordsFromSource = function (packageSource) { - var name = packageSource.name; - - // Create the package record. - self.packages.push({ - name: name, - maintainers: null, - lastUpdated: null }); - - // This doesn't have great birthday-paradox properties, but we - // don't have Random.id() here (since it comes from a - // isopack), and making an index so we can see if a value is - // already in use would complicated the code. Let's take the bet - // that by the time we have enough local packages that this is a - // problem, we either will have made tools into a star, or we'll - // have made Catalog be backed by a real database. - var versionId = "local-" + Math.floor(Math.random() * 1000000000); - - // Accurate version numbers are of supreme importance, because - // we use version numbers (of build-time dependencies such as - // the coffeescript plugin), together with source file hashes - // and the notion of a repeatable build, to decide when a - // package build is out of date and trigger a rebuild of the - // package. - // - // The package we have just loaded may declare its version to be - // 1.2.3, but that doesn't mean it's really the official version - // 1.2.3 of the package. It only gets that version number - // officially when it's published to the package server. So what - // we'd like to do here is give it a version number like - // '1.2.3+', where is a hash of everything - // that's necessary to repeat the build exactly: all of the - // package's source files, all of the package's build-time - // dependencies, and the version of the Meteor build tool used - // to build it. - // - // Unfortunately we can't actually compute such a buildid yet - // since it depends on knowing the build-time dependencies of - // the package, which requires that we run the constraint - // solver, which can only be done once we've populated the - // catalog, which is what we're trying to do right now. - // - // So we have a workaround. For local packages we will fake the - // version in the catalog by setting the buildid to 'local', as - // in '1.2.3+local'. This is enough for the constraint solver to - // run, but any code that actually relies on accurate versions - // (for example, code that checks if a build is up to date) - // needs to be careful to get the versions not from the catalog - // but from the actual built Isopack objects, which will have - // accurate versions (with precise buildids) even for local - // packages. - var version = packageSource.version; - if (version.indexOf('+') !== -1) - throw new Error("version already has a buildid?"); - version = version + "+local"; - - self.versions[name] = {}; - self.versions[name][version] = { - _id: versionId, - packageName: name, - testName: packageSource.testName, - version: version, - publishedBy: null, - earliestCompatibleVersion: packageSource.earliestCompatibleVersion, - description: packageSource.metadata.summary, - dependencies: packageSource.getDependencyMetadata(), - source: null, - lastUpdated: null, - published: null, - isTest: packageSource.isTest, - debugOnly: packageSource.debugOnly, - containsPlugins: packageSource.containsPlugins() - }; }; - // Load the package sources for packages and their tests into packageSources. - // XXX: We should make this work with parallel: true; right now it seems to hit node problems - buildmessage.forkJoin({ 'title': 'Initializing packages', parallel: false }, self.effectiveLocalPackages, function (x) { - initSourceFromDir(x); - }); - - // Go through the packageSources and create a catalog record for each. - _.each(self.packageSources, initCatalogRecordsFromSource); - - return allOK; + // Load the package sources for packages and their tests into + // self.packages. + // + // XXX We should make this work with parallel: true; right now it seems to + // hit node problems. + buildmessage.forkJoin( + { 'title': 'Initializing packages', parallel: false }, + self.effectiveLocalPackageDirs, + function (dir) { + initSourceFromDir(dir); + }); }, // Given a name and a version of a package, return a path on disk - // from which we can load it. If we don't have it on disk (we - // haven't downloaded it, or it just plain doesn't exist in the - // catalog) return null. - // - // Doesn't download packages. Downloading should be done at the time - // that .meteor/versions is updated. + // from which we can load it. If we don't have it on disk return null. // // HACK: Version can be null if you are certain that the package is to be // loaded from local packages. In the future, version should always be @@ -425,33 +374,24 @@ _.extend(LocalCatalog.prototype, { var self = this; self._requireInitialized(); buildmessage.assertInCapture(); - constraintSolverOpts = constraintSolverOpts || {}; + constraintSolverOpts = constraintSolverOpts || {}; // Check local packages first. - if (_.has(self.packageSources, name)) { - + if (_.has(self.packages, name)) { // If we don't have a build of this package, we need to rebuild it. self._build(name, {}, constraintSolverOpts); // Return the path. - return self.packageSources[name].sourceRoot; + return self.packages[name].packageSource.sourceRoot; } - if (! version) { - throw new Error(name + " not a local package, and no version specified?"); - } - - var packageDir = tropohouse.default.packagePath(name, version); - if (fs.existsSync(packageDir)) { - return packageDir; - } - return null; + return null; }, getLocalPackageNames: function () { var self = this; self._requireInitialized(); - return _.keys(self.packageSources); + return _.keys(self.packages); }, // Rebuild all source packages in our search paths. If two packages @@ -473,7 +413,7 @@ _.extend(LocalCatalog.prototype, { if (namedPackages) { var bad = false; _.each(namedPackages, function (namedPackage) { - if (!_.has(self.packageSources, namedPackage)) { + if (!_.has(self.packages, namedPackage)) { buildmessage.enterJob( { title: "rebuilding " + namedPackage }, function () { buildmessage.error("unknown package"); @@ -489,8 +429,8 @@ _.extend(LocalCatalog.prototype, { // directories. Now, no package will be up to date and all of them will have // to be rebuilt. var count = 0; - _.each(self.packageSources, function (packageSource, name) { - var loadPath = packageSource.sourceRoot; + _.each(self.packages, function (packageData, name) { + var loadPath = packageData.packageSource.sourceRoot; if (namedPackages && !_.contains(namedPackages, name)) return; var buildDir = path.join(loadPath, '.build.' + name); @@ -500,8 +440,8 @@ _.extend(LocalCatalog.prototype, { // Now, go (again) through the local packages and ask the packageCache to // load each one of them. Since the packageCache will not find any old // builds (and have no cache), it will be forced to recompile them. - _.each(self.packageSources, function (packageSource, name) { - var loadPath = packageSource.sourceRoot; + _.each(self.packages, function (packageData, name) { + var loadPath = packageData.packageSource.sourceRoot; if (namedPackages && !_.contains(namedPackages, name)) return; self.packageCache.loadPackageAtPath(name, loadPath); @@ -522,7 +462,7 @@ _.extend(LocalCatalog.prototype, { var self = this; self._requireInitialized(); - _.each(self.localPackageDirs, function (packageDir) { + _.each(self.localPackageSearchDirs, function (packageDir) { var packages = watch.readAndWatchDirectory(watchSet, { absPath: packageDir, include: [/\/$/] @@ -530,25 +470,21 @@ _.extend(LocalCatalog.prototype, { _.each(packages, function (p) { watch.readAndWatchFile(watchSet, path.join(packageDir, p, 'package.js')); - watch.readAndWatchFile(watchSet, - path.join(packageDir, p, 'unipackage.json')); - watch.readAndWatchFile(watchSet, - path.join(packageDir, p, 'isopack.json')); }); }); }, - // True if `name` is a local package (is to be loaded via - // localPackageDirs or addLocalPackage rather than from the package + // True if `name` is a local package (is to be loaded via + // localPackageSearchDirs or addLocalPackage rather than from the package // server) isLocalPackage: function (name) { var self = this; self._requireInitialized(); - return _.has(self.packageSources, name); + return _.has(self.packages, name); }, - // Add a local package to the catalog. `name` is the name to use for + // Add a local package to the catalog. `name` is the name to use for // the package and `directory` is the directory that contains the // source tree for the package. // @@ -562,13 +498,12 @@ _.extend(LocalCatalog.prototype, { self._requireInitialized(); var resolvedPath = path.resolve(directory); - self.localPackages.push(resolvedPath); + self.explicitlyAddedLocalPackageDirs.push(resolvedPath); // If we were making lots of calls to addLocalPackage, we would // want to coalesce the calls to refresh somehow, but I don't // think we'll actually be doing that so this should be fine. // #CallingRefreshEveryTimeLocalPackagesChange - self._recomputeEffectiveLocalPackages(); self.refresh(); }, @@ -595,21 +530,20 @@ _.extend(LocalCatalog.prototype, { var unip = null; - if (_.has(self.built, name)) { + if (_.has(self.buildRequested, name)) { return; } - self.built[name] = true; + self.buildRequested[name] = true; // Go through the build-time constraints. Make sure that they are built, // either because we have built them already, or because we are about to // build them. var deps = compiler.getBuildOrderConstraints( - self.packageSources[name], + self.packages[name].packageSource, constraintSolverOpts); _.each(deps, function (dep) { - // We don't need to build non-local packages. It has been built. Return. if (!self.isLocalPackage(dep.name)) { return; @@ -622,9 +556,9 @@ _.extend(LocalCatalog.prototype, { // // The catalog doesn't understand buildID versions, so let's strip out the // buildID. - var version = self._getLocalVersion(dep.version); - var packageVersion = - self._getLocalVersion(self.packageSources[dep.name].version); + var version = self._changeBuildIdToLocal(dep.version); + // This one is always already +local. + var packageVersion = self.packages[dep.name].versionRecord.version; if (version !== packageVersion) { throw new Error("unknown version for local package? " + name); } @@ -652,7 +586,7 @@ _.extend(LocalCatalog.prototype, { }); // Now build this package if it needs building - var sourcePath = self.packageSources[name].sourceRoot; + var sourcePath = self.packages[name].packageSource.sourceRoot; unip = self._maybeGetUpToDateBuild(name, constraintSolverOpts); if (! unip) { @@ -661,7 +595,7 @@ _.extend(LocalCatalog.prototype, { title: "Building package `" + name + "`", rootPath: sourcePath }, function () { - unip = compiler.compile(self.packageSources[name], { + unip = compiler.compile(self.packages[name].packageSource, { ignoreProjectDeps: constraintSolverOpts.ignoreProjectDeps }).isopack; if (! buildmessage.jobHasMessages()) { @@ -684,27 +618,28 @@ _.extend(LocalCatalog.prototype, { } // And put a build record for it in the catalog. There is only one version // for this package! - var versionId = _.values(self.versions[name])._id; + var versionId = self.packages[name].versionRecord._id; // XXX why isn't this build just happening through the package cache // directly? self.packageCache.cachePackageAtPath(name, sourcePath, unip); - self.builds.push({ + // XXX I'm not convinced that anything actually uses this build record. We + // mostly care about build records for packages we need to download. + self.packages[name].buildRecord = { buildArchitectures: unip.buildArchitectures(), builtBy: null, build: null, // this would be the URL and hash versionId: versionId, lastUpdated: null, buildPublished: null - }); + }; }, - // Given a version string that may or may not have a build ID, convert it into // the catalog's internal format for local versions -- [version // number]+local. (for example, 1.0.0+local). - _getLocalVersion: function (version) { + _changeBuildIdToLocal: function (version) { if (version) return version.split("+")[0] + "+local"; return version; @@ -716,7 +651,7 @@ _.extend(LocalCatalog.prototype, { var self = this; buildmessage.assertInCapture(); - var sourcePath = self.packageSources[name].sourceRoot; + var sourcePath = self.packages[name].packageSource.sourceRoot; var buildDir = path.join(sourcePath, '.build.' + name); if (fs.existsSync(buildDir)) { var unip = new isopack.Isopack; @@ -729,7 +664,7 @@ _.extend(LocalCatalog.prototype, { return null; } if (compiler.checkUpToDate( - self.packageSources[name], unip, constraintSolverOpts)) { + self.packages[name].packageSource, unip, constraintSolverOpts)) { return unip; } } diff --git a/tools/catalog.js b/tools/catalog.js index c67f819e71..1870c15fba 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -174,9 +174,21 @@ _.extend(LayeredCatalog.prototype, { return forgottenECVs; }, + // Doesn't download packages. Downloading should be done at the time + // that .meteor/versions is updated. getLoadPathForPackage: function (name, version, constraintSolverOpts) { var self = this; - return self.localCatalog.getLoadPathForPackage(name, version, constraintSolverOpts); + var loadPath = self.localCatalog.getLoadPathForPackage( + name, version, constraintSolverOpts); + if (loadPath) + return loadPath; + + if (! version) { + throw new Error(name + " not a local package, and no version specified?"); + } + + return self.otherCatalog.getLoadPathForPackage( + name, version, constraintSolverOpts); }, getLocalPackageNames: function () { diff --git a/tools/commands-packages.js b/tools/commands-packages.js index 954433b21d..11d9f1ea70 100644 --- a/tools/commands-packages.js +++ b/tools/commands-packages.js @@ -806,7 +806,7 @@ main.registerCommand({ var packageDir = path.resolve(path.join(localPackageDir, item)); // Consider a directory to be a package source tree if it // contains 'package.js'. (We used to support isopacks in - // localPackageDirs, but no longer.) + // local package directories, but no longer.) if (fs.existsSync(path.join(packageDir, 'package.js'))) { var packageSource = new PackageSource(catalog.complete); buildmessage.enterJob( diff --git a/tools/commands.js b/tools/commands.js index 8e6328884b..3ab736b583 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -584,11 +584,11 @@ main.registerCommand({ // XXX copied from main.js // We need to re-initialize the complete catalog to know about the app we just // created, because it might have local packages. - var localPackageDirs = [path.resolve(appPath, 'packages')]; + var localPackageSearchDirs = [path.resolve(appPath, 'packages')]; if (process.env.PACKAGE_DIRS) { // User can provide additional package directories to search in PACKAGE_DIRS // (colon-separated). - localPackageDirs = localPackageDirs.concat( + localPackageSearchDirs = localPackageSearchDirs.concat( _.map(process.env.PACKAGE_DIRS.split(':'), function (p) { return path.resolve(p); })); @@ -596,7 +596,7 @@ main.registerCommand({ if (!files.usesWarehouse()) { // Running from a checkout, so use the Meteor core packages from // the checkout. - localPackageDirs.push(path.join( + localPackageSearchDirs.push(path.join( files.getCurrentToolsDir(), 'packages')); } @@ -604,7 +604,7 @@ main.registerCommand({ // XXX Hack. In the future we should just delay all use of catalog.complete // until this point. catalog.complete.initialize({ - localPackageDirs: localPackageDirs + localPackageSearchDirs: localPackageSearchDirs }); // Run the constraint solver. Override the assumption that using '--release' diff --git a/tools/main.js b/tools/main.js index 4ef32c5ae3..768238ccf5 100644 --- a/tools/main.js +++ b/tools/main.js @@ -685,7 +685,8 @@ Fiber(function () { // springboard). var messages = buildmessage.capture({ title: "Initializing local packages" }, function () { catalog.uniload.initialize({ - localPackageDirs: [path.join(files.getCurrentToolsDir(), 'packages')] + localPackageSearchDirs: [ + path.join(files.getCurrentToolsDir(), 'packages')] }); }); if (messages.hasMessages()) { @@ -1238,14 +1239,14 @@ commandName + ": You're not in a Meteor project directory.\n" + // Figure out the directories that we should search for local // packages (in addition to packages downloaded from the package // server) - var localPackageDirs = []; + var localPackageSearchDirs = []; if (appDir) - localPackageDirs.push(path.join(appDir, 'packages')); + localPackageSearchDirs.push(path.join(appDir, 'packages')); if (process.env.PACKAGE_DIRS) { // User can provide additional package directories to search in // PACKAGE_DIRS (colon-separated). - localPackageDirs = localPackageDirs.concat( + localPackageSearchDirs = localPackageSearchDirs.concat( _.map(process.env.PACKAGE_DIRS.split(':'), function (p) { return path.resolve(p); })); @@ -1254,13 +1255,13 @@ commandName + ": You're not in a Meteor project directory.\n" + if (!files.usesWarehouse()) { // Running from a checkout, so use the Meteor core packages from // the checkout. - localPackageDirs.push(path.join( + localPackageSearchDirs.push(path.join( files.getCurrentToolsDir(), 'packages')); } var messages = buildmessage.capture({ title: "Initializing catalog" }, function () { catalog.complete.initialize({ - localPackageDirs: localPackageDirs + localPackageSearchDirs: localPackageSearchDirs }); }); if (messages.hasMessages()) { diff --git a/tools/tests/old/test-bundler-assets.js b/tools/tests/old/test-bundler-assets.js index dd67bbc977..cde2b0b29a 100644 --- a/tools/tests/old/test-bundler-assets.js +++ b/tools/tests/old/test-bundler-assets.js @@ -31,10 +31,10 @@ var setAppDir = function (appDir) { doOrThrow(function () { catalog.uniload.initialize({ - localPackageDirs: [checkoutPackageDir] + localPackageSearchDirs: [checkoutPackageDir] }); catalog.complete.initialize({ - localPackageDirs: [appPackageDir, checkoutPackageDir] + localPackageSearchDirs: [appPackageDir, checkoutPackageDir] }); }); }; diff --git a/tools/tests/old/test-bundler-npm.js b/tools/tests/old/test-bundler-npm.js index d7b6540226..442619e3b3 100644 --- a/tools/tests/old/test-bundler-npm.js +++ b/tools/tests/old/test-bundler-npm.js @@ -22,7 +22,7 @@ var setAppDir = function (appDir) { var checkoutPackageDir = path.join( files.getCurrentToolsDir(), 'packages'); - var localPackageDirs = [tmpPackageDirContainer, checkoutPackageDir]; + var localPackageSearchDirs = [tmpPackageDirContainer, checkoutPackageDir]; if (files.usesWarehouse()) { throw Error("This old test doesn't support non-checkout"); @@ -30,10 +30,10 @@ var setAppDir = function (appDir) { doOrThrow(function () { catalog.uniload.initialize({ - localPackageDirs: [checkoutPackageDir] + localPackageSearchDirs: [checkoutPackageDir] }); catalog.complete.initialize({ - localPackageDirs: localPackageDirs + localPackageSearchDirs: localPackageSearchDirs }); }); }; diff --git a/tools/tests/old/test-bundler-options.js b/tools/tests/old/test-bundler-options.js index 6322de8b5f..66a7150286 100644 --- a/tools/tests/old/test-bundler-options.js +++ b/tools/tests/old/test-bundler-options.js @@ -30,10 +30,10 @@ var setAppDir = function (appDir) { doOrThrow(function () { catalog.uniload.initialize({ - localPackageDirs: [checkoutPackageDir] + localPackageSearchDirs: [checkoutPackageDir] }); catalog.complete.initialize({ - localPackageDirs: [appPackageDir, checkoutPackageDir] + localPackageSearchDirs: [appPackageDir, checkoutPackageDir] }); }); };