Delete previous source outputs (#856)

Author: jakemac53

build_runner/lib/src/generate/build_definition.dart

@@ -75,12 +75,12 @@ class _Loader {
     _checkBuildActions();
 
     _logger.info('Initializing inputs');
+
+    var assetGraph = await _tryReadCachedAssetGraph();
     var inputSources = await _findInputSources();
     var cacheDirSources = await _findCacheDirSources();
     var internalSources = await _findInternalSources();
 
-    var assetGraph = await _tryReadCachedAssetGraph();
-
     BuildScriptUpdates buildScriptUpdates;
     if (assetGraph != null) {
       var updates = await logTimedAsync(
@@ -94,7 +94,8 @@ class _Loader {
       if (!_options.skipBuildScriptCheck &&
           buildScriptUpdates.hasBeenUpdated(updates.keys.toSet())) {
         _logger.warning('Invalidating asset graph due to build script update');
-        await _deleteGeneratedDir();
+        var deletedSourceOutputs = await _cleanupOldOutputs(assetGraph);
+        inputSources.removeAll(deletedSourceOutputs);
         assetGraph = null;
         buildScriptUpdates = null;
       }
@@ -191,10 +192,10 @@ class _Loader {
             cachedGraph.buildActionsDigest) {
           _logger.warning(
               'Throwing away cached asset graph because the build actions have '
-              'changed. This could happen as a result of adding a new '
-              'dependency, or if you are using a build script which changes '
-              'the build structure based on command line flags or other '
-              'configuration.');
+              'changed. This most commonly would happen as a result of adding a '
+              'new dependency or updating your dependencies.');
+
+          await _cleanupOldOutputs(cachedGraph);
           return null;
         }
         return cachedGraph;
@@ -209,6 +210,26 @@ class _Loader {
     });
   }
 
+  /// Deletes all the old outputs from [graph] that were written to the source
+  /// tree, and deletes the entire generated directory.
+  Future<Iterable<AssetId>> _cleanupOldOutputs(AssetGraph graph) async {
+    var deletedSources = <AssetId>[];
+    await logTimedAsync(_logger, 'Cleaning up outputs from previous builds.',
+        () async {
+      // Delete all the non-hidden outputs.
+      await Future.wait(graph.outputs.map((id) {
+        var node = graph.get(id) as GeneratedAssetNode;
+        if (node.wasOutput && !node.isHidden) {
+          deletedSources.add(id);
+          return _environment.writer.delete(id);
+        }
+      }).where((v) => v is Future));
+
+      await _deleteGeneratedDir();
+    });
+    return deletedSources;
+  }
+
   /// Updates [assetGraph] based on a the new view of the world.
   ///
   /// Once done, this returns a map of [AssetId] to [ChangeType] for all the

build_runner/lib/src/generate/watch_impl.dart

@@ -21,6 +21,7 @@ import '../asset_graph/node.dart';
 import '../environment/build_environment.dart';
 import '../environment/io_environment.dart';
 import '../environment/overridable_environment.dart';
+import '../logging/logging.dart';
 import '../package_graph/apply_builders.dart';
 import '../package_graph/build_config_overrides.dart';
 import '../package_graph/package_graph.dart';
@@ -191,10 +192,11 @@ class WatchImpl implements BuildState {
     final rootPackagesId = new AssetId(packageGraph.root.name, '.packages');
 
     // Start watching files immediately, before the first build is even started.
-    new PackageGraphWatcher(packageGraph,
-            logger: _logger,
-            watch: (node) =>
-                new PackageNodeWatcher(node, watch: _directoryWatcherFactory))
+    var graphWatcher = new PackageGraphWatcher(packageGraph,
+        logger: _logger,
+        watch: (node) =>
+            new PackageNodeWatcher(node, watch: _directoryWatcherFactory));
+    graphWatcher
         .watch()
         .asyncMap<AssetChange>((change) {
           // Delay any events until the first build is completed.
@@ -243,9 +245,10 @@ class WatchImpl implements BuildState {
     // Schedule the actual first build for the future so we can return the
     // stream synchronously.
     () async {
+      await logTimedAsync(_logger, 'Waiting for all file watchers to be ready',
+          () => graphWatcher.ready);
       originalRootPackagesDigest =
           md5.convert(await environment.reader.readAsBytes(rootPackagesId));
-
       _buildDefinition = await BuildDefinition.prepareWorkspace(
           environment, options, buildActions,
           onDelete: _expectedDeletes.add);

build_runner/lib/src/watcher/graph_watcher.dart

@@ -21,6 +21,11 @@ class PackageGraphWatcher {
   final _NodeWatcherStrategy _strategy;
   final PackageGraph _graph;
 
+  var _readyCompleter = new Completer<Null>();
+  Future<Null> get ready => _readyCompleter.future;
+
+  StreamController<AssetChange> controller;
+
   /// Creates a new watcher for a [PackageGraph].
   ///
   /// May optionally specify a [watch] strategy, otherwise will attempt a
@@ -35,7 +40,7 @@ class PackageGraphWatcher {
 
   /// Returns a stream of records for assets that changed in the package graph.
   Stream<AssetChange> watch() {
-    StreamController<AssetChange> controller;
+    if (controller != null) return controller.stream;
     List<StreamSubscription> subscriptions;
     controller = new StreamController<AssetChange>(
       sync: true,
@@ -50,17 +55,24 @@ class PackageGraphWatcher {
         for (final subscription in subscriptions) {
           subscription.cancel();
         }
+        _readyCompleter = new Completer<Null>();
+        var done = controller.close();
+        controller = null;
+        return done;
       },
     );
     return controller.stream;
   }
 
   List<StreamSubscription> _watch(StreamSink<AssetChange> sink) {
     final subscriptions = <StreamSubscription>[];
+    var allWatchers = <PackageNodeWatcher>[];
     _graph.allPackages.forEach((name, node) {
       final nestedPackages = _nestedPaths(node);
       _logger.fine('Setting up watcher at ${node.path}');
-      subscriptions.add(_strategy(node).watch().listen((event) {
+      var nodeWatcher = _strategy(node);
+      allWatchers.add(nodeWatcher);
+      subscriptions.add(nodeWatcher.watch().listen((event) {
         // TODO: Consider a faster filtering strategy.
         if (nestedPackages.any((path) => event.id.path.startsWith(path))) {
           return;
@@ -70,6 +82,13 @@ class PackageGraphWatcher {
         sink.add(event);
       }));
     });
+    // Asynchronously complete the `_readyCompleter` once all the watchers
+    // are done.
+    () async {
+      await Future
+          .wait(allWatchers.map((nodeWatcher) => nodeWatcher.watcher.ready));
+      _readyCompleter.complete();
+    }();
     return subscriptions;
   }
 

build_runner/test/common/runner_asset_writer_spy.dart

@@ -0,0 +1,23 @@
+// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:build_runner/build_runner.dart';
+import 'package:build/build.dart';
+
+class RunnerAssetWriterSpy extends AssetWriterSpy implements RunnerAssetWriter {
+  final RunnerAssetWriter _delegate;
+
+  final _assetsDeleted = new Set<AssetId>();
+  Iterable<AssetId> get assetsDeleted => _assetsDeleted;
+
+  RunnerAssetWriterSpy(this._delegate) : super(_delegate);
+
+  @override
+  Future delete(AssetId id) {
+    _assetsDeleted.add(id);
+    return _delegate.delete(id);
+  }
+}

build_runner/test/generate/build_definition_test.dart

@@ -26,6 +26,7 @@ import 'package:build_runner/src/util/constants.dart';
 
 import '../common/common.dart';
 import '../common/package_graphs.dart';
+import '../common/runner_asset_writer_spy.dart';
 
 main() {
   final aPackageGraph = buildPackageGraph({rootPackage('a'): []});
@@ -388,5 +389,43 @@ main() {
       expect(originalAssetGraph.buildActionsDigest,
           equals(newAssetGraph.buildActionsDigest));
     });
+
+    test('deletes old source outputs if the build actions change', () async {
+      var buildActions = [
+        new BuildAction(new TestBuilder(), 'a', hideOutput: false)
+      ];
+      var aTxt = new AssetId('a', 'lib/a.txt');
+      await createFile(aTxt.path, 'hello');
+
+      var writerSpy = new RunnerAssetWriterSpy(environment.writer);
+      environment = new OverrideableEnvironment(environment, writer: writerSpy);
+      options = new BuildOptions(environment,
+          packageGraph: options.packageGraph,
+          skipBuildScriptCheck: true,
+          logLevel: Level.OFF);
+
+      var originalAssetGraph = await AssetGraph.build(
+          buildActions,
+          <AssetId>[aTxt].toSet(),
+          new Set(),
+          aPackageGraph,
+          environment.reader);
+
+      var aTxtCopy = new AssetId('a', 'lib/a.txt.copy');
+      // Pretend we already output this without actually running a build.
+      (originalAssetGraph.get(aTxtCopy) as GeneratedAssetNode).wasOutput = true;
+      await createFile(aTxtCopy.path, 'hello');
+
+      await createFile(
+          assetGraphPath, JSON.encode(originalAssetGraph.serialize()));
+
+      buildActions.add(new BuildAction(new TestBuilder(), 'a',
+          targetSources: const InputSet(include: const ['.copy']),
+          hideOutput: true));
+
+      await BuildDefinition.prepareWorkspace(
+          environment, options, buildActions);
+      expect(writerSpy.assetsDeleted, contains(aTxtCopy));
+    });
   });
 }

build_runner/test/watcher/graph_watcher_test.dart

@@ -66,6 +66,38 @@ void main() {
             new AssetChange(new AssetId('b', 'lib/b.dart'), ChangeType.ADD),
           ]));
     });
+
+    test('ready waits for all node watchers to be ready', () async {
+      final graph = buildPackageGraph({
+        rootPackage('a', path: '/g/a'): ['b'],
+        package('b', path: '/g/b'): []
+      });
+      final nodes = {
+        'a': new FakeNodeWatcher(graph['a']),
+        'b': new FakeNodeWatcher(graph['b']),
+        r'$sdk': new FakeNodeWatcher(null),
+      };
+      final watcher = new PackageGraphWatcher(graph, watch: (node) {
+        return nodes[node.name];
+      });
+      // We have to listen in order for `ready` to complete.
+      // ignore: unawaited_futures
+      watcher.watch().drain();
+
+      var done = false;
+      // ignore: unawaited_futures
+      watcher.ready.then((_) => done = true);
+      await new Future.value();
+
+      for (final node in nodes.values) {
+        expect(done, isFalse);
+        node.markReady();
+        await new Future.value();
+      }
+
+      await new Future.value();
+      expect(done, isTrue);
+    });
   });
 }
 
@@ -76,7 +108,10 @@ class FakeNodeWatcher implements PackageNodeWatcher {
   FakeNodeWatcher(this._package);
 
   @override
-  Watcher get watcher => throw new UnimplementedError();
+  Watcher get watcher => _watcher;
+  final _watcher = new _FakeWatcher();
+
+  void markReady() => _watcher._readyCompleter.complete();
 
   void emitAdd(String path) {
     _events.add(
@@ -90,3 +125,18 @@ class FakeNodeWatcher implements PackageNodeWatcher {
   @override
   Stream<AssetChange> watch([_]) => _events.stream;
 }
+
+class _FakeWatcher implements Watcher {
+  @override
+  Stream<WatchEvent> get events => throw new UnimplementedError();
+
+  @override
+  bool get isReady => _readyCompleter.isCompleted;
+
+  @override
+  String get path => throw new UnimplementedError();
+
+  @override
+  Future get ready => _readyCompleter.future;
+  final _readyCompleter = new Completer();
+}

e2e_example/test/common/utils.dart

@@ -122,8 +122,9 @@ Future<Null> stopServer({bool cleanUp}) async {
   _stdOutLines = null;
   _stdErrLines = null;
 
-  if (cleanUp && await _toolDir.exists())
+  if (cleanUp && await _toolDir.exists()) {
     await _toolDir.delete(recursive: true);
+  }
 }
 
 /// Checks whether the current git client is "clean" (no pending changes) for