Show detailed report on upgrade.

sdk/lib/_internal/pub/lib/src/solver/backtracking_solver.dart

Index: sdk/lib/_internal/pub/lib/src/solver/backtracking_solver.dart
diff --git a/sdk/lib/_internal/pub/lib/src/solver/backtracking_solver.dart b/sdk/lib/_internal/pub/lib/src/solver/backtracking_solver.dart
index a466c2cfc890b2582ea1075a2153e3a6d4152311..1fff63da7d2fecbb1f8629dc09f3b51d39675152 100644
--- a/sdk/lib/_internal/pub/lib/src/solver/backtracking_solver.dart
+++ b/sdk/lib/_internal/pub/lib/src/solver/backtracking_solver.dart
@@ -55,13 +55,19 @@ import 'version_solver.dart';
 class BacktrackingSolver {
   final SourceRegistry sources;
   final Package root;
+
+  /// The lockfile that was present before solving.
   final LockFile lockFile;
+
   final PubspecCache cache;
 
   /// The set of packages that are being explicitly upgraded. The solver will
   /// only allow the very latest version for each of these packages.
   final _forceLatest = new Set<String>();
 
+  /// If this is set, the contents of [lockFile] are ignored while solving.
+  final bool _upgradeAll;
+
   /// The set of packages whose dependecy is being overridden by the root
   /// package, keyed by the name of the package.
   ///
@@ -93,12 +99,12 @@ class BacktrackingSolver {
   var _attemptedSolutions = 1;
 
   BacktrackingSolver(SourceRegistry sources, this.root, this.lockFile,
-                     List<String> useLatest)
+                     List<String> useLatest, {bool upgradeAll})
       : sources = sources,
-        cache = new PubspecCache(sources) {
+        cache = new PubspecCache(sources),
+        _upgradeAll = upgradeAll == true {

[nweiz, 2013/12/11 06:48:32]

Again, I think this would be much cleaner with an explicit default for
[upgradeAll].

Also, keeping this as a flag as opposed to using it to modify [lockFile] means
that you have to be sure that every time you access [lockFile] you remember to
check [_upgradeAll] as well, which seems sub-optimal.

[Bob Nystrom, 2013/12/11 22:36:59]

See previous comment.
I was never crazy about mutating the lockfile in the solver. (I'm actually
tempted to make the class actually immutable.) With this patch, it's actually
important that the lock file not be changed because the report uses it to see
what changed from the previous lock file.

You're right that the solver has to check here (and does now), but that turned
out to just be a couple of places since accessing the lockfile goes through a
narrow set of methods already.

     for (var package in useLatest) {
-      forceLatestVersion(package);
-      lockFile.packages.remove(package);
+      _forceLatest.add(package);
     }
 
     for (var override in root.dependencyOverrides) {
@@ -126,12 +132,31 @@ class BacktrackingSolver {
       _validateSdkConstraint(root.pubspec);
       return _traverseSolution();
     }).then((packages) {
-      return new SolveResult(packages, overrides, null, attemptedSolutions);
+      var availableVersions = new Map<String, List<Version>>();
+      for (var package in packages) {
+        var cached = cache.getCachedVersions(package.toRef());
+        var versions;
+        if (cached != null) {
+          versions = cached.map((id) => id.version).toList();
+        } else {
+          // If we didn't get the version list, that means package is either
+          // the root package, or its a package that we didn't unlock because
+          // we weren't trying to upgrade it. Just use the selected version in
+          // that case.
+          versions = [package.version];
+        }
+
+        availableVersions[package.name] = versions;

[nweiz, 2013/12/11 06:48:32]

Factor this chunk out into _getAvailableVersions(PackageId package).

[Bob Nystrom, 2013/12/11 22:36:59]

Done.

+      }
+
+      return new SolveResult(sources, root, lockFile, packages, overrides,
+          availableVersions, null, attemptedSolutions);

[nweiz, 2013/12/11 06:48:32]

This constructor is getting pretty unwieldy, especially with each invocation
passing null to different parameters. At least make the parameters where one or
the other invocation passes null or {} into named parameters.

[Bob Nystrom, 2013/12/11 22:36:59]

Split into two named constructors.

     }).catchError((error) {
       if (error is! SolveFailure) throw error;
 
       // Wrap a failure in a result so we can attach some other data.
-      return new SolveResult(null, overrides, error, attemptedSolutions);
+      return new SolveResult(sources, root, lockFile, null, overrides,
+          {}, error, attemptedSolutions);
     }).whenComplete(() {
       // Gather some solving metrics.
       var buffer = new StringBuffer();
@@ -148,10 +173,6 @@ class BacktrackingSolver {
     });
   }
 
-  void forceLatestVersion(String package) {
-    _forceLatest.add(package);
-  }
-
   /// Adds [versions], which is the list of all allowed versions of a given
   /// package, to the set of versions to consider for solutions. The first item
   /// in the list will be the currently selected version of that package.
@@ -179,7 +200,12 @@ class BacktrackingSolver {
 
   /// Gets the version of [package] currently locked in the lock file. Returns
   /// `null` if it isn't in the lockfile (or has been unlocked).
-  PackageId getLocked(String package) => lockFile.packages[package];
+  PackageId getLocked(String package) {
+    if (_upgradeAll) return null;
+    if (_forceLatest.contains(package)) return null;
+
+    return lockFile.packages[package];
+  }
 
   /// Traverses the root package's dependency graph using the current potential
   /// solution. If successful, completes to the solution. If not, backtracks
@@ -333,7 +359,7 @@ class BacktrackingSolver {
       buffer.write("- $package");
       if (_forceLatest.contains(package.name)) {
         buffer.write(" (use latest)");
-      } else if (lockFile.packages.containsKey(package.name)) {
+      } else if (!_upgradeAll && lockFile.packages.containsKey(package.name)) {

[nweiz, 2013/12/11 23:31:27]

If you're going to keep _upgradeAll, then all accesses to [lockFile.packages]
should go through [getLocked] to ensure that they respect the same logic.

[Bob Nystrom, 2013/12/12 00:14:58]

Done.

         var version = lockFile.packages[package.name].version;
         buffer.write(" (locked to $version)");
       }