From 22c9ac644d14ef456158aa789f7b97ed54ded003 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 10:10:59 -0700 Subject: [PATCH 01/18] autoupdate_client improvements - use observe instead of observeChanges so that assets is there even if not changing - protect against nonexistence doc.assets --- packages/autoupdate/autoupdate_client.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/autoupdate/autoupdate_client.js b/packages/autoupdate/autoupdate_client.js index cbbbfcbd7d..684da3d7f8 100644 --- a/packages/autoupdate/autoupdate_client.js +++ b/packages/autoupdate/autoupdate_client.js @@ -76,14 +76,14 @@ Autoupdate._retrySubscription = function () { }, onReady: function () { if (Package.reload) { - var checkNewVersionDocument = function (id, fields) { + var checkNewVersionDocument = function (doc) { var self = this; - if (id === 'version-refreshable' && - fields.version !== autoupdateVersionRefreshable) { - autoupdateVersionRefreshable = fields.version; + if (doc._id === 'version-refreshable' && + doc.version !== autoupdateVersionRefreshable) { + autoupdateVersionRefreshable = doc.version; // Switch out old css links for the new css links. Inspired by: // https://github.com/guard/guard-livereload/blob/master/js/livereload.js#L710 - var newCss = fields.assets.allCss; + var newCss = (doc.assets && doc.assets.allCss) || []; var oldLinks = []; _.each(document.getElementsByTagName('link'), function (link) { if (link.className === '__meteor-css__') { @@ -130,14 +130,14 @@ Autoupdate._retrySubscription = function () { attachStylesheetLink(newLink); }); } - else if (id === 'version' && - fields.version !== autoupdateVersion && handle) { + else if (doc._id === 'version' && + doc.version !== autoupdateVersion && handle) { handle.stop(); Package.reload.Reload._reload(); } }; - var handle = ClientVersions.find().observeChanges({ + var handle = ClientVersions.find().observe({ added: checkNewVersionDocument, changed: checkNewVersionDocument }); @@ -145,4 +145,4 @@ Autoupdate._retrySubscription = function () { } }); }; -Autoupdate._retrySubscription(); \ No newline at end of file +Autoupdate._retrySubscription(); From e8437bd91b5e58db72a891f8507d7501f83af6f1 Mon Sep 17 00:00:00 2001 From: Matthew Arbesfeld Date: Tue, 26 Aug 2014 11:53:52 -0700 Subject: [PATCH 02/18] Fix for autoupdate refresh not firing on initial document If you have an old version of the app loaded, startup won't pick up the new version. --- packages/autoupdate/autoupdate_client.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/autoupdate/autoupdate_client.js b/packages/autoupdate/autoupdate_client.js index 684da3d7f8..8fe72c5a8b 100644 --- a/packages/autoupdate/autoupdate_client.js +++ b/packages/autoupdate/autoupdate_client.js @@ -130,9 +130,8 @@ Autoupdate._retrySubscription = function () { attachStylesheetLink(newLink); }); } - else if (doc._id === 'version' && - doc.version !== autoupdateVersion && handle) { - handle.stop(); + else if (doc._id === 'version' && doc.version !== autoupdateVersion) { + handle && handle.stop(); Package.reload.Reload._reload(); } }; From 35d816a36347e9f4d21d0c57a0f894cb848203f0 Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Tue, 26 Aug 2014 15:58:52 -0700 Subject: [PATCH 03/18] Fix infinite reload loop running tests (just a guess at the fix; arbesfeld to fix the fix if necessary) --- packages/autoupdate/autoupdate_client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/autoupdate/autoupdate_client.js b/packages/autoupdate/autoupdate_client.js index 8fe72c5a8b..2b9f79a380 100644 --- a/packages/autoupdate/autoupdate_client.js +++ b/packages/autoupdate/autoupdate_client.js @@ -130,7 +130,7 @@ Autoupdate._retrySubscription = function () { attachStylesheetLink(newLink); }); } - else if (doc._id === 'version' && doc.version !== autoupdateVersion) { + else if (doc._id === 'version' && doc.version !== autoupdateVersion && handle) { handle && handle.stop(); Package.reload.Reload._reload(); } From 1ad3dd17af7e7bc8973b1dc0169b88f59e5f2c9e Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Tue, 26 Aug 2014 16:28:07 -0700 Subject: [PATCH 04/18] Fix the fix to the infinite reload loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …after talking to arbesfeld --- packages/autoupdate/autoupdate_client.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/autoupdate/autoupdate_client.js b/packages/autoupdate/autoupdate_client.js index 2b9f79a380..18be887a85 100644 --- a/packages/autoupdate/autoupdate_client.js +++ b/packages/autoupdate/autoupdate_client.js @@ -130,7 +130,8 @@ Autoupdate._retrySubscription = function () { attachStylesheetLink(newLink); }); } - else if (doc._id === 'version' && doc.version !== autoupdateVersion && handle) { + else if (doc._id === 'version' && autoupdateVersion !== 'unknown' && + doc.version !== autoupdateVersion) { handle && handle.stop(); Package.reload.Reload._reload(); } From 5d4a93d6277f1c6a5058bc2d9f7edb29688c660e Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 20:43:21 -0700 Subject: [PATCH 05/18] Revert "Fix the fix to the infinite reload loop" This reverts commit 1ad3dd17af7e7bc8973b1dc0169b88f59e5f2c9e. --- packages/autoupdate/autoupdate_client.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/autoupdate/autoupdate_client.js b/packages/autoupdate/autoupdate_client.js index 18be887a85..2b9f79a380 100644 --- a/packages/autoupdate/autoupdate_client.js +++ b/packages/autoupdate/autoupdate_client.js @@ -130,8 +130,7 @@ Autoupdate._retrySubscription = function () { attachStylesheetLink(newLink); }); } - else if (doc._id === 'version' && autoupdateVersion !== 'unknown' && - doc.version !== autoupdateVersion) { + else if (doc._id === 'version' && doc.version !== autoupdateVersion && handle) { handle && handle.stop(); Package.reload.Reload._reload(); } From 7358986f131648ee61b1f4e9bf93aa2267454294 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 20:43:22 -0700 Subject: [PATCH 06/18] Revert "Fix infinite reload loop running tests" This reverts commit 35d816a36347e9f4d21d0c57a0f894cb848203f0. --- packages/autoupdate/autoupdate_client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/autoupdate/autoupdate_client.js b/packages/autoupdate/autoupdate_client.js index 2b9f79a380..8fe72c5a8b 100644 --- a/packages/autoupdate/autoupdate_client.js +++ b/packages/autoupdate/autoupdate_client.js @@ -130,7 +130,7 @@ Autoupdate._retrySubscription = function () { attachStylesheetLink(newLink); }); } - else if (doc._id === 'version' && doc.version !== autoupdateVersion && handle) { + else if (doc._id === 'version' && doc.version !== autoupdateVersion) { handle && handle.stop(); Package.reload.Reload._reload(); } From c2c67b128e119959dba9f73d48424831527e5876 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 20:45:07 -0700 Subject: [PATCH 07/18] Fix infinite reload *AND* no-reload bugs in tests The root of the problem David Greenspan tried to fix was that __meteor_runtime_config__.autoupdateVersion was incorrectly 'unknown' when running tests (due to packages/test-in-browser/autoupdate.js being handled wrong by autoupdate_server.js). Turns out the right solution is to ensure the version is known, not to avoid reloading when the version is unknown. --- packages/autoupdate/autoupdate_server.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/autoupdate/autoupdate_server.js b/packages/autoupdate/autoupdate_server.js index 310f03bb45..bc91575c1a 100644 --- a/packages/autoupdate/autoupdate_server.js +++ b/packages/autoupdate/autoupdate_server.js @@ -49,11 +49,12 @@ ClientVersions = new Meteor.Collection("meteor_autoupdate_clientVersions", // runtime config before using the client hash as our default auto // update version id. +// Note: Tests allow people to override Autoupdate.autoupdateVersion before +// startup. Autoupdate.autoupdateVersion = null; Autoupdate.autoupdateVersionRefreshable = null; var syncQueue = new Meteor._SynchronousQueue(); -var startupVersion = null; // updateVersions can only be called after the server has fully loaded. var updateVersions = function (shouldReloadClientProgram) { @@ -64,13 +65,18 @@ var updateVersions = function (shouldReloadClientProgram) { WebAppInternals.reloadClientProgram(); } - if (startupVersion === null) { + // If we just re-read the client program, or if we don't have an autoupdate + // version, calculate it. + if (shouldReloadClientProgram || Autoupdate.autoupdateVersion === null) { Autoupdate.autoupdateVersion = - __meteor_runtime_config__.autoupdateVersion = - process.env.AUTOUPDATE_VERSION || - process.env.SERVER_ID || // XXX COMPAT 0.6.6 - WebApp.calculateClientHashNonRefreshable(); + process.env.AUTOUPDATE_VERSION || + process.env.SERVER_ID || // XXX COMPAT 0.6.6 + WebApp.calculateClientHashNonRefreshable(); } + // If we just recalculated it OR if it was set by (eg) test-in-browser, + // ensure it ends up in __meteor_runtime_config__. + __meteor_runtime_config__.autoupdateVersion = + Autoupdate.autoupdateVersion; Autoupdate.autoupdateVersionRefreshable = __meteor_runtime_config__.autoupdateVersionRefreshable = @@ -111,9 +117,6 @@ var updateVersions = function (shouldReloadClientProgram) { }; Meteor.startup(function () { - // Allow people to override Autoupdate.autoupdateVersion before startup. - // Tests do this. - startupVersion = Autoupdate.autoupdateVersion; updateVersions(false); }); From 4bb1c3f6cefd947bf593d923e3dd7b626dc5c448 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 26 Aug 2014 17:04:06 -0700 Subject: [PATCH 08/18] Do login prompt when talking to package server with expired credential --- tools/auth.js | 36 ++++++++++++++++--------------- tools/package-client.js | 37 ++++++++++++++++++++++++++------ tools/tests/package-tests.js | 41 ++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 23 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index 4e9d0c9d5d..faeec8262b 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -17,7 +17,7 @@ var auth = exports; var getLoadedPackages = function () { return uniload.load({ - packages: [ 'meteor', 'livedata' ] + packages: [ 'meteor', 'livedata', 'mongo-livedata' ] }); }; @@ -1045,24 +1045,10 @@ exports.loggedInUsername = function () { return loggedIn(data) ? currentUsername(data) : false; }; -// Given a ServiceConnection, log in with OAuth using Meteor developer -// accounts. Assumes the user is already logged in to the developer -// accounts server. -exports.loginWithTokenOrOAuth = function (conn, url, domain, sessionType) { - var setUpOnReconnect = function () { - conn.onReconnect = function () { - conn.apply('login', [{ - resume: auth.getSessionToken(domain) - }], { wait: true }, function () { }); - }; - }; - - var Package = uniload.load({ - packages: [ 'meteor', 'livedata', 'mongo-livedata' ] - }); - +exports.getAccountsConfiguration = function (conn) { // Subscribe to the package server's service configurations so that we // can get the OAuth client ID to kick off the OAuth flow. + var Package = getLoadedPackages(); var serviceConfigurations = new Package.meteor.Meteor.Collection( 'meteor_accounts_loginServiceConfiguration', { connection: conn.connection } @@ -1078,6 +1064,22 @@ exports.loginWithTokenOrOAuth = function (conn, url, domain, sessionType) { throw new Error('no-accounts-configuration'); } + return accountsConfiguration; +}; + +// Given a ServiceConnection, log in with OAuth using Meteor developer +// accounts. Assumes the user is already logged in to the developer +// accounts server. +exports.loginWithTokenOrOAuth = function (conn, accountsConfiguration, + url, domain, sessionType) { + var setUpOnReconnect = function () { + conn.onReconnect = function () { + conn.apply('login', [{ + resume: auth.getSessionToken(domain) + }], { wait: true }, function () { }); + }; + }; + var clientId = accountsConfiguration.clientId; var loginResult; diff --git a/tools/package-client.js b/tools/package-client.js index e5c85cd1ce..c9a5e79b74 100644 --- a/tools/package-client.js +++ b/tools/package-client.js @@ -288,12 +288,37 @@ exports.loggedInPackagesConnection = function () { } var conn = openPackageServerConnection(); - auth.loginWithTokenOrOAuth( - conn, - config.getPackageServerUrl(), - config.getPackageServerDomain(), - "package-server" - ); + + var accountsConfiguration = auth.getAccountsConfiguration(conn); + + try { + auth.loginWithTokenOrOAuth( + conn, + accountsConfiguration, + config.getPackageServerUrl(), + config.getPackageServerDomain(), + "package-server" + ); + } catch (err) { + if (err.message === "access-denied") { + // Maybe we thought we were logged in, but our token had been + // revoked. + process.stderr.write( +"It looks like you have been logged out! Please log in with your Meteor\n" + +"developer account. If you don't have one, you can quickly create one\n" + +"at www.meteor.com.\n"); + auth.doUsernamePasswordLogin({ retry: true }); + auth.loginWithTokenOrOAuth( + conn, + accountsConfiguration, + config.getPackageServerUrl(), + config.getPackageServerDomain(), + "package-server" + ); + } else { + throw err; + } + } return conn; }; diff --git a/tools/tests/package-tests.js b/tools/tests/package-tests.js index c09d2d6b51..5532c26580 100644 --- a/tools/tests/package-tests.js +++ b/tools/tests/package-tests.js @@ -584,3 +584,44 @@ selftest.define("package with --name", ['test-package-server'], function () { run.match("overriding accounts-base!"); run.stop(); }); + +selftest.define("talk to package server with expired or no accounts token", ['test-package-server'], function () { + var s = new Sandbox(); + testUtils.login(s, "test", "testtest"); + + // Revoke our credential by logging out. + var session = s.readSessionFile(); + testUtils.logout(s); + + // When we are not logged in, we should get prompted to log in when we + // run 'meteor admin maintainers --add'. + var run = s.run("admin", "maintainers", "standard-app-packages", + "--add", "foo"); + run.waitSecs(15); + run.matchErr("Username:"); + run.write("test\n"); + run.matchErr("Password:"); + run.write("testtest\n"); + run.waitSecs(15); + // The 'test' user should not be a maintainer of + // standard-app-packages. So this command should fail. + run.matchErr("You are not an authorized maintainer"); + run.expectExit(1); + + // Now restore our previous session, so that we now have an expired + // accounts token. + s.writeSessionFile(session); + + run = s.run("admin", "maintainers", "standard-app-packages", "--add", "foo"); + run.waitSecs(15); + run.matchErr("have been logged out"); + run.matchErr("Please log in"); + run.matchErr("Username"); + run.write("test\n"); + run.matchErr("Password:"); + run.write("testtest\n"); + run.waitSecs(15); + + run.matchErr("You are not an authorized maintainer"); + run.expectExit(1); +}); From 2e870042d391a80f06b3e0f29c1b1c01e1f37b55 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 26 Aug 2014 17:53:23 -0700 Subject: [PATCH 09/18] Add new argument to another loginWithTokenOrOAuth call --- tools/stats.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/stats.js b/tools/stats.js index fb418b86d1..c32d41ca1c 100644 --- a/tools/stats.js +++ b/tools/stats.js @@ -88,11 +88,13 @@ var recordPackages = function (what, site) { try { var conn = connectToPackagesStatsServer(); + var accountsConfiguration = auth.getAccountsConfiguration(conn); if (auth.isLoggedIn()) { try { auth.loginWithTokenOrOAuth( conn, + accountsConfiguration, config.getPackageStatsServerUrl(), config.getPackageStatsServerDomain(), "package-stats-server" From 64ecaaf13642ec0ec80750e8eb32d45b7ef53faa Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 26 Aug 2014 18:19:24 -0700 Subject: [PATCH 10/18] Add 'net' tag to "talk to package server" test --- tools/tests/package-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tests/package-tests.js b/tools/tests/package-tests.js index 5532c26580..b0f8f60beb 100644 --- a/tools/tests/package-tests.js +++ b/tools/tests/package-tests.js @@ -585,7 +585,7 @@ selftest.define("package with --name", ['test-package-server'], function () { run.stop(); }); -selftest.define("talk to package server with expired or no accounts token", ['test-package-server'], function () { +selftest.define("talk to package server with expired or no accounts token", ['net', 'test-package-server'], function () { var s = new Sandbox(); testUtils.login(s, "test", "testtest"); From 43e01c09eb9ba890a51721901b7ceb66d3322dc8 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 20:26:23 -0700 Subject: [PATCH 11/18] Improve treatment of prerelease (dashed) packages Drop the "at-least" constraint type entirely. It was not user-accessible and was only used in the form ">=0.0.0" to represent a constraint with no version constraint at all. This type of constraint is now called "any-reasonable". The definition of "any-reasonable" is: - Any version that is not a pre-release (has no dash) - Or a pre-release version that is explicitly mentioned in a TOP-LEVEL constraint passed to the constraint solver For example, constraints from .meteor/packages, constraints from the release, and constraints from the command line of "meteor add" end up being top-level. Why only top-level-constrained pre-release versions, and not versions we find explicitly desired by some other desired version while walking the graph? The constraint solver assumes that adding a constraint to the resolver state can't make previously impossible choices now possible. If pre-releases mentioned anywhere worked, then applying the constraints "any reasonable" followed by "1.2.3-rc1" would result in "1.2.3-rc1" ruled first impossible and then possible again. That's no good, so we have to fix the meaning based on something at the start. (We could try to apply our prerelease-avoidance tactics solely in the cost functions, but then it becomes a much less strict rule.) At the very least, this change should allow you to run meteor on a preview branch like cordova-hcp without getting a conflict between the prerelease package on the branch/release and the lack of an explicit constraint in .meteor/packages on that package, because we are reintepreting the .meteor/packages constraint as meaning "anything reasonable" and the in-the-release version counts as reasonable. --- .../constraint-solver-tests.js | 6 +- .../constraint-solver/constraint-solver.js | 21 ++-- .../constraint-solver/constraints-list.js | 14 ++- packages/constraint-solver/resolver-state.js | 19 +++- packages/constraint-solver/resolver-tests.js | 33 +++--- packages/constraint-solver/resolver.js | 105 ++++++++++++------ .../package-version-parser-tests.js | 34 ++---- tools/catalog.js | 14 ++- tools/compiler.js | 14 +-- tools/package-version-parser.js | 43 ++++--- tools/project.js | 2 +- 11 files changed, 171 insertions(+), 134 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-tests.js b/packages/constraint-solver/constraint-solver-tests.js index a73ea11dcf..0de31653df 100644 --- a/packages/constraint-solver/constraint-solver-tests.js +++ b/packages/constraint-solver/constraint-solver-tests.js @@ -565,12 +565,10 @@ function convertConstraints (inp) { // '>=1.2.3' => '1.2.3' .map(function (s) { if (s.indexOf(">= 0") === 0) - return "none"; + return ""; var x = s.split(' '); - if (x[0] === '~>') + if (x[0] === '~>' || x[0] === '>' || x[0] === '>=') x[0] = ''; - else if (x[0] === '>' || x[0] === '>=') - x[0] = '>='; else if (x[0] === '=') x[0] = '='; else diff --git a/packages/constraint-solver/constraint-solver.js b/packages/constraint-solver/constraint-solver.js index 19f9f97010..3ed7d372bd 100644 --- a/packages/constraint-solver/constraint-solver.js +++ b/packages/constraint-solver/constraint-solver.js @@ -141,8 +141,10 @@ ConstraintSolver.PackagesResolver.prototype._loadPackageInfo = function ( // dependencies - an array of string names of packages (not slices) // constraints - an array of objects: +// (almost, but not quite, what PackageVersion.parseConstraint returns) // - packageName - string name -// - version - string constraint (ex.: "1.2.3", ">=2.3.4", "=3.3.3") +// - version - string constraint +// - type - constraint type // options: // - upgrade - list of dependencies for which upgrade is prioritized higher // than keeping the old version @@ -161,7 +163,9 @@ ConstraintSolver.PackagesResolver.prototype.resolve = function ( check(dependencies, [String]); check(constraints, [{ - packageName: String, version: String, type: String, + packageName: String, + version: Match.OneOf(String, null), + type: String, constraintString: Match.Optional(Match.OneOf(String, null)) }]); @@ -281,15 +285,7 @@ ConstraintSolver.PackagesResolver.prototype._splitDepsToConstraints = }); _.each(inputConstraints, function (constraint) { - if (!semver.valid(constraint.version)) - throw Error("Bad semver: " + constraint.version); - var operator = ""; - if (constraint.type === "exactly") - operator = "="; - if (constraint.type === "at-least") - operator = ">="; - var constraintStr = operator + constraint.version; - + var constraintStr = PackageVersion.constraintToVersionString(constraint); _.each(self._unibuildsForPackage(constraint.packageName), function (unibuildName) { constraints.push(self.resolver.getConstraint(unibuildName, constraintStr)); }); @@ -414,7 +410,6 @@ ConstraintSolver.PackagesResolver.prototype._getResolverOptions = resolverOptions.estimateCostFunction = function (state, options) { options = options || {}; - var constraints = state.constraints; var cost = [0, 0, 0, 0]; state.eachDependency(function (dep, alternatives) { @@ -426,7 +421,7 @@ ConstraintSolver.PackagesResolver.prototype._getResolverOptions = if (_.has(prevSolMapping, dep)) { var prev = prevSolMapping[dep]; - var prevVersionMatches = constraints.isSatisfied(prev, self.resolver); + var prevVersionMatches = state.isSatisfied(prev); // if it matches, assume we would pick it and the cost doesn't // increase diff --git a/packages/constraint-solver/constraints-list.js b/packages/constraint-solver/constraints-list.js index b927e1df0f..c19f19fad9 100644 --- a/packages/constraint-solver/constraints-list.js +++ b/packages/constraint-solver/constraints-list.js @@ -63,10 +63,12 @@ ConstraintSolver.ConstraintsList.prototype.push = function (c) { // Note that this is one of the only pieces of the constraint solver that // actually does logic on constraints (and thus relies on the restricted set // of constraints that we support). - var minimal = mori.get(newList.minimalVersion, c.name); - if (!minimal || semver.lt(c.version, minimal)) { - newList.minimalVersion = mori.assoc( - newList.minimalVersion, c.name, c.version); + if (c.type !== 'any-reasonable') { + var minimal = mori.get(newList.minimalVersion, c.name); + if (!minimal || semver.lt(c.version, minimal)) { + newList.minimalVersion = mori.assoc( + newList.minimalVersion, c.name, c.version); + } } return newList; }; @@ -102,13 +104,13 @@ ConstraintSolver.ConstraintsList.prototype.each = function (iter) { // Checks if the passed unit version satisfies all of the constraints. ConstraintSolver.ConstraintsList.prototype.isSatisfied = function ( - uv, resolver) { + uv, resolver, resolveContext) { var self = this; var satisfied = true; self.forPackage(uv.name, function (c) { - if (! c.isSatisfied(uv, resolver)) { + if (! c.isSatisfied(uv, resolver, resolveContext)) { satisfied = false; return BREAK; } diff --git a/packages/constraint-solver/resolver-state.js b/packages/constraint-solver/resolver-state.js index 6c4ebb4b11..875a48b8ce 100644 --- a/packages/constraint-solver/resolver-state.js +++ b/packages/constraint-solver/resolver-state.js @@ -1,6 +1,7 @@ -ResolverState = function (resolver) { +ResolverState = function (resolver, resolveContext) { var self = this; self._resolver = resolver; + self._resolveContext = resolveContext; // The versions we've already chosen. // unitName -> UnitVersion self.choices = mori.hash_map(); @@ -23,7 +24,8 @@ _.extend(ResolverState.prototype, { self.constraints = self.constraints.push(constraint); var chosen = mori.get(self.choices, constraint.name); - if (chosen && !constraint.isSatisfied(chosen, self._resolver)) { + if (chosen && + !constraint.isSatisfied(chosen, self._resolver, self._resolveContext)) { // This constraint conflicts with a choice we've already made! self.error = "conflict: " + constraint.toString({removeUnibuild: true}) + " vs " + chosen.version; @@ -34,7 +36,8 @@ _.extend(ResolverState.prototype, { if (alternatives) { // Note: filter preserves order, which is important. var newAlternatives = filter(alternatives, function (unitVersion) { - return constraint.isSatisfied(unitVersion, self._resolver); + return constraint.isSatisfied( + unitVersion, self._resolver, self._resolveContext); }); if (mori.is_empty(newAlternatives)) { // XXX we should mention other constraints that are active @@ -68,7 +71,7 @@ _.extend(ResolverState.prototype, { // Note: relying on sortedness of unitsVersions so that alternatives is // sorted too (the estimation function uses this). var alternatives = filter(self._resolver.unitsVersions[unitName], function (uv) { - return self.constraints.isSatisfied(uv, self._resolver); + return self.isSatisfied(uv); // XXX hang on to list of violated constraints and use it in error // message }); @@ -99,7 +102,7 @@ _.extend(ResolverState.prototype, { self = self._clone(); // Does adding this choice break some constraints we already have? - if (!self.constraints.isSatisfied(uv, self._resolver)) { + if (!self.isSatisfied(uv)) { // XXX improve error self.error = "conflict: " + uv.toString({removeUnibuild: true}) + " can't be chosen"; @@ -132,9 +135,13 @@ _.extend(ResolverState.prototype, { mori.last(nameAndAlternatives)); }, self._dependencies); }, + isSatisfied: function (uv) { + var self = this; + return self.constraints.isSatisfied(uv, self._resolver, self._resolveContext); + }, _clone: function () { var self = this; - var clone = new ResolverState(self._resolver); + var clone = new ResolverState(self._resolver, self._resolveContext); _.each(['choices', '_dependencies', 'constraints', 'error'], function (field) { clone[field] = self[field]; }); diff --git a/packages/constraint-solver/resolver-tests.js b/packages/constraint-solver/resolver-tests.js index 51e4661d4c..ce0bc12b77 100644 --- a/packages/constraint-solver/resolver-tests.js +++ b/packages/constraint-solver/resolver-tests.js @@ -153,22 +153,29 @@ Tinytest.add("constraint solver - resolver, don't pick rcs", function (test) { resolver.addUnitVersion(A100rc1); resolver.addUnitVersion(A100); - var initialConstraint = resolver.getConstraint("A", ">=0.0.0"); + var basicConstraint = resolver.getConstraint("A", ""); + var rcConstraint = resolver.getConstraint("A", "1.0.0-rc1"); - var solution = resolver.resolve(["A"], [initialConstraint], { - costFunction: function (state) { - return mori.reduce(mori.sum, 0, mori.map(function (nameAndUv) { - var name = mori.first(nameAndUv); - var uv = mori.last(nameAndUv); - // Make the non-rc one more costly. But we still shouldn't choose it! - if (uv.version === "1.0.0") - return 100; - return 0; - }, state.choices)); - } - }); + // Make the non-rc one more costly. But we still shouldn't choose it unless it + // was specified in an initial constraint! + var proRcCostFunction = function (state) { + return mori.reduce(mori.sum, 0, mori.map(function (nameAndUv) { + var name = mori.first(nameAndUv); + var uv = mori.last(nameAndUv); + // Make the non-rc one more costly. But we still shouldn't choose it! + if (uv.version === "1.0.0") + return 100; + return 0; + }, state.choices)); + }; + var solution = resolver.resolve( + ["A"], [basicConstraint], {costFunction: proRcCostFunction }); resultEquals(test, solution, [A100]); + + solution = resolver.resolve( + ["A"], [rcConstraint], {costFunction: proRcCostFunction }); + resultEquals(test, solution, [A100rc1]); }); function semver2number (semverStr) { diff --git a/packages/constraint-solver/resolver.js b/packages/constraint-solver/resolver.js index 1adef00487..897ebe676c 100644 --- a/packages/constraint-solver/resolver.js +++ b/packages/constraint-solver/resolver.js @@ -140,17 +140,41 @@ ConstraintSolver.Resolver.prototype.resolve = function ( } }, options); + var resolveContext = new ResolveContext; + // Mapping that assigns every package an integer priority. We compute this // dynamically and in the process of resolution we try to resolve packages // with higher priority first. This helps the resolver a lot because if some // package has a higher weight to the solution (like a direct dependency) or // is more likely to break our solution in the future than others, it would be // great to try out and evaluate all versions early in the decision tree. + // XXX this could go on ResolveContext var resolutionPriority = {}; - var startState = new ResolverState(self); + var startState = new ResolverState(self, resolveContext); _.each(constraints, function (constraint) { startState = startState.addConstraint(constraint); + + // Keep track of any top-level constraints that mention a pre-release. + // These will be the only pre-release versions that count as "reasonable" + // for "any-reasonable" (ie, unconstrained) constraints. + // + // Why only top-level mentions, and not mentions we find while walking the + // graph? The constraint solver assumes that adding a constraint to the + // resolver state can't make previously impossible choices now possible. If + // pre-releases mentioned anywhere worked, then applying the constraints + // "any reasonable" followed by "1.2.3-rc1" would result in "1.2.3-rc1" + // ruled first impossible and then possible again. That's no good, so we + // have to fix the meaning based on something at the start. (We could try + // to apply our prerelease-avoidance tactics solely in the cost functions, + // but then it becomes a much less strict rule.) + if (constraint.version && /-/.test(constraint.version)) { + if (!_.has(resolveContext.topLevelPrereleases, constraint.name)) { + resolveContext.topLevelPrereleases[constraint.name] = {}; + } + resolveContext.topLevelPrereleases[constraint.name][constraint.version] + = true; + } }); _.each(dependencies, function (unitName) { startState = startState.addDependency(unitName); @@ -329,58 +353,75 @@ _.extend(ConstraintSolver.UnitVersion.prototype, { ConstraintSolver.Constraint = function (name, versionString) { var self = this; - if (versionString) { + if (versionString !== undefined) { _.extend(self, - PackageVersion.parseVersionConstraint( - versionString, {allowAtLeast: true})); + PackageVersion.parseVersionConstraint(versionString)); self.name = name; } else { // borrows the structure from the parseVersionConstraint format: - // - type - String [compatibl-with|exactly|at-least] + // - type - String [compatible-with|exactly|any-reasonable] // - version - String - semver string - _.extend(self, PackageVersion.parseConstraint(name, {allowAtLeast: true})); + _.extend(self, PackageVersion.parseConstraint(name)); } // See comment in UnitVersion constructor. - self.version = self.version.replace(/\+.*$/, ''); + if (self.version) + self.version = self.version.replace(/\+.*$/, ''); }; ConstraintSolver.Constraint.prototype.toString = function (options) { var self = this; options = options || {}; - var operator = ""; - if (self.type === "exactly") - operator = "="; - if (self.type === "at-least") - operator = ">="; var name = options.removeUnibuild ? removeUnibuild(self.name) : self.name; - return name + "@" + operator + self.version; + return name + "@" + PackageVersion.constraintToVersionString(self); }; -ConstraintSolver.Constraint.prototype.isSatisfied = function (candidateUV, - resolver) { +ConstraintSolver.Constraint.prototype.isSatisfied = function ( + candidateUV, resolver, resolveContext) { var self = this; check(candidateUV, ConstraintSolver.UnitVersion); + if (self.name !== candidateUV.name) { + throw Error("asking constraint on " + self.name + " about " + + candidateUV.name); + } + + if (self.type === "any-reasonable") { + // Non-prerelease versions are always reasonable. + if (!/-/.test(candidateUV.version)) + return true; + + // Is it a pre-release version that was explicitly mentioned at the top + // level? + if (_.has(resolveContext.topLevelPrereleases, self.name) && + _.has(resolveContext.topLevelPrereleases[self.name], + candidateUV.version)) { + return true; + } + + // Otherwise, not this pre-release! + return false; + } + + if (self.type === "exactly") { + return self.version === candidateUV.version; + } + + if (self.type !== "compatible-with") { + throw Error("Unknown constraint type: " + self.type); + } + // Pre-releases only match precisely; @1.2.3-rc1 doesn't necessarily match // 1.2.4, and @1.2.3 doesn't necessarily match 1.2.4-rc1. if (/-/.test(candidateUV.version) || /-/.test(self.version)) { return self.version === candidateUV.version; } - if (self.type === "exactly") - return self.version === candidateUV.version; - // If the candidate version is less than the version named in the constraint, // we are not satisfied. if (semver.lt(candidateUV.version, self.version)) return false; - // If we only care about "at-least" and not backwards-incompatible changes in - // the middle, then candidateUV is good enough. - if (self.type === "at-least") - return true; - var myECV = resolver.getEarliestCompatibleVersion(self.name, self.version); // If the constraint is "@1.2.3" and 1.2.3 doesn't exist, then nothing can // match. This is because we don't know the ECV (compatibility class) of @@ -395,17 +436,11 @@ ConstraintSolver.Constraint.prototype.isSatisfied = function (candidateUV, return myECV === candidateUV.earliestCompatibleVersion; }; -// Returns any unit version satisfying the constraint in the resolver -ConstraintSolver.Constraint.prototype.getSatisfyingUnitVersion = function ( - resolver) { +// An object that records the general context of a resolve call. It can be +// different for different resolve calls on the same Resolver, but is the same +// for every ResolverState in a given call. +var ResolveContext = function () { var self = this; - - if (self.type === "exactly") { - return resolver.getUnitVersion(self.name, self.version); - } - - // XXX this chooses a random UV, not the earliest or latest. Is that OK? - return _.find(resolver.unitsVersions[self.name], function (uv) { - return self.isSatisfied(uv, resolver); - }); + // unitName -> version string -> true + self.topLevelPrereleases = {}; }; diff --git a/packages/package-version-parser/package-version-parser-tests.js b/packages/package-version-parser/package-version-parser-tests.js index 30c0fbc1f3..e855069185 100644 --- a/packages/package-version-parser/package-version-parser-tests.js +++ b/packages/package-version-parser/package-version-parser-tests.js @@ -17,8 +17,8 @@ var FAIL = function (versionString) { Tinytest.add("Smart Package version string parsing - old format", function (test) { currentTest = test; - t("foo", { name: "foo", version: null, type: "compatible-with" }); - t("foo-1234", { name: "foo-1234", version: null, type: "compatible-with" }); + t("foo", { name: "foo", version: null, type: "any-reasonable" }); + t("foo-1234", { name: "foo-1234", version: null, type: "any-reasonable" }); FAIL("my_awesome_InconsitentPackage123"); }); @@ -37,6 +37,8 @@ Tinytest.add("Smart Package version string parsing - compatible version, compati FAIL("foo@x.y.z"); FAIL("foo@<1.2"); FAIL("foo<1.2"); + + t("foo", { name: "foo", version: null, type: "any-reasonable" }); }); Tinytest.add("Smart Package version string parsing - compatible version, exactly", function (test) { @@ -54,32 +56,10 @@ Tinytest.add("Smart Package version string parsing - compatible version, exactly FAIL("foo@=<1.2"); FAIL("foo@<=1.2"); FAIL("foo<=1.2"); -}); -Tinytest.add("Smart Package version string parsing - compatible version, at-least", function (test) { - var t = function (versionString, expected, descr) { - test.equal( - _.omit(PackageVersion.parseConstraint(versionString, - {allowAtLeast: true}), - 'constraintString'), - expected, - descr); - test.throws(function () { - PackageVersion.parseConstraint(versionString); - }); - }; - - var FAIL = function (versionString) { - test.throws(function () { - PackageVersion.parseConstraint(versionString, {allowAtLeast: true}); - }); - test.throws(function () { - PackageVersion.parseConstraint(versionString); - }); - }; - - t("foo@>=1.2.3", { name: "foo", version: "1.2.3", type: "at-least" }); - t("foo-bar@>=3.2.1", { name: "foo-bar", version: "3.2.1", type: "at-least" }); + // We no longer support @>=. + FAIL("foo@>=1.2.3"); + FAIL("foo-bar@>=3.2.1"); FAIL("42@>=0.2.0"); FAIL("foo@>=1.2.3.4"); FAIL("foo@>=1.4"); diff --git a/tools/catalog.js b/tools/catalog.js index ea2469d427..71f2a7e255 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -453,7 +453,8 @@ _.extend(CompleteCatalog.prototype, { // Constraints for uniload should just be packages with no version // constraint and one local version (since they should all be in core). - if (!_.has(constraint, 'packageName') || _.size(constraint) !== 1) { + if (!_.has(constraint, 'packageName') || + constraint.type !== 'any-reasonable') { throw Error("Surprising constraint: " + JSON.stringify(constraint)); } if (!_.has(self.versions, constraint.packageName)) { @@ -499,9 +500,7 @@ _.extend(CompleteCatalog.prototype, { deps.push(constraint.packageName); } delete constraint.weak; - if (constraint.version) { - constr.push(constraint); - } + constr.push(constraint); }); // If we are called with 'ignore projectDeps', then we don't even look to @@ -519,6 +518,13 @@ _.extend(CompleteCatalog.prototype, { }); } + // Local packages can only be loaded from the version we have the source + // for: that's a weak exact constraint. + _.each(self.packageSources, function (packageSource, name) { + constr.push({packageName: name, version: packageSource.version, + type: 'exactly'}); + }); + var patience = new utils.Patience({ messageAfterMs: 1000, message: "Figuring out the best package versions to use. This may take a moment." diff --git a/tools/compiler.js b/tools/compiler.js index 3f1145fb1c..fb5f7bd95e 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -179,10 +179,7 @@ var determineBuildTimeDependencies = function (packageSource, var constraints_array = []; _.each(dependencyMetadata, function (info, packageName) { constraints[packageName] = info.constraint; - var version = null; - if (info.constraint) { - version = utils.parseVersionConstraint(info.constraint) ; - } + var version = utils.parseVersionConstraint(info.constraint || '') ; constraints_array.push(_.extend({ packageName: packageName }, version)); }); @@ -230,12 +227,9 @@ var determineBuildTimeDependencies = function (packageSource, _.each(info.use, function (spec) { var parsedSpec = utils.splitConstraint(spec); constraints[parsedSpec.package] = parsedSpec.constraint || null; - var version = null; - if (parsedSpec.constraint) { - version = utils.parseVersionConstraint(info.constraint) ; - } - constraints_array.push({packageName: parsedSpec.package, - version: version }); + var version = utils.parseVersionConstraint(info.constraint || ''); + constraints_array.push(_.extend({packageName: parsedSpec.package}, + version)); }); var pluginVersion = pluginVersions[info.name] || {}; diff --git a/tools/package-version-parser.js b/tools/package-version-parser.js index 8e7827e81a..d7f9238333 100644 --- a/tools/package-version-parser.js +++ b/tools/package-version-parser.js @@ -23,32 +23,28 @@ var __ = inTool ? require('underscore') : _; // 2. "exactly" - A@=x.y.z - constraints package A only to version x.y.z and // nothing else. // "pick A exactly at x.y.z" -// 3. "at-least" - A@>=x.y.z - constraints package A to version x.y.z or higher. -// "pick A at least at x.y.z" -// This one is only used internally by the constraint solver --- end users -// shouldn't be allowed to specify it, and you need to specially request it -// with the "allowAtLeast" option. +// 3. "any-reasonable" - "A" +// Basically, this means any version of A ... other than ones that have +// dashes in the version (ie, are prerelease) ... unless the prerelease +// version has been explicitly selected (which at this stage in the game +// means they are mentioned in a top-level constraint in the top-level +// call to the resolver). PV.parseVersionConstraint = function (versionString, options) { options = options || {}; - var versionDesc = { version: null, type: "compatible-with", + var versionDesc = { version: null, type: "any-reasonable", constraintString: versionString }; - if (versionString === "none" || versionString === null) { - versionDesc.type = "at-least"; - versionDesc.version = "0.0.0"; + if (!versionString) { return versionDesc; } if (versionString.charAt(0) === '=') { versionDesc.type = "exactly"; versionString = versionString.substr(1); - } else if (options.allowAtLeast && versionString.substr(0, 2) === '>=') { - versionDesc.type = "at-least"; - versionString = versionString.substr(2); + } else { + versionDesc.type = "compatible-with"; } - // XXX check for a dash in the version in case of foo@1.2.3-rc0 - if (! semver.valid(versionString)) { throwVersionParserError( "Version string must look like semver (eg '1.2.3'), not '" @@ -68,7 +64,7 @@ PV.parseConstraint = function (constraintString, options) { var splitted = constraintString.split('@'); var constraint = { name: "", version: null, - type: "compatible-with", constraintString: null }; + type: "any-reasonable", constraintString: null }; var name = splitted[0]; var versionString = splitted[1]; @@ -124,3 +120,20 @@ var throwVersionParserError = function (message) { e.versionParserError = true; throw e; }; + +// XXX if we were better about consistently only using functions in this file, +// we could just do this using the constraintString field +PV.constraintToVersionString = function (parsedConstraint) { + if (parsedConstraint.type === "any-reasonable") + return ""; + if (parsedConstraint.type === "compatible-with") + return parsedConstraint.version; + if (parsedConstraint.type === "exactly") + return "=" + parsedConstraint.version; + throw Error("Unknown constraint type: " + parsedConstraint.type); +}; + +PV.constraintToFullString = function (parsedConstraint) { + return parsedConstraint.name + "@" + PV.constraintToVersionString( + parsedConstraint); +}; diff --git a/tools/project.js b/tools/project.js index 3fdeac7bc0..34e8f0f7de 100644 --- a/tools/project.js +++ b/tools/project.js @@ -281,7 +281,7 @@ _.extend(Project.prototype, { // someday, this will make sense. (The conditional here allows us to work // in tests with releases that have no packages.) if (catalog.complete.getPackage("ctl")) { - allDeps.push({packageName: "ctl", version: null }); + allDeps.push({packageName: "ctl", version: null, type: 'any-reasonable'}); } return allDeps; From 6a6837e32c0ef014bcef51ca387170ccd9ca852f Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 22:16:09 -0700 Subject: [PATCH 12/18] Fix constraint solver benchmark --- packages/constraint-solver/constraint-solver-tests.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-tests.js b/packages/constraint-solver/constraint-solver-tests.js index 0de31653df..446ebb56df 100644 --- a/packages/constraint-solver/constraint-solver-tests.js +++ b/packages/constraint-solver/constraint-solver-tests.js @@ -515,8 +515,10 @@ function getCatalogStub (gems) { }); var ecv = function (version) { - // hard-code to "x.0.0" - return parseInt(version) + ".0.0"; + // hard-coded, because lots of the constraints are > or >= which we + // don't support anymore. But constant ECV means that "compatible-with" + // is interpreted as >=. + return "0.0.0"; }; var packageVersion = { From 8373507288160bd97ff3b54d2a8f564d53940688 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Tue, 26 Aug 2014 22:16:03 -0700 Subject: [PATCH 13/18] Fix hot code push from pre-0.9.0 apps For 0.9.0, we changed the structure of documents in the ClientVersions collection. So now we just throw in a single dummy document in the old format, triggering a reload. Fixes #2447 --- packages/autoupdate/autoupdate_server.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/autoupdate/autoupdate_server.js b/packages/autoupdate/autoupdate_server.js index bc91575c1a..9586351df0 100644 --- a/packages/autoupdate/autoupdate_server.js +++ b/packages/autoupdate/autoupdate_server.js @@ -90,6 +90,13 @@ var updateVersions = function (shouldReloadClientProgram) { WebAppInternals.generateBoilerplate(); } + if (! ClientVersions.findOne({current: true})) { + // To ensure apps with version of Meteor prior to 0.9.0 (in + // which the structure of documents in `ClientVersions` was + // different) also reload. + ClientVersions.insert({current: true}); + } + if (! ClientVersions.findOne({_id: "version"})) { ClientVersions.insert({ _id: "version", From 3564c3ac4e6915e57827eee54470681e610b13c3 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 23:01:48 -0700 Subject: [PATCH 14/18] Fix 'meteor add x@version' over x@other Before, we were running the constraint solver with both the new and the old constraint, which would fail if they were not simultaneously satisfiable. (We were writing the right thing to disk if it succeeded, at least.) --- tools/commands-packages.js | 18 ++++++++++++++++++ tools/project.js | 1 + tools/tests/publish.js | 23 +++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/tools/commands-packages.js b/tools/commands-packages.js index 7bcf83b492..4eff18754b 100644 --- a/tools/commands-packages.js +++ b/tools/commands-packages.js @@ -1738,6 +1738,24 @@ main.registerCommand({ } else { process.stdout.write("The version constraint will be removed.\n"); } + // Now remove the old constraint from what we're going to calculate + // with. + // This matches code in calculateCombinedConstraints. + var oldConstraint = _.extend( + {packageName: constraint.name}, + utils.parseVersionConstraint(packages[constraint.name])); + var removed = false; + for (var i = 0; i < allPackages.length; ++i) { + if (_.isEqual(oldConstraint, allPackages[i])) { + removed = true; + allPackages.splice(i, 1); + break; + } + } + if (!removed) { + throw Error("Couldn't find constraint to remove: " + + JSON.stringify(oldConstraint)); + } } } diff --git a/tools/project.js b/tools/project.js index 34e8f0f7de..dd323c81d4 100644 --- a/tools/project.js +++ b/tools/project.js @@ -234,6 +234,7 @@ _.extend(Project.prototype, { var allDeps = []; // First, we process the contents of the .meteor/packages file. The // self.constraints variable is always up to date. + // Note that two parts of the "add" command run code that matches this. _.each(self.constraints, function (constraint, packageName) { allDeps.push(_.extend({packageName: packageName}, utils.parseVersionConstraint(constraint))); diff --git a/tools/tests/publish.js b/tools/tests/publish.js index 63c43192cc..fa04dbb3cb 100644 --- a/tools/tests/publish.js +++ b/tools/tests/publish.js @@ -194,6 +194,29 @@ selftest.define("list-with-a-new-version", run.match("New versions"); run.match("meteor update"); run.expectExit(0); + + // Switch to the other version. + run = s.run("add", fullPackageName + "@1.0.1"); + run.waitSecs(100); + run.expectExit(0); + run = s.run("list"); + run.waitSecs(10); + run.match(fullPackageName); + run.match("1.0.1"); + run.forbidAll("New versions"); + run.expectExit(0); + + // Switch back to the first version. + run = s.run("add", fullPackageName + "@=1.0.0"); + run.waitSecs(100); + run.expectExit(0); + run = s.run("list"); + run.waitSecs(10); + run.match(fullPackageName); + run.match("1.0.0*"); + run.match("New versions"); + run.match("meteor update"); + run.expectExit(0); }); }); From a5eea24d4781fe1a05b7094ede783366b18c7aa1 Mon Sep 17 00:00:00 2001 From: Matthew Arbesfeld Date: Tue, 26 Aug 2014 23:04:03 -0700 Subject: [PATCH 15/18] Update autoupdate description --- packages/autoupdate/autoupdate_server.js | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/autoupdate/autoupdate_server.js b/packages/autoupdate/autoupdate_server.js index 9586351df0..3f94447901 100644 --- a/packages/autoupdate/autoupdate_server.js +++ b/packages/autoupdate/autoupdate_server.js @@ -19,19 +19,15 @@ // AUTOUPDATE_VERSION. // // The server publishes a `meteor_autoupdate_clientVersions` -// collection. The contract of this collection is that each document -// in the collection represents an acceptable client version, with the -// `_id` field of the document set to the client id. -// -// An "unacceptable" client version, for example, might be a version -// of the client code which has a severe UI bug, or is incompatible -// with the server. An "acceptable" client version could be one that -// is older than the latest client code available on the server but -// still works. -// -// One of the published documents in the collection will have its -// `current` field set to `true`. This is the version of the client -// code that the browser will receive from the server if it reloads. +// collection. There are two documents in this collection, a document +// with _id 'version' which represnets the non refreshable client assets, +// and a document with _id 'version-refreshable' which represents the +// refreshable client assets. Each document has a 'version' field +// which is equivalent to the hash of the relevant assets. The refreshable +// document also contains a list of the refreshable assets, so that the client +// can swap in the new assets without forcing a page refresh. Clients can +// observe changes on these documents to detect when there is a new +// version available. // // In this implementation only two documents are present in the collection // the current refreshable client version and the current nonRefreshable client From 9c9eadc079aeb59f7ab2ac97ca650e668fabc5a0 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Tue, 26 Aug 2014 23:15:38 -0700 Subject: [PATCH 16/18] Add COMPAT WITH 0.8.3 comment --- packages/autoupdate/autoupdate_server.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/autoupdate/autoupdate_server.js b/packages/autoupdate/autoupdate_server.js index 3f94447901..60358afaa2 100644 --- a/packages/autoupdate/autoupdate_server.js +++ b/packages/autoupdate/autoupdate_server.js @@ -86,6 +86,7 @@ var updateVersions = function (shouldReloadClientProgram) { WebAppInternals.generateBoilerplate(); } + // XXX COMPAT WITH 0.8.3 if (! ClientVersions.findOne({current: true})) { // To ensure apps with version of Meteor prior to 0.9.0 (in // which the structure of documents in `ClientVersions` was From 07d9a5e36c7c1d0ad5faa43017c0c88f51b39bea Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 23:24:51 -0700 Subject: [PATCH 17/18] Arch-specific plugins make a package arch-specific. Fixes #2449. --- tools/unipackage.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/unipackage.js b/tools/unipackage.js index a6f09dc1d6..2891fe5508 100644 --- a/tools/unipackage.js +++ b/tools/unipackage.js @@ -307,9 +307,19 @@ _.extend(Unipackage.prototype, { // An sorted array of all the architectures included in this package. architectures: function () { var self = this; - var arches = _.uniq( - _.pluck(self.unibuilds, 'arch').concat(self._toolArchitectures()) - ).sort(); + var archSet = {}; + _.each(self.unibuilds, function (unibuild) { + archSet[unibuild.arch] = true; + }); + _.each(self._toolArchitectures(), function (arch) { + archSet[arch] = true; + }); + _.each(self.plugins, function (plugin, name) { + _.each(plugin, function (plug, arch) { + archSet[arch] = true; + }); + }); + var arches = _.keys(archSet).sort(); // Ensure that our buildArchitectures string does not look like // web+os+os.osx.x86_64 // This would happen if there is an 'os' unibuild but a platform-specific From 273b70bea48a189bd73d7b0fbf45d8d4f0da5a6a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 23:56:51 -0700 Subject: [PATCH 18/18] prerelease versions shouldn't prompt you do update Replace catalog.getLatestVersion with catalog.getLatestMainlineVersion, which skips prerelease versions (those with dashes in the version). Ensure that this function is only used by high-level commands like 'meteor list'. Replace other uses of that function with other equivalent functions. Also, don't stack trace on 'meteor add' constraint failure. --- tools/catalog-base.js | 11 ++- tools/catalog.js | 5 +- tools/commands-packages.js | 70 ++++++++++++------- tools/commands.js | 46 +++--------- tools/selftest.js | 4 +- .../package-of-two-versions/packagerc.js | 4 ++ tools/tests/publish.js | 49 ++++++++++++- 7 files changed, 118 insertions(+), 71 deletions(-) create mode 100644 tools/tests/packages/package-of-two-versions/packagerc.js diff --git a/tools/catalog-base.js b/tools/catalog-base.js index 3cc8045376..e479f6ee2f 100644 --- a/tools/catalog-base.js +++ b/tools/catalog-base.js @@ -197,15 +197,20 @@ _.extend(baseCatalog.BaseCatalog.prototype, { // As getVersion, but returns info on the latest version of the // package, or null if the package doesn't exist or has no versions. - getLatestVersion: function (name) { + // It does not include prereleases (with dashes in the version); + getLatestMainlineVersion: function (name) { var self = this; self._requireInitialized(); buildmessage.assertInCapture(); var versions = self.getSortedVersions(name); - if (versions.length === 0) + versions.reverse(); + var latest = _.find(versions, function (version) { + return !/-/.test(version); + }); + if (!latest) return null; - return self.getVersion(name, versions[versions.length - 1]); + return self.getVersion(name, latest); }, // If this package has any builds at this version, return an array of builds diff --git a/tools/catalog.js b/tools/catalog.js index 71f2a7e255..4429545a03 100644 --- a/tools/catalog.js +++ b/tools/catalog.js @@ -1010,8 +1010,9 @@ _.extend(CompleteCatalog.prototype, { } }); } - // And put a build record for it in the catalog - var versionId = self.getLatestVersion(name); + // 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; // XXX why isn't this build just happening through the package cache // directly? diff --git a/tools/commands-packages.js b/tools/commands-packages.js index 4eff18754b..d573896b38 100644 --- a/tools/commands-packages.js +++ b/tools/commands-packages.js @@ -966,6 +966,7 @@ main.registerCommand({ return _.extend({ buildArchitectures: myStringBuilds }, versionRecord); }; + // XXX should this skip pre-releases? var versions = catalog.official.getSortedVersions(name); if (full.length > 1) { versions = [full[1]]; @@ -1081,7 +1082,7 @@ main.registerCommand({ var vr; doOrDie(function () { - vr = catalog.official.getLatestVersion(name); + vr = catalog.official.getLatestMainlineVersion(name); }); return vr && !vr.unmigrated; }; @@ -1118,7 +1119,7 @@ main.registerCommand({ _.each(allPackages, function (pack) { if (selector(pack, false)) { var vr = doOrDie(function () { - return catalog.official.getLatestVersion(pack); + return catalog.official.getLatestMainlineVersion(pack); }); if (vr) { matchingPackages.push( @@ -1200,8 +1201,12 @@ main.registerCommand({ } var versionAddendum = "" ; - var latest = catalog.complete.getLatestVersion(name, version); + var latest = catalog.complete.getLatestMainlineVersion(name, version); + var semver = require('semver'); if (version !== latest.version && + // If we're currently running a prerelease, "latest" may be older than + // what we're at, so don't tell us we're outdated! + semver.lt(version, latest.version) && !catalog.complete.isLocalPackage(name)) { versionAddendum = "*"; newVersionsAvailable = true; @@ -1779,33 +1784,44 @@ main.registerCommand({ } var downloaded, versions, newVersions; - var messages = buildmessage.capture(function () { - // Get the contents of our versions file. We need to pass them to the - // constraint solver, because our contract with the user says that we will - // never downgrade a dependency. - versions = project.getVersions(); - // Call the constraint solver. - newVersions = catalog.complete.resolveConstraints( - allPackages, - { previousSolution: versions }, - { ignoreProjectDeps: true }); - if ( ! newVersions) { - // XXX: Better error handling. - process.stderr.write("Cannot resolve package dependencies.\n"); - return; - } + try { + var messages = buildmessage.capture(function () { + // Get the contents of our versions file. We need to pass them to the + // constraint solver, because our contract with the user says that we will + // never downgrade a dependency. + versions = project.getVersions(); - // Don't tell the user what all the operations were until we finish -- we - // don't want to give a false sense of completeness until everything is - // written to disk. - var messageLog = []; + // Call the constraint solver. + newVersions = catalog.complete.resolveConstraints( + allPackages, + { previousSolution: versions }, + { ignoreProjectDeps: true }); + if ( ! newVersions) { + // XXX: Better error handling. + process.stderr.write("Cannot resolve package dependencies.\n"); + return; + } - // Install the new versions. If all new versions were installed - // successfully, then change the .meteor/packages and .meteor/versions to - // match expected reality. - downloaded = project.addPackages(constraints, newVersions); - }); + // Don't tell the user what all the operations were until we finish -- we + // don't want to give a false sense of completeness until everything is + // written to disk. + var messageLog = []; + + // Install the new versions. If all new versions were installed + // successfully, then change the .meteor/packages and .meteor/versions to + // match expected reality. + downloaded = project.addPackages(constraints, newVersions); + }); + } catch (e) { + if (!e.constraintSolverError) + throw e; + // XXX this is too many forms of error handling! + process.stderr.write( + "Could not satisfy all the specified constraints:\n" + + e + "\n"); + return 1; + } if (messages.hasMessages()) { process.stderr.write(messages.formatMessages()); return 1; diff --git a/tools/commands.js b/tools/commands.js index d060bff043..587d6bccf9 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -54,22 +54,6 @@ var hostedWithGalaxy = function (site) { return !! require('./deploy-galaxy.js').discoverGalaxy(site); }; -// Get all local packages available. Returns a map from the package name to the -// version record for that package. -var getLocalPackages = function () { - var ret = {}; - buildmessage.assertInCapture(); - - var names = catalog.complete.getAllPackageNames(); - _.each(names, function (name) { - if (catalog.complete.isLocalPackage(name)) { - ret[name] = catalog.complete.getLatestVersion(name); - } - }); - - return ret; -}; - /////////////////////////////////////////////////////////////////////////////// // options that act like commands @@ -950,18 +934,8 @@ main.registerCommand({ }, function (options) { var testPackages; if (options.args.length === 0) { - // Only test local packages if no package is specified. - // XXX should this use the new getLocalPackageNames? - var packageList = commandsPackages.doOrDie(function () { - return getLocalPackages(); - }); - if (! packageList) { - // Couldn't load the package list, probably because some package - // has a parse error. Bail out -- this kind of sucks; we would - // like to find a way to get reloading. - return 1; - } - testPackages = _.keys(packageList); + // Test all local packages if no package is specified. + testPackages = catalog.complete.getLocalPackageNames(); } else { var messages = buildmessage.capture(function () { testPackages = _.map(options.args, function (p) { @@ -978,16 +952,15 @@ main.registerCommand({ } // Check to see if this is a real package, and if it is a real // package, if it has tests. - var versionRec = catalog.complete.getLatestVersion(p); - if (!versionRec) { - buildmessage.error( - "Unknown package: " + p ); - } if (!catalog.complete.isLocalPackage(p)) { buildmessage.error( - "Not a local package, cannot test: " + p ); + "Not a known local package, cannot test: " + p ); return p; } + var versionNames = catalog.complete.getSortedVersions(p); + if (versionNames.length !== 1) + throw Error("local package should have one version?"); + var versionRec = catalog.complete.getVersion(p, versionNames[0]); if (versionRec && !versionRec.testName) { buildmessage.error( "There are no tests for package: " + p ); @@ -1058,7 +1031,10 @@ main.registerCommand({ var tests = []; var messages = buildmessage.capture(function () { _.each(testPackages, function(name) { - var versionRecord = catalog.complete.getLatestVersion(name); + var versionNames = catalog.complete.getSortedVersions(name); + if (versionNames.length !== 1) + throw Error("local package should have one version?"); + var versionRecord = catalog.complete.getVersion(name, versionNames[0]); if (versionRecord && versionRecord.testName) { tests.push(versionRecord.testName); } diff --git a/tools/selftest.js b/tools/selftest.js index 6f4116d12d..5603ae6de0 100644 --- a/tools/selftest.js +++ b/tools/selftest.js @@ -713,7 +713,7 @@ _.extend(Sandbox.prototype, { ['autopublish', 'standard-app-packages', 'insecure'], function (name) { var versionRec = doOrThrow(function () { - return catalog.official.getLatestVersion(name); + return catalog.official.getLatestMainlineVersion(name); }); if (!versionRec) { catalog.official.offline = false; @@ -722,7 +722,7 @@ _.extend(Sandbox.prototype, { }); catalog.official.offline = true; versionRec = doOrThrow(function () { - return catalog.official.getLatestVersion(name); + return catalog.official.getLatestMainlineVersion(name); }); if (!versionRec) { throw new Error(" hack fails for " + name); diff --git a/tools/tests/packages/package-of-two-versions/packagerc.js b/tools/tests/packages/package-of-two-versions/packagerc.js new file mode 100644 index 0000000000..1a8172641b --- /dev/null +++ b/tools/tests/packages/package-of-two-versions/packagerc.js @@ -0,0 +1,4 @@ +Package.describe({ + summary: "Test package.", + version: "1.0.4-rc3" +}); diff --git a/tools/tests/publish.js b/tools/tests/publish.js index fa04dbb3cb..18104a6f07 100644 --- a/tools/tests/publish.js +++ b/tools/tests/publish.js @@ -172,7 +172,7 @@ selftest.define("list-with-a-new-version", run = s.run("list"); run.waitSecs(10); run.match(fullPackageName); - run.match("1.0.0"); + run.match("1.0.0 "); run.forbidAll("New versions"); run.expectExit(0); }); @@ -202,7 +202,7 @@ selftest.define("list-with-a-new-version", run = s.run("list"); run.waitSecs(10); run.match(fullPackageName); - run.match("1.0.1"); + run.match("1.0.1 "); run.forbidAll("New versions"); run.expectExit(0); @@ -217,6 +217,51 @@ selftest.define("list-with-a-new-version", run.match("New versions"); run.match("meteor update"); run.expectExit(0); + + // ... and back to the second version + run = s.run("add", fullPackageName + "@=1.0.1"); + run.waitSecs(100); + run.expectExit(0); + run = s.run("list"); + run.waitSecs(10); + run.match(fullPackageName); + run.match("1.0.1 "); + run.forbidAll("New versions"); + run.expectExit(0); }); + // Now publish an 1.0.4-rc4. + s.cp(fullPackageName+'/packagerc.js', fullPackageName+'/package.js'); + s.cd(fullPackageName, function () { + run = s.run("publish"); + run.waitSecs(15); + run.expectExit(0); + run.match("Done"); + }); + + s.cd('mapp', function () { + // // + // run = s.run("search", "asdf"); + // run.waitSecs(100); + // run.expectExit(0); + + // Because it's an RC, we shouldn't see an update message. + run = s.run("list"); + run.waitSecs(10); + run.match(fullPackageName); + run.match("1.0.1 "); + run.forbidAll("New versions"); + run.expectExit(0); + + // It works if ask for it, though. + run = s.run("add", fullPackageName + "@1.0.4-rc3"); + run.waitSecs(100); + run.expectExit(0); + run = s.run("list"); + run.waitSecs(10); + run.match(fullPackageName); + run.match("1.0.4-rc3 "); + run.forbidAll("New versions"); + run.expectExit(0); + }); });