Get rid of RootSource.

This was a sort of hackish source only used internally during
version solving. But it causes a couple of subtle bugs because it
never gets registered with the SourceRegistry. That in turn makes
toString() fail on any PackageId with that source since it relies
on the Source being registered.

The simplest fix is to just remove the hacked source and have a
null source in PackageId/Ref mean "root" and handle it specially.

Review URL: https://codereview.chromium.org//12047096

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@17646 260f80e4-7a28-3924-810f-c04153c831b5

utils/pub/entrypoint.dart

@@ -9,7 +9,6 @@ import 'io.dart';
 import 'lock_file.dart';
 import 'log.dart' as log;
 import 'package.dart';
-import 'root_source.dart';
 import 'system_cache.dart';
 import 'utils.dart';
 import 'version.dart';
@@ -132,7 +131,7 @@ class Entrypoint {
   Future _installDependencies(List<PackageId> packageVersions) {
     return cleanDir(path).then((_) {
       return Future.wait(packageVersions.mappedBy((id) {
-        if (id.source is RootSource) return new Future.immediate(id);
+        if (id.isRoot) return new Future.immediate(id);
         return install(id);
       }));
     }).then(_saveLockFile)
@@ -161,7 +160,7 @@ class Entrypoint {
   Future _saveLockFile(List<PackageId> packageIds) {
     var lockFile = new LockFile.empty();
     for (var id in packageIds) {
-      if (id.source is! RootSource) lockFile.packages[id.name] = id;
+      if (!id.isRoot) lockFile.packages[id.name] = id;
     }
 
     var lockFilePath = join(root.dir, 'pubspec.lock');

utils/pub/package.dart

@@ -82,7 +82,8 @@ class PackageId implements Comparable {
   /// The name of the package being identified.
   final String name;
 
-  /// The [Source] used to look up this package given its [description].
+  /// The [Source] used to look up this package given its [description]. If
+  /// this is a root package ID, this will be `null`.
   final Source source;
 
   /// The package's version.
@@ -96,9 +97,10 @@ class PackageId implements Comparable {
 
   PackageId(this.name, this.source, this.version, this.description);
 
-  int get hashCode => name.hashCode ^
-                      source.name.hashCode ^
-                      version.hashCode;
+  /// Whether this ID identifies the root package.
+  bool get isRoot => source == null;
+
+  int get hashCode => name.hashCode ^ source.hashCode ^ version.hashCode;
 
   bool operator ==(other) {
     if (other is! PackageId) return false;
@@ -106,11 +108,12 @@ class PackageId implements Comparable {
     // enough to uniquely identify the package and that we don't need to delve
     // into the description.
     return other.name == name &&
-           other.source.name == source.name &&
+           other.source == source &&
            other.version == version;
   }
 
   String toString() {
+    if (isRoot) return "$name $version (root)";
     if (source.isDefault) return "$name $version";
     return "$name $version from $source";
   }
@@ -141,7 +144,8 @@ class PackageRef {
   /// The name of the package being identified.
   final String name;
 
-  /// The [Source] used to look up the package.
+  /// The [Source] used to look up the package. If this refers to a root
+  /// package, this will be `null`.
   final Source source;
 
   /// The allowed package versions.
@@ -153,7 +157,20 @@ class PackageRef {
 
   PackageRef(this.name, this.source, this.constraint, this.description);
 
-  String toString() => "$name $constraint from $source ($description)";
+  /// Creates a reference to the given root package.
+  PackageRef.root(Package package)
+      : name = package.name,
+        source = null,
+        constraint = package.version,
+        description = package.name;
+
+  /// Whether this refers to the root package.
+  bool get isRoot => source == null;
+
+  String toString() {
+    if (isRoot) return "$name $constraint (root)";
+    return "$name $constraint from $source ($description)";
+  }
 
   /// Returns a [PackageId] generated from this [PackageRef] with the given
   /// concrete version.

utils/pub/root_source.dart

@@ -1,30 +0,0 @@
-// 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.
-
-library root_source;
-
-import 'dart:async';
-import 'package.dart';
-import 'pubspec.dart';
-import 'source.dart';
-
-/// A source used only for the root package when doing version resolution. It
-/// contains only the root package and is unable to install packages.
-///
-/// This source cannot be referenced from a pubspec.
-class RootSource extends Source {
-  final String name = "root";
-  final bool shouldCache = false;
-  final Package package;
-
-  RootSource(this.package);
-
-  Future<Pubspec> describe(PackageId id) {
-    return new Future<Pubspec>.immediate(package.pubspec);
-  }
-
-  Future<bool> install(PackageId id, String destPath) {
-    throw new UnsupportedError("Can't install from a root source.");
-  }
-}

utils/pub/sdk.dart

@@ -45,7 +45,7 @@ Version _getVersion() {
   var match = _versionPattern.firstMatch(version);
   if (match == null) {
     throw new FormatException("The Dart SDK's 'version' file was not in a "
-        "format pub could recognize. Found: $revision");
+        "format pub could recognize. Found: $version");
   }
 
   var build = match[4];

utils/pub/version_solver.dart

@@ -43,7 +43,6 @@ import 'lock_file.dart';
 import 'log.dart' as log;
 import 'package.dart';
 import 'pubspec.dart';
-import 'root_source.dart';
 import 'source.dart';
 import 'source_registry.dart';
 import 'utils.dart';
@@ -91,8 +90,7 @@ class VersionSolver {
   Future<List<PackageId>> solve() {
     // Kick off the work by adding the root package at its concrete version to
     // the dependency graph.
-    var ref = new PackageRef(
-        _root.name, new RootSource(_root), _root.version, _root.name);
+    var ref = new PackageRef.root(_root);
     enqueue(new AddConstraint('(entrypoint)', ref));
     _pubspecs.cache(ref.atVersion(_root.version), _root.pubspec);
 
@@ -242,9 +240,7 @@ class ChangeVersion implements WorkItem {
   /// The new selected version.
   final Version version;
 
-  ChangeVersion(this.package, this.source, this.description, this.version) {
-    if (source == null) throw "null source";
-  }
+  ChangeVersion(this.package, this.source, this.description, this.version);
 
   Future process(VersionSolver solver) {
     log.fine("Changing $package to version $version.");
@@ -529,14 +525,14 @@ class Dependency {
   }
 
   /// Return the PackageRef that has the canonical source and description for
-  /// this package. If any dependency requires that this package come from a
-  /// [RootSource], that will be used; otherwise, it will be the source and
-  /// description that all dependencies agree upon.
+  /// this package. If any dependency is on the root package, that will be used;
+  /// otherwise, it will be the source and description that all dependencies
+  /// agree upon.
   PackageRef _canonicalRef() {
     if (_refs.isEmpty) return null;
     var refs = _refs.values;
     for (var ref in refs) {
-      if (ref is RootSource) return ref;
+      if (ref.isRoot) return ref;
     }
     return refs.first;
   }
@@ -582,7 +578,7 @@ class Dependency {
     var dependers = _refs.keys.toList();
     if (dependers.length == 1) {
       var depender = dependers[0];
-      if (_refs[depender].source is RootSource) return null;
+      if (_refs[depender].isRoot) return null;
       return depender;
     }
 

utils/tests/pub/version_solver_test.dart

@@ -10,7 +10,6 @@ import 'dart:io';
 import '../../pub/lock_file.dart';
 import '../../pub/package.dart';
 import '../../pub/pubspec.dart';
-import '../../pub/root_source.dart';
 import '../../pub/source.dart';
 import '../../pub/source_registry.dart';
 import '../../pub/system_cache.dart';
@@ -69,7 +68,6 @@ Matcher sourceMismatch(String package1, String package2) {
 MockSource source1;
 MockSource source2;
 Source versionlessSource;
-Source rootSource;
 
 main() {
   testResolve('no dependencies', {
@@ -405,8 +403,6 @@ testResolve(description, packages, {lockfile, result, Matcher error}) {
         // doesn't try to look up information about the local package on the
         // remote server.
         root = package;
-        rootSource = new RootSource(root);
-        cache.register(rootSource);
       } else {
         source.addPackage(package);
       }
@@ -545,7 +541,7 @@ Pair<String, Source> parseSource(String name) {
   switch (match[2]) {
   case 'mock1': return new Pair<String, Source>(match[1], source1);
   case 'mock2': return new Pair<String, Source>(match[1], source2);
-  case 'root': return new Pair<String, Source>(match[1], rootSource);
+  case 'root': return new Pair<String, Source>(match[1], null);
   case 'versionless':
     return new Pair<String, Source>(match[1], versionlessSource);
   }