Expand barback's notion of dirtiness to understand declaredness.

Expand barback's notion of dirtiness to understand declaredness.

Barback is now able to distinguish between a section of the
transformer graph that could produce arbitrary new assets and one for
which all the transforms have declared their outputs, and so can only
produce a subset of those outputs. This allows requests for assets
that we know won't be generated to be short-circuited, and will
eventually allow us to finish providing inputs to aggregate primary
transformers without waiting for the entire previous phase to complete.

R=rnystrom@google.com
BUG=18226
Committed: https://code.google.com/p/dart/source/detail?r=35546

[Bob Nystrom, 2014-04-25 22:02:33]

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/asset_ca...
File pkg/barback/lib/src/asset_cascade.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/asset_ca...
pkg/barback/lib/src/asset_cascade.dart:59: /// The subscription to the
[Phase.onStatusChange] stream of the last [Phase] in
Long line.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
File pkg/barback/lib/src/node_status.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
pkg/barback/lib/src/node_status.dart:26: /// [DECLARED].
This is super confusing. Why isn't it DECLARED?

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/package_gra...
File pkg/barback/test/package_graph/declaring_transformer_test.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/package_gra...
pkg/barback/test/package_graph/declaring_transformer_test.dart:116: 
There's a bunch of code (all the .dirtiest() calls) for handling things like
concurrent declares and applies in transformer groups and phases, but no tests
for them. The behavior there seems complex, so we should cover it better.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/package_gra...
pkg/barback/test/package_graph/declaring_transformer_test.dart:186: // changing
[TransformNode.deferred] value.
Tracking bug?

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
File pkg/barback/test/transformer/check_content_and_rename.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
pkg/barback/test/transformer/check_content_and_rename.dart:22:
this.newExtension, this.newContent);
These would be easier to read at callsites if they were named parameters.

[nweiz, 2014-04-28 21:02:00]

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/asset_ca...
File pkg/barback/lib/src/asset_cascade.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/asset_ca...
pkg/barback/lib/src/asset_cascade.dart:59: /// The subscription to the
[Phase.onStatusChange] stream of the last [Phase] in
On 2014/04/25 22:02:33, Bob Nystrom wrote:
> Long line.

Done.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
File pkg/barback/lib/src/node_status.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
pkg/barback/lib/src/node_status.dart:26: /// [DECLARED].
On 2014/04/25 22:02:33, Bob Nystrom wrote:
> This is super confusing. Why isn't it DECLARED?

Because the node's concrete value isn't being generated, which is one of the
conditions for being marked DECLARED.

Think about it in terms of the package graph as a whole. We want to emit a
BuildResult once all the work is done and nothing else will change without
external input—this is what "DONE" indicates. If some transformers are lazy, we
don't want to wait until they're materialized to emit the BuildResult, we want
to emit it as soon as they're declared.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/package_gra...
File pkg/barback/test/package_graph/declaring_transformer_test.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/package_gra...
pkg/barback/test/package_graph/declaring_transformer_test.dart:116: 
On 2014/04/25 22:02:33, Bob Nystrom wrote:
> There's a bunch of code (all the .dirtiest() calls) for handling things like
> concurrent declares and applies in transformer groups and phases, but no tests
> for them. The behavior there seems complex, so we should cover it better.

Done.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/package_gra...
pkg/barback/test/package_graph/declaring_transformer_test.dart:186: // changing
[TransformNode.deferred] value.
On 2014/04/25 22:02:33, Bob Nystrom wrote:
> Tracking bug?

I already have a patch for this, I don't think it's worth the effort to make a
bug.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
File pkg/barback/test/transformer/check_content_and_rename.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
pkg/barback/test/transformer/check_content_and_rename.dart:22:
this.newExtension, this.newContent);
On 2014/04/25 22:02:33, Bob Nystrom wrote:
> These would be easier to read at callsites if they were named parameters.

I'm not sure what the defaults would be...

[Bob Nystrom, 2014-04-29 18:34:42]

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
File pkg/barback/lib/src/node_status.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
pkg/barback/lib/src/node_status.dart:26: /// [DECLARED].
On 2014/04/28 21:02:00, nweiz wrote:
> On 2014/04/25 22:02:33, Bob Nystrom wrote:
> > This is super confusing. Why isn't it DECLARED?
> 
> Because the node's concrete value isn't being generated, which is one of the
> conditions for being marked DECLARED.
> 
> Think about it in terms of the package graph as a whole. We want to emit a
> BuildResult once all the work is done and nothing else will change without
> external input—this is what "DONE" indicates. If some transformers are lazy,
we
> don't want to wait until they're materialized to emit the BuildResult, we want
> to emit it as soon as they're declared.

That makes sense, but the terminology is misleading. How about:

DONE -> IDLE
DIRTY -> RUNNING
DECLARED -> MATERIALIZING

?

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
File pkg/barback/test/transformer/check_content_and_rename.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
pkg/barback/test/transformer/check_content_and_rename.dart:22:
this.newExtension, this.newContent);
On 2014/04/28 21:02:00, nweiz wrote:
> On 2014/04/25 22:02:33, Bob Nystrom wrote:
> > These would be easier to read at callsites if they were named parameters.
> 
> I'm not sure what the defaults would be...

I would treat them as non-optional and assert the args are non-null. It's really
just to get the parameter name there, similar to how we handle boolean params.

[nweiz, 2014-04-29 20:11:26]

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
File pkg/barback/lib/src/node_status.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/lib/src/node_sta...
pkg/barback/lib/src/node_status.dart:26: /// [DECLARED].
On 2014/04/29 18:34:43, Bob Nystrom wrote:
> On 2014/04/28 21:02:00, nweiz wrote:
> > On 2014/04/25 22:02:33, Bob Nystrom wrote:
> > > This is super confusing. Why isn't it DECLARED?
> > 
> > Because the node's concrete value isn't being generated, which is one of the
> > conditions for being marked DECLARED.
> > 
> > Think about it in terms of the package graph as a whole. We want to emit a
> > BuildResult once all the work is done and nothing else will change without
> > external input—this is what "DONE" indicates. If some transformers are lazy,
> we
> > don't want to wait until they're materialized to emit the BuildResult, we
want
> > to emit it as soon as they're declared.
> 
> That makes sense, but the terminology is misleading. How about:
> 
> DONE -> IDLE
> DIRTY -> RUNNING
> DECLARED -> MATERIALIZING
> 
> ?

Done.

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
File pkg/barback/test/transformer/check_content_and_rename.dart (right):

https://codereview.chromium.org/255483002/diff/1/pkg/barback/test/transformer...
pkg/barback/test/transformer/check_content_and_rename.dart:22:
this.newExtension, this.newContent);
On 2014/04/29 18:34:43, Bob Nystrom wrote:
> On 2014/04/28 21:02:00, nweiz wrote:
> > On 2014/04/25 22:02:33, Bob Nystrom wrote:
> > > These would be easier to read at callsites if they were named parameters.
> > 
> > I'm not sure what the defaults would be...
> 
> I would treat them as non-optional and assert the args are non-null. It's
really
> just to get the parameter name there, similar to how we handle boolean params.

Done.

[Bob Nystrom, 2014-04-29 21:01:17]

LGTM!

[nweiz, 2014-04-29 21:32:49]

Committed patchset #2 manually as r35546 (presubmit successful).