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.

Sign up for GitHub

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

incremental rebuilds with watch #46

Merged
merged 2 commits into from
Feb 16, 2016
Merged

incremental rebuilds with watch #46

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c37e8eb
incremental rebuilds with watch
jakemac53 Feb 12, 2016
41806f1
Fix build_outputs.json for rebuilds, and speed up rebuilds
jakemac53 Feb 16, 2016
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
48 changes: 36 additions & 12 deletions lib/src/generate/build_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:logging/logging.dart';
import 'package:path/path.dart' as path;

import '../asset/asset.dart';
import '../asset/cache.dart';
import '../asset/exceptions.dart';
import '../asset/id.dart';
import '../asset/reader.dart';
Expand Down Expand Up @@ -85,15 +84,21 @@ class BuildImpl {
/// Deletes all previous output files.
Future _deletePreviousOutputs() async {
if (await _reader.hasInput(_buildOutputsId)) {
// Cache file exists, just delete all outputs contained in it.
/// Cache file exists, delete all outputs which don't appear in the
/// [_assetGraph], or are marked as needing an update.
///
/// Removes all files from [_inputsByPackage] regardless of state.
var previousOutputs =
JSON.decode(await _reader.readAsString(_buildOutputsId));
await _writer.delete(_buildOutputsId);
_inputsByPackage[_buildOutputsId.package]?.remove(_buildOutputsId);
await Future.wait(previousOutputs.map((output) {
await Future.wait(previousOutputs.map((output) async {
var outputId = new AssetId.deserialize(output);
_inputsByPackage[outputId.package]?.remove(outputId);
return _writer.delete(outputId);
var node = _assetGraph.get(outputId);
if (node == null || (node as GeneratedAssetNode).needsUpdate) {
Copy link

Choose a reason for hiding this comment

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

Will this always be a safe cast to GeneratedAssetNode? If so, why not type _assetGraph?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should always be a GeneratedAssetNode, otherwise it would be an error (all outputs are GeneratedAssetNodes).

await _writer.delete(outputId);
}
}));
return;
}
Expand Down Expand Up @@ -176,26 +181,29 @@ class BuildImpl {
final outputs = <Asset>[];
int phaseGroupNum = 0;
for (var group in _phaseGroups) {
final groupOutputs = <Asset>[];
/// Collects all the ids for files which are output by this stage. This
/// also includes files which didn't get regenerated because they weren't,
/// dirty unlike [outputs] which only gets files which were explicitly
/// generated in this build.
final groupOutputIds = new Set<AssetId>();
for (var phase in group) {
var inputs = _matchingInputs(phase.inputSets);
for (var builder in phase.builders) {
// TODO(jakemac): Optimize, we can run all the builders in a phase
// at the same time instead of sequentially.
await for (var output
in _runBuilder(builder, inputs, phaseGroupNum)) {
groupOutputs.add(output);
in _runBuilder(builder, inputs, phaseGroupNum, groupOutputIds)) {
outputs.add(output);
}
}
}

/// Once the group is done, add all outputs so they can be used in the next
/// phase.
for (var output in groupOutputs) {
for (var outputId in groupOutputIds) {
_inputsByPackage.putIfAbsent(
output.id.package, () => new Set<AssetId>());
_inputsByPackage[output.id.package].add(output.id);
outputId.package, () => new Set<AssetId>());
_inputsByPackage[outputId.package].add(outputId);
}
phaseGroupNum++;
}
Expand Down Expand Up @@ -250,7 +258,7 @@ class BuildImpl {

/// Runs [builder] with [inputs] as inputs.
Stream<Asset> _runBuilder(Builder builder, Iterable<AssetId> primaryInputs,
int phaseGroupNum) async* {
int phaseGroupNum, Set<AssetId> groupOutputs) async* {
for (var input in primaryInputs) {
var expectedOutputs = builder.declareOutputs(input);

Expand Down Expand Up @@ -278,14 +286,29 @@ class BuildImpl {
/// Skip the build step if none of the outputs need updating.
var skipBuild = !expectedOutputs.any((output) =>
(_assetGraph.get(output) as GeneratedAssetNode).needsUpdate);
if (skipBuild) continue;
if (skipBuild) {
/// If we skip the build, we still need to add the ids as outputs for
/// any files which were output last time, so they can be used by
/// subsequent phases.
for (var output in expectedOutputs) {
if (await _reader.hasInput(output)) {
groupOutputs.add(output);
}
}
continue;
}

var inputAsset = new Asset(input, await _reader.readAsString(input));
var buildStep = new BuildStepImpl(inputAsset, expectedOutputs, _reader,
_writer, _packageGraph.root.name);
await builder.build(buildStep);
await buildStep.complete();

/// Mark all outputs as no longer needing an update.
for (var output in expectedOutputs) {
(_assetGraph.get(output) as GeneratedAssetNode).needsUpdate = false;
}

/// Update the asset graph based on the dependencies discovered.
for (var dependency in buildStep.dependencies) {
var dependencyNode = _assetGraph.addIfAbsent(
Expand All @@ -297,6 +320,7 @@ class BuildImpl {

/// Yield the outputs.
for (var output in buildStep.outputs) {
groupOutputs.add(output.id);
yield output;
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/src/generate/watch_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ class WatchImpl {
void clearNodeAndDeps(AssetId id) {
var node = _assetGraph.get(id);
if (node == null) return;
if (node is GeneratedAssetNode) node.needsUpdate = true;
_assetCache.remove(id);
for (var output in node.outputs) {
clearNodeAndDeps(output);
Expand Down
21 changes: 19 additions & 2 deletions test/common/copy_builder.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,33 @@ import 'dart:async';
import 'package:build/build.dart';

class CopyBuilder implements Builder {
/// If > 0, then multiple copies will be output, using the copy number as an
/// additional extension.
final int numCopies;

/// The file extension to add to files.
final String extension;

/// The package in which to output files.
final String outputPackage;

CopyBuilder({this.numCopies: 1, this.extension: 'copy', this.outputPackage});
/// Copies content from this asset into all files, instead of the primary
/// asset.
final AssetId copyFromAsset;

CopyBuilder(
{this.numCopies: 1,
this.extension: 'copy',
this.outputPackage,
this.copyFromAsset});

Future build(BuildStep buildStep) async {
var ids = declareOutputs(buildStep.input.id);
for (var id in ids) {
buildStep.writeAsString(new Asset(id, buildStep.input.stringContents));
var content = copyFromAsset == null
? buildStep.input.stringContents
: await buildStep.readAsString(copyFromAsset);
buildStep.writeAsString(new Asset(id, content));
}
}

Expand Down
185 changes: 182 additions & 3 deletions test/generate/watch_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ main() {
.notifyWatchers(new WatchEvent(ChangeType.ADD, 'a/web/b.txt'));

result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'a', 'a|web/b.txt.copy': 'b',},
result, writer.assets);
checkOutputs({'a|web/b.txt.copy': 'b',}, result, writer.assets);
// Previous outputs should still exist.
expect(writer.assets[makeAssetId('a|web/a.txt.copy')], 'a');
});

test('rebuilds on deleted files', () async {
Expand All @@ -88,7 +89,185 @@ main() {
.notifyWatchers(new WatchEvent(ChangeType.REMOVE, 'a/web/a.txt'));

result = await nextResult(results);
checkOutputs({'a|web/b.txt.copy': 'b',}, result, writer.assets);

// Shouldn't rebuild anything, no outputs.
checkOutputs({}, result, writer.assets);

// The old output file should no longer exist either.
expect(writer.assets[makeAssetId('a|web/a.txt.copy')], isNull);
// Previous outputs should still exist.
expect(writer.assets[makeAssetId('a|web/b.txt.copy')], 'b');
});
});

group('multiple phases', () {
test('edits propagate through all phases', () async {
var phases = [
[
new Phase([new CopyBuilder()], [new InputSet('a')]),
],
[
new Phase([
new CopyBuilder()
], [
new InputSet('a', filePatterns: ['**/*.copy'])
]),
]
];
var writer = new InMemoryAssetWriter();
var results = [];
startWatch(phases, {'a|web/a.txt': 'a'}, writer).listen(results.add);

var result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'a', 'a|web/a.txt.copy.copy': 'a'},
result, writer.assets);

await writer.writeAsString(makeAsset('a|web/a.txt', 'b'));
FakeWatcher
.notifyWatchers(new WatchEvent(ChangeType.MODIFY, 'a/web/a.txt'));

result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'b', 'a|web/a.txt.copy.copy': 'b'},
result, writer.assets);
});

test('adds propagate through all phases', () async {
var phases = [
[
new Phase([new CopyBuilder()], [new InputSet('a')]),
],
[
new Phase([
new CopyBuilder()
], [
new InputSet('a', filePatterns: ['**/*.copy'])
]),
]
];
var writer = new InMemoryAssetWriter();
var results = [];
startWatch(phases, {'a|web/a.txt': 'a'}, writer).listen(results.add);

var result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'a', 'a|web/a.txt.copy.copy': 'a'},
result, writer.assets);

await writer.writeAsString(makeAsset('a|web/b.txt', 'b'));
FakeWatcher
.notifyWatchers(new WatchEvent(ChangeType.ADD, 'a/web/b.txt'));

result = await nextResult(results);
checkOutputs({'a|web/b.txt.copy': 'b', 'a|web/b.txt.copy.copy': 'b'},
result, writer.assets);
// Previous outputs should still exist.
expect(writer.assets[makeAssetId('a|web/a.txt.copy')], 'a');
expect(writer.assets[makeAssetId('a|web/a.txt.copy.copy')], 'a');
});

test('deletes propagate through all phases', () async {
var phases = [
[
new Phase([new CopyBuilder()], [new InputSet('a')]),
],
[
new Phase([
new CopyBuilder()
], [
new InputSet('a', filePatterns: ['**/*.copy'])
]),
]
];
var writer = new InMemoryAssetWriter();
var results = [];
startWatch(phases, {'a|web/a.txt': 'a', 'a|web/b.txt': 'b'}, writer)
.listen(results.add);

var result = await nextResult(results);
checkOutputs({
'a|web/a.txt.copy': 'a',
'a|web/a.txt.copy.copy': 'a',
'a|web/b.txt.copy': 'b',
'a|web/b.txt.copy.copy': 'b'
}, result, writer.assets);

await writer.delete(makeAssetId('a|web/a.txt'));
FakeWatcher
.notifyWatchers(new WatchEvent(ChangeType.REMOVE, 'a/web/a.txt'));

result = await nextResult(results);
// Shouldn't rebuild anything, no outputs.
checkOutputs({}, result, writer.assets);

// Derived outputs should no longer exist.
expect(writer.assets[makeAssetId('a|web/a.txt.copy')], isNull);
expect(writer.assets[makeAssetId('a|web/a.txt.copy.copy')], isNull);
// Other outputs should still exist.
expect(writer.assets[makeAssetId('a|web/b.txt.copy')], 'b');
expect(writer.assets[makeAssetId('a|web/b.txt.copy.copy')], 'b');
});
});

/// Tests for updates
group('secondary dependency', () {
test('of an output file is edited', () async {
var phases = [
[
new Phase([
new CopyBuilder(copyFromAsset: makeAssetId('a|web/b.txt'))
], [
new InputSet('a', filePatterns: ['web/a.txt'])
]),
],
];
var writer = new InMemoryAssetWriter();
var results = [];
startWatch(phases, {'a|web/a.txt': 'a', 'a|web/b.txt': 'b'}, writer)
.listen(results.add);

var result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'b'}, result, writer.assets);

await writer.writeAsString(makeAsset('a|web/b.txt', 'c'));
FakeWatcher
.notifyWatchers(new WatchEvent(ChangeType.MODIFY, 'a/web/b.txt'));

result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'c'}, result, writer.assets);
});

test(
'of an output which is derived from another generated file is edited',
() async {
var phases = [
[
new Phase([
new CopyBuilder()
], [
new InputSet('a', filePatterns: ['web/a.txt'])
]),
],
[
new Phase([
new CopyBuilder(copyFromAsset: makeAssetId('a|web/b.txt'))
], [
new InputSet('a', filePatterns: ['web/a.txt.copy'])
]),
],
];
var writer = new InMemoryAssetWriter();
var results = [];
startWatch(phases, {'a|web/a.txt': 'a', 'a|web/b.txt': 'b'}, writer)
.listen(results.add);

var result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy': 'a', 'a|web/a.txt.copy.copy': 'b'}, result, writer.assets);

await writer.writeAsString(makeAsset('a|web/b.txt', 'c'));
FakeWatcher
.notifyWatchers(new WatchEvent(ChangeType.MODIFY, 'a/web/b.txt'));

result = await nextResult(results);
checkOutputs({'a|web/a.txt.copy.copy': 'c'}, result, writer.assets);
});
});
});
Expand Down