From 9f3ff7da0f732d1d531ee4be6227108495aa0da4 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 10:49:52 -0700 Subject: [PATCH 01/13] WIP: compute build ids and add to versions. Utterly untested. --- tools/compiler.js | 33 ++++++++++++++++++++- tools/unipackage.js | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/tools/compiler.js b/tools/compiler.js index 260556ebee..47b2851743 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -599,16 +599,27 @@ var compileSlice = function (unipackage, inputSlice, packageLoader, // the appropriate compiler plugins. Once build has completed, any errors // detected in the package will have been emitted to buildmessage. // +// Options: +// - officialBuild: defaults to false. If false, then we will compute a +// build identifier (a hash of the package's dependency versions and +// source files) and include it as part of the unipackage's version +// string. If true, then we will use the version that is contained in +// the package's source. You should set it to true when you are +// building a package to publish as an official build with the +// package server. +// // Returns an object with keys: // - unipackage: the build Unipackage // - sources: array of source files (identified by their path on local // disk) that were used by the build (the source files you'd have to // ship to a different machine to replicate the build there) -compiler.compile = function (packageSource) { +compiler.compile = function (packageSource, options) { var sources = []; var pluginWatchSet = packageSource.pluginWatchSet.clone(); var plugins = {}; + options = _.extend({ officialBuild: false }, options); + // Determine versions of build-time dependencies var buildTimeDeps = determineBuildTimeDependencies(packageSource); @@ -723,6 +734,26 @@ compiler.compile = function (packageSource) { sources.push.apply(sources, sliceSources); }); + + if (options.officialBuild) { + // XXX I have no idea if this should be using buildmessage.enterJob + // or not. test what happens on error + buildmessage.enterJob({ + title: "compute build identifier for package `" + + packageSource.name + "`", + rootPath: packageSource.sourceRoot + }, function () { + if (packageSource.version.indexOf("+") !== -1) { + buildmessage.error("cannot compute build identifier for package `" + + packageSource.name + "` version " + + packageSource.version + "because it already " + + "has a build identifier"); + } else { + unipackage.addBuildIdentifierToVersion(); + } + }); + } + return { sources: _.uniq(sources), unipackage: unipackage diff --git a/tools/unipackage.js b/tools/unipackage.js index e462e51183..043d751b0f 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -8,6 +8,7 @@ var path = require('path'); var Builder = require('./builder.js'); var bundler = require('./bundler.js'); var watch = require('./watch.js'); +var PackageLoader = require('./package-loader.js'); var rejectBadPath = function (p) { if (p.match(/\.\./)) @@ -851,6 +852,77 @@ _.extend(Unipackage.prototype, { builder.abort(); throw e; } + }, + + // Computes a hash of the versions of all the package's dependencies + // (direct and plugin dependencies) and the slices' and plugins' watch + // sets. Adds the result as a build identifier to the unipackage's + // version. The caller is responsible for checking whether the + // existing version has a build identifier already. + addBuildIdentifierToVersion: function () { + var self = this; + // Gather all the dependencies' versions and organize them into + // arrays. We use arrays to avoid relying on the order of + // stringified object keys. + var directDeps = []; + var directDepsLoader = new PackageLoader(self.buildTimeDirectDependencies); + _.each(self.buildTimeDirectDependencies, function (version, packageName) { + var unipackage = directDepsLoader.getPackage(packageName); + directDeps.push([unipackage.name, unipackage.version]); + }); + + // Sort direct dependencies by package name (which is the "0" property + // of each element in the array). + directDeps = _.sortBy(directDeps, "0"); + + var pluginDeps = []; + _.each(self.buildTimePluginDependencies, function (versions, pluginName) { + var pluginDepsLoader = new PackageLoader(versions); + var singlePluginDeps = []; + _.each(versions, function (version, packageName) { + var unipackage = pluginDepsLoader.getPackage(packageName); + singlePluginDeps.push([unipackage.name, unipackage.version]); + }); + singlePluginDeps = _.sortBy(singlePluginDeps, "0"); + pluginDeps.push([pluginName, singlePluginDeps]); + }); + pluginDeps = _.sortBy(pluginDeps, "0"); + + // Now that we have versions for all our dependencies, canonicalize + // the slices' and plugins' watch sets. + // XXX Do we need to relativize paths? Why? + var sliceFiles = []; + _.each(self.slices, function (slice) { + var watchSetFiles = []; + _.each(slice.watchSet.files, function (hash, fileAbsPath) { + watchSetFiles.push([fileAbsPath, hash]);; + }); + watchSetFiles = _.sortBy(watchSetFiles, "0"); + sliceFiles.push([slice.sliceName, slice.arch, watchSetFiles]); + }); + // Sort by the combination of slice name and architecture. + sliceFiles = _.sortBy(sliceFiles, function (sliceInfo) { + return sliceInfo[0] + " " + sliceInfo[1]; + }); + + var pluginFiles = []; + _.each(self.pluginWatchSet.files, function (hash, fileAbsPath) { + pluginFiles.push([fileAbsPath, hash]); + }); + pluginFiles = _.sortBy(pluginFiles, "0"); + + // Stick all our info into one big array, stringify it, and hash it. + var buildIdInfo = [ + directDeps, + pluginDeps, + sliceFiles, + pluginFiles + ]; + var crypto = require('crypto'); + var hasher = crypto.createHash('sha1'); + hasher.update(JSON.stringify(buildIdInfo)); + var buildId = hasher.digest('hex'); + self.version = self.version + "+" + buildId; } }); From 077437bea6cbf12d9d281808b09cfa1e4ee84f0a Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 11:18:40 -0700 Subject: [PATCH 02/13] Merge plugin/slice watch sets before including in build id --- tools/unipackage.js | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/tools/unipackage.js b/tools/unipackage.js index 043d751b0f..e4c49b7d31 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -891,32 +891,22 @@ _.extend(Unipackage.prototype, { // Now that we have versions for all our dependencies, canonicalize // the slices' and plugins' watch sets. // XXX Do we need to relativize paths? Why? - var sliceFiles = []; + var watchFiles = []; + var watchSet = new watch.WatchSet(); + watchSet.merge(self.pluginWatchSet); _.each(self.slices, function (slice) { - var watchSetFiles = []; - _.each(slice.watchSet.files, function (hash, fileAbsPath) { - watchSetFiles.push([fileAbsPath, hash]);; - }); - watchSetFiles = _.sortBy(watchSetFiles, "0"); - sliceFiles.push([slice.sliceName, slice.arch, watchSetFiles]); + watchSet.merge(slice.watchSet); }); - // Sort by the combination of slice name and architecture. - sliceFiles = _.sortBy(sliceFiles, function (sliceInfo) { - return sliceInfo[0] + " " + sliceInfo[1]; + _.each(watchSet.files, function (hash, fileAbsPath) { + watchFiles.push([fileAbsPath, hash]); }); - - var pluginFiles = []; - _.each(self.pluginWatchSet.files, function (hash, fileAbsPath) { - pluginFiles.push([fileAbsPath, hash]); - }); - pluginFiles = _.sortBy(pluginFiles, "0"); + watchFiles = _.sortBy(watchFiles, "0"); // Stick all our info into one big array, stringify it, and hash it. var buildIdInfo = [ directDeps, pluginDeps, - sliceFiles, - pluginFiles + watchFiles ]; var crypto = require('crypto'); var hasher = crypto.createHash('sha1'); From 8d0f57ef598941807ac039d353717e6a7b6c3023 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 12:34:45 -0700 Subject: [PATCH 03/13] serverPackageData can be null --- tools/catalog.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/catalog.js b/tools/catalog.js index 9f5e4ec92e..aac54278bc 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -135,7 +135,9 @@ _.extend(Catalog.prototype, { self.packages = []; self.versions = []; self.builds = []; - self._insertServerPackages(serverPackageData); + if (serverPackageData) { + self._insertServerPackages(serverPackageData); + } self._addLocalPackageOverrides(true /* setInitialized */); }, From 601393d90faa03e407fc1ab8c923a7252db53216 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 12:35:30 -0700 Subject: [PATCH 04/13] Set officialBuild flags on 'publish' and 'publish-for-arch' --- tools/commands.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index 472412c11b..cf97c2ef46 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1585,7 +1585,7 @@ main.registerCommand({ if (buildmessage.jobHasMessages()) return; // already have errors, so skip the build - compileResult = compiler.compile(packageSource); + compileResult = compiler.compile(packageSource, { officialBuild: true }); }); if (messages.hasMessages()) { @@ -1720,7 +1720,9 @@ main.registerCommand({ var packageSource = new PackageSource(packageDir); packageSource.initFromPackageDir(options.name, packageDir); - var unipackage = compiler.compile(packageSource).unipackage; + var unipackage = compiler.compile(packageSource, { + officialBuild: true + }).unipackage; unipackage.saveToPath(path.join(packageDir, '.build')); var conn; From f55c501ae9984ec0920484b33de3d8f503152966 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 12:36:30 -0700 Subject: [PATCH 05/13] Fix checks before computing build id --- tools/compiler.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/compiler.js b/tools/compiler.js index 47b2851743..acae842af3 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -734,8 +734,9 @@ compiler.compile = function (packageSource, options) { sources.push.apply(sources, sliceSources); }); - - if (options.officialBuild) { + // XXX what should we do if the PackageSource doesn't have a version? + // (e.g. a plugin) + if (! options.officialBuild && packageSource.version) { // XXX I have no idea if this should be using buildmessage.enterJob // or not. test what happens on error buildmessage.enterJob({ From 58e4a31df8702c83ffe7f14046b632734d646194 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 13:51:23 -0700 Subject: [PATCH 06/13] Fix WatchSet.merge to actually merge files. Not sure why this was commented out? --- tools/watch.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/watch.js b/tools/watch.js index 04f86e2f48..aed13b978e 100644 --- a/tools/watch.js +++ b/tools/watch.js @@ -138,9 +138,9 @@ _.extend(WatchSet.prototype, { self.alwaysFire = true; return; } -// _.each(other.files, function (hash, name) { -// self.addFile(name, hash); -// }); + _.each(other.files, function (hash, name) { + self.addFile(name, hash); + }); _.each(other.directories, function (dir) { // XXX this doesn't deep-clone the directory, but I think these objects // are never mutated #WatchSetShallowClone From 9d174696e73841dd0b12409680031dff5824f5b1 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 13:52:26 -0700 Subject: [PATCH 07/13] Remove outdated comment --- tools/compiler.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/compiler.js b/tools/compiler.js index acae842af3..15ed154dba 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -805,11 +805,6 @@ compiler.getBuildOrderConstraints = function (packageSource) { // identical code). True if we have dependency info and it // says that the package is up-to-date. False if a source file or // build-time dependency has changed. -// -// 'what' identifies the build to check for up-to-dateness and is an -// object with exactly one of the following keys: -// - path: a path on disk to a unipackage -// - unipackage: a Unipackage object compiler.checkUpToDate = function (packageSource, unipackage) { if (unipackage.forceNotUpToDate) return false; From af195e2f488ee89c4283fa26c2a72de54f19019b Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 15:45:06 -0700 Subject: [PATCH 08/13] Store and read build-time deps in unipackage buildinfo.json --- tools/unipackage.js | 67 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/tools/unipackage.js b/tools/unipackage.js index e4c49b7d31..e79086cf04 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -464,6 +464,10 @@ _.extend(Unipackage.prototype, { // Read basic buildinfo.json info self.builtBy = buildInfoJson.builtBy || null; + self.buildTimeDirectDependencies = + buildInfoJson.buildTimeDirectDependencies || null; + self.buildTimePluginDependencies = + buildInfoJson.buildTimePluginDependencies || null; if (options.buildOfPath && (buildInfoJson.source !== options.buildOfPath)) { @@ -658,7 +662,9 @@ _.extend(Unipackage.prototype, { sliceDependencies: { }, pluginDependencies: self.pluginWatchSet.toJSON(), pluginProviderPackages: self.pluginProviderPackageDirs, - source: options.buildOfPath || undefined + source: options.buildOfPath || undefined, + buildTimeDirectDependencies: self._buildTimeDirectDependenciesWithBuildIds(), + buildTimePluginDependencies: self._buildTimePluginDependenciesWithBuildIds() }; builder.reserve("unipackage.json"); @@ -854,6 +860,31 @@ _.extend(Unipackage.prototype, { } }, + _buildTimeDirectDependenciesWithBuildIds: function () { + var self = this; + var directDepsLoader = new PackageLoader(self.buildTimeDirectDependencies); + var result = {}; + _.each(self.buildTimeDirectDependencies, function (version, packageName) { + var unipackage = directDepsLoader.getPackage(packageName); + result[packageName] = unipackage.version; + }); + return result; + }, + + _buildTimePluginDependenciesWithBuildIds: function () { + var self = this; + var result = {}; + _.each(self.buildTimePluginDependencies, function (deps, pluginName) { + var pluginPackageLoader = new PackageLoader(deps); + result[pluginName] = {}; + _.each(deps, function (version, packageName) { + var unipackage = pluginPackageLoader.getPackage(packageName); + result[pluginName][packageName] = version; + }); + }); + return result; + }, + // Computes a hash of the versions of all the package's dependencies // (direct and plugin dependencies) and the slices' and plugins' watch // sets. Adds the result as a build identifier to the unipackage's @@ -865,27 +896,31 @@ _.extend(Unipackage.prototype, { // arrays. We use arrays to avoid relying on the order of // stringified object keys. var directDeps = []; - var directDepsLoader = new PackageLoader(self.buildTimeDirectDependencies); - _.each(self.buildTimeDirectDependencies, function (version, packageName) { - var unipackage = directDepsLoader.getPackage(packageName); - directDeps.push([unipackage.name, unipackage.version]); - }); + _.each( + self._buildTimeDirectDependenciesWithBuildIds(), + function (version, packageName) { + directDeps.push([packageName, version]); + } + ); // Sort direct dependencies by package name (which is the "0" property // of each element in the array). directDeps = _.sortBy(directDeps, "0"); var pluginDeps = []; - _.each(self.buildTimePluginDependencies, function (versions, pluginName) { - var pluginDepsLoader = new PackageLoader(versions); - var singlePluginDeps = []; - _.each(versions, function (version, packageName) { - var unipackage = pluginDepsLoader.getPackage(packageName); - singlePluginDeps.push([unipackage.name, unipackage.version]); - }); - singlePluginDeps = _.sortBy(singlePluginDeps, "0"); - pluginDeps.push([pluginName, singlePluginDeps]); - }); + _.each( + self._buildTimePluginDependenciesWithBuildIds(), + function (versions, pluginName) { + var pluginDepsLoader = new PackageLoader(versions); + var singlePluginDeps = []; + _.each(versions, function (version, packageName) { + var unipackage = pluginDepsLoader.getPackage(packageName); + singlePluginDeps.push([unipackage.name, unipackage.version]); + }); + singlePluginDeps = _.sortBy(singlePluginDeps, "0"); + pluginDeps.push([pluginName, singlePluginDeps]); + } + ); pluginDeps = _.sortBy(pluginDeps, "0"); // Now that we have versions for all our dependencies, canonicalize From 1b66cf60d749afe9c623ca72bee4188751208140 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 28 Mar 2014 15:45:30 -0700 Subject: [PATCH 09/13] Use build-time deps in `checkUpToDate` --- tools/compiler.js | 82 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/tools/compiler.js b/tools/compiler.js index 15ed154dba..c981abe5f7 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -26,7 +26,7 @@ var compiler = exports; // end up as watched dependencies. (At least for now, packages only used in // target creation (eg minifiers and dev-bundle-fetcher) don't require you to // update BUILT_BY, though you will need to quit and rerun "meteor run".) -compiler.BUILT_BY = 'meteor/10'; +compiler.BUILT_BY = 'meteor/11'; // XXX where should this go? I'll make it a random utility function // for now @@ -811,11 +811,82 @@ compiler.checkUpToDate = function (packageSource, unipackage) { // Do we think we'd generate different contents than the tool that // built this package? - if (unipackage.builtBy !== compiler.BUILT_BY) + if (unipackage.builtBy !== compiler.BUILT_BY) { + // XXX XXX XXX XXX XXX XXX XXX + // + // This branch is not currently in a state where we can build + // packages with plugins, so we explicitly do NOT want to trigger + // re-builds of packages built by different versions of meteor. + // + // Once this branch is in a state where we CAN build packages + // a-fresh, then we should change this back to "return false". + // + // XXX XXX XXX XXX XXX XXX XXX + console.log("XXX warning: considering package", + packageSource.name, "to be up to date because", + "it was built by <", compiler.BUILT_BY, + "and this makes no sense at all"); + return true; return false; + } - // XXX XXX XXX - var pluginProviderPackageDirs = unipackage.pluginProviderPackageDirs; + var buildTimeDeps = determineBuildTimeDependencies(packageSource); + + // Compute the unipackage's direct and plugin dependencies to + // `buildTimeDeps`, by comparing versions (including build + // identifiers). + + if (_.keys(buildTimeDeps.directDependencies).length !== + _.keys(unipackage.buildTimeDirectDependencies).length) { + return false; + } + + var directDepsPackageLoader = new PackageLoader( + buildTimeDeps.directDependencies); + var directDepsMatch = _.all( + buildTimeDeps.directDependencies, + function (version, packageName) { + var loadedPackage = directDepsPackageLoader.getPackage(packageName); + // XXX Check that `versionWithBuildId` is the same as `version` + // except for the build id? + return (loadedPackage && + unipackage.buildTimeDirectDependencies[packageName] === + loadedPackage.version); + } + ); + if (! directDepsMatch) { + return false; + } + + if (_.keys(buildTimeDeps.pluginDependencies).length !== + _.keys(unipackage.buildTimePluginDependencies).length) { + return false; + } + + var pluginDepsMatch = _.all( + buildTimeDeps.pluginDependencies, + function (pluginDeps, pluginName) { + // For each plugin, check that the resolved build-time deps for + // that plugin match the unipackage's build time deps for it. + var packageLoaderForPlugin = new PackageLoader( + buildTimeDeps.pluginDependencies + ); + var unipackagePluginDeps = unipackage.buildTimePluginDependencies[pluginName]; + if (! unipackagePluginDeps || + _.keys(pluginDeps).length !== _.keys(unipackagePluginDeps).length) { + return false; + } + return _.all(pluginDeps, function (version, packageName) { + var loadedPackage = packageLoaderForPlugin.getPackage(packageName); + return loadedPackage && + unipackagePluginDeps[packageName] === loadedPackage.version; + }); + } + ); + + if (! pluginDepsMatch) { + return false; + } /* // XXX XXX this shouldn't work this way at all. instead it should @@ -848,8 +919,9 @@ compiler.checkUpToDate = function (packageSource, unipackage) { watchSet.merge(slice.watchSet); }); - if (! watch.isUpToDate(watchSet)) + if (! watch.isUpToDate(watchSet)) { return false; + } return true; }; From 62153ce0b079ed7cbda47a3256b82db2876623ec Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sun, 30 Mar 2014 09:01:42 -0700 Subject: [PATCH 10/13] Remove a couple completed XXXs --- tools/compiler.js | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/tools/compiler.js b/tools/compiler.js index c981abe5f7..05b150adb5 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -888,31 +888,6 @@ compiler.checkUpToDate = function (packageSource, unipackage) { return false; } - /* - // XXX XXX this shouldn't work this way at all. instead it should - // just get the resolved build-time dependencies from packageSource - // and make sure they match the versions that were used for the - // build. - var packageLoader = XXX; - - // Are all of the packages we directly use (which can provide - // plugins which affect compilation) resolving to the same - // directory? (eg, have we updated our release version to something - // with a new version of a package?) - var packageResolutionsSame = _.all( - _pluginProviderPackageDirs, function (packageDir, name) { - return packageLoader.getLoadPathForPackage(name) === packageDir; - }); - if (! packageResolutionsSame) - return false; - */ - - // XXX as we're checking build-time dependency freshness in the - // future, remember to not rely on - // packageSource.directBuildTimeDependencies, which may contain - // versions like 1.2.3+local, but instead get versions with real - // build ids through the catalog - var watchSet = new watch.WatchSet(); watchSet.merge(unipackage.pluginWatchSet); _.each(unipackage.slices, function (slice) { From ae506b67071325402afb96b4235bb3bca032cddb Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sun, 30 Mar 2014 09:07:53 -0700 Subject: [PATCH 11/13] Remove `packageDirectoryForBuildInfo`. We used to keep the `packageDirectoryForBuildInfo` of every package that a package directly depends on in its buildinfo.json; if direct dependencies change, then we might need to rebuild because they could have registered or unregistered a plugin. Now we just keep the package name and version (including build for our local packages) of our direct dependencies. --- tools/catalog.js | 4 ++-- tools/commands.js | 4 ++-- tools/compiler.js | 3 +-- tools/package-cache.js | 11 ++++++----- tools/package-source.js | 11 +---------- tools/unipackage.js | 8 +------- 6 files changed, 13 insertions(+), 28 deletions(-) diff --git a/tools/catalog.js b/tools/catalog.js index aac54278bc..b777d4fffc 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -214,7 +214,7 @@ _.extend(Catalog.prototype, { var packageSources = {}; // name to PackageSource var versionIds = {}; // name to _id of the created Version record _.each(self.effectiveLocalPackages, function (packageDir, name) { - var packageSource = new PackageSource(packageDir); + var packageSource = new PackageSource; packageSource.initFromPackageDir(name, packageDir); packageSources[name] = packageSource; @@ -326,7 +326,7 @@ _.extend(Catalog.prototype, { var sourcePath = self.effectiveLocalPackages[name]; var buildDir = path.join(sourcePath, '.build'); if (fs.existsSync(buildDir)) { - var unipackage = new Unipackage(sourcePath); + var unipackage = new Unipackage; unipackage.initFromPath(name, buildDir, { buildOfPath: sourcePath }); if (compiler.checkUpToDate(packageSources[name], unipackage)) { return unipackage; diff --git a/tools/commands.js b/tools/commands.js index cf97c2ef46..ac97270718 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1580,7 +1580,7 @@ main.registerCommand({ // still confirming that it matches the name of the directory) var packageName = path.basename(options.packageDir.toLowerCase()); - packageSource = new PackageSource(options.packageDir); + packageSource = new PackageSource; packageSource.initFromPackageDir(packageName, options.packageDir); if (buildmessage.jobHasMessages()) return; // already have errors, so skip the build @@ -1718,7 +1718,7 @@ main.registerCommand({ return 1; } - var packageSource = new PackageSource(packageDir); + var packageSource = new PackageSource; packageSource.initFromPackageDir(options.name, packageDir); var unipackage = compiler.compile(packageSource, { officialBuild: true diff --git a/tools/compiler.js b/tools/compiler.js index 05b150adb5..406d5e35f1 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -707,7 +707,7 @@ compiler.compile = function (packageSource, options) { // by adding + to the version (it's an error if you already // had one, I guess). - var unipackage = new Unipackage(); + var unipackage = new Unipackage; unipackage.initFromOptions({ name: packageSource.name, metadata: packageSource.metadata, @@ -715,7 +715,6 @@ compiler.compile = function (packageSource, options) { earliestCompatibleVersion: packageSource.earliestCompatibleVersion, defaultSlices: packageSource.defaultSlices, testSlices: packageSource.testSlices, - packageDirectoryForBuildInfo: packageSource.packageDirectoryForBuildInfo, plugins: plugins, pluginWatchSet: pluginWatchSet, buildTimeDirectDependencies: buildTimeDeps.directDependencies, diff --git a/tools/package-cache.js b/tools/package-cache.js index c0bff38ffe..41216f4a6b 100644 --- a/tools/package-cache.js +++ b/tools/package-cache.js @@ -70,14 +70,15 @@ _.extend(PackageCache.prototype, { delete self.softReloadCache[loadPath]; var isUpToDate; + var unipackage; if (fs.existsSync(path.join(loadPath, 'unipackage.json'))) { // We don't even have the source to this package, so it must // be up to date. isUpToDate = true; } else { - var packageSource = new PackageSource(loadPath); + var packageSource = new PackageSource; packageSource.initFromPackageDir(name, loadPath); - var unipackage = new Unipackage(loadPath); + unipackage = new Unipackage; unipackage.initFromPath(name, entry.buildDir); isUpToDate = compiler.checkUpToDate(packageSource, entry.pkg); } @@ -94,7 +95,7 @@ _.extend(PackageCache.prototype, { // Does loadPath point directly at a unipackage (rather than a // source tree?) if (fs.existsSync(path.join(loadPath, 'unipackage.json'))) { - var unipackage = new Unipackage(loadPath); + unipackage = new Unipackage; unipackage.initFromPath(name, loadPath); self.loadedPackages[loadPath] = { pkg: unipackage, @@ -105,13 +106,13 @@ _.extend(PackageCache.prototype, { }; // It's a source tree. Load it. - var packageSource = new PackageSource(loadPath); + var packageSource = new PackageSource; packageSource.initFromPackageDir(name, loadPath); // Does it have an up-to-date build? var buildDir = path.join(loadPath, '.build'); if (fs.existsSync(buildDir)) { - var unipackage = new Unipackage(loadPath); + unipackage = new Unipackage; unipackage.initFromPath(name, buildDir); if (compiler.checkUpToDate(packageSource, unipackage)) { self.loadedPackages[loadPath] = { pkg: unipackage, diff --git a/tools/package-source.js b/tools/package-source.js index 080c7cf5c6..e48ebbd436 100644 --- a/tools/package-source.js +++ b/tools/package-source.js @@ -171,7 +171,7 @@ var SourceSlice = function (pkg, options) { // PackageSource /////////////////////////////////////////////////////////////////////////////// -var PackageSource = function (packageDirectoryForBuildInfo) { +var PackageSource = function () { var self = this; // The name of the package, or null for an app pseudo-package or @@ -193,15 +193,6 @@ var PackageSource = function (packageDirectoryForBuildInfo) { // it's still nice to get it right). self.serveRoot = null; - // The package's directory. This is used only by other packages that use this - // package in their buildinfo.json (to detect that they need to be rebuilt if - // the PackageLoader resolves it to a different package); it is not used to - // read files or anything else. Notably, it should be the same if a package is - // read from a source tree or read from the .build unipackage inside that - // source tree. - // XXX can this go away now? - self.packageDirectoryForBuildInfo = packageDirectoryForBuildInfo; - // Package metadata. Keys are 'summary' and 'internal'. Currently // both of these are optional. self.metadata = {}; diff --git a/tools/unipackage.js b/tools/unipackage.js index e79086cf04..fb90af4d1e 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -180,7 +180,7 @@ _.extend(UnipackageSlice.prototype, { /////////////////////////////////////////////////////////////////////////////// // XXX document -var Unipackage = function (packageDirectoryForBuildInfo) { +var Unipackage = function () { var self = this; // These have the same meaning as in PackageSource. @@ -191,10 +191,6 @@ var Unipackage = function (packageDirectoryForBuildInfo) { self.defaultSlices = {}; self.testSlices = {}; - // XXX this is likely to go away once we have build versions - // (also in PackageSource) - self.packageDirectoryForBuildInfo = packageDirectoryForBuildInfo; - // Build slices. Array of UnipackageSlice. self.slices = []; @@ -212,7 +208,6 @@ var Unipackage = function (packageDirectoryForBuildInfo) { // The versions that we used at build time for each of our direct // dependencies. Map from package name to version string. - // XXX save to disk self.buildTimeDirectDependencies = null; // The complete list of versions (including transitive dependencies) @@ -263,7 +258,6 @@ _.extend(Unipackage.prototype, { self.earliestCompatibleVersion = options.earliestCompatibleVersion; self.defaultSlices = options.defaultSlices; self.testSlices = options.testSlices; - self.packageDirectoryForBuildInfo = options.packageDirectoryForBuildInfo; self.plugins = options.plugins; self.pluginWatchSet = options.pluginWatchSet; self.buildTimeDirectDependencies = options.buildTimeDirectDependencies; From e3b9c09319330f74d6045b82729e298a3be1579d Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sun, 30 Mar 2014 09:18:56 -0700 Subject: [PATCH 12/13] Remove another XXX --- tools/compiler.js | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tools/compiler.js b/tools/compiler.js index 406d5e35f1..04acb9b414 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -692,21 +692,6 @@ compiler.compile = function (packageSource, options) { } } - // XXX XXX HERE HERE - // - // Unless the 'officalBuild' option was set, compute a build - // identifier by finding the versions of all of our package - // dependencies (direct and plugin dependencies) -- real versions, - // not +local version, which means a lookup in the catalog -- and - // hashing them together (in a structured way with what they go - // with, canoncialized as well as possible) with the contents of the - // merged watchsets of our slices and our plugins, with paths - // relativized somehow (that last bit may be tricky!) - // - // Then -- again, unless 'officialBuild' was set -- modify version - // by adding + to the version (it's an error if you already - // had one, I guess). - var unipackage = new Unipackage; unipackage.initFromOptions({ name: packageSource.name, From 2ca550ae6ea754767677d2639c66e5648bcadd4b Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sun, 30 Mar 2014 21:55:52 -0700 Subject: [PATCH 13/13] Fix incorrect PackageLoader constructor arguments --- tools/unipackage.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/unipackage.js b/tools/unipackage.js index fb90af4d1e..21a9c1a4aa 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -856,7 +856,9 @@ _.extend(Unipackage.prototype, { _buildTimeDirectDependenciesWithBuildIds: function () { var self = this; - var directDepsLoader = new PackageLoader(self.buildTimeDirectDependencies); + var directDepsLoader = new PackageLoader({ + versions: self.buildTimeDirectDependencies + }); var result = {}; _.each(self.buildTimeDirectDependencies, function (version, packageName) { var unipackage = directDepsLoader.getPackage(packageName); @@ -869,11 +871,11 @@ _.extend(Unipackage.prototype, { var self = this; var result = {}; _.each(self.buildTimePluginDependencies, function (deps, pluginName) { - var pluginPackageLoader = new PackageLoader(deps); + var pluginPackageLoader = new PackageLoader({ versions: deps }); result[pluginName] = {}; _.each(deps, function (version, packageName) { var unipackage = pluginPackageLoader.getPackage(packageName); - result[pluginName][packageName] = version; + result[pluginName][packageName] = unipackage.version; }); }); return result; @@ -905,7 +907,7 @@ _.extend(Unipackage.prototype, { _.each( self._buildTimePluginDependenciesWithBuildIds(), function (versions, pluginName) { - var pluginDepsLoader = new PackageLoader(versions); + var pluginDepsLoader = new PackageLoader({ versions: versions }); var singlePluginDeps = []; _.each(versions, function (version, packageName) { var unipackage = pluginDepsLoader.getPackage(packageName);