diff --git a/tools/catalog-local.js b/tools/catalog-local.js index c966b98b62..9f38f1e09d 100644 --- a/tools/catalog-local.js +++ b/tools/catalog-local.js @@ -45,7 +45,13 @@ var LocalCatalog = function (options) { // need to process. self.effectiveLocalPackageDirs = []; - self.buildRequested = null; + // A WatchSet that detects when the set of packages and their locations + // changes. ie, the listings of 'packages' directories, and the contents of + // package.js files underneath. It does NOT track the rest of the source of + // the packages: that wouldn't be helpful to the runner since it would be too + // coarse to tell if a change is client-only or not. (But any change to the + // layout of where packages live counts as a non-client-only change.) + self.packageLocationWatchSet = new watch.WatchSet; self.nextVersionId = 1; }; @@ -74,29 +80,14 @@ _.extend(LocalCatalog.prototype, { options = options || {}; - // 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.localPackageSearchDirs = []; - _.each(options.localPackageSearchDirs, function (dir) { - dir = path.resolve(dir); - if (utils.isDirectory(dir)) - self.localPackageSearchDirs.push(dir); - }); + self.localPackageSearchDirs = _.map( + options.localPackageSearchDirs, function (p) { + return path.resolve(p); + }); - self.refresh(); - }, - - // Set all the collections to their initial values, which are mostly - // blank. This does not set self.initialized -- do that manually in the child - // class when applicable. - reset: function () { - var self = this; - - // Initialize everything to its default version. - self.packages = {}; - - self.buildRequested = {}; + self._recomputeEffectiveLocalPackages(); + self._loadLocalPackages(); + self.initialized = true; }, // Throw if the catalog's self.initialized value has not been set to true. @@ -163,22 +154,6 @@ _.extend(LocalCatalog.prototype, { return self.packages[name].versionRecord; }, - // Refresh the packages in the catalog, by re-scanning local packages. - // - // options: - // - watchSet: if provided, any files read in reloading packages will be added - // to this set. - refresh: function (options) { - var self = this; - options = options || {}; - buildmessage.assertInCapture(); - - self.reset(); - self._recomputeEffectiveLocalPackages(); - self._loadLocalPackages({ watchSet: options.watchSet }); - self.initialized = true; - }, - // Compute self.effectiveLocalPackageDirs from self.localPackageSearchDirs and // self.explicitlyAddedLocalPackageDirs. _recomputeEffectiveLocalPackages: function () { @@ -187,25 +162,35 @@ _.extend(LocalCatalog.prototype, { self.explicitlyAddedLocalPackageDirs); _.each(self.localPackageSearchDirs, function (searchDir) { - if (! utils.isDirectory(searchDir)) + var possiblePackageDirs = watch.readAndWatchDirectory( + self.packageLocationWatchSet, { + absPath: searchDir, + include: [/\/$/] + }); + // Not a directory? Ignore. + if (possiblePackageDirs === null) return; - var contents = fs.readdirSync(searchDir); - _.each(contents, function (item) { - var packageDir = path.join(searchDir, item); - if (! utils.isDirectory(packageDir)) - return; + + _.each(possiblePackageDirs, function (subdir) { + // readAndWatchDirectory adds a slash to the end of directory names to + // differentiate them from filenames. Remove it. + subdir = subdir.substr(0, subdir.length - 1); + var absPackageDir = path.join(searchDir, subdir); // 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'))) { + var packageJs = watch.readAndWatchFile( + self.packageLocationWatchSet, + path.join(absPackageDir, 'package.js')); + if (packageJs !== null) { // Let earlier package directories override later package // directories. // 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.effectiveLocalPackageDirs.push(packageDir); + self.effectiveLocalPackageDirs.push(absPackageDir); } }); }); @@ -247,20 +232,8 @@ _.extend(LocalCatalog.prototype, { opts["name"] = definiteName; } packageSource.initFromPackageDir(packageDir, opts); - - 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; - } + if (buildmessage.jobHasMessages()) + return; // recover by ignoring // Now that we have initialized the package from package.js, we know its // name. @@ -318,29 +291,6 @@ _.extend(LocalCatalog.prototype, { }); }, - // Register local package directories with a watchSet. We want to know if a - // package is created or deleted, which includes both its top-level source - // directory and its main package metadata file. - // - // This will watch the local package directories that are in effect when the - // function is called. (As set by the most recent call to - // setLocalPackageDirs.) - watchLocalPackageDirs: function (watchSet) { - var self = this; - self._requireInitialized(); - - _.each(self.localPackageSearchDirs, function (packageDir) { - var packages = watch.readAndWatchDirectory(watchSet, { - absPath: packageDir, - include: [/\/$/] - }); - _.each(packages, function (p) { - watch.readAndWatchFile(watchSet, - path.join(packageDir, p, 'package.js')); - }); - }); - }, - // True if `name` is a local package (is to be loaded via // localPackageSearchDirs or addLocalPackage rather than from the package // server) @@ -359,6 +309,7 @@ _.extend(LocalCatalog.prototype, { // be overridden (it will be as if that package doesn't exist on the // package server at all). And for now, it's an error to call this // function twice with the same `name`. + // XXX #3006 rewrite this addLocalPackage: function (directory) { var self = this; buildmessage.assertInCapture(); diff --git a/tools/catalog.js b/tools/catalog.js index d524d396ac..b9175aa1a5 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -208,24 +208,6 @@ _.extend(LayeredCatalog.prototype, { if (!latest) return null; return self.getVersion(name, latest); - }, - - // Refresh the catalogs referenced by this catalog. - // options: - // - watchSet: if provided, any files read in reloading packages will be added - // to this set. - refreshLocalPackages: function (options) { - var self = this; - self.localCatalog.refresh(options); - //// Note that otherCatalog can throw, if we fail to connect - //// XXX: Order of refreshes? Continue on error? - //self.otherCatalog.refresh(options); - // XXX #3006 do we need to refresh some IsopackCache too? - }, - - watchLocalPackageDirs: function (watchSet) { - var self = this; - self.localCatalog.watchLocalPackageDirs(watchSet); } }); diff --git a/tools/commands.js b/tools/commands.js index 380e4720d2..70be271ea6 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1450,6 +1450,7 @@ var getPackagesForTest = function (packages) { // main package. This is not ideal (I hate how this mutates global // state) but it'll do for now. var packageDir = path.resolve(p); + // XXX #3006 rework addLocalPackage catalog.complete.addLocalPackage(packageDir); if (buildmessage.jobHasMessages()) { diff --git a/tools/isopack-cache.js b/tools/isopack-cache.js index 3cbd6dc031..f9103028cf 100644 --- a/tools/isopack-cache.js +++ b/tools/isopack-cache.js @@ -2,7 +2,6 @@ var _ = require('underscore'); var path = require('path'); var buildmessage = require('./buildmessage.js'); -var catalog = require('./catalog.js'); var compiler = require('./compiler.js'); var files = require('./files.js'); var isopackModule = require('./isopack.js'); diff --git a/tools/project-context.js b/tools/project-context.js index 6484244ab9..bcee4e7aa1 100644 --- a/tools/project-context.js +++ b/tools/project-context.js @@ -53,6 +53,7 @@ _.extend(exports.ProjectContext.prototype, { self.finishedUpgraders = null; // Initialized by _resolveConstraints. + self.localCatalog = null; self.packageMap = null; self.isopackCache = null; }, @@ -153,6 +154,9 @@ _.extend(exports.ProjectContext.prototype, { }); }, + // This is a WatchSet that ends up being the WatchSet for the app's + // initFromAppDir PackageSource. Changes to this will cause the whole app to + // be rebuilt (client and server). getProjectWatchSet: function () { // We don't cache a projectWatchSet on this object, since some of the // metadata files can be written by us (eg .meteor/versions @@ -165,9 +169,18 @@ _.extend(exports.ProjectContext.prototype, { function (metadataFile) { metadataFile && watchSet.merge(metadataFile.watchSet); }); + + if (self.localCatalog) { + watchSet.merge(self.localCatalog.packageLocationWatchSet); + } + return watchSet; }, + // This WatchSet encompasses everything that users can change to restart an + // app. We only watch this for failed bundles; for successful bundles, we have + // more precise server-specific and client-specific WatchSets that add up to + // this one. getProjectAndLocalPackagesWatchSet: function () { var self = this; var watchSet = self.getProjectWatchSet(); @@ -270,8 +283,10 @@ _.extend(exports.ProjectContext.prototype, { buildmessage.assertInCapture(); var cat = new catalog.LayeredCatalog; - cat.setCatalogs(new catalogLocal.LocalCatalog({ containingCatalog: cat }), - catalog.official); + self.localCatalog = new catalogLocal.LocalCatalog({ + containingCatalog: cat + }); + cat.setCatalogs(self.localCatalog, catalog.official); var searchDirs = self._localPackageSearchDirs(); buildmessage.enterJob({ title: "scanning local packages" }, function () { diff --git a/tools/run-app.js b/tools/run-app.js index a3ed6f0840..1ae3203a80 100644 --- a/tools/run-app.js +++ b/tools/run-app.js @@ -534,11 +534,6 @@ _.extend(AppRunner.prototype, { bundleResult.errors.merge(settingsMessages); } - // HACK: Also make sure we notice when somebody adds a package to - // the app packages dir that may override a catalog package. - // XXX #3006 do this better - // catalog.complete.watchLocalPackageDirs(serverWatchSet); - // Atomically (1) see if we've been stop()'d, (2) if not, create a // future that can be used to stop() us once we start running. if (self.exitFuture) diff --git a/tools/tests/old/test-bundler-npm.js b/tools/tests/old/test-bundler-npm.js index 44de398f1d..2566f209b4 100644 --- a/tools/tests/old/test-bundler-npm.js +++ b/tools/tests/old/test-bundler-npm.js @@ -57,6 +57,7 @@ var testPackageDir = path.join(tmpPackageDirContainer, 'test-package'); var reloadPackages = function () { doOrThrow(function () { + // XXX #3006 this doesn't exist catalog.complete.refreshLocalPackages(); }); };