Tweak SDK constraint checking a bit.

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

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

lib/src/entrypoint.dart

@@ -9,6 +9,7 @@ import 'io.dart';
 import 'lock_file.dart';
 import 'log.dart' as log;
 import 'package.dart';
+import 'pubspec.dart';
 import 'sdk.dart' as sdk;
 import 'system_cache.dart';
 import 'utils.dart';
@@ -44,7 +45,7 @@ class Entrypoint {
 
   /// Loads the entrypoint from a package at [rootDir].
   Entrypoint(String rootDir, SystemCache cache)
-      : root = new Package(null, rootDir, cache.sources),
+      : root = new Package.load(null, rootDir, cache.sources),
         cache = cache;
 
   // TODO(rnystrom): Make this path configurable.
@@ -139,17 +140,17 @@ class Entrypoint {
   /// Traverses the root's package dependency graph and loads each of the
   /// reached packages. This should only be called after the lockfile has been
   /// successfully generated.
-  Future<List<Package>> walkDependencies() {
+  Future<List<Pubspec>> walkDependencies() {
     return defer(() {
       var lockFile = loadLockFile();
-      var group = new FutureGroup<Package>();
+      var group = new FutureGroup<Pubspec>();
       var visited = new Set<String>();
 
       // Include the root package in the results.
-      group.add(new Future.immediate(root));
+      group.add(new Future.immediate(root.pubspec));
 
-      visitPackage(Package package) {
-        for (var ref in package.dependencies) {
+      visitPackage(Pubspec pubspec) {
+        for (var ref in pubspec.dependencies) {
           if (visited.contains(ref.name)) continue;
 
           // Look up the concrete version.
@@ -160,10 +161,10 @@ class Entrypoint {
           group.add(future.then(visitPackage));
         }
 
-        return package;
+        return pubspec;
       }
 
-      visitPackage(root);
+      visitPackage(root.pubspec);
       return group.future;
     });
   }
@@ -172,13 +173,13 @@ class Entrypoint {
   /// of every package in the dependency graph. If a package's constraint does
   /// not match, prints an error.
   Future validateSdkConstraints() {
-    return walkDependencies().then((packages) {
+    return walkDependencies().then((pubspecs) {
       var errors = [];
 
-      for (var package in packages) {
-        var sdkConstraint = package.pubspec.environment.sdkVersion;
+      for (var pubspec in pubspecs) {
+        var sdkConstraint = pubspec.environment.sdkVersion;
         if (!sdkConstraint.allows(sdk.version)) {
-          errors.add("- '${package.name}' requires ${sdkConstraint}");
+          errors.add("- '${pubspec.name}' requires ${sdkConstraint}");
         }
       }
 

lib/src/git_source.dart

@@ -44,8 +44,7 @@ class GitSource extends Source {
 
       ensureDir(join(systemCacheRoot, 'cache'));
       return _ensureRepoCache(id);
-    }).then((_) => _revisionCachePath(id))
-      .then((path) {
+    }).then((_) => systemCacheDirectory(id)).then((path) {
       revisionCachePath = path;
       if (entryExists(revisionCachePath)) return;
       return _clone(_repoCachePath(id), revisionCachePath, mirror: false);
@@ -54,12 +53,17 @@ class GitSource extends Source {
       if (ref == 'HEAD') return;
       return _checkOut(revisionCachePath, ref);
     }).then((_) {
-      return new Package(id.name, revisionCachePath, systemCache.sources);
+      return new Package.load(id.name, revisionCachePath, systemCache.sources);
     });
   }
 
-  Future<String> systemCacheDirectory(PackageId id) => _revisionCachePath(id);
-
+  /// Returns the path to the revision-specific cache of [id].
+  Future<String> systemCacheDirectory(PackageId id) {
+    return _revisionAt(id).then((rev) {
+      var revisionCacheName = '${id.name}-$rev';
+      return join(systemCacheRoot, revisionCacheName);
+    });
+  }
   /// Ensures [description] is a Git URL.
   void validateDescription(description, {bool fromLockFile: false}) {
     // A single string is assumed to be a Git URL.
@@ -117,14 +121,6 @@ class GitSource extends Source {
         workingDir: _repoCachePath(id)).then((result) => result[0]);
   }
 
-  /// Returns the path to the revision-specific cache of [id].
-  Future<String> _revisionCachePath(PackageId id) {
-    return _revisionAt(id).then((rev) {
-      var revisionCacheName = '${id.name}-$rev';
-      return join(systemCacheRoot, revisionCacheName);
-    });
-  }
-
   /// Clones the repo at the URI [from] to the path [to] on the local
   /// filesystem.
   ///

lib/src/package.dart

@@ -58,26 +58,9 @@ class Package {
   /// Loads the package whose root directory is [packageDir]. [name] is the
   /// expected name of that package (e.g. the name given in the dependency), or
   /// `null` if the package being loaded is the entrypoint package.
-  factory Package(String name, String packageDir, SourceRegistry sources) {
-    var pubspecPath = join(packageDir, 'pubspec.yaml');
-    if (!fileExists(pubspecPath)) throw new PubspecNotFoundException(name);
-
-    try {
-      var pubspec = new Pubspec.parse(readTextFile(pubspecPath), sources);
-
-      if (pubspec.name == null) {
-        throw new PubspecHasNoNameException(name);
-      }
-
-      if (name != null && pubspec.name != name) {
-        throw new PubspecNameMismatchException(name, pubspec.name);
-      }
-
-      return new Package._(packageDir, pubspec);
-    } on FormatException catch (ex) {
-      throw 'Could not parse $pubspecPath:\n${ex.message}';
-    }
-  }
+  Package.load(String name, String packageDir, SourceRegistry sources)
+      : dir = packageDir,
+        pubspec = new Pubspec.load(name, packageDir, sources);
 
   /// Constructs a package with the given pubspec. The package will have no
   /// directory associated with it.

lib/src/pubspec.dart

@@ -5,7 +5,9 @@
 library pubspec;
 
 import '../../pkg/yaml/lib/yaml.dart';
+import '../../pkg/path/lib/path.dart' as path;
 
+import 'io.dart';
 import 'package.dart';
 import 'source.dart';
 import 'source_registry.dart';
@@ -30,6 +32,28 @@ class Pubspec {
   /// are derived.
   final Map<String, Object> fields;
 
+  /// Loads the pubspec for a package [name] located in [packageDir].
+  factory Pubspec.load(String name, String packageDir, SourceRegistry sources) {
+    var pubspecPath = path.join(packageDir, 'pubspec.yaml');
+    if (!fileExists(pubspecPath)) throw new PubspecNotFoundException(name);
+
+    try {
+      var pubspec = new Pubspec.parse(readTextFile(pubspecPath), sources);
+
+      if (pubspec.name == null) {
+        throw new PubspecHasNoNameException(name);
+      }
+
+      if (name != null && pubspec.name != name) {
+        throw new PubspecNameMismatchException(name, pubspec.name);
+      }
+
+      return pubspec;
+    } on FormatException catch (ex) {
+      throw 'Could not parse $pubspecPath:\n${ex.message}';
+    }
+  }
+
   Pubspec(this.name, this.version, this.dependencies, this.environment,
       [Map<String, Object> fields])
     : this.fields = fields == null ? {} : fields;

lib/src/sdk_source.dart

@@ -24,10 +24,10 @@ class SdkSource extends Source {
     return defer(() {
       var packageDir = _getPackagePath(id);
       // TODO(rnystrom): What if packageDir is null?
-      var package = new Package(id.name, packageDir, systemCache.sources);
+      var pubspec = new Pubspec.load(id.name, packageDir, systemCache.sources);
       // Ignore the pubspec's version, and use the SDK's.
-      return new Pubspec(id.name, sdk.version, package.pubspec.dependencies,
-          package.pubspec.environment);
+      return new Pubspec(id.name, sdk.version, pubspec.dependencies,
+          pubspec.environment);
     });
   }
 

lib/src/source.dart

@@ -113,7 +113,7 @@ abstract class Source {
       return install(id, path);
     }).then((found) {
       if (!found) throw 'Package $id not found.';
-      return new Package(id.name, path, systemCache.sources);
+      return new Package.load(id.name, path, systemCache.sources);
     });
   }
 

lib/src/system_cache.dart

@@ -62,22 +62,17 @@ class SystemCache {
 
   /// Gets the package identified by [id]. If the package is already cached,
   /// reads it from the cache. Otherwise, requests it from the source.
-  Future<Package> describe(PackageId id) {
-    Future<Package> getUncached() {
-      // Not cached, so get it from the source.
-      return id.describe().then((pubspec) => new Package.inMemory(pubspec));
-    }
-
+  Future<Pubspec> describe(PackageId id) {
     // Try to get it from the system cache first.
     if (id.source.shouldCache) {
       return id.systemCacheDirectory.then((packageDir) {
-        if (!dirExists(packageDir)) return getUncached();
-        return new Package(id.name, packageDir, sources);
+        if (!dirExists(packageDir)) return id.describe();
+        return new Pubspec.load(id.name, packageDir, sources);
       });
     }
 
     // Not cached, so get it from the source.
-    return getUncached();
+    return id.describe();
   }
 
   /// Ensures that the package identified by [id] is installed to the cache,