Encapsulate graph and build script updates in BuildSeries. (#4223)

Author: davidmorgan

build_runner/lib/src/build/build_series.dart

@@ -8,13 +8,17 @@ import 'package:build/build.dart';
 import 'package:built_collection/built_collection.dart';
 import 'package:watcher/watcher.dart';
 
-import '../bootstrap/build_script_updates.dart';
 import '../build_plan/build_directory.dart';
 import '../build_plan/build_filter.dart';
 import '../build_plan/build_plan.dart';
+import '../commands/watch/asset_change.dart';
+import '../constants.dart';
+import '../io/asset_tracker.dart';
+import '../io/build_output_reader.dart';
 import '../io/filesystem_cache.dart';
 import '../io/reader_writer.dart';
 import 'asset_graph/graph.dart';
+import 'asset_graph/node.dart';
 import 'build.dart';
 import 'build_result.dart';
 
@@ -31,46 +35,126 @@ import 'build_result.dart';
 /// this serialized state is not actually used: the `AssetGraph` instance
 /// already in memory is used directly.
 class BuildSeries {
-  final BuildPlan buildPlan;
+  final BuildPlan _buildPlan;
 
-  final AssetGraph assetGraph;
-  final BuildScriptUpdates? buildScriptUpdates;
+  final AssetGraph _assetGraph;
 
-  final ReaderWriter readerWriter;
-  final ResourceManager resourceManager = ResourceManager();
+  final ReaderWriter _readerWriter;
+  final ResourceManager _resourceManager = ResourceManager();
 
   /// For the first build only, updates from the previous serialized build
   /// state.
   ///
   /// Null after the first build, or if there was no serialized build state, or
   /// if the serialized build state was discarded.
-  BuiltMap<AssetId, ChangeType>? updatesFromLoad;
+  BuiltMap<AssetId, ChangeType>? _updatesFromLoad;
 
   final StreamController<BuildResult> _buildResultsController =
       StreamController.broadcast();
 
   /// Whether the next build is the first build.
   bool firstBuild = true;
 
-  Future<void> beforeExit() => resourceManager.beforeExit();
-
-  BuildSeries._(
-    this.buildPlan,
-    this.assetGraph,
-    this.buildScriptUpdates,
-    this.updatesFromLoad,
-  ) : readerWriter = buildPlan.readerWriter.copyWith(
-        generatedAssetHider: assetGraph,
-        cache:
-            buildPlan.buildOptions.enableLowResourcesMode
-                ? const PassthroughFilesystemCache()
-                : InMemoryFilesystemCache(),
-      );
+  BuildSeries._({
+    required BuildPlan buildPlan,
+    required AssetGraph assetGraph,
+    required ReaderWriter readerWriter,
+    required BuiltMap<AssetId, ChangeType>? updatesFromLoad,
+  }) : _buildPlan = buildPlan,
+       _assetGraph = assetGraph,
+       _readerWriter = readerWriter,
+       _updatesFromLoad = updatesFromLoad;
+
+  factory BuildSeries(BuildPlan buildPlan) {
+    final assetGraph = buildPlan.takeAssetGraph();
+    final readerWriter = buildPlan.readerWriter.copyWith(
+      generatedAssetHider: assetGraph,
+      cache:
+          buildPlan.buildOptions.enableLowResourcesMode
+              ? const PassthroughFilesystemCache()
+              : InMemoryFilesystemCache(),
+    );
+    return BuildSeries._(
+      buildPlan: buildPlan,
+      assetGraph: assetGraph,
+      readerWriter: readerWriter,
+      updatesFromLoad: buildPlan.updates,
+    );
+  }
 
   /// Broadcast stream of build results.
   Stream<BuildResult> get buildResults => _buildResultsController.stream;
   Future<BuildResult>? _currentBuildResult;
 
+  bool _hasBuildScriptChanged(Set<AssetId> changes) {
+    if (_buildPlan.buildOptions.skipBuildScriptCheck) return false;
+    if (_buildPlan.buildScriptUpdates == null) return true;
+    return _buildPlan.buildScriptUpdates!.hasBeenUpdated(changes);
+  }
+
+  /// Returns whether [change] might trigger a build.
+  ///
+  /// Pass expected deletes in [expectedDeletes]. Expected deletes do not
+  /// trigger a build. A delete that matches is removed from the set.
+  Future<bool> shouldProcess(
+    AssetChange change,
+    Set<AssetId> expectedDeletes,
+  ) async {
+    // Ignore any expected delete once.
+    if (change.type == ChangeType.REMOVE && expectedDeletes.remove(change.id)) {
+      return false;
+    }
+
+    final node =
+        _assetGraph.contains(change.id) ? _assetGraph.get(change.id) : null;
+
+    // Changes to files that are not currently part of the build.
+    if (node == null) {
+      // Ignore under `.dart_tool/build`.
+      if (change.id.path.startsWith(cacheDir)) return false;
+
+      // Ignore modifications and deletes.
+      if (change.type != ChangeType.ADD) return false;
+
+      // It's an add: return whether it's a new input.
+      return _buildPlan.targetGraph.anyMatchesAsset(change.id);
+    }
+
+    // Changes to files that are part of the build.
+
+    // If not copying to a merged output directory, ignore changes to files with
+    // no outputs.
+    if (!_buildPlan.buildOptions.anyMergedOutputDirectory &&
+        !node.changesRequireRebuild) {
+      return false;
+    }
+
+    // Ignore creation or modification of outputs.
+    if (node.type == NodeType.generated && change.type != ChangeType.REMOVE) {
+      return false;
+    }
+
+    // For modifications, confirm that the content actually changed.
+    if (change.type == ChangeType.MODIFY) {
+      _readerWriter.cache.invalidate([change.id]);
+      final newDigest = await _readerWriter.digest(change.id);
+      return node.digest != newDigest;
+    }
+
+    // It's an add of "missing source" node or a deletion of an input.
+    return true;
+  }
+
+  Future<List<WatchEvent>> checkForChanges() async {
+    final updates = await AssetTracker(
+      _buildPlan.readerWriter,
+      _buildPlan.targetGraph,
+    ).collectChanges(_assetGraph);
+    return List.of(
+      updates.entries.map((entry) => WatchEvent(entry.value, '${entry.key}')),
+    );
+  }
+
   /// If a build is running, the build result when it's done.
   ///
   /// If no build has ever run, returns the first build result when it's
@@ -93,27 +177,39 @@ class BuildSeries {
     BuiltSet<BuildDirectory>? buildDirs,
     BuiltSet<BuildFilter>? buildFilters,
   }) async {
-    buildDirs ??= buildPlan.buildOptions.buildDirs;
-    buildFilters ??= buildPlan.buildOptions.buildFilters;
+    if (_hasBuildScriptChanged(updates.keys.toSet())) {
+      return BuildResult(
+        status: BuildStatus.failure,
+        failureType: FailureType.buildScriptChanged,
+        buildOutputReader: BuildOutputReader(
+          buildPlan: _buildPlan,
+          readerWriter: _readerWriter,
+          assetGraph: _assetGraph,
+        ),
+      );
+    }
+
+    buildDirs ??= _buildPlan.buildOptions.buildDirs;
+    buildFilters ??= _buildPlan.buildOptions.buildFilters;
     if (firstBuild) {
-      if (updatesFromLoad != null) {
-        updates = updatesFromLoad!.toMap()..addAll(updates);
-        updatesFromLoad = null;
+      if (_updatesFromLoad != null) {
+        updates = _updatesFromLoad!.toMap()..addAll(updates);
+        _updatesFromLoad = null;
       }
     } else {
-      if (updatesFromLoad != null) {
+      if (_updatesFromLoad != null) {
         throw StateError('Only first build can have updates from load.');
       }
     }
 
     final build = Build(
-      buildPlan: buildPlan.copyWith(
+      buildPlan: _buildPlan.copyWith(
         buildDirs: buildDirs,
         buildFilters: buildFilters,
       ),
-      assetGraph: assetGraph,
-      readerWriter: readerWriter,
-      resourceManager: resourceManager,
+      assetGraph: _assetGraph,
+      readerWriter: _readerWriter,
+      resourceManager: _resourceManager,
     );
     if (firstBuild) firstBuild = false;
 
@@ -123,14 +219,5 @@ class BuildSeries {
     return result;
   }
 
-  static Future<BuildSeries> create({required BuildPlan buildPlan}) async {
-    final assetGraph = buildPlan.takeAssetGraph();
-    final build = BuildSeries._(
-      buildPlan,
-      assetGraph,
-      buildPlan.buildScriptUpdates,
-      buildPlan.updates,
-    );
-    return build;
-  }
+  Future<void> beforeExit() => _resourceManager.beforeExit();
 }

build_runner/lib/src/build_plan/build_options.dart

@@ -30,6 +30,10 @@ class BuildOptions {
   final bool trackPerformance;
   final bool verbose;
 
+  late final bool anyMergedOutputDirectory = buildDirs.any(
+    (target) => target.outputLocation?.path.isNotEmpty ?? false,
+  );
+
   BuildOptions({
     required this.buildDirs,
     required this.builderConfigOverrides,

build_runner/lib/src/commands/build_command.dart

@@ -56,9 +56,9 @@ class BuildCommand implements BuildRunnerCommand {
       return BuildResult.buildScriptChanged();
     }
 
-    final build = await BuildSeries.create(buildPlan: buildPlan);
-    final result = await build.run({});
-    await build.beforeExit();
+    final buildSeries = BuildSeries(buildPlan);
+    final result = await buildSeries.run({});
+    await buildSeries.beforeExit();
     return result;
   }
 }

build_runner/lib/src/commands/daemon/change_providers.dart

@@ -7,9 +7,6 @@ import 'dart:async';
 import 'package:build_daemon/change_provider.dart';
 import 'package:watcher/watcher.dart' show WatchEvent;
 
-import '../../build/asset_graph/graph.dart';
-import '../../io/asset_tracker.dart';
-
 /// Continually updates the [changes] stream as watch events are seen on the
 /// input stream.
 class AutoChangeProviderImpl implements AutoChangeProvider {
@@ -22,16 +19,12 @@ class AutoChangeProviderImpl implements AutoChangeProvider {
 /// Computes changes with a file scan when requested by a call to
 /// [collectChanges].
 class ManualChangeProviderImpl implements ManualChangeProvider {
-  final AssetGraph _assetGraph;
-  final AssetTracker _assetTracker;
+  final Future<List<WatchEvent>> Function() _function;
 
-  ManualChangeProviderImpl(this._assetTracker, this._assetGraph);
+  ManualChangeProviderImpl(this._function);
 
   @override
   Future<List<WatchEvent>> collectChanges() async {
-    final updates = await _assetTracker.collectChanges(_assetGraph);
-    return List.of(
-      updates.entries.map((entry) => WatchEvent(entry.value, '${entry.key}')),
-    );
+    return _function();
   }
 }

build_runner/lib/src/commands/daemon/daemon_builder.dart

@@ -20,11 +20,9 @@ import '../../build/build_series.dart';
 import '../../build_plan/build_directory.dart';
 import '../../build_plan/build_filter.dart';
 import '../../build_plan/build_plan.dart';
-import '../../io/asset_tracker.dart' show AssetTracker;
 import '../../logging/build_log.dart';
 import '../daemon_options.dart';
 import '../watch/asset_change.dart';
-import '../watch/change_filter.dart';
 import '../watch/collect_changes.dart';
 import '../watch/graph_watcher.dart';
 import '../watch/node_watcher.dart';
@@ -76,15 +74,6 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
             )
             .toList();
 
-    if (!_buildPlan.buildOptions.skipBuildScriptCheck &&
-        buildSeries.buildScriptUpdates!.hasBeenUpdated(
-          changes.map<AssetId>((change) => change.id).toSet(),
-        )) {
-      if (!_buildScriptUpdateCompleter.isCompleted) {
-        _buildScriptUpdateCompleter.complete();
-      }
-      return;
-    }
     final targetNames = targets.map((t) => t.target).toSet();
     _logMessage(Level.INFO, 'About to build ${targetNames.toList()}...');
     _signalStart(targetNames);
@@ -124,6 +113,12 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
         buildDirs: buildDirs.build(),
         buildFilters: buildFilters.build(),
       );
+      if (result.failureType == core.FailureType.buildScriptChanged) {
+        if (!_buildScriptUpdateCompleter.isCompleted) {
+          _buildScriptUpdateCompleter.complete();
+        }
+        return;
+      }
       final interestedInOutputs = targets.any(
         (e) => e is DefaultBuildTarget && e.reportChangedAssets,
       );
@@ -230,7 +225,7 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
       ),
     );
 
-    final buildSeries = await BuildSeries.create(buildPlan: buildPlan);
+    final buildSeries = BuildSeries(buildPlan);
 
     // Only actually used for the AutoChangeProvider.
     Stream<List<WatchEvent>> graphEvents() => PackageGraphWatcher(
@@ -239,15 +234,7 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
         )
         .watch()
         .asyncWhere(
-          (change) => shouldProcess(
-            change,
-            buildSeries.assetGraph,
-            buildPlan.targetGraph,
-            // Assume we will create an outputDir.
-            true,
-            expectedDeletes,
-            buildPlan.readerWriter,
-          ),
+          (change) => buildSeries.shouldProcess(change, expectedDeletes),
         )
         .map((data) => WatchEvent(data.type, '${data.id}'))
         .debounceBuffer(
@@ -258,10 +245,7 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
     final changeProvider =
         daemonOptions.buildMode == BuildMode.Auto
             ? AutoChangeProviderImpl(graphEvents())
-            : ManualChangeProviderImpl(
-              AssetTracker(buildPlan.readerWriter, buildPlan.targetGraph),
-              buildSeries.assetGraph,
-            );
+            : ManualChangeProviderImpl(buildSeries.checkForChanges);
 
     return BuildRunnerDaemonBuilder._(
       buildPlan,

build_runner/lib/src/commands/serve/server.dart

@@ -63,12 +63,6 @@ class ServeHandler {
         'Only top level directories such as `web` or `test` can be served, got',
       );
     }
-    _watcher.currentBuildResult.then((_) {
-      // If the first build fails with a handled exception, we might not have
-      // an asset graph and can't do this check.
-      if (_watcher.assetGraph == null) return;
-      _warnForEmptyDirectory(rootDir);
-    });
     var cascade = shelf.Cascade();
     if (liveReload) {
       cascade = cascade.add(_webSocketHandler.createHandlerByRootDir(rootDir));
@@ -136,18 +130,6 @@ class ServeHandler {
       headers: {HttpHeaders.contentTypeHeader: 'application/json'},
     );
   }
-
-  void _warnForEmptyDirectory(String rootDir) {
-    if (!_watcher.assetGraph!
-        .packageNodes(_watcher.packageGraph.root.name)
-        .any((n) => n.id.path.startsWith('$rootDir/'))) {
-      buildLog.warning(
-        'Requested a server for `$rootDir` but this directory '
-        'has no assets in the build. You may need to add some sources or '
-        'include this directory in some target in your `build.yaml`.',
-      );
-    }
-  }
 }
 
 /// Class that manages web socket connection handler to inform clients about