Better error messages for dependency conflicts.

utils/pub/version_solver.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Attempts to resolve a set of version constraints for a package dependency
  * graph and select an appropriate set of best specific versions for all
  * dependent packages. It works iteratively and tries to reach a stable
  * solution where the constraints of all dependencies are met. If it fails to
  * reach a solution after a certain number of iterations, it assumes the
@@ skipping 146 matching lines @@
       for (var version in versions) {
         if (dependency.useLatestVersion ||
             dependency.constraint.allows(version)) {
           if (best == null || version > best) best = version;
         }
       }
 
       // TODO(rnystrom): Better exception.
       if (best == null) {
         if (tryUnlockDepender(dependency)) return null;
-        throw new NoVersionException(dependency.name, dependency.constraint);
+        throw new NoVersionException(dependency.name, dependency.constraint,
+            dependency._refs);
       } else if (!dependency.constraint.allows(best)) {
         if (tryUnlockDepender(dependency)) return null;
         throw new CouldNotUpdateException(
             dependency.name, dependency.constraint, best);
       }
 
       return best;
     });
   }
 
@@ skipping 168 matching lines @@
 
     // If the package is over-constrained, i.e. the packages depending have
     // disjoint constraints, then try unlocking a depender that's locked by the
     // lockfile. If there are no remaining locked dependencies, throw an error.
     if (newConstraint != null && newConstraint.isEmpty) {
       if (solver.tryUnlockDepender(newDependency)) {
         undo(solver);
         return null;
       }
 
-      throw new DisjointConstraintException(name);
+      throw new DisjointConstraintException(name, newDependency._refs);
     }
 
     // If this constraint change didn't cause the overall constraint on the
     // package to change, then we don't need to do any further work.
     if (oldConstraint == newConstraint) return null;
 
     // If the dependency has been cut free from the graph, just remove it.
     if (!newDependency.isDependedOn) {
       solver.enqueue(new ChangeVersion(name, source, description, null));
       return null;
@@ skipping 232 matching lines @@
   /** Creates a copy of this dependency. */
   Dependency clone() => new Dependency._clone(this);
 
   /// Return a list of available versions for this dependency.
   Future<List<Version>> getVersions() => source.getVersions(name, description);
 
   /**
    * Places [ref] as a constraint from [package] onto this.
    */
   void placeConstraint(String package, PackageRef ref) {
-    var required = _requiredRef();
-    if (required != null) {
+    var requiredDepender = _requiredDepender();
+    if (requiredDepender != null) {
+      var required = _refs[requiredDepender];
       if (required.source.name != ref.source.name) {
-        throw new SourceMismatchException(name, required.source, ref.source);
+        throw new SourceMismatchException(name,
+            requiredDepender, required.source, package, ref.source);
       } else if (!required.source.descriptionsEqual(
                      required.description, ref.description)) {
-        throw new DescriptionMismatchException(
-            name, required.description, ref.description);
+        throw new DescriptionMismatchException(name,
+            requiredDepender, required.description, package, ref.description);
       }
     }
 
     _refs[package] = ref;
   }
 
-  /// Returns a PackageRef whose source and description any new constraints are
-  /// required to match. Returns null if there are no requirements on new
-  /// constraints.
-  PackageRef _requiredRef() {
+  /// Returns the name of a package whose constraint source and description

[nweiz, 2012/12/06 00:58:55]

"constraint" -> ""?

[Bob Nystrom, 2012/12/06 17:44:41]

It returns the name of the depender, and it's not the depender's source and
description that have to match, it's the depender's constraint's source and
description.

[nweiz, 2012/12/06 19:43:10]

Then maybe "constraint's"?

+  /// all other constraints must match. Returns null if there are no
+  /// requirements on new constraints.
+  String _requiredDepender() {
     if (_refs.isEmpty) return null;
-    var refs = _refs.values;
-    var first = refs[0];
-    if (refs.length == 1) {
-      if (first.source is RootSource) return null;
-      return first;
+
+    var dependers = _refs.keys;
+    if (dependers.length == 1) {
+      var depender = dependers[0];
+      if (_refs[depender].source is RootSource) return null;
+      return depender;
     }
-    return refs[1];
+
+    return dependers[1];
   }
 
   /**
    * Removes the constraint from [package] onto this.
    */
   PackageRef removeConstraint(String package) => _refs.remove(package);
 }
 
-// TODO(rnystrom): Report the last of depending packages and their constraints.
 /**
  * Exception thrown when the [VersionConstraint] used to match a package is
  * valid (i.e. non-empty), but there are no released versions of the package
  * that fit that constraint.
  */
 class NoVersionException implements Exception {
   final String package;
   final VersionConstraint constraint;
+  final Map<String, PackageRef> _dependencies;
 
-  NoVersionException(this.package, this.constraint);
+  NoVersionException(this.package, this.constraint, this._dependencies);
 
-  String toString() =>
-      "Package '$package' has no versions that match $constraint.";
+  String toString() {
+    var buffer = new StringBuffer();
+    buffer.add("Package '$package' has no versions that match $constraint derived from:\n");

[nweiz, 2012/12/06 00:58:55]

Long line

[Bob Nystrom, 2012/12/06 17:44:41]

Done.

+
+    var keys = new List.from(_dependencies.keys);
+    keys.sort();
+
+    for (var key in keys) {
+      buffer.add("- '$key' depends on version "
+          "${_dependencies[key].constraint}\n");
+    }
+
+    return buffer.toString();
+  }
 }
 
 // TODO(rnystrom): Report the list of depending packages and their constraints.
 /**
  * Exception thrown when the most recent version of [package] must be selected,
  * but doesn't match the [VersionConstraint] imposed on the package.
  */
 class CouldNotUpdateException implements Exception {
   final String package;
   final VersionConstraint constraint;
   final Version best;
 
   CouldNotUpdateException(this.package, this.constraint, this.best);
 
   String toString() =>
       "The latest version of '$package', $best, does not match $constraint.";
 }
 
-// TODO(rnystrom): Report the last of depending packages and their constraints.
 /**
  * Exception thrown when the [VersionConstraint] used to match a package is
  * the empty set: in other words, multiple packages depend on it and have
  * conflicting constraints that have no overlap.
  */
 class DisjointConstraintException implements Exception {
   final String package;
+  final Map<String, PackageRef> _dependencies;
 
-  DisjointConstraintException(this.package);
+  DisjointConstraintException(this.package, this._dependencies);
 
-  String toString() =>
-      "Package '$package' has disjoint constraints.";
+  String toString() {
+    var buffer = new StringBuffer();
+    buffer.add("Shared dependency version conflict on '$package':\n");

[nweiz, 2012/12/06 00:58:55]

I don't think "shared dependency version conflict" will mean a whole lot to most
of our users. Maybe "incompatible version constraints"? Similar issue with the
other error messages below.

[Bob Nystrom, 2012/12/06 17:44:41]

Done.

+
+    var keys = new List.from(_dependencies.keys);
+    keys.sort();
+
+    for (var key in keys) {
+      buffer.add("- '$key' depends on version "
+          "${_dependencies[key].constraint}\n");
+    }
+
+    return buffer.toString();
+  }
 }
 
 /**
  * Exception thrown when the [VersionSolver] fails to find a solution after a
  * certain number of iterations.
  */
 class CouldNotSolveException implements Exception {
   CouldNotSolveException();
 
   String toString() =>
       "Could not find a solution that met all version constraints.";
 }
 
 /**
  * Exception thrown when two packages with the same name but different sources
  * are depended upon.
  */
 class SourceMismatchException implements Exception {
   final String package;
+  final String depender1;
   final Source source1;
+  final String depender2;
   final Source source2;
 
-  SourceMismatchException(this.package, this.source1, this.source2);
+  SourceMismatchException(this.package, this.depender1, this.source1,
+      this.depender2, this.source2);
 
   String toString() {
-    return "Package '$package' is depended on from both sources "
-    "'${source1.name}' and '${source2.name}'.";
+    return "Shared dependency conflict on '$package':\n"
+        "- '$depender1' depends on it from source '$source1'\n"
+        "- '$depender2' depends on it from source '$source2'";
   }
 }
 
 /**
  * Exception thrown when two packages with the same name and source but
  * different descriptions are depended upon.
  */
 class DescriptionMismatchException implements Exception {
   final String package;
+  final String depender1;
   final description1;
+  final String depender2;
   final description2;
 
-  DescriptionMismatchException(this.package, this.description1,
-      this.description2);
+  DescriptionMismatchException(this.package, this.depender1, this.description1,
+      this.depender2, this.description2);
 
-  // TODO(nweiz): Dump to YAML when that's supported
-  String toString() => "Package '$package' has conflicting descriptions "
-    "'${JSON.stringify(description1)}' and '${JSON.stringify(description2)}'";
+  String toString() {
+    // TODO(nweiz): Dump descriptions to YAML when that's supported.
+    return "Shared dependency conflict on '$package':\n"
+        "- '$depender1' depends on it with description "
+        "${JSON.stringify(description1)}\n"
+        "- '$depender2' depends on it with description "
+        "${JSON.stringify(description2)}";
+  }
 }