Get codebase ready for actually supporting versioning.

utils/pub/package.dart

Index: utils/pub/package.dart
diff --git a/utils/pub/package.dart b/utils/pub/package.dart
index af585ab058af4e73138c987a203d4e6023b1e88a..8b3f047391585c88e7461ef04b6a1054ac6f7ca8 100644
--- a/utils/pub/package.dart
+++ b/utils/pub/package.dart
@@ -5,15 +5,15 @@
 /**
  * A named, versioned, unit of code and resource reuse.
  */
-class Package implements Hashable {
+class Package {
   /**
    * Loads the package whose root directory is [packageDir].
    */
-  static Future<Package> load(String packageDir, SourceRegistry sources) {
-    final pubspecPath = join(packageDir, 'pubspec');
+  static Future<Package> load(String packageDir, SystemCache cache) {
+    var pubspecPath = join(packageDir, 'pubspec');
 
-    return _parsePubspec(pubspecPath, sources).transform((dependencies) {
-      return new Package._(packageDir, dependencies);
+    return Pubspec.parse(pubspecPath, cache.sources).transform((pubspec) {
+      return new Package._(packageDir, cache, pubspec);
     });
   }
 
@@ -28,179 +28,156 @@ class Package implements Hashable {
   final String name;
 
   /**
+   * The package's version.
+   */
+  Version get version() => pubspec.version;
+
+  /**
+   * The parsed pubspec associated with this package.
+   */
+  final Pubspec pubspec;
+
+  /**
+   * The "packages" directory that this package installs its dependencies into.
+   */
+  final PackagesDir packagesDir;
+
+  /**
    * The ids of the packages that this package depends on. This is what is
    * specified in the pubspec when this package depends on another.
    */
-  final Collection<PackageId> dependencies;
+  Collection<PackageRef> get dependencies() => pubspec.dependencies;
 
   /**
    * Constructs a package. This should not be called directly. Instead, acquire
    * packages from [load()].
    */
-  Package._(String dir, this.dependencies)
+  Package._(String dir, SystemCache cache, this.pubspec)

[nweiz, 2012/05/18 00:08:24]

I'm not sure I like Package having a reference to the SystemCache, and
especially not to the PackagesDir. I think we need to figure out exactly what
the Package class represents.

I've been thinking of Package as a thin data class similar to PackageId and now
PackageRef. My conceptual model is that you start with a PackageRef, resolve it
to get a PackageId, and install+load that to get a Package. The actual
resolution, installation, and loading would be handled externally to the Package
class (although the loading may be part of a factory constructor).

I suggest using PackagesDir#installTransitively directly rather than
Package#installDependencies, and moving Package#resolve to PackagesDir. Once we
have a lockfile, that seems like it should be managed by PackagesDir as well.

It's possible PackagesDir should be renamed to EntrypointCache or something like
that.

[Bob Nystrom, 2012/05/18 20:02:38]

Done.

   : dir = dir,
-    name = basename(dir);
+    name = basename(dir),
+    packagesDir = new PackagesDir(join(dir, 'packages'), cache);
 
   /**
-   * Generates a hashcode for the package.
+   * Installs all dependencies of this package to its "packages" directory.
+   * Returns a [Future] that completes when all dependencies are installed.
    */
-  // TODO(rnystrom): Do something more sophisticated here once we care about
-  // versioning and different package sources.
-  int hashCode() => name.hashCode();
+  Future installDependencies() {
+    return packagesDir.installTransitively(this, this.dependencies);
+  }
 
   /**
-   * Returns a debug string for the package.
+   * Given [ref], which ambiguously identifies a dependent package, selects an
+   * appropriate precise package to use when this package is the entrypoint.
+   * In other words, given a loose refence like "foo >= 2.0", figures out what
+   * concrete package *this* app wants to use.
    */
-  String toString() => '$name ($dir)';
+  Future<PackageId> resolve(PackageRef ref) {
+    // TODO(rnystrom): This should use the lockfile to select the right version
+    // once that's implemented. If the lockfile doesn't exist, it should
+    // generate it. In the meantime, here's a dumb implementation:
+    return new Future.immediate(
+        new PackageId(ref.source, Version.none, ref.description));
+  }
 
   /**
-   * Parses the pubspec at the given path and returns the list of package
-   * dependencies it exposes.
+   * Returns a debug string for the package.
    */
-  static Future<List<PackageId>> _parsePubspec(String path,
-      SourceRegistry sources) {
-    final completer = new Completer<List<PackageId>>();
-
-    // TODO(rnystrom): Handle the directory not existing.
-    // TODO(rnystrom): Error-handling.
-    final readFuture = readTextFile(path);
-    readFuture.handleException((error) {
-      // If there is no pubspec, we implicitly treat that as a package with no
-      // dependencies.
-      // TODO(rnystrom): Distinguish file not found from other real errors.
-      completer.complete(<PackageId>[]);
-      return true;
-    });
-
-    readFuture.then((pubspec) {
-      if (pubspec.trim() == '') {
-        completer.complete(<String>[]);
-        return;
-      }
-
-      var parsedPubspec = loadYaml(pubspec);
-      if (parsedPubspec is! Map) {
-        completer.completeException('The pubspec must be a YAML mapping.');
-      }
-
-      if (!parsedPubspec.containsKey('dependencies')) {
-        completer.complete(<String>[]);
-        return;
-      }
-
-      var dependencies = parsedPubspec['dependencies'];
-      if (dependencies is! Map ||
-          dependencies.getKeys().some((e) => e is! String)) {
-        completer.completeException(
-            'The pubspec dependencies must be a map of package names.');
-      }
-
-      var dependencyIds = <PackageId>[];
-      dependencies.forEach((name, spec) {
-        var fullName, source;
-        // TODO(nweiz): parse the version once we have version handling
-        if (spec == null || spec is String) {
-          fullName = name;
-          source = sources.defaultSource;
-        } else if (spec is Map) {
-          spec.remove('version');
-
-          var sourceNames = spec.getKeys();
-          if (sourceNames.length > 1) {
-            completer.completeException(
-                'Dependency $name may not have multiple sources: '
-                '$sourceNames.');
-            return;
-          }
-
-          var sourceName = only(sourceNames);
-          if (sourceName is! String) {
-            completer.completeException(
-                'Source name $sourceName must be a string.');
-            return;
-          }
-          source = sources[sourceName];
-
-          // TODO(nweiz): At some point we want fullName to be able to be an
-          // arbitrary object that's parsed by the source.
-          fullName = spec[sourceName];
-          if (fullName is! String) {
-            completer.completeException(
-                'Source identifier $fullName must be a string.');
-            return;
-          }
-        } else {
-          completer.completeException(
-              'Dependency specification $spec must be a string or a mapping.');
-          return;
-        }
-
-        var id = new PackageId(fullName, source);
-        var nameFromSource = source.packageName(id);
-        if (nameFromSource != name) {
-          completer.completeException(
-              'Dependency name "$name" doesn\'t match name "$nameFromSource" '
-              'from source "${source.name}".');
-          return;
-        }
-
-        dependencyIds.add(id);
-      });
-      completer.complete(dependencyIds);
-    });
-
-    return completer.future;
-  }
+  String toString() => '$name ($dir)';
 }
 
 /**
- * A unique identifier for a package. A given package id specifies a single
- * chunk of code and resources.
+ * An unambiguous resolved reference to a package. A package ID contains enough
+ * information to correctly install the package.
  *
- * Note that it's possible for multiple package ids to point to identical
- * packages. For example, the same package may be available from multiple
- * sources. As far as Pub is concerned, those packages are different.
+ * Note that it's possible for multiple distinct package IDs to point to
+ * different directories that happen to contain identical packages. For example,
+ * the same package may be available from multiple sources. As far as Pub is
+ * concerned, those packages are different.
  */
-// TODO(nweiz, rnystrom): this should include version eventually
-class PackageId implements Hashable, Comparable {
+class PackageId implements Comparable, Hashable {
   /**
-   * The name used by the [source] to look up the package.
-   *
-   * Note that this may be distinct from [name], which is the name of the
-   * package itself. The [source] uses this name to locate the package and
-   * returns the true package name. For example, for a Git source [fullName]
-   * might be the URL "git://github.com/dart/uilib.git", while [name] would just
-   * be "uilib". It would be up to the source to take the URL and extract the
-   * package name.
+   * The [Source] used to look up this package given its [description].
    */
-  final String fullName;
+  final Source source;
 
   /**
-   * The [Source] used to look up the package given the [fullName].
+   * The package's version.
    */
-  final Source source;
+  final Version version;
+
+  /**
+   * The metadata used by the package's [source] to identify and locate it. It
+   * contains whatever [Source]-specific data it needs to be able to install
+   * the package. For example, the description of a git sourced package might
+   * by the URL "git://github.com/dart/uilib.git".
+   */
+  final description;
 
-  PackageId(String this.fullName, Source this.source);
+  PackageId(this.source, this.version, this.description);
 
   /**
-   * The name of the package being imported. Not necessarily the same as
-   * [fullName].
+   * The name of the package being identified. This will be the human-friendly
+   * name like "uilib".
    */
   String get name() => source.packageName(this);
 
-  int hashCode() => fullName.hashCode() ^ source.name.hashCode();
+  int hashCode() => name.hashCode() ^
+                    source.name.hashCode() ^
+                    version.hashCode();
 
   bool operator ==(other) {
     if (other is! PackageId) return false;
-    return other.fullName == fullName && other.source.name == source.name;
+    // TODO(rnystrom): We're assuming here the name/version/source tuple is
+    // 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.version == version;
   }
 
-  String toString() => "$fullName from ${source.name}";
+  String toString() => "$name $version from ${source.name}";
 
   int compareTo(Comparable other) {
     if (other is! PackageId) throw new IllegalArgumentException(other);
-    var sourceComp = this.source.name.compareTo(other.source.name);
+
+    var sourceComp = source.name.compareTo(other.source.name);
     if (sourceComp != 0) return sourceComp;
-    return this.fullName.compareTo(other.fullName);
+
+    var nameComp = name.compareTo(other.name);
+    if (nameComp != 0) return nameComp;
+
+    return version.compareTo(other.version);
   }
 }
+
+/**
+ * A reference to a package. Unlike a [PackageId], a PackageRef may not
+ * unambiguously refer to a single package. It may describe a range of allowed
+ * packages.
+ */
+class PackageRef {

[nweiz, 2012/05/18 00:08:24]

What do you think about PackageRange or PackageConstraint as a name for this? I
feel like "Ref" isn't very descriptive.

[Bob Nystrom, 2012/05/18 20:02:38]

I <3 Ref as per our discussion.

+  /**
+   * The name of the package being referenced.
+   */
+  final String name;
+
+  /**
+   * The [Source] used to look up the package.
+   */
+  final Source source;
+
+  /**
+   * The allowed package versions.
+   */
+  final VersionConstraint version;
+
+  /**
+   * The metadata used to identify the package being referenced. The
+   * interpretation of this will vary based on the [source].
+   */
+  final description;
+
+  PackageRef(this.name, this.source, this.version, this.description);
+
+  String toString() => "$name $version from $source ($description)";
+}