Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track inputs for GeneratedAssetNodes #584

Merged
merged 4 commits into from
Nov 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build_runner/lib/src/asset/reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class SingleStepReader implements AssetReader {
SingleStepReader(this._delegate, this._assetGraph, this._phaseNumber,
this._primaryPackage, this._runPhaseForInput);

Iterable<AssetId> get assetsRead => _assetsRead;
Set<AssetId> get assetsRead => _assetsRead;

/// The [Glob]s which have been searched with [findAssets].
///
Expand Down
12 changes: 8 additions & 4 deletions build_runner/lib/src/asset_graph/graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ class AssetGraph {
for (var output in node.primaryOutputs) {
_remove(output, removedIds: removedIds);
}
for (var output in node.outputs) {
var generatedNode = get(output) as GeneratedAssetNode;
if (generatedNode != null) {
generatedNode.inputs.remove(id);
}
}
_nodesByPackage[id.package].remove(id.path);
return removedIds;
}
Expand Down Expand Up @@ -145,12 +151,10 @@ class AssetGraph {

// Builds up `idsToDelete` and `idsToRemove` by recursively invalidating
// the outputs of `id`.
void clearNodeAndDeps(AssetId id, ChangeType rootChangeType,
{bool rootIsSource}) {
void clearNodeAndDeps(AssetId id, ChangeType rootChangeType) {
var node = this.get(id);
if (node == null) return;
if (!invalidatedIds.add(id)) return;
rootIsSource ??= node is SourceAssetNode;

if (node is GeneratedAssetNode) {
idsToDelete.add(id);
Expand All @@ -161,7 +165,7 @@ class AssetGraph {

// Update all outputs of this asset as well.
for (var output in node.outputs) {
clearNodeAndDeps(output, rootChangeType, rootIsSource: rootIsSource);
clearNodeAndDeps(output, rootChangeType);
}
}

Expand Down
7 changes: 6 additions & 1 deletion build_runner/lib/src/asset_graph/node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,15 @@ class GeneratedAssetNode extends AssetNode {
/// Any new or deleted files matching this glob should invalidate this node.
Set<Glob> globs;

/// All the inputs that were read when generating this asset, or deciding not
/// to generate it.
final Set<AssetId> inputs;

GeneratedAssetNode(this.phaseNumber, this.primaryInput, this.needsUpdate,
this.wasOutput, AssetId id,
{Digest lastKnownDigest, Set<Glob> globs})
{Digest lastKnownDigest, Set<Glob> globs, Iterable<AssetId> inputs})
: this.globs = globs ?? new Set<Glob>(),
this.inputs = inputs?.toSet() ?? new Set<AssetId>(),
super(id, lastKnownDigest: lastKnownDigest);

@override
Expand Down
13 changes: 13 additions & 0 deletions build_runner/lib/src/asset_graph/serialization.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,23 @@ class _AssetGraphDeserializer {
new AssetId(descriptor[1] as String, descriptor[2] as String);
}

// Read in all the nodes and their outputs.
//
// Note that this does not read in the inputs of generated nodes.
for (var serializedItem in _serializedGraph['nodes']) {
graph._add(_deserializeAssetNode(serializedItem as List));
}

// Update the inputs of all generated nodes based on the outputs of the
// current nodes.
for (var node in graph.allNodes) {
for (var output in node.outputs) {
var generatedNode = graph.get(output) as GeneratedAssetNode;
assert(generatedNode != null, 'Asset Graph is missing $output');
generatedNode.inputs.add(node.id);
}
}

return graph;
}

Expand Down
37 changes: 27 additions & 10 deletions build_runner/lib/src/generate/build_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -411,12 +411,38 @@ class BuildImpl {
SingleStepReader reader, AssetWriterSpy writer) async {
// Reset the state for each output, setting `wasOutput` to false for now
// (will set to true in the next loop for written assets).
//
// Also updates the `inputs` set for each output, and the `outputs` sets for
// all inputs.
for (var output in declaredOutputs) {
(_assetGraph.get(output) as GeneratedAssetNode)
var node = _assetGraph.get(output) as GeneratedAssetNode;
node
..needsUpdate = false
..wasOutput = false
..lastKnownDigest = null
..globs = reader.globsRan.toSet();

// Update dependencies that don't exist any more.
var removedInputs = node.inputs.difference(reader.assetsRead);
node.inputs.removeAll(removedInputs);
for (var input in removedInputs) {
// TODO: special type of dependency here? This means the primary input
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's fine to always treat the primary input like it was read. The edge cases shouldn't break anything, at worst builds are slightly less optimal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya, thats pretty much my same line of thinking for now.

// was never actually read.
if (input == node.primaryInput) continue;

var inputNode = _assetGraph.get(input);
assert(inputNode != null, 'Asset Graph is missing $input');
inputNode.outputs.remove(output);
}

// Add the new dependencies.
var newInputs = reader.assetsRead.difference(node.inputs);
node.inputs.addAll(newInputs);
for (var input in newInputs) {
var inputNode = _assetGraph.get(input);
assert(inputNode != null, 'Asset Graph is missing $input');
inputNode.outputs.add(output);
}
}

// Mark the actual outputs as output.
Expand All @@ -425,15 +451,6 @@ class BuildImpl {
..wasOutput = true
..lastKnownDigest = await _reader.digest(output);
}));

// Update the asset graph based on the dependencies discovered.
for (var dependency in reader.assetsRead) {
var dependencyNode = _assetGraph.get(dependency);
assert(dependencyNode != null, 'Asset Graph is missing $dependency');
// We care about all builderOutputs, not just real outputs. Updates
// to dependencies may cause a file to be output which wasn't before.
dependencyNode.outputs.addAll(declaredOutputs);
}
}

Future _delete(AssetId id) {
Expand Down
2 changes: 2 additions & 0 deletions build_runner/test/asset_graph/graph_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ void main() {
var syntheticNode = new SyntheticAssetNode(makeAssetId());
syntheticNode.outputs.add(generatedNode.id);

generatedNode.inputs.addAll([node.id, syntheticNode.id]);

graph.add(syntheticNode);
graph.add(generatedNode);
}
Expand Down
8 changes: 8 additions & 0 deletions build_runner/test/common/matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ class _AssetGraphMatcher extends Matcher {
];
matches = false;
}
if (!unorderedEquals(node.inputs)
.matches(expectedNode.inputs, null)) {
matchState['Inputs of ${node.id}'] = [
node.inputs,
expectedNode.inputs
];
matches = false;
}
} else {
matchState['Type of ${node.id}'] = [
node.runtimeType,
Expand Down
Loading