Gracefully handle pubspecs with dependencies using unknown sources.

sdk/lib/_internal/pub/lib/src/source.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 source;
 
 import 'dart:async';
 
 import 'package:pathos/path.dart' as path;
 
@@ skipping 50 matching lines @@
   /// Get the list of all versions that exist for the package described by
   /// [description]. [name] is the expected name of the package.
   ///
   /// Note that this does *not* require the packages to be installed, which is
   /// the point. This is used during version resolution to determine which
   /// package versions are available to be installed (or already installed).
   ///
   /// By default, this assumes that each description has a single version and
   /// uses [describe] to get that version.
   Future<List<Version>> getVersions(String name, description) {
-    return describe(new PackageId(name, this, Version.none, description))
-      .then((pubspec) => [pubspec.version]);
+    var id = new PackageId(name, this.name, Version.none, description);
+    return describeUncached(id).then((pubspec) => [pubspec.version]);
   }
 
   /// Loads the (possibly remote) pubspec for the package version identified by
   /// [id]. This may be called for packages that have not yet been installed
   /// during the version resolution process.
   ///
+  /// If the package has been installed to the system cache, the cached pubspec
+  /// will be used. Otherwise, it delegates to host-specific lookup behavior.
+  ///
   /// For cached sources, by default this uses [installToSystemCache] to get the
   /// pubspec. There is no default implementation for non-cached sources; they
   /// must implement it manually.
   Future<Pubspec> describe(PackageId id) {

[nweiz, 2013/05/17 23:55:55]

I don't understand why the describe/describeUncached change is part of this CL
either.

[Bob Nystrom, 2013/05/20 20:06:00]

This moves the cache checking into Source from SystemCache. We want to make sure
we use cached pubspecs whenever possible when describing packages. In the
solver, there was at least one place where the SystemCache wasn't handy, so I
thought it made the most sense to just move that logic directly into Source. I
think I also found a place that was inadvertently not checking the cache first
because it just called describe() directly on the source. This makes that less
likely.

[nweiz, 2013/05/20 23:54:05]

It would be nice if this sort of change were separated out into a different CL
in the future.

+    if (id.isRoot) throw new ArgumentError("Cannot describe the root package.");
+    if (id.source != name) throw new ArgumentError(

[nweiz, 2013/05/17 23:55:55]

Use {} if the value is smaller than a single line.

[Bob Nystrom, 2013/05/20 20:06:00]

Done.

+        "Package $id does not use this source.");

[nweiz, 2013/05/17 23:55:55]

Include the name of this source.

[Bob Nystrom, 2013/05/20 20:06:00]

Done.

+
+    // Try to get it from the system cache first.
+    if (shouldCache) {
+      return systemCacheDirectory(id).then((packageDir) {
+        if (!fileExists(path.join(packageDir, "pubspec.yaml"))) {
+          return describeUncached(id);
+        }
+
+        return new Pubspec.load(id.name, packageDir, _systemCache.sources);
+      });
+    }
+
+    // Not cached, so get it from the source.
+    return describeUncached(id);
+  }
+
+  /// Loads the pubspec for the package version identified by [id] which is not
+  /// already in the system cache. For cached sources, by default this uses

[nweiz, 2013/05/17 23:55:55]

I learned recently that docstrings are supposed to begin with a single-sentence
paragraph.

[Bob Nystrom, 2013/05/20 20:06:00]

Done.

+  /// [installToSystemCache] to get the pubspec. There is no default
+  /// implementation for non-cached sources; they must implement it manually.

[nweiz, 2013/05/17 23:55:55]

Mention that this is intended to be a protected method, and that people should
call [describe] instead.

[Bob Nystrom, 2013/05/20 20:06:00]

Done.

+  Future<Pubspec> describeUncached(PackageId id) {
     if (!shouldCache) {
-      throw new UnimplementedError("Source $name must implement describe(id).");
+      throw new UnimplementedError(
+          "Source $name must implement describeUncached(id).");
     }
     return installToSystemCache(id).then((package) => package.pubspec);
   }
 
   /// Installs the package identified by [id] to [path]. Returns a [Future] that
   /// completes when the installation was finished. The [Future] should resolve
   /// to true if the package was found in the source and false if it wasn't. For
   /// all other error conditions, it should complete with an exception.
   ///
   /// [path] is guaranteed not to exist, and its parent directory is guaranteed
@@ skipping 118 matching lines @@
   /// Returns the [Package]s that have been installed in the system cache.
   List<Package> getCachedPackages() {
     if (shouldCache) {
       throw new UnimplementedError("Source $name must implement this.");
     }
   }
 
   /// Returns the source's name.
   String toString() => name;
 }