Make pub build use barback.

Make pub build use barback.

BUG=https://code.google.com/p/dart/issues/detail?id=13880

[Bob Nystrom, 2013-10-10 22:16:05]

Alan, Nathan is working on SASS stuff this week. Is there any chance you can
take a look at this?

This builds on https://codereview.chromium.org/26572010/, so please review that
one first. I'll rebase this one against that once it lands.

Thank you!

[Bob Nystrom, 2013-10-11 00:02:54]

Rebased now. It's all yours!

- bob

[Alan Knight, 2013-10-11 00:24:59]

lgtm

https://codereview.chromium.org/26933003/diff/4001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/command/build.dart (right):

https://codereview.chromium.org/26933003/diff/4001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/command/build.dart:94: /// If this package depends
non-transitively on the `browser` package, this
What does it mean to depends non-transitively?  Directly? Indirectly? Something
else?

[Bob Nystrom, 2013-10-11 01:00:15]

Thanks!

https://codereview.chromium.org/26933003/diff/4001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/command/build.dart (right):

https://codereview.chromium.org/26933003/diff/4001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/command/build.dart:94: /// If this package depends
non-transitively on the `browser` package, this
On 2013/10/11 00:25:00, Alan Knight wrote:
> What does it mean to depends non-transitively?  Directly? Indirectly?
Something
> else?

Directly. Reworded.

[Bob Nystrom, 2013-10-11 16:53:00]

On 2013/10/11 01:00:15, Bob Nystrom wrote:
> Thanks!
> 
>
https://codereview.chromium.org/26933003/diff/4001/sdk/lib/_internal/pub/lib/...
> File sdk/lib/_internal/pub/lib/src/command/build.dart (right):
> 
>
https://codereview.chromium.org/26933003/diff/4001/sdk/lib/_internal/pub/lib/...
> sdk/lib/_internal/pub/lib/src/command/build.dart:94: /// If this package
depends
> non-transitively on the `browser` package, this
> On 2013/10/11 00:25:00, Alan Knight wrote:
> > What does it mean to depends non-transitively?  Directly? Indirectly?
> Something
> > else?
> 
> Directly. Reworded.

git cl dcommit flaked out in the middle, but this is committed now.

[nweiz, 2013-10-16 21:40:21]

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:31: //
TODO(rnystrom): Do something cleaner for this, or eliminate those files.
A low-hanging way to do this would be to just put a browser.js file next to any
HTML file in web. There's already a TODO for this.

In the long term, though, we'll hopefully have a story for supporting assets in
general, and a way to refer to them in HTML. At that point we won't need any
explicit browser.js handling.

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/command/build.dart (right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/command/build.dart:69: return
Future.forEach(assets, (asset) {
Why are we doing these sequentially rather than in parallel?

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/command/build.dart:100: dep.name == 'browser' &&
dep.source == 'hosted')) {
Nit: I like indenting this line more (probably four spaces more) since it
"belongs to" the previous line.

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/tes...
File
sdk/lib/_internal/pub/test/build/compiles_dart_entrypoints_to_dart_and_js_test.dart
(right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/tes...
sdk/lib/_internal/pub/test/build/compiles_dart_entrypoints_to_dart_and_js_test.dart:13:
// TODO(rnystrom): Should also add tests that other transformers work.
Is there a bug for this?

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/tes...
sdk/lib/_internal/pub/test/build/compiles_dart_entrypoints_to_dart_and_js_test.dart:32:
output: new RegExp(r"Built 2 files!"),
It feels weird not to test any other aspects of the output here. The
user-friendliness of the output is part of the UI.

[Bob Nystrom, 2013-10-29 00:03:30]

Thanks! Revised here: https://codereview.chromium.org/48833007/

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:31: //
TODO(rnystrom): Do something cleaner for this, or eliminate those files.
On 2013/10/16 21:40:22, nweiz wrote:
> A low-hanging way to do this would be to just put a browser.js file next to
any
> HTML file in web. There's already a TODO for this.
> 
> In the long term, though, we'll hopefully have a story for supporting assets
in
> general, and a way to refer to them in HTML. At that point we won't need any
> explicit browser.js handling.

Yeah, I talked to Vijay about this a bit while you were out and I think we'll
just eventually remove this stuff entirely once it isn't needed.

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/command/build.dart (right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/command/build.dart:69: return
Future.forEach(assets, (asset) {
On 2013/10/16 21:40:22, nweiz wrote:
> Why are we doing these sequentially rather than in parallel?

It doesn't matter much right now. The original code did, mainly to keep the
output deterministic, I think.

The output here could definitely be improved so when/if that happens, we'll
probably want this to be sequential then too.

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/command/build.dart:100: dep.name == 'browser' &&
dep.source == 'hosted')) {
On 2013/10/16 21:40:22, nweiz wrote:
> Nit: I like indenting this line more (probably four spaces more) since it
> "belongs to" the previous line.

Moved the "(dep) =>" to the next line, which I think works a bit better.

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/tes...
File
sdk/lib/_internal/pub/test/build/compiles_dart_entrypoints_to_dart_and_js_test.dart
(right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/tes...
sdk/lib/_internal/pub/test/build/compiles_dart_entrypoints_to_dart_and_js_test.dart:13:
// TODO(rnystrom): Should also add tests that other transformers work.
On 2013/10/16 21:40:22, nweiz wrote:
> Is there a bug for this?

There is now.

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/tes...
sdk/lib/_internal/pub/test/build/compiles_dart_entrypoints_to_dart_and_js_test.dart:32:
output: new RegExp(r"Built 2 files!"),
On 2013/10/16 21:40:22, nweiz wrote:
> It feels weird not to test any other aspects of the output here. The
> user-friendliness of the output is part of the UI.

Added a TODO. The current command-line output for this is more bare-bones than
I'd like, but I wanted to get the core functionality landed first. (There's a
TODO for the output already.)

[nweiz, 2013-10-29 00:48:55]

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/command/build.dart (right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/command/build.dart:69: return
Future.forEach(assets, (asset) {
On 2013/10/29 00:03:30, Bob Nystrom wrote:
> On 2013/10/16 21:40:22, nweiz wrote:
> > Why are we doing these sequentially rather than in parallel?
> 
> It doesn't matter much right now. The original code did, mainly to keep the
> output deterministic, I think.
> 
> The output here could definitely be improved so when/if that happens, we'll
> probably want this to be sequential then too.

It's only deterministic inasmuch as the return value of [barback.getAllAssets]
is deterministic, which it isn't. I think we should make this [Future.wait] for
now so it doesn't appear to be doing something important that it's not actually
doing.

[Bob Nystrom, 2013-10-29 19:39:39]

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/command/build.dart (right):

https://codereview.chromium.org/26933003/diff/14001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/command/build.dart:69: return
Future.forEach(assets, (asset) {
On 2013/10/29 00:48:56, nweiz wrote:
> On 2013/10/29 00:03:30, Bob Nystrom wrote:
> > On 2013/10/16 21:40:22, nweiz wrote:
> > > Why are we doing these sequentially rather than in parallel?
> > 
> > It doesn't matter much right now. The original code did, mainly to keep the
> > output deterministic, I think.
> > 
> > The output here could definitely be improved so when/if that happens, we'll
> > probably want this to be sequential then too.
> 
> It's only deterministic inasmuch as the return value of [barback.getAllAssets]
> is deterministic, which it isn't. I think we should make this [Future.wait]
for
> now so it doesn't appear to be doing something important that it's not
actually
> doing.

Done.