From 56f96cc6df2685e6eb2d2fbb036be6affcabf7d1 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 15 Aug 2014 22:42:09 -0700 Subject: [PATCH] First try locking all previousSolution versions. This is how some of us thought previousSolution already worked. --- .../constraint-solver-tests.js | 50 +++++++++++++++ .../constraint-solver/constraint-solver.js | 63 +++++++++++++------ packages/constraint-solver/resolver.js | 7 ++- 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-tests.js b/packages/constraint-solver/constraint-solver-tests.js index 4284e30e8e..1e8be2d543 100644 --- a/packages/constraint-solver/constraint-solver-tests.js +++ b/packages/constraint-solver/constraint-solver-tests.js @@ -164,6 +164,56 @@ Tinytest.add("constraint solver - non-exact direct dependency", function (test) }, { _testing: true }); }); +Tinytest.add("constraint solver - previousSolution", function (test) { + currentTest = test; + // This is what you get if you lock sparky-forms to 1.0.0. + t({ "sparky-forms": "=1.0.0" }, { + "sparky-forms": "1.0.0", + "awesome-dropdown": "1.4.0", + "dropdown": "1.2.2", + "jquery-widgets": "1.0.0", + "jquery": "1.8.2", + "sparkle": "2.1.1" + }, { _testing: true }); + + // If you just requires something compatible with 1.0.0, we end up choosing + // 1.1.2. + t({ "sparky-forms": "1.0.0" }, { + "sparky-forms": "1.1.2", + "forms": "1.0.1", + "sparkle": "2.1.1", + "jquery-widgets": "1.0.0", + "jquery": "1.8.2" + }, { _testing: true }); + + // But if you ask for something compatible with 1.0.0 and have a previous + // solution with 1.0.0, the previous solution works (since it is achievable). + t({ "sparky-forms": "1.0.0" }, { + "sparky-forms": "1.0.0", + "awesome-dropdown": "1.4.0", + "dropdown": "1.2.2", + "jquery-widgets": "1.0.0", + "jquery": "1.8.2", + "sparkle": "2.1.1" + }, { _testing: true, previousSolution: { + "sparky-forms": "1.0.0" + }}); + + // On the other hand, if the previous solution is incompatible with the + // constraints, it's not an error: we can try something that isn't the + // previous solution in this case! + t({ "sparky-forms": "1.1.2" }, { + "sparky-forms": "1.1.2", + "forms": "1.0.1", + "sparkle": "2.1.1", + "jquery-widgets": "1.0.0", + "jquery": "1.8.2" + }, { _testing: true, previousSolution: { + "sparky-forms": "1.0.0" + }}); + +}); + Tinytest.add("constraint solver - no constraint dependency - anything", function (test) { currentTest = test; var versions = resolver.resolve(["sparkle"], [], { _testing: true }); diff --git a/packages/constraint-solver/constraint-solver.js b/packages/constraint-solver/constraint-solver.js index 718cab29f0..9c8d45b4b4 100644 --- a/packages/constraint-solver/constraint-solver.js +++ b/packages/constraint-solver/constraint-solver.js @@ -204,28 +204,53 @@ ConstraintSolver.PackagesResolver.prototype.resolve = function ( var dc = self._splitDepsToConstraints(dependencies, constraints); - // Never allow to downgrade a version of a direct dependency in regards to the - // previous solution. - // Depending on whether the option `breaking` is set or not, allow only - // compatible upgrades or any upgrades. - _.each(options.previousSolution, function (uv) { - // if not a root dependency, there is no 'no-upgrade' constraint - if (! _.contains(dependencies, uv.name)) - return; - - var constrType = options.breaking ? ">=" : ""; - dc.constraints.push( - self.resolver.getConstraint(uv.name, constrType + uv.version)); - }); - options.rootDependencies = dc.dependencies; + var resolverOptions = self._getResolverOptions(options); + var res = null; + // If a previous solution existed, try resolving with additional (weak) + // equality constraints on all the versions from the previous solution. If + // it's possible to solve the constraints without changing any of the previous + // versions (though we may add more choices in addition, or remove some + // now-unnecessary choices) then that's our first try. + if (!_.isEmpty(options.previousSolution)) { + var constraintsWithPreviousSolutionLock = _.clone(dc.constraints); + _.each(options.previousSolution, function (uv) { + constraintsWithPreviousSolutionLock.push( + self.resolver.getConstraint(uv.name, '=' + uv.version)); + }); + try { + // Try running the resolver. If it fails to resolve, that's OK, we'll keep + // working. + res = self.resolver.resolve( + dc.dependencies, constraintsWithPreviousSolutionLock, resolverOptions); + } catch (e) { + if (!(e.constraintSolverError)) + throw e; + } + } - var resolverOptions = self._getResolverOptions(options, dc); + if (!res) { + // Either we didn't have a previous solution, or it doesn't work. Try again + // without locking in the previous solution as strict equality. + // + // However, we still don't want you to downgrade a version of a direct + // dependency in regards to the previous solution. + // Depending on whether the option `breaking` is set or not, allow only + // compatible upgrades or any upgrades. + _.each(options.previousSolution, function (uv) { + // if not a root dependency, there is no 'no-upgrade' constraint + if (! _.contains(dependencies, uv.name)) + return; - // XXX resolver.resolve can throw an error, should have error handling with - // proper error translation. - var res = self.resolver.resolve(dc.dependencies, dc.constraints, resolverOptions); + var constrType = options.breaking ? ">=" : ""; + dc.constraints.push( + self.resolver.getConstraint(uv.name, constrType + uv.version)); + }); + + res = self.resolver.resolve( + dc.dependencies, dc.constraints, resolverOptions); + } var resultChoices = {}; _.each(res, function (uv) { @@ -324,7 +349,7 @@ ConstraintSolver.PackagesResolver.prototype._unibuildsForPackage = }; ConstraintSolver.PackagesResolver.prototype._getResolverOptions = - function (options, dc) { + function (options) { var self = this; var semverToNum = function (version) { diff --git a/packages/constraint-solver/resolver.js b/packages/constraint-solver/resolver.js index d45eff0fdf..8d8923f9fa 100644 --- a/packages/constraint-solver/resolver.js +++ b/packages/constraint-solver/resolver.js @@ -211,8 +211,11 @@ ConstraintSolver.Resolver.prototype.resolve = return solution; // XXX should be much much better - if (someError) - throw new Error(someError); + if (someError) { + var e = new Error(someError); + e.constraintSolverError = true; + throw e; + } throw new Error("Couldn't resolve, I am sorry"); };