From 57f6d25ef28f08dca938fc93a81815ee7aa3abac Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 16 Jul 2013 17:28:03 -0700 Subject: [PATCH 1/2] Make api.add_files("f") default to ["client","server"]. --- tools/packages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/packages.js b/tools/packages.js index 065f5718b6..dc68444b9c 100644 --- a/tools/packages.js +++ b/tools/packages.js @@ -1436,7 +1436,7 @@ _.extend(Package.prototype, { paths = paths ? [paths] : []; if (!(where instanceof Array)) - where = where ? [where] : []; + where = where ? [where] : ["client", "server"]; _.each(paths, function (path) { _.each(where, function (w) { From 6970a89ee0c96e08d2317792e8acb0669090d65a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 16 Jul 2013 09:46:45 -0700 Subject: [PATCH 2/2] Implement "api.imply". Make all accounts packages imply accounts-base. If X uses Y and Y implies Z, then X is also treated as using Z. This can be used to create umbrella packages, etc. --- packages/accounts-facebook/package.js | 2 + packages/accounts-github/package.js | 2 + packages/accounts-google/package.js | 2 + packages/accounts-meetup/package.js | 2 + packages/accounts-oauth/package.js | 2 + packages/accounts-password/package.js | 2 + packages/accounts-twitter/package.js | 2 + packages/accounts-ui-unstyled/package.js | 2 + packages/accounts-ui/package.js | 2 + packages/accounts-weibo/package.js | 2 + tools/bundler.js | 26 ++-- tools/packages.js | 173 +++++++++++++++++++---- 12 files changed, 171 insertions(+), 48 deletions(-) diff --git a/packages/accounts-facebook/package.js b/packages/accounts-facebook/package.js index afb98a391a..667f56bb32 100644 --- a/packages/accounts-facebook/package.js +++ b/packages/accounts-facebook/package.js @@ -4,6 +4,8 @@ Package.describe({ Package.on_use(function(api) { api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-oauth', ['client', 'server']); api.use('facebook', ['client', 'server']); diff --git a/packages/accounts-github/package.js b/packages/accounts-github/package.js index 7b740bea5e..3855b12c23 100644 --- a/packages/accounts-github/package.js +++ b/packages/accounts-github/package.js @@ -4,6 +4,8 @@ Package.describe({ Package.on_use(function(api) { api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-oauth', ['client', 'server']); api.use('github', ['client', 'server']); diff --git a/packages/accounts-google/package.js b/packages/accounts-google/package.js index aebf8bcb0f..aac4af35c2 100644 --- a/packages/accounts-google/package.js +++ b/packages/accounts-google/package.js @@ -5,6 +5,8 @@ Package.describe({ Package.on_use(function(api) { api.use(['underscore', 'random']); api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-oauth', ['client', 'server']); api.use('google', ['client', 'server']); diff --git a/packages/accounts-meetup/package.js b/packages/accounts-meetup/package.js index db4f222b39..4616c99246 100644 --- a/packages/accounts-meetup/package.js +++ b/packages/accounts-meetup/package.js @@ -4,6 +4,8 @@ Package.describe({ Package.on_use(function(api) { api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-oauth', ['client', 'server']); api.use('meetup', ['client', 'server']); diff --git a/packages/accounts-oauth/package.js b/packages/accounts-oauth/package.js index c97299ce44..d3fb7b984b 100644 --- a/packages/accounts-oauth/package.js +++ b/packages/accounts-oauth/package.js @@ -9,6 +9,8 @@ Package.on_use(function (api) { api.use('check', ['client', 'server']); api.use('webapp', 'server'); api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('oauth', 'server'); api.add_files('oauth_common.js', ['client', 'server']); diff --git a/packages/accounts-password/package.js b/packages/accounts-password/package.js index db3b78367a..a8d2c7c16a 100644 --- a/packages/accounts-password/package.js +++ b/packages/accounts-password/package.js @@ -4,6 +4,8 @@ Package.describe({ Package.on_use(function(api) { api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('srp', ['client', 'server']); api.use('email', ['server']); api.use('random', ['server']); diff --git a/packages/accounts-twitter/package.js b/packages/accounts-twitter/package.js index efcd66a53a..a29c8f00ef 100644 --- a/packages/accounts-twitter/package.js +++ b/packages/accounts-twitter/package.js @@ -5,6 +5,8 @@ Package.describe({ Package.on_use(function(api) { api.use('underscore', ['server']); api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-oauth', ['client', 'server']); api.use('twitter', ['client', 'server']); diff --git a/packages/accounts-ui-unstyled/package.js b/packages/accounts-ui-unstyled/package.js index 0c75e3ee7c..1acad8282a 100644 --- a/packages/accounts-ui-unstyled/package.js +++ b/packages/accounts-ui-unstyled/package.js @@ -6,6 +6,8 @@ Package.on_use(function (api) { api.use(['deps', 'service-configuration', 'accounts-urls', 'accounts-base', 'underscore', 'templating', 'handlebars', 'spark', 'session'], 'client'); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.add_files([ 'accounts_ui.js', diff --git a/packages/accounts-ui/package.js b/packages/accounts-ui/package.js index 93680b7c15..97d6ec4e26 100644 --- a/packages/accounts-ui/package.js +++ b/packages/accounts-ui/package.js @@ -3,6 +3,8 @@ Package.describe({ }); Package.on_use(function (api) { + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-ui-unstyled', 'client'); api.use('less', 'client'); diff --git a/packages/accounts-weibo/package.js b/packages/accounts-weibo/package.js index 2d88b4294e..22329991f2 100644 --- a/packages/accounts-weibo/package.js +++ b/packages/accounts-weibo/package.js @@ -4,6 +4,8 @@ Package.describe({ Package.on_use(function(api) { api.use('accounts-base', ['client', 'server']); + // Export Accounts (etc) to packages using this one. + api.imply('accounts-base', ['client', 'server']); api.use('accounts-oauth', ['client', 'server']); api.use('weibo', ['client', 'server']); diff --git a/tools/bundler.js b/tools/bundler.js index 80237fc8b4..529db4c358 100644 --- a/tools/bundler.js +++ b/tools/bundler.js @@ -534,11 +534,7 @@ _.extend(Target.prototype, { if (_.has(getsUsed, slice.id)) return; getsUsed[slice.id] = slice; - _.each(slice.uses, function (u) { - if (u.weak) - return; - _.each(library.getSlices(u.spec, self.arch), addToGetsUsed); - }); + slice.eachUsedSlice(self.arch, {skipWeak: true}, addToGetsUsed); }; _.each(rootSlices, addToGetsUsed); @@ -563,19 +559,16 @@ _.extend(Target.prototype, { if (!_.has(needed, slice.id)) return; - _.each(slice.uses, function (u) { - // If this is an unordered dependency, then there's no reason to add it - // *now*, and for all we know, `u` will depend on `slice` and need to be - // added after it. So we ignore this edge. Because we did follow this - // edge in Phase 1, usedSlice was at some point in `needed` and will not - // be left out. - if (u.unordered) - return; - - _.each(library.getSlices(u.spec, self.arch), function (usedSlice) { + // Process each ordered dependency. (If we have an unordered dependency + // `u`, then there's no reason to add it *now*, and for all we know, `u` + // will depend on `slice` and need to be added after it. So we ignore + // those edge. Because we did follow those edges in Phase 1, any unordered + // slices were at some point in `needed` and will not be left out.) + slice.eachUsedSlice( + self.arch, {skipUnordered: true}, function (usedSlice, useOptions) { // If this is a weak dependency, and nothing else in the target had a // strong dependency on it, then ignore this edge. - if (u.weak && ! _.has(getsUsed, usedSlice.id)) + if (useOptions.weak && ! _.has(getsUsed, usedSlice.id)) return; if (onStack[usedSlice.id]) { @@ -588,7 +581,6 @@ _.extend(Target.prototype, { add(usedSlice); delete onStack[usedSlice.id]; }); - }); self.slices.push(slice); delete needed[slice.id]; }; diff --git a/tools/packages.js b/tools/packages.js index dc68444b9c..9762b33d93 100644 --- a/tools/packages.js +++ b/tools/packages.js @@ -84,6 +84,7 @@ var rejectBadPath = function (p) { // - name [required] // - arch [required] // - uses +// - implies // - getSourcesFunc // - forceExport // - dependencyInfo @@ -124,8 +125,18 @@ var Slice = function (pkg, options) { // or plugins. // It is an error for both unordered and weak to be true, because // such a dependency would have no effect. + // + // In most places, you want to use slice.eachUsedSlice() instead of + // slice.uses, which also takes into account implied packages. self.uses = options.uses; + // Packages which are "implied" by using this package. If a slice X uses this + // slice Y, and Y implies Z, then X will effectively use Z as well (and get + // its imports and plugins). An array of objects of the same type as the + // elements of self.uses (although for now unordered and weak are not + // allowed). + self.implies = options.implies || []; + // A function that returns the source files for this slice. Array of objects // with keys "relPath" and "fileOptions". Null if loaded from unipackage. // @@ -226,17 +237,19 @@ _.extend(Slice.prototype, { // way we get one error about it instead of a new error at each // stage in the build process in which we try to retrieve the // package. - var scrubbedUses = []; - _.each(self.uses, function (u) { - var parts = u.spec.split('.'); - var pkg = self.pkg.library.get(parts[0], /* throwOnError */ false); - if (! pkg) { - buildmessage.error("no such package: '" + parts[0] + "'"); - // recover by omitting this package from 'uses' - } else - scrubbedUses.push(u); + _.each(['uses', 'implies'], function (field) { + var scrubbed = []; + _.each(self[field], function (u) { + var parts = u.spec.split('.'); + var pkg = self.pkg.library.get(parts[0], /* throwOnError */ false); + if (! pkg) { + buildmessage.error("no such package: '" + parts[0] + "'"); + // recover by omitting this package from the field + } else + scrubbed.push(u); + }); + self[field] = scrubbed; }); - self.uses = scrubbedUses; _.each(self.getSourcesFunc(), function (source) { var relPath = source.relPath; @@ -530,22 +543,20 @@ _.extend(Slice.prototype, { // Compute imports by merging the exports of all of the packages // we use. Note that in the case of conflicting symbols, later // packages get precedence. + // + // We don't get imports from unordered dependencies (since they may not be + // defined yet) or from weak dependencies (because the meaning of a name + // shouldn't be affected by the non-local decision of whether or not an + // unrelated package in the target depends on something). var imports = {}; // map from symbol to supplying package name - _.each(_.values(self.uses), function (u) { - // We don't get imports from unordered dependencies (since they may not be - // defined yet) or from weak dependencies (because the meaning of a name - // shouldn't be affected by the non-local decision of whether or not - // an unrelated package in the target depends on something). - if (! u.unordered && ! u.weak) { - _.each(library.getSlices(u.spec, bundleArch), function (otherSlice) { - if (! otherSlice.isBuilt) - throw new Error("dependency wasn't built?"); - _.each(otherSlice.exports, function (symbol) { - imports[symbol] = otherSlice.pkg.name; - }); + self.eachUsedSlice( + bundleArch, {skipWeak: true, skipUnordered: true}, function (otherSlice) { + if (! otherSlice.isBuilt) + throw new Error("dependency wasn't built?"); + _.each(otherSlice.exports, function (symbol) { + imports[symbol] = otherSlice.pkg.name; }); - } - }); + }); // Phase 2 link var isApp = ! self.pkg.name; @@ -575,6 +586,48 @@ _.extend(Slice.prototype, { return _.union(self.resources, jsResources); // union preserves order }, + // Calls `callback` with each slice (of architecture matching `arch`) that is + // "used" by this slice. This includes directly used slices, and slices that + // are transitively "implied" by used slices. (But not slices that are used by + // slices that we use!) Options are skipWeak and skipUnordered, meaning to + // ignore direct "uses" that are weak or unordered. + eachUsedSlice: function (arch, options, callback) { + var self = this; + if (typeof options === "function") { + callback = options; + options = {}; + } + + var processedSliceId = {}; + var usesToProcess = []; + _.each(self.uses, function (use) { + if (options.skipUnordered && use.unordered) + return; + if (options.skipWeak && use.weak) + return; + usesToProcess.push(use); + }); + + while (!_.isEmpty(usesToProcess)) { + var use = usesToProcess.shift(); + + var slices = self.pkg.library.getSlices(use.spec, arch); + _.each(slices, function (slice) { + if (_.has(processedSliceId, slice.id)) + return; + processedSliceId[slice.id] = true; + callback(slice, { + unordered: !!use.unordered, + weak: !!use.weak + }); + + _.each(slice.implies, function (implied) { + usesToProcess.push(implied); + }); + }); + } + }, + // Return an array of all plugins that are active in this slice, as // a list of Packages. _activePluginPackages: function () { @@ -586,16 +639,25 @@ _.extend(Slice.prototype, { // what the correct behavior should be -- we need to resolve // whether we think about extensions as being global to a package // or particular to a slice. + // (there's also some weirdness here with handling implies, because + // the implies field is on the target slice, but we really only care + // about packages.) var ret = [self.pkg]; - _.each(self.uses, function (u) { - // We don't use plugins from weak dependencies, because the ability to - // compile a certain type of file shouldn't depend on whether or not some - // unrelated package in the target has a dependency. - if (! u.weak) - ret.push(self.pkg.library.get(u.spec.split('.')[0])); + // We don't use plugins from weak dependencies, because the ability to + // compile a certain type of file shouldn't depend on whether or not some + // unrelated package in the target has a dependency. + // + // We pass archinfo.host here, not self.arch, because it may be more + // specific, and because plugins always have to run on the host + // architecture. + self.eachUsedSlice(archinfo.host(), {skipWeak: true}, function (usedSlice) { + ret.push(usedSlice.pkg); }); + // Only need one copy of each package. + ret = _.uniq(ret); + _.each(ret, function (pkg) { pkg._ensurePluginsInitialized(); }); @@ -1342,9 +1404,13 @@ _.extend(Package.prototype, { var forceExport = {use: {client: [], server: []}, test: {client: [], server: []}}; - // packages used (keys are 'spec', 'unordered', and 'weak') + // packages used and implied (keys are 'spec', 'unordered', and 'weak'). an + // "implied" package is a package that will be used by a slice which uses + // us. (since you can't use a test slice, only the use slice can have + // "implies".) var uses = {use: {client: [], server: []}, test: {client: [], server: []}}; + var implies = {client: [], server: []}; // For this old-style, on_use/on_test/where-based package, figure // out its dependencies by calling its on_xxx functions and seeing @@ -1428,6 +1494,35 @@ _.extend(Package.prototype, { }); }, + // Called when this package wants packages using it to also use + // another package. eg, for umbrella packages which want packages + // using them to also get symbols or plugins from their components. + imply: function (names, where) { + if (role === "test") { + buildmessage.error( + "api.imply() is only allowed in on_use, not on_test.", + { useMyCaller: true }); + // recover by ignoring + return; + } + + if (!(names instanceof Array)) + names = names ? [names] : []; + + if (!(where instanceof Array)) + where = where ? [where] : ["client", "server"]; + + _.each(names, function (name) { + _.each(where, function (w) { + implies[w].push({ + spec: name + // We don't allow weak or unordered implies, since the main + // purpose of imply is to provide imports and plugins. + }); + }); + }); + }, + // Top-level call to add a source file to a package. It will // be processed according to its extension (eg, *.coffee // files will be compiled to JavaScript.) @@ -1584,6 +1679,7 @@ _.extend(Package.prototype, { name: ({ use: "main", test: "tests" })[role], arch: arch, uses: uses[role][where], + implies: role === "use" && implies[where] || undefined, getSourcesFunc: function () { return sources[role][where]; }, forceExport: forceExport[role][where], dependencyInfo: dependencyInfo, @@ -1875,6 +1971,11 @@ _.extend(Package.prototype, { weak: u.weak }; }), + implies: _.map(sliceJson.implies, function (u) { + return { + spec: u['package'] + (u.slice ? "." + u.slice : "") + }; + }), staticDirectory: staticDirectory }); @@ -2033,6 +2134,16 @@ _.extend(Package.prototype, { weak: u.weak || undefined }; }), + implies: (_.isEmpty(slice.implies) ? undefined : + _.map(slice.implies, function (u) { + var specParts = u.spec.split('.'); + if (specParts.length > 2) + throw new Error("Bad package spec: " + u.spec); + return { + 'package': specParts[0], + slice: specParts[1] || undefined + }; + })), node_modules: slice.nodeModulesPath ? 'npm/node_modules' : undefined, resources: [], staticDirectory: path.join(sliceDir, self.serveRoot)