Support activating packages from local paths.

sdk/lib/_internal/pub/lib/src/global_packages.dart

Index: sdk/lib/_internal/pub/lib/src/global_packages.dart
diff --git a/sdk/lib/_internal/pub/lib/src/global_packages.dart b/sdk/lib/_internal/pub/lib/src/global_packages.dart
index b9bb54881fc29e904daf9b3afdc69add61c7c97e..3f9f8db05397e85243c7f270aaa19b18580508b8 100644
--- a/sdk/lib/_internal/pub/lib/src/global_packages.dart
+++ b/sdk/lib/_internal/pub/lib/src/global_packages.dart
@@ -16,7 +16,9 @@ import 'log.dart' as log;
 import 'package.dart';
 import 'system_cache.dart';
 import 'solver/version_solver.dart';
+import 'source.dart';
 import 'source/cached.dart';
+import 'source/path.dart';
 import 'utils.dart';
 import 'version.dart';
 
@@ -29,6 +31,21 @@ import 'version.dart';
 /// Only one version of a given package name can be globally activated at a
 /// time. Activating a different version of a package will deactivate the
 /// previous one.
+///
+/// This handles packages from uncached and cached sources a little differently.
+/// For a cached source, the package is physically in the user's pub cache and
+/// we don't want to mess with it by putting a lockfile in there. Instead, when
+/// we activate the package, we create a full lockfile and put it in the
+/// "global_packages" directory. It's named "<package>.lock". Unlike a normal
+/// lockfile, it also contains an entry for the root package itself, so that we
+/// know the version and description that was activated.
+///
+/// Uncached packages (i.e. "path" packages) are somewhere else on the user's
+/// local file system and can have a lockfile directly in place. (And, in fact,
+/// we want to ensure we honor the user's lockfile there.) To activate it, we
+/// just need to know where that package directory is. For that, we create a
+/// lockfile that *only* contains the root package's [PackageId] -- basically
+/// just the path to the directory where the real lockfile lives.
 class GlobalPackages {
   /// The [SystemCache] containing the global packages.
   final SystemCache cache;
@@ -36,10 +53,6 @@ class GlobalPackages {
   /// The directory where the lockfiles for activated packages are stored.
   String get _directory => p.join(cache.rootDir, "global_packages");
 
-  /// The source that global packages can be activated from.
-  // TODO(rnystrom): Allow activating packages from other sources.
-  CachedSource get _source => cache.sources["hosted"] as CachedSource;
-
   /// Creates a new global package registry backed by the given directory on
   /// the user's file system.
   ///
@@ -47,111 +60,213 @@ class GlobalPackages {
   /// when needed.
   GlobalPackages(this.cache);
 
+  /// Activates the Git repo described by [ref] for package [name].
+  Future activateGit(String name, String ref) {
+    // See if we already have it activated.
+    var lockFile = _describeActive(name);
+    var id;
+    if (lockFile != null) {
+      id = lockFile.packages[name];

[nweiz, 2014/07/31 20:01:53]

I don't understand this logic. If there's already a lockfile for this package,
shouldn't we override it with the (potentially) new ref? If we're not overriding
it, why are we running _installInCache?

[Bob Nystrom, 2014/08/04 23:41:51]

Killing git support for now.

+    } else {
+      id = new PackageId(name, "git", Version.none, ref);
+    }
+
+    return _installInCache(id, lockFile);
+  }
+
   /// Finds the latest version of the hosted package with [name] that matches
   /// [constraint] and makes it the active global version.
-  Future activate(String name, VersionConstraint constraint) {
+  Future activateHosted(String name, VersionConstraint constraint) {
     // See if we already have it activated.
-    var lockFile;
+    var lockFile = _describeActive(name);
     var currentVersion;
-    try {
-      lockFile = new LockFile.load(_getLockFilePath(name), cache.sources);
-      currentVersion = lockFile.packages[name].version;
+    if (lockFile != null) {
+      var id = lockFile.packages[name];
+
+      // Try to preserve the current version if we've already activated the
+      // hosted package.
+      if (id.source == "hosted") currentVersion = id.version;
 
       // Pull the root package out of the lock file so the solver doesn't see
       // it.
       lockFile.packages.remove(name);
-
-      log.message("Package ${log.bold(name)} is already active at "
-          "version ${log.bold(currentVersion)}.");
-    } on IOException catch (error) {
-      // If we couldn't read the lock file, it's not activated.
+    } else {
       lockFile = new LockFile.empty();
     }
 
-    var package;
-    var id;
     return _selectVersion(name, currentVersion, constraint).then((version) {
       // Make sure it's in the cache.
-      id = new PackageId(name, _source.name, version, name);
-      return _source.downloadToSystemCache(id);
-    }).then((p) {
-      package = p;
-      // Resolve it and download its dependencies.
-      return resolveVersions(SolveType.GET, cache.sources, package,
-          lockFile: lockFile);
+      var id = new PackageId(name, "hosted", version, name);
+      return _installInCache(id, lockFile);
+    });
+  }
+
+  /// Makes the local package at [path] globally active.
+  Future activatePath(String path) {
+    return syncFuture(() {
+      var entrypoint = new Entrypoint(path, cache);
+
+      var name = entrypoint.root.name;
+      _describeActive(name);

[nweiz, 2014/07/31 20:01:53]

Add a comment about why this is is being called despite its return value going
unused.

[Bob Nystrom, 2014/08/04 23:41:50]

Done.

+
+      // Write a lockfile that points to the local package.
+      var fullPath = canonicalize(entrypoint.root.dir);
+      var id = new PackageId(name, "path", entrypoint.root.version,
+          PathSource.describePath(fullPath));
+      _finishActivation(id, new LockFile.empty());
+    });
+  }
+
+  /// Installs the package [id] with [lockFile] into the system cache.

[nweiz, 2014/07/31 20:01:54]

Clarify that this also installs [id]'s dependencies.

[Bob Nystrom, 2014/08/04 23:41:50]

Done.

+  Future _installInCache(PackageId id, LockFile lockFile) {
+    var source = cache.sources[id.source];
+
+    // Put the main package in the cache.
+    return source.downloadToSystemCache(id).then((package) {
+      // If we didn't know the version for the ID (which is true for Git
+      // packages), look it up now that we have it.
+      if (id.version == Version.none) {
+        id = id.atVersion(package.version);
+      }
+
+      return source.resolveId(id).then((id_) {
+        id = id_;
+
+        // Resolve it and download its dependencies.
+        return resolveVersions(SolveType.GET, cache.sources, package,
+            lockFile: lockFile);
+      });
     }).then((result) {
       if (!result.succeeded) throw result.error;
       result.showReport(SolveType.GET);
 
       // Make sure all of the dependencies are locally installed.
-      return Future.wait(result.packages.map((id) {
-        var source = cache.sources[id.source];
-        if (source is! CachedSource) return new Future.value();
-        return source.downloadToSystemCache(id)
-            .then((_) => source.resolveId(id));
-      }));
+      return Future.wait(result.packages.map(_cacheDependency));
     }).then((ids) {
-      var lockFile = new LockFile(ids);
+      _finishActivation(id, new LockFile(ids));
+    });
+  }
 
-      // Add the root package itself to the lockfile.
-      lockFile.packages[name] = id;
+  /// Downloads [id] into the system cache if it's a cached package.
+  ///
+  /// Returns the resolved [PackageId] for [id].
+  Future<PackageId> _cacheDependency(PackageId id) {
+    var source = cache.sources[id.source];
 
-      ensureDir(_directory);
-      writeTextFile(_getLockFilePath(name),
-          lockFile.serialize(cache.rootDir, cache.sources));
+    return syncFuture(() {
+      if (id.isRoot) return null;
+      if (source is! CachedSource) return null;
 
-      log.message("Activated ${log.bold(package.name)} ${package.version}.");
-      // TODO(rnystrom): Look in "bin" and display list of binaries that
-      // user can run.
-    });
+      return source.downloadToSystemCache(id);
+    }).then((_) => source.resolveId(id));
   }
 
-  /// Deactivates a previously-activated package named [name] or fails with
-  /// an error if [name] is not an active package.
-  void deactivate(String name) {
-    // See if we already have it activated.
+  /// Finishes activating package [id] by saving [lockFile] in the cache.
+  void _finishActivation(PackageId id, LockFile lockFile) {

[nweiz, 2014/07/31 20:01:53]

It would be nice to have a more descriptive method name here. Maybe
"_writeLockFile"?

[Bob Nystrom, 2014/08/04 23:41:50]

Done.

+    deactivate(id.name);
+
+    // Add the root package to the lockfile.
+    lockFile.packages[id.name] = id;
+
+    ensureDir(_directory);
+    writeTextFile(_getLockFilePath(id.name),
+        lockFile.serialize(cache.rootDir, cache.sources));
+
+    log.message("Activated ${log.bold(id.name)} ${id.version}.");
+    // TODO(rnystrom): Look in "bin" and display list of binaries that
+    // user can run.
+  }
+
+  /// Gets the lock file for the currently activate package with [name].

[nweiz, 2014/07/31 20:01:53]

"activate" -> "active"

[Bob Nystrom, 2014/08/04 23:41:51]

Done.

+  ///
+  /// Displays a message to the user about the current package, if any. Returns
+  /// the [LockFile] for the active package or `null` otherwise.
+  LockFile _describeActive(String package) {
     try {
-      var lockFilePath = p.join(_directory, "$name.lock");
-      var lockFile = new LockFile.load(lockFilePath, cache.sources);
-      var version = lockFile.packages[name].version;
+      var lockFile = new LockFile.load(_getLockFilePath(package),
+          cache.sources);
+      var id = lockFile.packages[package];
 
-      deleteEntry(lockFilePath);
-      log.message("Deactivated package ${log.bold(name)} $version.");
+      if (id.source == "path") {
+        var path = PathSource.pathFromDescription(id.description);
+        log.message('Package ${log.bold(package)} is already active at '
+            'path "$path".');
+      } else {

[nweiz, 2014/07/31 20:01:54]

Shouldn't a git source display its URL and ref?

[Bob Nystrom, 2014/08/04 23:41:50]

Killing git support for now.

+        log.message("Package ${log.bold(package)} is already active at "
+            "version ${log.bold(id.version)}.");

[nweiz, 2014/07/31 20:01:53]

"is already active" sounds to me like the sort of thing you'd see in an error
message describing why activation failed. Maybe "was active" instead to make it
clear that it's being replaced with the new source/version?

[Bob Nystrom, 2014/08/04 23:41:50]

Putting it in the past tense may be confusing if activating the new package
fails. Changed to "is currently active". Is that better?

[nweiz, 2014/08/05 19:14:42]

SGTM

+      }
+
+      return lockFile;
     } on IOException catch (error) {
-      dataError("No active package ${log.bold(name)}.");
+      // If we couldn't read the lock file, it's not activated.

[nweiz, 2014/07/31 20:01:53]

Return null here or the analyzer will complain.

[Bob Nystrom, 2014/08/04 23:41:51]

Done.

     }
   }
 
-  /// Finds the active packge with [name].
+  /// Deactivates a previously-activated package named [name].
+  ///
+  /// If [logDeletion] is true, displays to the user when a package is
+  /// deactivated. Otherwise, deactivates silently.
+  ///
+  /// Returns `false` if no package with [name] was currently active.
+  bool deactivate(String name, {bool logDeletion: false}) {
+    var lockFilePath = _getLockFilePath(name);
+    if (!fileExists(lockFilePath)) return false;
+
+    var lockFile = new LockFile.load(lockFilePath, cache.sources);
+    var id = lockFile.packages[name];
+
+    deleteEntry(lockFilePath);
+
+    if (logDeletion) {
+      if (id.source == "path") {
+        var path = PathSource.pathFromDescription(id.description);
+        log.message('Deactivated package ${log.bold(name)} at path "$path".');
+      } else {
+        log.message("Deactivated package ${log.bold(name)} ${id.version}.");
+      }
+    }
+
+    return true;
+  }
+
+  /// Finds the active package with [name].
   ///
   /// Returns an [Entrypoint] loaded with the active package if found.
   Future<Entrypoint> find(String name) {
-    var lockFile;
-    var version;
     return syncFuture(() {
+      var lockFile;
       try {
         lockFile = new LockFile.load(_getLockFilePath(name), cache.sources);
-        version = lockFile.packages[name].version;
       } on IOException catch (error) {
         // If we couldn't read the lock file, it's not activated.
         dataError("No active package ${log.bold(name)}.");
       }
-    }).then((_) {
+
       // Load the package from the cache.
-      var id = new PackageId(name, _source.name, version, name);
-      return _source.getDirectory(id);
-    }).then((dir) {
-      return new Package.load(name, dir, cache.sources);
-    }).then((package) {
-      // Pull the root package out of the lock file so the solver doesn't see
-      // it.
+      var id = lockFile.packages[name];
       lockFile.packages.remove(name);
 
-      return new Entrypoint.inMemory(package, lockFile, cache);
+      var source = cache.sources[id.source];
+      if (source is CachedSource) {
+        // For cached sources, the package itself is in the cache and the
+        // lockfile is the one we just loaded.
+        return cache.sources[id.source].getDirectory(id)
+            .then((dir) => new Package.load(name, dir, cache.sources))
+            .then((package) {
+          return new Entrypoint.inMemory(package, lockFile, cache);
+        });
+      }
+
+      // For uncached sources (i.e. path), the ID just points to the real
+      // directory for the package.
+      assert(id.source == "path");
+      return new Entrypoint(PathSource.pathFromDescription(id.description),
+          cache);
     });
   }
 
-  /// Picks the best version of [package] to activate that meets [constraint].
+  /// Picks the best hosted version of [package] to activate that meets
+  /// [constraint].
   ///
   /// If [version] is not `null`, this tries to maintain that version if
   /// possible.
@@ -163,7 +278,8 @@ class GlobalPackages {
     }
 
     // Otherwise, select the best version the matches the constraint.
-    return _source.getVersions(package, package).then((versions) {
+    var source = cache.sources["hosted"];
+    return source.getVersions(package, package).then((versions) {
       versions = versions.where(constraint.allows).toList();
 
       if (versions.isEmpty) {
@@ -178,6 +294,7 @@ class GlobalPackages {
     });
   }
 
-  /// Gets the path to the lock file for an activated package with [name].
+  /// Gets the path to the lock file for an activated cached package with
+  /// [name].
   String _getLockFilePath(name) => p.join(_directory, name + ".lock");
 }