Removing polymer-specific code from barback runner

pkg/polymer/lib/src/build/runner.dart

 // Copyright (c) 2013, 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.
 
 /**
  * Definitions used to run the polymer linter and deploy tools without using
  * pub serve or pub deploy.
  */
 library polymer.src.build.runner;
 
 import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
 import 'package:barback/barback.dart';
 import 'package:path/path.dart' as path;
 import 'package:stack_trace/stack_trace.dart';
 import 'package:yaml/yaml.dart';
 
 
 /** Collects different parameters needed to configure and run barback. */
 class BarbackOptions {
-  /** Phases of transformers to run. */
+  /** Phases of transformers to run for the current package.

[Siggi Cherem (dart-lang), 2014/02/11 23:15:20]

nit: fix the multiline style of this comment (extra \n)

[blois, 2014/02/12 02:39:27]

Done.

+   * Use packagePhases to specify phases for other packages.
+   */
   final List<List<Transformer>> phases;
 
   /** Package to treat as the current package in barback. */
   final String currentPackage;
 
+  /** Directory root for the current package. */
+  final String packageHome;
+
   /**
    * Mapping between package names and the path in the file system where
    * to find the sources of such package.
    */
   final Map<String, String> packageDirs;
 
   /** Whether to run transformers on the test folder. */
   final bool transformTests;
 
-  /** Whether to apply transformers on polymer dependencies. */
-  final bool transformPolymerDependencies;
-
   /** Directory where to generate code, if any. */
   final String outDir;
 
   /**
    * Whether to print error messages using a json-format that tools, such as the
    * Dart Editor, can process.
    */
   final bool machineFormat;
 
   /**
    * Whether to follow symlinks when listing directories. By default this is
    * false because directories have symlinks for the packages directory created
    * by pub, but it can be turned on for custom uses of this library.
    */
   final bool followLinks;
 
-  BarbackOptions(this.phases, this.outDir, {currentPackage, packageDirs,
-      this.transformTests: false, this.transformPolymerDependencies: false,
-      this.machineFormat: false, this.followLinks: false})
+  /** Phases of transformers to apply to packages other than the current

[Siggi Cherem (dart-lang), 2014/02/11 23:15:20]

same here (style should be:
/**
 * Phases ...
)

[blois, 2014/02/12 02:39:27]

Done.

+   * package, keyed by the package name.
+   */
+  final Map<String, List<List<Transformer>>> packagePhases;
+
+  BarbackOptions(this.phases, this.outDir, {currentPackage, packageHome,
+      packageDirs, this.transformTests: false, this.machineFormat: false,
+      this.followLinks: false, packagePhases})

[Siggi Cherem (dart-lang), 2014/02/11 23:15:20]

lets add a type for packagePhases here

[blois, 2014/02/12 02:39:27]

Done.

       : currentPackage = (currentPackage != null
           ? currentPackage : readCurrentPackageFromPubspec()),
         packageDirs = (packageDirs != null
-          ? packageDirs : _readPackageDirsFromPub(currentPackage));
+          ? packageDirs : readPackageDirsFromPub(packageHome)),
+        packageHome = (packageHome != null
+          ? packageHome : Directory.current.path),

[Siggi Cherem (dart-lang), 2014/02/11 23:15:20]

you seem to already default to Directory.current in the logic below. Consider
just making this null by default?

[blois, 2014/02/12 02:39:27]

Done.

+        packagePhases = (packagePhases != null ? packagePhases : {});
 
 }
 
 /**
  * Creates a barback system as specified by [options] and runs it.  Returns a
  * future that contains the list of assets generated after barback runs to
  * completion.
  */
 Future<AssetSet> runBarback(BarbackOptions options) {
-  var barback = new Barback(new _PolymerPackageProvider(options.packageDirs));
+  var barback = new Barback(new _PackageProvider(options.packageDirs));
   _initBarback(barback, options);
   _attachListeners(barback, options);
   if (options.outDir == null) return barback.getAllAssets();
   return _emitAllFiles(barback, options);
 }
 
 /** Extract the current package from the pubspec.yaml file. */
 String readCurrentPackageFromPubspec([String dir]) {
   var pubspec = new File(
       dir == null ? 'pubspec.yaml' : path.join(dir, 'pubspec.yaml'));
   if (!pubspec.existsSync()) {
     print('error: pubspec.yaml file not found, please run this script from '
         'your package root directory.');
     return null;
   }
   return loadYaml(pubspec.readAsStringSync())['name'];
 }
 
 /**
  * Extract a mapping between package names and the path in the file system where
  * to find the sources of such package. This map will contain an entry for the
  * current package and everything it depends on (extracted via `pub
  * list-package-dirs`).
  */
-Map<String, String> _readPackageDirsFromPub(String currentPackage) {
+Map<String, String> readPackageDirsFromPub([String packageHome]) {
+  var cachedDir = Directory.current;
+  if (packageHome != null) {
+    Directory.current = new Directory(packageHome);
+  }
+
   var dartExec = Platform.executable;
   // If dartExec == dart, then dart and pub are in standard PATH.
   var sdkDir = dartExec == 'dart' ? '' : path.dirname(dartExec);
   var pub = path.join(sdkDir, Platform.isWindows ? 'pub.bat' : 'pub');
   var result = Process.runSync(pub, ['list-package-dirs']);
   if (result.exitCode != 0) {
     print("unexpected error invoking 'pub':");
     print(result.stdout);
     print(result.stderr);
     exit(result.exitCode);
   }
   var map = JSON.decode(result.stdout)["packages"];
-  map.forEach((k, v) { map[k] = path.dirname(v); });
-  map[currentPackage] = '.';
+  map.forEach((k, v) { map[k] = path.absolute(packageHome, path.dirname(v)); });
+  map[readCurrentPackageFromPubspec(packageHome)] = packageHome;

[Siggi Cherem (dart-lang), 2014/02/11 23:15:20]

seems that we are computing the current-package twice, could we pass it in?

seems that the old code had the argument, but was buggy (seems like I was
incorrectly passing the argument instead of the field value, so we might need to
move some of the initialization to the body of the constructor to pass the right
thing)

[blois, 2014/02/12 02:39:27]

Updated to only read if currentPackage is null.

+
+  Directory.current = cachedDir;
   return map;
 }
 
-/** Internal packages used by polymer. */
-// TODO(sigmund): consider computing this list by recursively parsing
-// pubspec.yaml files in the `Options.packageDirs`.
-final Set<String> _polymerPackageDependencies = [
-    'analyzer', 'args', 'barback', 'browser', 'custom_element', 'html5lib',
-    'html_import', 'js', 'logging', 'mutation_observer', 'observe', 'path'
-    'polymer_expressions', 'serialization', 'shadow_dom', 'source_maps',
-    'stack_trace', 'template_binding', 'unittest', 'unmodifiable_collection',
-    'yaml'].toSet();
-
 /** Return the relative path of each file under [subDir] in [package]. */
 Iterable<String> _listPackageDir(String package, String subDir,
     BarbackOptions options) {
   var packageDir = options.packageDirs[package];
   if (packageDir == null) return const [];
   var dir = new Directory(path.join(packageDir, subDir));
   if (!dir.existsSync()) return const [];
   return dir.listSync(recursive: true, followLinks: options.followLinks)
       .where((f) => f is File)
       .map((f) => path.relative(f.path, from: packageDir));
 }
 
 /** A simple provider that reads files directly from the pub cache. */
-class _PolymerPackageProvider implements PackageProvider {
+class _PackageProvider implements PackageProvider {
   Map<String, String> packageDirs;
   Iterable<String> get packages => packageDirs.keys;
 
-  _PolymerPackageProvider(this.packageDirs);
+  _PackageProvider(this.packageDirs);
 
   Future<Asset> getAsset(AssetId id) => new Future.value(
       new Asset.fromPath(id, path.join(packageDirs[id.package],
       _toSystemPath(id.path))));
 }
 
 /** Convert asset paths to system paths (Assets always use the posix style). */
 String _toSystemPath(String assetPath) {
   if (path.Style.platform != path.Style.windows) return assetPath;
   return path.joinAll(path.posix.split(assetPath));
 }
 
 /** Tell barback which transformers to use and which assets to process. */
 void _initBarback(Barback barback, BarbackOptions options) {
   var assets = [];
   void addAssets(String package, String subDir) {
     for (var filepath in _listPackageDir(package, subDir, options)) {
       assets.add(new AssetId(package, filepath));
     }
   }
 
   for (var package in options.packageDirs.keys) {
-    // There is nothing to do in the polymer package dependencies.
-    // However: in Polymer package *itself*, we need to replace Observable
-    // with ChangeNotifier.
-    if (!options.transformPolymerDependencies &&
-        _polymerPackageDependencies.contains(package)) continue;
-    barback.updateTransformers(package, options.phases);
-
     // Notify barback to process anything under 'lib' and 'asset'.
     addAssets(package, 'lib');
     addAssets(package, 'asset');
+
+    if (options.packagePhases.containsKey(package)) {
+      barback.updateTransformers(package, options.packagePhases[package]);
+    }
   }
+  barback.updateTransformers(options.currentPackage, options.phases);
 
   // In case of the current package, include also 'web'.
   addAssets(options.currentPackage, 'web');
   if (options.transformTests) addAssets(options.currentPackage, 'test');
 
   barback.updateSources(assets);

[Siggi Cherem (dart-lang), 2014/02/11 23:15:20]

would you mind adding a small comment here saying that we add the sources after
the transformers so that barback has everything it needs before it starts
processing these assets?

(I got confused briefly because the updateTransformers was before 'addAssets',
but what matters is that it's before updateSources).

[blois, 2014/02/12 02:39:27]

Done.

 }
 
 /** Attach error listeners on [barback] so we can report errors. */
 void _attachListeners(Barback barback, BarbackOptions options) {
   // Listen for errors and results
   barback.errors.listen((e) {
     var trace = null;
     if (e is Error) trace = e.stackTrace;
     if (trace != null) {
       print(Trace.format(trace));
@@ skipping 13 matching lines @@
     if (options.machineFormat) {
       print(_jsonFormatter(entry));
     } else {
       print(_consoleFormatter(entry));
     }
   });
 }
 
 /**
  * Emits all outputs of [barback] and copies files that we didn't process (like
- * polymer's libraries).
+ * dependent package's libraries).
  */
 Future _emitAllFiles(Barback barback, BarbackOptions options) {
   return barback.getAllAssets().then((assets) {
     // Delete existing output folder before we generate anything
     var dir = new Directory(options.outDir);
     if (dir.existsSync()) dir.deleteSync(recursive: true);
     return _emitPackagesDir(options)
       .then((_) => _emitTransformedFiles(assets, options))
       .then((_) => _addPackagesSymlinks(assets, options))
       .then((_) => assets);
@@ skipping 55 matching lines @@
       }
     }
   }
 }
 
 /**
  * Emits a 'packages' directory directly under `out/packages` with the contents
  * of every file that was not transformed by barback.
  */
 Future _emitPackagesDir(BarbackOptions options) {
-  if (options.transformPolymerDependencies) return new Future.value(null);
   var outPackages = path.join(options.outDir, 'packages');
   _ensureDir(outPackages);
 
   // Copy all the files we didn't process
   var dirs = options.packageDirs;
 
-  return Future.forEach(_polymerPackageDependencies, (package) {
+  return Future.forEach(dirs.keys, (package) {
     return Future.forEach(_listPackageDir(package, 'lib', options), (relpath) {
       var inpath = path.join(dirs[package], relpath);
       var outpath = path.join(outPackages, package, relpath.substring(4));
       return _copyFile(inpath, outpath);
     });
   });
 }
 
 /** Ensure [dirpath] exists. */
 void _ensureDir(String dirpath) {
@@ skipping 65 matching lines @@
     output.write(entry.span.getLocationMessage(entry.message,
           useColors: useColors,
           color: levelColor));
   }
   return output.toString();
 }
 
 const String _RED_COLOR = '\u001b[31m';
 const String _MAGENTA_COLOR = '\u001b[35m';
 const String _NO_COLOR = '\u001b[0m';