From e98cd49b3bf5e757f537320b72e39aa96149ef4a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 20 Aug 2014 21:00:49 -0700 Subject: [PATCH] A more specific error for nonexistent package This helps with #2410 though that particular case doesn't work since paths are explored in a bad order. --- .../constraint-solver-tests.js | 2 +- .../constraint-solver/constraint-solver.js | 11 +++++-- packages/constraint-solver/resolver.js | 32 ++++++++++++++++--- tools/catalog-base.js | 10 ++++-- tools/project.js | 1 + 5 files changed, 45 insertions(+), 11 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-tests.js b/packages/constraint-solver/constraint-solver-tests.js index 1e8be2d543..865ed5f537 100644 --- a/packages/constraint-solver/constraint-solver-tests.js +++ b/packages/constraint-solver/constraint-solver-tests.js @@ -513,7 +513,7 @@ function getCatalogStub (gems) { return _.uniq(_.pluck(gems, 'name')); }, getPackage: function (name) { - throw new Error("Not implemeneted"); + return !!_.findWhere(gems, {name: name}); }, getSortedVersions: function (name) { return _.chain(gems) diff --git a/packages/constraint-solver/constraint-solver.js b/packages/constraint-solver/constraint-solver.js index c0348509ac..f82f18d7f6 100644 --- a/packages/constraint-solver/constraint-solver.js +++ b/packages/constraint-solver/constraint-solver.js @@ -65,10 +65,15 @@ ConstraintSolver.PackagesResolver.prototype._loadPackageInfo = function ( // actually have different archs used. var allArchs = ["os", "web.browser", "web.cordova"]; - // XXX is sortedness actually relevant? is there a minor optimization here - // where we can only talk to self.catalog once? + if (!self.catalog.getPackage(packageName, { noRefresh: true })) { + _.each(allArchs, function (arch) { + var unitName = packageName + "#" + arch; + self.resolver.noUnitVersionsExist(unitName); + }); + } + + // XXX is sortedness actually relevant? var sortedVersions = self.catalog.getSortedVersions(packageName); - // XXX throw error if the package doesn't exist? _.each(sortedVersions, function (version) { var versionDef = self.catalog.getVersion(packageName, version); diff --git a/packages/constraint-solver/resolver.js b/packages/constraint-solver/resolver.js index a38cbfe247..fdccbab781 100644 --- a/packages/constraint-solver/resolver.js +++ b/packages/constraint-solver/resolver.js @@ -24,6 +24,11 @@ ConstraintSolver.Resolver = function (options) { // Maps name@version string to a unit version self._unitsVersionsMap = {}; + // A set of unit names which have no unit versions (ie, the package does not + // exist). Note that we only set this for nonexistent packages, not for + // version-less packages, though maybe that's wrong. + self._noUnitVersionsExist = {}; + // Maps unit name string to the greatest version string we have self._latestVersion = {}; @@ -46,6 +51,9 @@ ConstraintSolver.Resolver.prototype.addUnitVersion = function (unitVersion) { check(unitVersion, ConstraintSolver.UnitVersion); + if (_.has(self._noUnitVersionsExist, unitVersion.name)) + throw Error("but no unit versions exist for " + unitVersion.name + "!"); + if (! _.has(self.unitsVersions, unitVersion.name)) { self.unitsVersions[unitVersion.name] = []; self._latestVersion[unitVersion.name] = unitVersion.version; @@ -60,6 +68,16 @@ ConstraintSolver.Resolver.prototype.addUnitVersion = function (unitVersion) { self._latestVersion[unitVersion.name] = unitVersion.version; }; +ConstraintSolver.Resolver.prototype.noUnitVersionsExist = function (unitName) { + var self = this; + + if (_.has(self.unitsVersions, unitName)) + throw Error("already have unit versions for " + unitName + "!"); + self._noUnitVersionsExist[unitName] = true; +}; + + + ConstraintSolver.Resolver.prototype.getUnitVersion = function (unitName, version) { var self = this; return self._unitsVersionsMap[unitName + "@" + version]; @@ -247,8 +265,8 @@ ConstraintSolver.Resolver.prototype.resolve = function ( // } // // NOTE: assumes that exact dependencies are already propagated -ConstraintSolver.Resolver.prototype._stateNeighbors = - function (state, resolutionPriority) { +ConstraintSolver.Resolver.prototype._stateNeighbors = function ( + state, resolutionPriority) { var self = this; var dependencies = state.dependencies; @@ -267,13 +285,19 @@ ConstraintSolver.Resolver.prototype._stateNeighbors = } }); - dependencies = dependencies.remove(candidateName); - var edgeVersions = constraints.edgeMatchingVersionsFor(candidateName, self); edgeVersions.earliest = edgeVersions.earliest || { version: "1000.1000.1000" }; edgeVersions.latest = edgeVersions.latest || { version: "0.0.0" }; + if (_.has(self._noUnitVersionsExist, candidateName)) { + return { + success: false, + failureMsg: "No such package: " + candidateName, + conflictingUnit: candidateName + }; + } + var candidateVersions = _.filter(self.unitsVersions[candidateName], function (uv) { // reject immideately if not in acceptable range diff --git a/tools/catalog-base.js b/tools/catalog-base.js index 35503a7c27..1b2a187ae0 100644 --- a/tools/catalog-base.js +++ b/tools/catalog-base.js @@ -128,13 +128,17 @@ _.extend(baseCatalog.BaseCatalog.prototype, { // Returns general (non-version-specific) information about a // package, or null if there is no such package. - getPackage: function (name) { + getPackage: function (name, options) { var self = this; buildmessage.assertInCapture(); self._requireInitialized(); - return self._recordOrRefresh(function () { + options = options || {}; + + var get = function () { return _.findWhere(self.packages, { name: name }); - }); + }; + + return options.noRefresh ? get() : self._recordOrRefresh(get); }, // Given a package, returns an array of the versions available for diff --git a/tools/project.js b/tools/project.js index c66398a9e2..f1e0763d40 100644 --- a/tools/project.js +++ b/tools/project.js @@ -245,6 +245,7 @@ _.extend(Project.prototype, { utils.parseVersionConstraint(constraint))); }); + // Now we have to go through the programs directory, go through each of the // programs, get their dependencies and use them. (We could have memorized // this value, but this is called very rarely outside the first