From ba9ec26b97a67651fa1243968e1669ceb9087151 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Fri, 12 Sep 2014 12:16:26 -0700
Subject: [PATCH] constraint solver errors: ignore unibuilds

Most people think of package deps in terms of packages, not unibuilds,
but our constraint solver has all these generated "if you're using
foo#os@1.0.0, use foo#web.browser@=1.0.0" constraints. They are just
distracting in errors (which already hide the unibuild part for
clarity) so make sure we don't see them in paths either.

(I could imagine a flag/env var that skips all the removeUnibuild calls
and this special processing, for "gory details" mode.)
---
 .../constraint-solver-tests.js                | 16 +++++++--
 packages/constraint-solver/resolver-state.js  | 34 +++++++++++++++----
 packages/tinytest/tinytest.js                 |  2 +-
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/packages/constraint-solver/constraint-solver-tests.js b/packages/constraint-solver/constraint-solver-tests.js
index 021aa24334..de7a9b6848 100644
--- a/packages/constraint-solver/constraint-solver-tests.js
+++ b/packages/constraint-solver/constraint-solver-tests.js
@@ -160,8 +160,20 @@ Tinytest.add("constraint solver - no results", function (test) {
     ["indirect", "2.0.0"]
   ]);
   testWithResolver(test, resolver, function (t, FAIL) {
-    FAIL({ "bad-1": "1.0.0", "bad-2": "" },
-         /indirect@2\.0\.0 is not satisfied by 1.0.0[^]+bad-1[^]+bad-2/);
+    FAIL({ "bad-1": "1.0.0", "bad-2": "" }, function (error) {
+      return error.message.match(/indirect@2\.0\.0 is not satisfied by 1\.0\.0/)
+        && error.message.match(/bad-1@1\.0\.0/)
+        && error.message.match(/bad-2@1\.0\.0/)
+        // We shouldn't get shown indirect itself in a pathway: that would just
+        // be an artifact of there being a path that passes through another
+        // unibuild.  (Note: we might change our mind and decide that all these
+        // lines should end in the relevant constraint, which would probably be
+        // nice! But in that case, we should test that no line ends with TWO
+        // mentions of indirect.)
+        && ! error.message.match(/-> indirect/)
+        // Lines should be unique.
+        && ! error.message.match(/bad-1[^]+bad-1/);
+    });
   });
 
   resolver = makeResolver([
diff --git a/packages/constraint-solver/resolver-state.js b/packages/constraint-solver/resolver-state.js
index 07595f385c..fdc0513c35 100644
--- a/packages/constraint-solver/resolver-state.js
+++ b/packages/constraint-solver/resolver-state.js
@@ -188,9 +188,12 @@ _.extend(ResolverState.prototype, {
   },
   _shownPathwaysForConstraints: function (unitName) {
     var self = this;
-    return mori.into_array(mori.map(function (pathway) {
-      return showPathway(pathway);
+    var pathways = mori.into_array(mori.map(function (pathway) {
+      return showPathway(pathway, unitName);
     }, mori.get(self._unitPathways, unitName)));
+    pathways.sort();
+    pathways = _.uniq(pathways, true);
+    return pathways;
   },
   _shownPathwaysForConstraintsIndented: function (unitName) {
     var self = this;
@@ -217,8 +220,27 @@ removeUnibuild = function (unitName) {
   return unitName.split('#')[0];
 };
 
-var showPathway = function (pathway) {
-  return mori.into_array(mori.map(function (uv) {
-    return uv.toString({removeUnibuild: true});
-  }, mori.reverse(pathway))).join(' -> ');
+// XXX from Underscore.String (http://epeli.github.com/underscore.string/)
+// XXX how many copies of this do we have in Meteor?
+var startsWith = function(str, starts) {
+  return str.length >= starts.length &&
+    str.substring(0, starts.length) === starts;
+};
+
+var showPathway = function (pathway, dropIfFinal) {
+  var pathUnits = mori.into_array(mori.map(function (uv) {
+    return uv.toString({removeUnibuild: true});
+  }, mori.reverse(pathway)));
+
+  var dropPrefix = removeUnibuild(dropIfFinal) + '@';
+  while (pathUnits.length && startsWith(_.last(pathUnits), dropPrefix)) {
+    pathUnits.pop();
+  }
+
+  // This is a bit of a hack: we're using _.uniq in "it's sorted" mode, whose
+  // implementation is "drop adjacent duplicates". This is what we want (we're
+  // trying to avoid seeing "foo -> foo" which represents "foo#os ->
+  // foo#web.browser") even though it's not actually sorted.
+  pathUnits = _.uniq(pathUnits, true);
+  return pathUnits.join(' -> ');
 };
diff --git a/packages/tinytest/tinytest.js b/packages/tinytest/tinytest.js
index b97f2943b3..b4992a4075 100644
--- a/packages/tinytest/tinytest.js
+++ b/packages/tinytest/tinytest.js
@@ -206,7 +206,7 @@ _.extend(TestCaseResults.prototype, {
       };
     else if (expected instanceof RegExp)
       predicate = function (actual) {
-        return expected.test(actual.message)
+        return expected.test(actual.message);
       };
     else if (typeof expected === 'function')
       predicate = expected;