From 8d1d80f541b12451a75ee288a7df393d96b99e59 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Thu, 1 Dec 2016 15:06:05 -0800
Subject: [PATCH] Avoid barback for global activate if possible.

Similarly 98ecbdedeba96cb63d1efa74101f9b312dea1f10, this provides better
error messages and faster installation times.

See #1473
See #1477
---
 lib/src/global_packages.dart                  | 84 ++++++++++++++-----
 .../repair/recompiles_snapshots_test.dart     |  1 -
 .../activate_git_after_hosted_test.dart       |  1 -
 .../activate_hosted_after_git_test.dart       |  1 -
 .../activate_hosted_after_path_test.dart      |  1 -
 test/global/activate/cached_package_test.dart |  1 -
 .../activate/different_version_test.dart      |  1 -
 test/global/activate/git_package_test.dart    |  1 -
 .../activate/ignores_active_version_test.dart |  1 -
 .../reactivating_git_upgrades_test.dart       |  2 -
 .../activate/uncached_package_test.dart       |  1 -
 ...eactivate_and_reactivate_package_test.dart |  1 -
 12 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart
index 2265e53a..117f701d 100644
--- a/lib/src/global_packages.dart
+++ b/lib/src/global_packages.dart
@@ -10,6 +10,7 @@ import 'package:barback/barback.dart';
 import 'package:pub_semver/pub_semver.dart';
 
 import 'barback/asset_environment.dart';
+import 'dart.dart' as dart;
 import 'entrypoint.dart';
 import 'exceptions.dart';
 import 'executable.dart' as exe;
@@ -146,6 +147,7 @@ class GlobalPackages {
 
     _updateBinStubs(entrypoint.root, executables,
         overwriteBinStubs: overwriteBinStubs);
+    log.message('Activated ${_formatPackage(id)}.');
   }
 
   /// Installs the package [dep] and its dependencies into the system cache.
@@ -169,41 +171,88 @@ class GlobalPackages {
     // Make sure all of the dependencies are locally installed.
     await Future.wait(result.packages.map(_cacheDependency));
 
+    var lockFile = result.lockFile;
+    _writeLockFile(dep.name, lockFile);
+    writeTextFile(_getPackagesFilePath(dep.name), lockFile.packagesFile(cache));
+
     // Load the package graph from [result] so we don't need to re-parse all
     // the pubspecs.
     var entrypoint = new Entrypoint.fromSolveResult(root, cache, result,
         isGlobal: true);
     var snapshots = await _precompileExecutables(entrypoint, dep.name);
 
-    var lockFile = result.lockFile;
-    _writeLockFile(dep.name, lockFile);
-    writeTextFile(_getPackagesFilePath(dep.name), lockFile.packagesFile(cache));
-
     _updateBinStubs(entrypoint.packageGraph.packages[dep.name], executables,
         overwriteBinStubs: overwriteBinStubs, snapshots: snapshots);
+
+    var id = lockFile.packages[dep.name];
+    log.message('Activated ${_formatPackage(id)}.');
   }
 
-  /// Precompiles the executables for [package] and saves them in the global
+  /// Precompiles the executables for [packageName] and saves them in the global
   /// cache.
   ///
   /// Returns a map from executable name to path for the snapshots that were
   /// successfully precompiled.
   Future<Map<String, String>> _precompileExecutables(Entrypoint entrypoint,
-      String package) {
-    return log.progress("Precompiling executables", () async {
-      var binDir = p.join(_directory, package, 'bin');
+      String packageName) {
+    return log.progress("Precompiling executables", () {
+      var binDir = p.join(_directory, packageName, 'bin');
       cleanDir(binDir);
 
-      var environment = await AssetEnvironment.create(
-          entrypoint, BarbackMode.RELEASE,
-          entrypoints: entrypoint.packageGraph.packages[package].executableIds,
-          useDart2JS: false);
-      environment.barback.errors.listen((error) {
-        log.error(log.red("Build error:\n$error"));
-      });
+      // Try to avoid starting up an asset server to precompile packages if
+      // possible. This is faster and produces better error messages.
+      var package = entrypoint.packageGraph.packages[packageName];
+      if (entrypoint.packageGraph.transitiveDependencies(packageName)
+          .every((package) => package.pubspec.transformers.isEmpty)) {
+        return _precompileExecutablesWithoutBarback(package, binDir);
+      } else {
+        return _precompileExecutablesWithBarback(entrypoint, package, binDir);
+      }
+    });
+  }
 
-      return environment.precompileExecutables(package, binDir);
+  //// Precompiles all executables in [package] to snapshots from the
+  //// filesystem.
+  ////
+  //// The snapshots are placed in [dir].
+  ////
+  //// Returns a map from executable basenames without extensions to the paths
+  //// to those executables.
+  Future<Map<String, String>> _precompileExecutablesWithoutBarback(
+      Package package, String dir) async {
+    var precompiled = {};
+    await waitAndPrintErrors(package.executableIds.map((id) async {
+      var url = p.toUri(package.dir);
+      url = url.replace(path: p.url.join(url.path, id.path));
+      var basename = p.url.basename(id.path);
+      var snapshotPath = p.join(dir, '$basename.snapshot');
+      await dart.snapshot(
+          url, snapshotPath,
+          packagesFile: p.toUri(_getPackagesFilePath(package.name)),
+          id: id);
+      precompiled[p.withoutExtension(basename)] = snapshotPath;
+    }));
+    return precompiled;
+  }
+
+  //// Precompiles all executables in [package] to snapshots from a barback
+  //// asset environment.
+  ////
+  //// The snapshots are placed in [dir].
+  ////
+  //// Returns a map from executable basenames without extensions to the paths
+  //// to those executables.
+  Future<Map<String, String>> _precompileExecutablesWithBarback(
+      Entrypoint entrypoint, Package package, String dir) async {
+    var environment = await AssetEnvironment.create(
+        entrypoint, BarbackMode.RELEASE,
+        entrypoints: package.executableIds,
+        useDart2JS: false);
+    environment.barback.errors.listen((error) {
+      log.error(log.red("Build error:\n$error"));
     });
+
+    return environment.precompileExecutables(package.name, dir);
   }
 
   /// Downloads [id] into the system cache if it's a cached package.
@@ -225,9 +274,6 @@ class GlobalPackages {
     if (fileExists(oldPath)) deleteEntry(oldPath);
 
     writeTextFile(_getLockFilePath(package), lockFile.serialize(cache.rootDir));
-
-    var id = lockFile.packages[package];
-    log.message('Activated ${_formatPackage(id)}.');
   }
 
   /// Shows the user the currently active package with [name], if any.
diff --git a/test/cache/repair/recompiles_snapshots_test.dart b/test/cache/repair/recompiles_snapshots_test.dart
index a831c8ca..e0d37a7c 100644
--- a/test/cache/repair/recompiles_snapshots_test.dart
+++ b/test/cache/repair/recompiles_snapshots_test.dart
@@ -29,7 +29,6 @@ main() {
           Reinstalled 1 package.
           Reactivating foo 1.0.0...
           Precompiling executables...
-          Loading source assets...
           Precompiled foo:script.
           Reactivated 1 package.''');
 
diff --git a/test/global/activate/activate_git_after_hosted_test.dart b/test/global/activate/activate_git_after_hosted_test.dart
index 2e552101..16b2c740 100644
--- a/test/global/activate/activate_git_after_hosted_test.dart
+++ b/test/global/activate/activate_git_after_hosted_test.dart
@@ -37,7 +37,6 @@ main() {
             // Specific revision number goes here.
             endsWith(
                 'Precompiling executables...\n'
-                'Loading source assets...\n'
                 'Precompiled foo:foo.\n'
                 'Activated foo 1.0.0 from Git repository "../foo.git".')));
 
diff --git a/test/global/activate/activate_hosted_after_git_test.dart b/test/global/activate/activate_hosted_after_git_test.dart
index 000f0411..d7329bb6 100644
--- a/test/global/activate/activate_hosted_after_git_test.dart
+++ b/test/global/activate/activate_hosted_after_git_test.dart
@@ -30,7 +30,6 @@ main() {
         + foo 2.0.0
         Downloading foo 2.0.0...
         Precompiling executables...
-        Loading source assets...
         Precompiled foo:foo.
         Activated foo 2.0.0.""");
 
diff --git a/test/global/activate/activate_hosted_after_path_test.dart b/test/global/activate/activate_hosted_after_path_test.dart
index bc0ac010..92b12346 100644
--- a/test/global/activate/activate_hosted_after_path_test.dart
+++ b/test/global/activate/activate_hosted_after_path_test.dart
@@ -34,7 +34,6 @@ main() {
         + foo 2.0.0
         Downloading foo 2.0.0...
         Precompiling executables...
-        Loading source assets...
         Precompiled foo:foo.
         Activated foo 2.0.0.""");
 
diff --git a/test/global/activate/cached_package_test.dart b/test/global/activate/cached_package_test.dart
index ef5be8f4..af24e7c5 100644
--- a/test/global/activate/cached_package_test.dart
+++ b/test/global/activate/cached_package_test.dart
@@ -19,7 +19,6 @@ main() {
         Resolving dependencies...
         + foo 1.0.0
         Precompiling executables...
-        Loading source assets...
         Activated foo 1.0.0.""");
 
     // Should be in global package cache.
diff --git a/test/global/activate/different_version_test.dart b/test/global/activate/different_version_test.dart
index a920df36..9d78d0c3 100644
--- a/test/global/activate/different_version_test.dart
+++ b/test/global/activate/different_version_test.dart
@@ -22,7 +22,6 @@ main() {
         + foo 2.0.0
         Downloading foo 2.0.0...
         Precompiling executables...
-        Loading source assets...
         Activated foo 2.0.0.""");
   });
 }
diff --git a/test/global/activate/git_package_test.dart b/test/global/activate/git_package_test.dart
index fe2a9b4b..4d53ead9 100644
--- a/test/global/activate/git_package_test.dart
+++ b/test/global/activate/git_package_test.dart
@@ -26,7 +26,6 @@ main() {
             // Specific revision number goes here.
             endsWith(
                 'Precompiling executables...\n'
-                'Loading source assets...\n'
                 'Precompiled foo:foo.\n'
                 'Activated foo 1.0.0 from Git repository "../foo.git".')));
   });
diff --git a/test/global/activate/ignores_active_version_test.dart b/test/global/activate/ignores_active_version_test.dart
index e6969d18..d66c146a 100644
--- a/test/global/activate/ignores_active_version_test.dart
+++ b/test/global/activate/ignores_active_version_test.dart
@@ -22,7 +22,6 @@ main() {
         + foo 1.3.0
         Downloading foo 1.3.0...
         Precompiling executables...
-        Loading source assets...
         Activated foo 1.3.0.""");
   });
 }
diff --git a/test/global/activate/reactivating_git_upgrades_test.dart b/test/global/activate/reactivating_git_upgrades_test.dart
index d75382e4..148ffe89 100644
--- a/test/global/activate/reactivating_git_upgrades_test.dart
+++ b/test/global/activate/reactivating_git_upgrades_test.dart
@@ -24,7 +24,6 @@ main() {
             // Specific revision number goes here.
             endsWith(
                 'Precompiling executables...\n'
-                'Loading source assets...\n'
                 'Activated foo 1.0.0 from Git repository "../foo.git".')));
 
     d.git('foo.git', [
@@ -42,7 +41,6 @@ main() {
             // Specific revision number goes here.
             endsWith(
                 'Precompiling executables...\n'
-                'Loading source assets...\n'
                 'Activated foo 1.0.1 from Git repository "../foo.git".')));
   });
 }
diff --git a/test/global/activate/uncached_package_test.dart b/test/global/activate/uncached_package_test.dart
index 8b706faa..129be88c 100644
--- a/test/global/activate/uncached_package_test.dart
+++ b/test/global/activate/uncached_package_test.dart
@@ -20,7 +20,6 @@ main() {
         + foo 1.2.3 (2.0.0-wildly.unstable available)
         Downloading foo 1.2.3...
         Precompiling executables...
-        Loading source assets...
         Activated foo 1.2.3.""");
 
     // Should be in global package cache.
diff --git a/test/global/deactivate/deactivate_and_reactivate_package_test.dart b/test/global/deactivate/deactivate_and_reactivate_package_test.dart
index 0f8f6d85..b4bf9482 100644
--- a/test/global/deactivate/deactivate_and_reactivate_package_test.dart
+++ b/test/global/deactivate/deactivate_and_reactivate_package_test.dart
@@ -23,7 +23,6 @@ main() {
         + foo 2.0.0
         Downloading foo 2.0.0...
         Precompiling executables...
-        Loading source assets...
         Activated foo 2.0.0.""");
   });
 }
-- 
GitLab