Validate packages against their SDK constraints.

utils/pub/system_cache.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.
 
 library system_cache;
 
 import 'dart:io';
 import 'dart:async';
 
 import 'git_source.dart';
 import 'hosted_source.dart';
 import 'io.dart';
 import 'io.dart' as io show createTempDir;
 import 'log.dart' as log;
 import 'package.dart';
+import 'pubspec.dart';
 import 'sdk_source.dart';
 import 'source.dart';
 import 'source_registry.dart';
 import 'utils.dart';
 import 'version.dart';
 
 /// The system-wide cache of installed packages.
 ///
 /// This cache contains all packages that are downloaded from the internet.
 /// Packages that are available locally (e.g. from the SDK) don't use this
@@ skipping 26 matching lines @@
     return cache;
   }
 
   /// Registers a new source. This source must not have the same name as a
   /// source that's already been registered.
   void register(Source source) {
     source.bind(this);
     sources.register(source);
   }
 
+  /// 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) {

[nweiz, 2013/01/31 22:38:53]

I don't like that this returns a Package whereas other describe() methods return
Pubspecs. It looks to me like you could make walkDependencies work with Pubspecs
rather than Packages, so I'd do that.

[Bob Nystrom, 2013/02/01 16:53:29]

I had that originally, but Pubspec doesn't know how to find and read the file.
It's constructor just takes the contents of a pubspec file. So I had code here
that would locate the pubspec file, read it, and then create the pubspec.

That's exactly what Package's constructor does. So it seemed like Package was a
better object to use.

[nweiz, 2013/02/01 22:24:54]

I think it's fine to use Package internally to load the pubspec, but I don't
think it makes sense to return it. It's not useful for the known use-case, and
it's inconsistent with other methods called "describe".

If you really feel like this should return a package, at least rename it load()
or something like that.

[Bob Nystrom, 2013/02/02 00:02:31]

Done.

+    Future<Package> getUncached() {
+      // Not cached, so get it from the source.
+      return id.source.describe(id).then((pubspec) {

[nweiz, 2013/01/31 22:38:53]

"id.source.describe(id)" -> "id.describe()"

[Bob Nystrom, 2013/02/01 16:53:29]

Done.

+        return new Package.inMemory(pubspec);
+      });
+    }
+
+    // Try to get it from the system cache first.
+    if (id.source.shouldCache) {
+      return id.source.systemCacheDirectory(id).then((packageDir) {

[nweiz, 2013/01/31 22:38:53]

This would be a little cleaner with an "id.systemCacheDirectory" getter.

[Bob Nystrom, 2013/02/01 16:53:29]

Done.

+        if (!dirExistsSync(packageDir)) return getUncached();
+        return Package.load(id.name, packageDir, sources);
+      });
+    }
+
+    // Not cached, so get it from the source.
+    return getUncached();
+  }
+
   /// Ensures that the package identified by [id] is installed to the cache,
   /// loads it, and returns it.
   ///
   /// It is an error to try installing a package from a source with `shouldCache
   /// == false` to the system cache.
   Future<Package> install(PackageId id) {
     if (!id.source.shouldCache) {
       throw new ArgumentError("Package $id is not cacheable.");
     }
 
@@ skipping 19 matching lines @@
 
   /// Delete's the system cache's internal temp directory.
   Future deleteTempDir() {
     log.fine('Clean up system cache temp directory $tempDir.');
     return dirExists(tempDir).then((exists) {
       if (!exists) return;
       return deleteDir(tempDir);
     });
   }
 }