Support serving from multiple directories using "pub serve".

sdk/lib/_internal/pub/lib/src/barback/build_environment.dart

Index: sdk/lib/_internal/pub/lib/src/barback/build_environment.dart
diff --git a/sdk/lib/_internal/pub/lib/src/barback/build_environment.dart b/sdk/lib/_internal/pub/lib/src/barback/build_environment.dart
index af2eae8c906043bad774c0258d8914ed871e643a..67ecf7895a5955ff90d63947845ad5b3396f5f54 100644
--- a/sdk/lib/_internal/pub/lib/src/barback/build_environment.dart
+++ b/sdk/lib/_internal/pub/lib/src/barback/build_environment.dart
@@ -33,10 +33,16 @@ class BuildEnvironment {
   /// Creates a new build environment for working with the assets used by
   /// [entrypoint] and its dependencies.
   ///
-  /// Spawns an HTTP server on [hostname] and [port]. Loads all used
-  /// transformers using [mode] (including dart2js if [useDart2JS] is true).
+  /// Spawns an HTTP server for each directory in [rootDirectories]. These
+  /// servers will be on [hostname] and have ports based on [basePort].
+  /// [basePort] itself is reserved for "web/" and `basePort + 1` is reserved
+  /// for "test/"; further ports will be allocated for other root directories as
+  /// necessary. If [basePort] is zero, each server will have an arbitrarily-allocated port.

[Bob Nystrom, 2014/02/19 00:36:52]

Long line.

Also, "arbitrarily-allocated" -> "ephemeral".

[nweiz, 2014/02/19 01:25:58]

Done.

   ///
-  /// Includes [buildDirectories] in the root package, as well as "lib" and
+  /// Loads all used transformers using [mode] (including dart2js if
+  /// [useDart2JS] is true).
+  ///
+  /// Includes [rootDirectories] in the root package, as well as "lib" and
   /// "asset".
   ///
   /// If [watcherType] is not [WatcherType.NONE], watches source assets for
@@ -45,17 +51,17 @@ class BuildEnvironment {
   /// Returns a [Future] that completes to the environment once the inputs,
   /// transformers, and server are loaded and ready.
   static Future<BuildEnvironment> create(Entrypoint entrypoint,
-      String hostname, int port, BarbackMode mode, WatcherType watcherType,
-      Set<String> buildDirectories,
+      String hostname, int basePort, BarbackMode mode, WatcherType watcherType,
+      Iterable<String> rootDirectories,
       {bool useDart2JS: true}) {
     return entrypoint.loadPackageGraph().then((graph) {
       var barback = new Barback(new PubPackageProvider(graph));
       barback.log.listen(_log);
 
-      return BarbackServer.bind(hostname, port, barback,
-          graph.entrypoint.root.name).then((server) {
-        var environment = new BuildEnvironment._(graph, server, mode,
-            watcherType, buildDirectories);
+      return _startServers(hostname, basePort, mode, graph, barback,
+          rootDirectories).then((servers) {
+        var environment = new BuildEnvironment._(graph, servers, mode,
+            watcherType, rootDirectories);
 
         // If the entrypoint package manually configures the dart2js
         // transformer, don't include it in the built-in transformer list.
@@ -78,11 +84,42 @@ class BuildEnvironment {
     });
   }
 
-  /// The server serving this environment's assets.
-  final BarbackServer server;
+  /// Start the [BarbackServer]s that will serve [rootDirectories].
+  static Future<List<BarbackServer>> _startServers(String hostname,
+      int basePort, BarbackMode mode, PackageGraph graph, Barback barback,
+      Iterable<String> rootDirectories) {
+    _bind(port, rootDirectory) {
+      if (basePort == 0) port = 0;
+      return BarbackServer.bind(hostname, port, barback,
+          graph.entrypoint.root.name, rootDirectory);
+    }
+
+    rootDirectories = rootDirectories.toSet();
+
+    // For consistency, "web/" should always have the first available port and
+    // "test/" should always have the second. Other directories are assigned
+    // the following ports in alphabetical order.

[Bob Nystrom, 2014/02/19 00:36:52]

Since create() takes an iterable and not a set and the serve command defines
this policy, how about we just rely on serve passing in "web" and "test" as the
first two elements in the iterable instead of redundantly enforcing the policy
here too?

[nweiz, 2014/02/19 01:25:58]

The issue with that is that port 8080 should always mean "web" and 8081 should
always mean test; if the user writes "pub serve example", example should be on
8082, not 8080.

[Bob Nystrom, 2014/02/19 01:50:55]

I personally don't care that much about always guaranteeing the ports for web
and test, but if it's important to you, OK.

+    var serverFutures = [];
+    if (rootDirectories.remove('web')) {
+      serverFutures.add(_bind(basePort, 'web'));
+    }
+    if (rootDirectories.remove('test')) {
+      serverFutures.add(_bind(basePort + 1, 'test'));
+    }
+
+    var remainingDirectories = ordered(rootDirectories);

[Bob Nystrom, 2014/02/19 00:36:52]

I think it makes more sense to just preserve the order the user passed them in.
That may make it easier for users to script pub serve.

[nweiz, 2014/02/19 01:25:58]

Done.

+    for (var i = 0; i < remainingDirectories.length; i++) {
+      serverFutures.add(_bind(basePort + 2 + i, remainingDirectories[i]));
+    }
+
+    return Future.wait(serverFutures);
+  }
+
+  /// The servers serving this environment's assets.
+  final List<BarbackServer> servers;
 
   /// The [Barback] instance used to process assets in this environment.
-  Barback get barback => server.barback;
+  Barback get barback => servers.first.barback;
 
   /// The root package being built.
   Package get rootPackage => graph.entrypoint.root;
@@ -100,12 +137,13 @@ class BuildEnvironment {
   /// How source files should be watched.
   final WatcherType _watcherType;
 
-  /// The set of top-level directories in the entrypoint package that should be
-  /// built.
-  final Set<String> _buildDirectories;
+  /// The set of top-level directories in the entrypoint package that will be
+  /// exposed.
+  final Set<String> _rootDirectories;
 
-  BuildEnvironment._(this.graph, this.server, this.mode, this._watcherType,
-      this._buildDirectories);
+  BuildEnvironment._(this.graph, this.servers, this.mode, this._watcherType,
+      Iterable<String> rootDirectories)
+      : _rootDirectories = rootDirectories.toSet();
 
   /// Gets the built-in [Transformer]s that should be added to [package].
   ///
@@ -121,7 +159,7 @@ class BuildEnvironment {
     return _builtInTransformers;
   }
 
-  /// Creates a [BarbackServer] for this environment.
+  /// Loads the assets and transformers for this environment.
   ///
   /// This transforms and serves all library and asset files in all packages in
   /// the environment's package graph. It loads any transformer plugins defined
@@ -134,21 +172,22 @@ class BuildEnvironment {
     return _provideSources(barback).then((_) {
       var completer = new Completer();
 
-      // If any errors get emitted either by barback or by the server,
+      // If any errors get emitted either by barback or by the primary server,
       // including non-programmatic barback errors, they should take down the
       // whole program.
       var subscriptions = [
-        server.barback.errors.listen((error) {
+        barback.errors.listen((error) {
           if (error is TransformerException) error = error.error;
           if (!completer.isCompleted) {
             completer.completeError(error, new Chain.current());
           }
         }),
-        server.barback.results.listen((_) {}, onError: (error, stackTrace) {
+        barback.results.listen((_) {},
+            onError: (error, stackTrace) {
           if (completer.isCompleted) return;
           completer.completeError(error, stackTrace);
         }),
-        server.results.listen((_) {}, onError: (error, stackTrace) {
+        servers.first.results.listen((_) {}, onError: (error, stackTrace) {

[Bob Nystrom, 2014/02/19 00:36:52]

What about the others?

[nweiz, 2014/02/19 01:25:58]

This just catches errors during the transformer initialization, which only uses
the first server. I'll add a comment to that effect.

           if (completer.isCompleted) return;
           completer.completeError(error, stackTrace);
         })
@@ -296,7 +335,7 @@ class BuildEnvironment {
     var directories = ["asset", "lib"];
 
     if (package.name == entrypoint.root.name) {
-      directories.addAll(_buildDirectories);
+      directories.addAll(_rootDirectories);
     }
 
     return directories;