From 273b70bea48a189bd73d7b0fbf45d8d4f0da5a6a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 26 Aug 2014 23:56:51 -0700 Subject: [PATCH] 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); + }); });