From c77b28ed32ca87487beb01f9cb4d399db134e45d Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 5 May 2014 17:14:38 -0700 Subject: [PATCH] WIP: track which direct deps induced which constraints. Completely untested. Moving in the direction of error reporting. --- .../constraint-solver/constraint-solver.js | 3 +- .../constraint-solver/constraints-list.js | 24 ++++++++---- packages/constraint-solver/resolver.js | 37 +++++++++++++++++-- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/packages/constraint-solver/constraint-solver.js b/packages/constraint-solver/constraint-solver.js index a579b2152c..871382f9ae 100644 --- a/packages/constraint-solver/constraint-solver.js +++ b/packages/constraint-solver/constraint-solver.js @@ -294,7 +294,7 @@ ConstraintSolver.PackagesResolver.prototype._getResolverOptions = var latestFitting = null; for (var i = versions.length - 1; i >= 0; i--) { - if (! constraints.violated(versions[i])) { + if (_.isEmpty(constraints.violatedConstraints(versions[i]))) { latestFitting = versions[i]; break; } @@ -349,4 +349,3 @@ ConstraintSolver.PackagesResolver.prototype._getResolverOptions = return resolverOptions; }; - diff --git a/packages/constraint-solver/constraints-list.js b/packages/constraint-solver/constraints-list.js index c8f3b41cf9..a1f4ad41f2 100644 --- a/packages/constraint-solver/constraints-list.js +++ b/packages/constraint-solver/constraints-list.js @@ -111,17 +111,28 @@ ConstraintSolver.ConstraintsList.prototype.union = function (anotherList) { return newList; }; -// Checks if the passed unit version violates any of the constraints -ConstraintSolver.ConstraintsList.prototype.violated = function (uv) { +// Checks if the passed unit version violates any of the constraints. +// Returns a list of constraints that are violated (empty if the unit +// version does not violate any constraints). +// XXX Returns a regular array, not a ConstraintsList. +ConstraintSolver.ConstraintsList.prototype.violatedConstraints = function (uv) { var self = this; var forPackage = mori.get(self.byName, uv.name); var exact = mori.get(forPackage, "exact"); var inexact = mori.get(forPackage, "inexact"); - if (! mori.every(function (c) { return mori.last(c).isSatisfied(uv); }, exact)) - return true; - - return ! mori.every(function (c) { return mori.last(c).isSatisfied(uv); }, inexact); + var violated = []; + mori.each(exact, function (c) { + if (! mori.last(c).isSatisified(uv)) { + violated.push(c); + } + }); + mori.each(inexact, function (c) { + if (! mori.last(c).isSatisified(uv)) { + violated.push(c); + } + }); + return violated; }; // a weird method that returns a list of exact constraints those correspond to @@ -179,4 +190,3 @@ ConstraintSolver.ConstraintsList.fromArray = function (arr) { return list; }; - diff --git a/packages/constraint-solver/resolver.js b/packages/constraint-solver/resolver.js index dfdc3595a0..d9ddd7053e 100644 --- a/packages/constraint-solver/resolver.js +++ b/packages/constraint-solver/resolver.js @@ -154,6 +154,11 @@ ConstraintSolver.Resolver.prototype.resolve = // - dependencies: DependenciesList - remaining dependencies // - constraints: ConstraintsList - constraints to satisfy // - choices: array of UnitVersion - current fixed set of choices +// - constraintAncestors: Constraint (string representation) -> +// DependencyList. Used for error reporting to indicate which direct +// dependencies have caused a failure. For every constraint, this is +// the list of direct dependencies which led to this constraint being +// present. // // returns { // success: Boolean, @@ -169,19 +174,44 @@ ConstraintSolver.Resolver.prototype._stateNeighbors = var dependencies = state.dependencies; var constraints = state.constraints; var choices = state.choices; + var constraintAncestors = state.constraintAncestors; var candidateName = dependencies.peek(); dependencies = dependencies.remove(candidateName); var candidateVersions = _.filter(self.unitsVersions[candidateName], function (uv) { - return !constraints.violated(uv); + return _.isEmpty(constraints.violatedConstraints(uv)); }); - if (_.isEmpty(candidateVersions)) + if (_.isEmpty(candidateVersions)) { + var uv = self.unitsVersions[candidateName][0]; + if (! uv) { + // XXX Some stupid error message, this should not happen + } else { + var violatedConstraint = _.filter( + constraints.violatedConstraints(uv)[0], + function (constraint) { + return constraint.name === uv.name; + } + ); + // XXX Should we return information about all the violated + // constraints instead of just one? + var directDeps = constraintAncestors[violatedConstraint.toString()]; + return { + success: false, + // XXX We really want to say "directDep1 depends on X@1.0 and + // directDep2 depends on X@2.0" + failureMsg: "Direct dependencies " + directDeps.toString() + + " conflict on " + uv.name + ". " + violatedConstraint.toString() + }; + } + + return { success: false, failureMsg: "Cannot choose satisfying versions of package -- " + candidateName }; + } var lastInvalidNeighbor = null; @@ -291,7 +321,7 @@ ConstraintSolver.Resolver.prototype._propagateExactTransDeps = var choicesDontViolateConstraints = function (choices, constraints) { return _.all(choices, function (uv) { - return !constraints.violated(uv); + return _.isEmpty(constraints.violatedConstraints(uv)); }); }; @@ -446,4 +476,3 @@ ConstraintSolver.Constraint.prototype.getSatisfyingUnitVersion = _.bind(self.isSatisfied, self)); return unitVersion; }; -