Run a simple preprocessor on the sources pub loads in barback isolates.

Run a simple preprocessor on the sources pub loads in barback isolates.

Currently no actual preprocessing happens, but it will become necessary when
adding support for aggregate transformers.

R=rnystrom@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36939

[nweiz, 2014-05-30 01:01:18]

+efortuna, jmesserly for pubspec updates

[Jennifer Messerly, 2014-05-30 01:11:13]

On 2014/05/30 01:01:18, nweiz wrote:
> +efortuna, jmesserly for pubspec updates

I'm not seeing pubspec changes in patchset #2, maybe upload failed?

[nweiz, 2014-05-30 01:17:48]

On 2014/05/30 01:11:13, John Messerly wrote:
> On 2014/05/30 01:01:18, nweiz wrote:
> > +efortuna, jmesserly for pubspec updates
> 
> I'm not seeing pubspec changes in patchset #2, maybe upload failed?

Worse: wrong CL entirely!

[Bob Nystrom, 2014-06-02 23:33:55]

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:376:
.where(fileExists)
Stat-ing every path is a real perf hit. How about just checking for a ".dart"
extension?

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:403:
barback.errors.map((error) {
This is hard to parse. How about moving this out into a local variable:

var errorStream = barback.errors.map(...)

return _withStreamErrors(() { ... }, [errorStream, barback.results]);

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:423:
barback.errors.map((error) {
Ditto.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:425: // fatal.
We don't want transformer errors during transformer loading to be fatal? Why
not?

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/pub_package_provider.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/pub_package_provider.dart:38: // packages
being used at all. The "$pub" sources are still provided, but
"packages" -> "transformers".

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/pub_package_provider.dart:42: if (barback
!= null) {
If barback is null, it's more efficient to use a path asset instead of always
reading it in here.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/preprocess.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/preprocess.dart:73: _emitText();
This means any //> outside of an if will be inserted, right? Is that desired?

[nweiz, 2014-06-03 00:13:11]

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:376:
.where(fileExists)
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> Stat-ing every path is a real perf hit. How about just checking for a ".dart"
> extension?

Done.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:403:
barback.errors.map((error) {
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> This is hard to parse. How about moving this out into a local variable:
> 
> var errorStream = barback.errors.map(...)
> 
> return _withStreamErrors(() { ... }, [errorStream, barback.results]);

Done.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:423:
barback.errors.map((error) {
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> Ditto.

Done.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:425: // fatal.
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> We don't want transformer errors during transformer loading to be fatal? Why
> not?

As (eager) transformers load, they run eagerly on available sources. There may
be some errors in those sources, but those errors should just be displayed; we
only want to stop loading if a source *loaded by a transformer* can't be
transformed, but that will be caught later.

For example, suppose you're loading the async transformer and some unrelated
"in-progress.dart" file has a syntax error. The transformer will print an error
because it can't parse the file, but that doesn't mean you want to stop loading
the transformers.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/pub_package_provider.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/pub_package_provider.dart:38: // packages
being used at all. The "$pub" sources are still provided, but
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> "packages" -> "transformers".

Done.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/pub_package_provider.dart:42: if (barback
!= null) {
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> If barback is null, it's more efficient to use a path asset instead of always
> reading it in here.

Done.

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/preprocess.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/preprocess.dart:73: _emitText();
On 2014/06/02 23:33:55, Bob Nystrom wrote:
> This means any //> outside of an if will be inserted, right? Is that desired?

That was my intention. I think it's pretty reasonable behavior; it allows you to
define things that the preprocessor always produces but the analyzer can't see,
which may be necessary at some point.

[Bob Nystrom, 2014-06-03 20:03:44]

lgtm

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:425: // fatal.
On 2014/06/03 00:13:11, nweiz wrote:
> On 2014/06/02 23:33:55, Bob Nystrom wrote:
> > We don't want transformer errors during transformer loading to be fatal? Why
> > not?
> 
> As (eager) transformers load, they run eagerly on available sources. There may
> be some errors in those sources, but those errors should just be displayed; we
> only want to stop loading if a source *loaded by a transformer* can't be
> transformed, but that will be caught later.
> 
> For example, suppose you're loading the async transformer and some unrelated
> "in-progress.dart" file has a syntax error. The transformer will print an
error
> because it can't parse the file, but that doesn't mean you want to stop
loading
> the transformers.

SGTM. Can you leave a little note to this effect?

[nweiz, 2014-06-03 20:39:43]

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart (right):

https://codereview.chromium.org/300843007/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart:425: // fatal.
On 2014/06/03 20:03:44, Bob Nystrom wrote:
> On 2014/06/03 00:13:11, nweiz wrote:
> > On 2014/06/02 23:33:55, Bob Nystrom wrote:
> > > We don't want transformer errors during transformer loading to be fatal?
Why
> > > not?
> > 
> > As (eager) transformers load, they run eagerly on available sources. There
may
> > be some errors in those sources, but those errors should just be displayed;
we
> > only want to stop loading if a source *loaded by a transformer* can't be
> > transformed, but that will be caught later.
> > 
> > For example, suppose you're loading the async transformer and some unrelated
> > "in-progress.dart" file has a syntax error. The transformer will print an
> error
> > because it can't parse the file, but that doesn't mean you want to stop
> loading
> > the transformers.
> 
> SGTM. Can you leave a little note to this effect?

Done.

[nweiz, 2014-06-03 20:42:35]

Committed patchset #4 manually as r36939 (presubmit successful).