Fix build_outputs.json for rebuilds, and speed up rebuilds

Author: jakemac53

lib/src/asset_graph/graph.dart

@@ -36,6 +36,9 @@ class AssetGraph {
   /// Removes [node] from the graph.
   AssetNode remove(AssetId id) => _nodesById.remove(id);
 
+  /// Gets all nodes in the graph.
+  Iterable<AssetNode> get allNodes => _nodesById.values;
+
   @override
   toString() => _nodesById.values.toList().toString();
 }

lib/src/asset_graph/node.dart

@@ -34,8 +34,11 @@ class GeneratedAssetNode extends AssetNode {
   /// Whether or not this asset needs to be updated.
   bool needsUpdate;
 
+  /// Whether the asset was actually output.
+  bool wasOutput;
+
   GeneratedAssetNode(this.builder, this.primaryInput, this.generatingPhaseGroup,
-      this.needsUpdate, AssetId id)
+      this.needsUpdate, this.wasOutput, AssetId id)
       : super(id);
 
   @override

lib/src/generate/build_impl.dart

@@ -34,8 +34,8 @@ class BuildImpl {
   bool _buildRunning = false;
   final _logger = new Logger('Build');
 
-  BuildImpl(this._assetGraph, this._reader, this._writer,
-      this._packageGraph, this._phaseGroups);
+  BuildImpl(this._assetGraph, this._reader, this._writer, this._packageGraph,
+      this._phaseGroups);
 
   /// Runs a build
   ///
@@ -61,11 +61,13 @@ class BuildImpl {
       _logger.info('Running build phases');
       var result = await _runPhases();
 
-      // Write out the new build_outputs file.
+      /// Write out the new build_outputs file.
+      var allOuputs = _assetGraph.allNodes
+          .where((node) => node is GeneratedAssetNode && node.wasOutput);
       var buildOutputsAsset = new Asset(
           _buildOutputsId,
           JSON.encode(
-              result.outputs.map((output) => output.id.serialize()).toList()));
+              allOuputs.map((output) => output.id.serialize()).toList()));
       await _writer.writeAsString(buildOutputsAsset);
 
       return result;
@@ -280,7 +282,7 @@ class BuildImpl {
         _assetGraph.addIfAbsent(
             output,
             () => new GeneratedAssetNode(
-                builder, input, phaseGroupNum, true, output));
+                builder, input, phaseGroupNum, true, false, output));
       }
 
       /// Skip the build step if none of the outputs need updating.
@@ -291,7 +293,7 @@ class BuildImpl {
         /// 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)) {
+          if ((_assetGraph.get(output) as GeneratedAssetNode).wasOutput) {
             groupOutputs.add(output);
           }
         }
@@ -304,22 +306,27 @@ class BuildImpl {
       await builder.build(buildStep);
       await buildStep.complete();
 
-      /// Mark all outputs as no longer needing an update.
+      /// Mark all outputs as no longer needing an update, and mark `wasOutput`
+      /// as `false` for now (this will get reset to true later one).
       for (var output in expectedOutputs) {
-        (_assetGraph.get(output) as GeneratedAssetNode).needsUpdate = false;
+        (_assetGraph.get(output) as GeneratedAssetNode)
+          ..needsUpdate = false
+          ..wasOutput = false;
       }
 
       /// Update the asset graph based on the dependencies discovered.
       for (var dependency in buildStep.dependencies) {
         var dependencyNode = _assetGraph.addIfAbsent(
             dependency, () => new AssetNode(dependency));
 
-        /// We care about all [expectedOutputs], not just real outputs.
+        /// We care about all [expectedOutputs], not just real outputs. Updates
+        /// to dependencies may cause a file to be output which wasn't before.
         dependencyNode.outputs.addAll(expectedOutputs);
       }
 
       /// Yield the outputs.
       for (var output in buildStep.outputs) {
+        (_assetGraph.get(output.id) as GeneratedAssetNode).wasOutput = true;
         groupOutputs.add(output.id);
         yield output;
       }

lib/src/generate/watch_impl.dart

@@ -111,7 +111,7 @@ class WatchImpl {
     _runningWatch = true;
     _resultStreamController = new StreamController<BuildResult>();
     _nextBuildScheduled = false;
-    var updatedInputs = new Set<AssetId>();
+    var updatedInputs = new Map<AssetId, ChangeType>();
 
     doBuild([bool force = false]) {
       // Don't schedule more builds if we are turning down.
@@ -128,24 +128,32 @@ class WatchImpl {
 
       /// Remove any updates that were generated outputs or otherwise not
       /// interesting.
-      updatedInputs.removeWhere(_shouldSkipInput);
+      var updatesToRemove = updatedInputs.keys.where(_shouldSkipInput).toList();
+      updatesToRemove.forEach(updatedInputs.remove);
       if (updatedInputs.isEmpty && !force) {
         return;
       }
 
       _logger.info('Preparing for next build');
       _logger.info('Clearing cache for invalidated assets');
-      void clearNodeAndDeps(AssetId id) {
+      void clearNodeAndDeps(AssetId id, ChangeType rootChangeType) {
         var node = _assetGraph.get(id);
         if (node == null) return;
-        if (node is GeneratedAssetNode) node.needsUpdate = true;
+        if (node is GeneratedAssetNode) {
+          node.needsUpdate = true;
+        }
         _assetCache.remove(id);
         for (var output in node.outputs) {
-          clearNodeAndDeps(output);
+          clearNodeAndDeps(output, rootChangeType);
+        }
+
+        /// For deletes, prune the graph.
+        if (rootChangeType == ChangeType.REMOVE) {
+          _assetGraph.remove(id);
         }
       }
-      for (var input in updatedInputs) {
-        clearNodeAndDeps(input);
+      for (var input in updatedInputs.keys) {
+        clearNodeAndDeps(input, updatedInputs[input]);
       }
       updatedInputs.clear();
 
@@ -181,7 +189,7 @@ class WatchImpl {
       _allListeners.add(watcher.events.listen((WatchEvent e) {
         _logger.fine('Got WatchEvent for path ${e.path}');
         var id = new AssetId(package.name, path.normalize(e.path));
-        updatedInputs.add(id);
+        updatedInputs[id] = e.type;
         scheduleBuild();
       }));
     }

test/asset_graph/graph_test.dart

@@ -34,17 +34,19 @@ main() {
       return node;
     }
 
-    test('add, contains, get', () {
+    test('add, contains, get, allNodes', () {
+      var expectedNodes = [];
       for (int i = 0; i < 5; i++) {
-        testAddNode();
+        expectedNodes.add(testAddNode());
       }
+      expect(graph.allNodes, unorderedEquals(expectedNodes));
     });
 
     test('addIfAbsent', () {
       var node = makeAssetNode();
       expect(graph.addIfAbsent(node.id, () => node), same(node));
       expect(graph.contains(node.id), isTrue);
-      
+
       var otherNode = new AssetNode(node.id);
       expect(graph.addIfAbsent(otherNode.id, () => otherNode), same(node));
       expect(graph.contains(otherNode.id), isTrue);

test/generate/watch_test.dart

@@ -98,6 +98,35 @@ main() {
         // Previous outputs should still exist.
         expect(writer.assets[makeAssetId('a|web/b.txt.copy')], 'b');
       });
+
+      test('rebuilds properly update build_outputs.json', () async {
+        var phases = [
+          [
+            new Phase([new CopyBuilder()], [new InputSet('a')]),
+          ]
+        ];
+        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/b.txt.copy': 'b',},
+            result, writer.assets);
+
+        await writer.writeAsString(makeAsset('a|web/c.txt', 'c'));
+        FakeWatcher
+            .notifyWatchers(new WatchEvent(ChangeType.ADD, 'a/web/c.txt'));
+        await writer.delete(makeAssetId('a|web/a.txt'));
+        FakeWatcher
+            .notifyWatchers(new WatchEvent(ChangeType.REMOVE, 'a/web/a.txt'));
+
+        result = await nextResult(results);
+        checkOutputs({'a|web/c.txt.copy': 'c'}, result, writer.assets);
+
+        expect(writer.assets[makeAssetId('a|.build/build_outputs.json')],
+            '[["a","web/b.txt.copy"],["a","web/c.txt.copy"]]');
+      });
     });
 
     group('multiple phases', () {
@@ -260,7 +289,8 @@ main() {
             .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);
+        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