Make a bunch of stuff in pub synchronous.

utils/pub/entrypoint.dart

Index: utils/pub/entrypoint.dart
diff --git a/utils/pub/entrypoint.dart b/utils/pub/entrypoint.dart
index d562997f9aae2890a4404fb63b601072c6e79d05..2aca2575464cff4e5bbbf31610166f8907ec5af6 100644
--- a/utils/pub/entrypoint.dart
+++ b/utils/pub/entrypoint.dart
@@ -39,16 +39,12 @@ class Entrypoint {
 
   /// Packages which are either currently being asynchronously installed to the
   /// directory, or have already been installed.
-  final Map<PackageId, Future<PackageId>> _installs;
-
-  Entrypoint(this.root, this.cache)
-  : _installs = new Map<PackageId, Future<PackageId>>();
+  final _installs = new Map<PackageId, Future<PackageId>>();

[nweiz, 2013/02/01 02:05:55]

Can't we make this a Map literal, or are there still weird restrictions on
those?

[Bob Nystrom, 2013/02/01 23:17:21]

Map literals can only have string keys. Seems a bit silly to have that
restriction even for empty maps, but <shrug>.

 
   /// Loads the entrypoint from a package at [rootDir].
-  static Future<Entrypoint> load(String rootDir, SystemCache cache) {
-    return Package.load(null, rootDir, cache.sources).then((package) =>
-        new Entrypoint(package, cache));
-  }
+  Entrypoint(String rootDir, SystemCache cache)
+      : root = new Package(null, rootDir, cache.sources),
+        cache = cache;
 
   // TODO(rnystrom): Make this path configurable.
   /// The path to this "packages" directory.
@@ -70,10 +66,10 @@ class Entrypoint {
     if (pendingOrCompleted != null) return pendingOrCompleted;
 
     var packageDir = join(path, id.name);
-    var future = ensureDir(dirname(packageDir)).then((_) {
-      return exists(packageDir);
-    }).then((exists) {
-      if (!exists) return;
+    var future = defer(() {
+      ensureDir(dirname(packageDir));
+      if (!dirExists(packageDir)) return;
+
       // TODO(nweiz): figure out when to actually delete the directory, and when
       // we can just re-use the existing symlink.
       log.fine("Deleting package directory for ${id.name} before install.");
@@ -100,9 +96,10 @@ class Entrypoint {
   /// directory, respecting the [LockFile] if present. Returns a [Future] that
   /// completes when all dependencies are installed.
   Future installDependencies() {
-    return loadLockFile()
-      .then((lockFile) => resolveVersions(cache.sources, root, lockFile))
-      .then(_installDependencies);
+    // Make sure all errors propagate through future.

[nweiz, 2013/02/01 02:05:55]

I don't want to have to leave this comment on every use of defer(). We should
just mention this use case in the documentation of defer() itself.

[Bob Nystrom, 2013/02/01 23:17:21]

Done.

+    return defer(() {
+      return resolveVersions(cache.sources, root, loadLockFile());
+    }).then(_installDependencies);
   }
 
   /// Installs the latest available versions of all dependencies of the [root]
@@ -110,19 +107,20 @@ class Entrypoint {
   /// [Future] that completes when all dependencies are installed.
   Future updateAllDependencies() {
     return resolveVersions(cache.sources, root, new LockFile.empty())
-      .then(_installDependencies);
+        .then(_installDependencies);
   }
 
   /// Installs the latest available versions of [dependencies], while leaving
   /// other dependencies as specified by the [LockFile] if possible. Returns a
   /// [Future] that completes when all dependencies are installed.
   Future updateDependencies(List<String> dependencies) {
-    return loadLockFile().then((lockFile) {
-      var versionSolver = new VersionSolver(cache.sources, root, lockFile);
+    // Make sure all errors propagate through future.
+    return defer(() {
+      var solver = new VersionSolver(cache.sources, root, loadLockFile());
       for (var dependency in dependencies) {
-        versionSolver.useLatestVersion(dependency);
+        solver.useLatestVersion(dependency);
       }
-      return versionSolver.solve();
+      return solver.solve();
     }).then(_installDependencies);
   }
 
@@ -141,43 +139,34 @@ class Entrypoint {
 
   /// Loads the list of concrete package versions from the `pubspec.lock`, if it
   /// exists. If it doesn't, this completes to an empty [LockFile].
-  Future<LockFile> loadLockFile() {
+  LockFile loadLockFile() {
     var lockFilePath = join(root.dir, 'pubspec.lock');
-
-    log.fine("Loading lockfile.");
-    return fileExists(lockFilePath).then((exists) {
-      if (!exists) {
-        log.fine("No lock file at $lockFilePath, creating empty one.");
-        return new LockFile.empty();
-      }
-
-      return readTextFile(lockFilePath).then((text) =>
-          new LockFile.parse(text, cache.sources));
-    });
+    if (!fileExists(lockFilePath)) return new LockFile.empty();
+    return new LockFile.parse(readTextFile(lockFilePath), cache.sources);
   }
 
   /// Saves a list of concrete package versions to the `pubspec.lock` file.
-  Future _saveLockFile(List<PackageId> packageIds) {
+  void _saveLockFile(List<PackageId> packageIds) {
     var lockFile = new LockFile.empty();
     for (var id in packageIds) {
       if (!id.isRoot) lockFile.packages[id.name] = id;
     }
 
     var lockFilePath = join(root.dir, 'pubspec.lock');
-    log.fine("Saving lockfile.");
-    return writeTextFile(lockFilePath, lockFile.serialize());
+    writeTextFile(lockFilePath, lockFile.serialize());
   }
 
   /// Installs a self-referential symlink in the `packages` directory that will
   /// allow a package to import its own files using `package:`.
   Future _installSelfReference(_) {
-    var linkPath = join(path, root.name);
-    return exists(linkPath).then((exists) {
+    // Make sure all errors propagate through future.
+    return defer(() {
+      var linkPath = join(path, root.name);
       // Create the symlink if it doesn't exist.
-      if (exists) return;
-      return ensureDir(path).then(
-          (_) => createPackageSymlink(root.name, root.dir, linkPath,
-              isSelfLink: true));
+      if (entryExists(linkPath)) return;
+      ensureDir(path);
+      return createPackageSymlink(root.name, root.dir, linkPath,
+          isSelfLink: true);
     });
   }
 
@@ -190,8 +179,9 @@ class Entrypoint {
     var testDir = join(root.dir, 'test');
     var toolDir = join(root.dir, 'tool');
     var webDir = join(root.dir, 'web');
-    return dirExists(binDir).then((exists) {
-      if (!exists) return;
+    // Make sure all errors propagate through future.
+    return defer(() {
+      if (!dirExists(binDir)) return;
       return _linkSecondaryPackageDir(binDir);
     }).then((_) => _linkSecondaryPackageDirsRecursively(exampleDir))
       .then((_) => _linkSecondaryPackageDirsRecursively(testDir))
@@ -202,14 +192,16 @@ class Entrypoint {
   /// Creates a symlink to the `packages` directory in [dir] and all its
   /// subdirectories.
   Future _linkSecondaryPackageDirsRecursively(String dir) {
-    return dirExists(dir).then((exists) {
-      if (!exists) return;
+    // Make sure all errors propagate through future.
+    return defer(() {
+      if (!dirExists(dir)) return;
       return _linkSecondaryPackageDir(dir)
-        .then((_) => _listDirWithoutPackages(dir))
-        .then((files) {
+          .then((_) => _listDirWithoutPackages(dir))
+          .then((files) {
         return Future.wait(files.map((file) {
-          return dirExists(file).then((isDir) {
-            if (!isDir) return;
+          // Make sure all errors propagate through future.
+          return defer(() {
+            if (!dirExists(file)) return;
             return _linkSecondaryPackageDir(file);
           });
         }));
@@ -223,9 +215,9 @@ class Entrypoint {
   Future<List<String>> _listDirWithoutPackages(dir) {
     return listDir(dir).then((files) {
       return Future.wait(files.map((file) {
-        if (basename(file) == 'packages') return new Future.immediate([]);
-        return dirExists(file).then((isDir) {
-          if (!isDir) return [];
+        // Make sure all errors propagate through future.
+        return defer(() {
+          if (basename(file) == 'packages' || !dirExists(file)) return [];

[nweiz, 2013/02/01 02:05:55]

Moving the "packages" check in here changes the behavior so that "packages"
directories (although not their subfiles) are returned.

[Bob Nystrom, 2013/02/01 23:17:21]

Fixed.

           return _listDirWithoutPackages(file);
         }).then((subfiles) {
           var fileAndSubfiles = [file];
@@ -238,9 +230,10 @@ class Entrypoint {
 
   /// Creates a symlink to the `packages` directory in [dir] if none exists.
   Future _linkSecondaryPackageDir(String dir) {
-    var to = join(dir, 'packages');
-    return exists(to).then((exists) {
-      if (exists) return;
+    // Make sure all errors propagate through future.
+    return defer(() {
+      var to = join(dir, 'packages');
+      if (entryExists(to)) return;
       return createSymlink(path, to);
     });
   }