Support directories other than "web" in pub build.

sdk/lib/_internal/pub/lib/src/command/build.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.
 
 library pub.command.build;
 
 import 'dart:async';
 
 import 'package:barback/barback.dart';
 import 'package:path/path.dart' as path;
 
-import '../barback/dart2js_transformer.dart';
-import '../barback/dart_forwarding_transformer.dart';
+import '../barback/build_environment.dart';
 import '../barback.dart' as barback;
 import '../command.dart';
 import '../exit_codes.dart' as exit_codes;
 import '../io.dart';
 import '../log.dart' as log;
 import '../utils.dart';
 
 final _arrow = getSpecial('\u2192', '=>');
 
+/// The set of top level directories in the entrypoint package that can be
+/// built.
+final _allowedBuildDirectories = [
+  "benchmark", "bin", "example", "test", "web"
+].toSet();

[nweiz, 2014/01/28 03:21:51]

Style nit: I slightly prefer [new Set.from] because it puts the fact that this
is going to be a set right up front.

[Bob Nystrom, 2014/01/28 23:02:04]

Done.

+
 /// Handles the `build` pub command.
 class BuildCommand extends PubCommand {
   String get description =>
       "Copy and compile all Dart entrypoints in the 'web' directory.";
   String get usage => "pub build [options]";
   List<String> get aliases => const ["deploy", "settle-up"];
+  bool get takesArguments => true;
 
-  // TODO(nweiz): make these configurable.
-  /// The path to the source directory of the application.
-  String get source => path.join(entrypoint.root.dir, 'web');
-
+  // TODO(nweiz): make this configurable.
   /// The path to the application's build output directory.
   String get target => path.join(entrypoint.root.dir, 'build');
 
   /// The build mode.
   BarbackMode get mode => new BarbackMode(commandOptions['mode']);
 
+  /// The number of files that have been built and written to disc so far.
+  int builtFiles = 0;

[nweiz, 2014/01/28 03:21:51]

It's weird that this is public but [_buildDirectories] is private.

[Bob Nystrom, 2014/01/28 23:02:04]

Made _buildDirectories public. In the command classes, we tend to be lax about
making stuff private.

+
+  /// The names of the top-level build directories that will be built.
+  final _buildDirectories = new Set<String>();
+
   BuildCommand() {
     commandParser.addOption('mode', defaultsTo: BarbackMode.RELEASE.toString(),
         help: 'Mode to run transformers in.');
   }
 
   Future onRun() {
-    if (!dirExists(source)) {
-      throw new ApplicationException('There is no "$source" directory.');
-    }
-
+    _parseBuildDirectories();
     cleanDir(target);
 
-    var dart2jsTransformer;
-    var builtFiles = 0;
+    // Since this server will only be hit by the transformer loader and isn't
+    // user-facing, just use an IPv4 address to avoid a weird bug on the
+    // OS X buildbots.
+    return BuildEnvironment.create(entrypoint, "127.0.0.1", 0, mode,
+        WatcherType.NONE, _buildDirectories, useDart2JS: true)
+          .then((environment) {
 
-    return entrypoint.loadPackageGraph().then((graph) {
-      dart2jsTransformer = new Dart2JSTransformer(graph, mode);
-      var builtInTransformers = [
-        dart2jsTransformer,
-        new DartForwardingTransformer(mode)
-      ];
-
-      // Since this server will only be hit by the transformer loader and isn't
-      // user-facing, just use an IPv4 address to avoid a weird bug on the
-      // OS X buildbots.
-      // TODO(rnystrom): Allow specifying mode.
-      return barback.createServer("127.0.0.1", 0, graph, mode,
-          builtInTransformers: builtInTransformers,
-          watcher: barback.WatcherType.NONE);
-    }).then((server) {
       // Show in-progress errors, but not results. Those get handled implicitly
       // by getAllAssets().
-      server.barback.errors.listen((error) {
+      environment.server.barback.errors.listen((error) {
         log.error(log.red("Build error:\n$error"));
       });
 
       return log.progress("Building ${entrypoint.root.name}",
-          () => server.barback.getAllAssets());
-    }).then((assets) {
-      return Future.wait(assets.map((asset) {
-        // In release mode, strip out .dart files since all relevant ones have
-        // been compiled to JavaScript already.
-        if (mode == BarbackMode.RELEASE && asset.id.extension == ".dart") {
-          return new Future.value();
-        }
-
-        builtFiles++;
-
-        // Figure out the output directory for the asset, which is the same
-        // as the path pub serve would use to serve it.
-        var relativeUrl = barback.idtoUrlPath(entrypoint.root.name, asset.id);
-
-        // Remove the leading "/".
-        relativeUrl = relativeUrl.substring(1);
-
-        var relativePath = path.fromUri(new Uri(path: relativeUrl));
-        var destPath = path.join(target, relativePath);
-
-        ensureDir(path.dirname(destPath));
-        // TODO(rnystrom): Should we display this to the user?
-        return createFileFromStream(asset.read(), destPath);
-      })).then((_) {
-        builtFiles += _copyBrowserJsFiles(dart2jsTransformer.entrypoints);
-        log.message("Built $builtFiles ${pluralize('file', builtFiles)}!");
+          () => environment.server.barback.getAllAssets()).then((assets) {
+        return Future.wait(assets.map(_writeAsset)).then((_) {
+          builtFiles += _copyBrowserJsFiles(
+              environment.dart2JSTransformer.entrypoints);

[nweiz, 2014/01/28 03:21:51]

Accessing dart2JSTransformer here is gross. It also doesn't seem safe to assume
that the browser JS files only need to go next to the ".dart.js" files. What if
an HTML file imports a dart2js-compiled file that's not in the same directory?

If we do make this assumption, it seems cleaner to look for ".dart.js" files and
to copy the JS bootstrap files next to those, or to have the dart2js transformer
emit them itself.

[Bob Nystrom, 2014/01/28 23:02:04]

Done.

+          log.message("Built $builtFiles ${pluralize('file', builtFiles)}!");
+        });
       });
     }).catchError((error) {
       // If [getAllAssets()] throws a BarbackException, the error has already
       // been reported.
       if (error is! BarbackException) throw error;
 
       log.error(log.red("Build failed."));
       return flushThenExit(exit_codes.DATA);
     });
   }
 
+  /// Parses the command-line arguments to determine the set of top-level
+  /// directories to build.
+  ///
+  /// If there are no arguments to `pub build`, this will just be "web".
+  ///
+  /// If the argument is `*`, then it will be all buildable directories that
+  /// exist.

[nweiz, 2014/01/28 03:21:51]

Passing * on the command line is very confusing. It makes users think about the
difference between "pub build *" and "pub build \*", which is asking for
trouble. I think we should either do "pub build all" or "pub build --all",
depending on whether or not we think people will want to build a directory named
"all/" at some point.

[Bob Nystrom, 2014/01/28 23:02:04]

Done. --all.

+  ///
+  /// Otherwise, all arguments should be the names of directories to include.
+  ///
+  /// Throws if any of the specified directories cannot be found.
+  void _parseBuildDirectories() {
+    if (commandOptions.rest.length == 1 && commandOptions.rest.single == "*") {
+      // Include every build directory that exists in the package.
+      var allowed = _allowedBuildDirectories.where(
+          (d) => dirExists(path.join(entrypoint.root.dir, d)));
+
+      if (allowed.isEmpty) {
+        throw new ApplicationException("There are no buildable directories.");

[nweiz, 2014/01/28 03:21:51]

List which directories are considered buildable in this error message.

[Bob Nystrom, 2014/01/28 23:02:04]

Done.

+      }
+
+      _buildDirectories.addAll(allowed);
+      return;
+    }
+
+    _buildDirectories.addAll(commandOptions.rest);
+
+    // If no directory was specified, default to "web".
+    if (commandOptions.rest.isEmpty) {
+      _buildDirectories.add("web");
+    }
+
+    // Make sure the arguments are known directories.
+    var disallowed = commandOptions.rest.where(
+        (dir) => !_allowedBuildDirectories.contains(dir));
+    if (disallowed.isNotEmpty) {
+      var dirs = pluralize("directory", disallowed.length,
+          plural: "directories");
+      var names = toSentence(ordered(disallowed).map((name) => '"$name"'));
+      var allowed = toSentence(ordered(_allowedBuildDirectories.map(
+          (name) => '"$name"')));
+      throw new ApplicationException(
+          'Unsupported build $dirs $names.\n'
+          'The allowed directories are: $allowed.');

[nweiz, 2014/01/28 03:21:51]

Nit: since [allowed] is sentenceized, I think this would read better without the
colon.

[Bob Nystrom, 2014/01/28 23:02:04]

Done.

+    }
+
+    // Make sure all of the build directories exist.
+    var missing = _buildDirectories.where(
+        (dir) => !dirExists(path.join(entrypoint.root.dir, dir)));
+
+    if (missing.isNotEmpty) {
+      var dirs = pluralize("directory", missing.length, plural: "directories");
+      var names = toSentence(ordered(missing).map((name) => '"$name"'));
+      throw new ApplicationException('Could not find $dirs $names.');

[nweiz, 2014/01/28 03:21:51]

I feel like "Could not find" is more ambiguous than "$dirs $names do(es) not
exist."

[Bob Nystrom, 2014/01/28 23:02:04]

Done.

+    }
+  }
+
+  /// Writes [asset] to the appropriate build directory.
+  ///
+  /// If [asset] is in the special "assets" directory, writes it to every
+  /// build directory.
+  Future _writeAsset(Asset asset) {
+    // In release mode, strip out .dart files since all relevant ones have been
+    // compiled to JavaScript already.
+    if (mode == BarbackMode.RELEASE && asset.id.extension == ".dart") {
+      return new Future.value();
+    }
+
+    // If the asset is from a package's "lib" directory, we make it available
+    // as an input for transformers, but don't want it in the final output.
+    // (Any Dart code in there should be imported and compiled to JS, anything
+    // else we want to omit.)
+    if (asset.id.path.startsWith("lib/")) {
+      return new Future.value();
+    }
+
+    // Figure out the output directory for the asset, which is the same as the
+    // path pub serve would use to serve it.
+    var relativeUrl = barback.idtoUrlPath(entrypoint.root.name, asset.id,
+        useWebAsRoot: false);
+
+    // Remove the leading "/".
+    relativeUrl = relativeUrl.substring(1);
+
+    // If the asset is from a special "shared" directory package, copy it into

[nweiz, 2014/01/28 03:21:51]

"'shared' directory package" -> "'assets/' directory"

[Bob Nystrom, 2014/01/28 23:02:04]

Done.

+    // all of the top-level build directories.
+    if (relativeUrl.startsWith("assets/")) {
+      builtFiles += _buildDirectories.length;
+      return Future.wait(_buildDirectories.map(
+          (buildDir) => _writeOutputFile(asset,
+              path.url.join(buildDir, relativeUrl))));
+    }
+
+    builtFiles++;
+    return _writeOutputFile(asset, relativeUrl);
+  }
+
+  /// Writes the contents of [asset] to [relativeUrl] within the build
+  /// directory.
+  Future _writeOutputFile(Asset asset, String relativeUrl) {
+    var relativePath = path.fromUri(new Uri(path: relativeUrl));
+    var destPath = path.join(target, relativePath);
+
+    ensureDir(path.dirname(destPath));
+    return createFileFromStream(asset.read(), destPath);
+  }
+
   /// If this package depends directly on the `browser` package, this ensures
   /// that the JavaScript bootstrap files are copied into `packages/browser/`
   /// directories next to each entrypoint in [entrypoints].
   ///
   /// Returns the number of files it copied.
   int _copyBrowserJsFiles(Iterable<AssetId> entrypoints) {
     // Must depend on the browser package.
     if (!entrypoint.root.dependencies.any(
         (dep) => dep.name == 'browser' && dep.source == 'hosted')) {
       return 0;
     }
 
     // Get all of the directories that contain Dart entrypoints.
     var entrypointDirs = entrypoints
         .map((id) => path.url.split(id.path))
-        .map((parts) => parts.skip(1)) // Remove "web/".
         .map((relative) => path.dirname(path.joinAll(relative)))
         .toSet();
 
     for (var dir in entrypointDirs) {
       // TODO(nweiz): we should put browser JS files next to any HTML file
       // rather than any entrypoint. An HTML file could import an entrypoint
       // that's not adjacent.
       _addBrowserJs(dir, "dart");
       _addBrowserJs(dir, "interop");
     }
 
     return entrypointDirs.length * 2;
   }
 
   // TODO(nweiz): do something more principled when issue 6101 is fixed.
   /// Ensures that the [name].js file is copied into [directory] in [target],
   /// under `packages/browser/`.
   void _addBrowserJs(String directory, String name) {
     var jsPath = path.join(
         target, directory, 'packages', 'browser', '$name.js');
     ensureDir(path.dirname(jsPath));
+
+    // TODO(rnystrom): This won't work if we get rid of symlinks and the top
+    // level "packages" directory. Will need to copy from the browser
+    // directory.
     copyFile(path.join(entrypoint.packagesDir, 'browser', '$name.js'), jsPath);
   }
 }