Centralize file update handling, handle concurrent deletes.

build_runner/CHANGELOG.md

@@ -1,6 +1,9 @@
 ## 2.14.0-wip
 
 - Add OSC 8 hyperlinks for logged input paths.
+- Better handling of deletions of files during the build: if the file is not
+  needed ignore the deletion, if it's needed try to use the cached version,
+  as a last resort restart the build.
 - Bug fix: small correctness fix in input tracking.
 
 ## 2.13.1

build_runner/lib/src/bootstrap/bootstrapper.dart

@@ -6,7 +6,6 @@ import 'dart:async';
 import 'dart:io';
 
 import 'package:built_collection/built_collection.dart';
-import 'package:io/io.dart';
 
 import '../exceptions.dart';
 import '../internal.dart';
@@ -43,8 +42,10 @@ class Bootstrapper {
 
   /// Generates the entrypoint script, compiles it and runs it with [arguments].
   ///
-  /// If the entrypoint script exits with `ExitCode.tempFail` then regenerates
-  /// it and launches it again with the same arguments.
+  /// If the entrypoint script exits with
+  /// `ChildProcess.recompileBuildersExitCode` or
+  /// `ChildProcess.assetDeletedExitCode` then regenerates it and launches it
+  /// again with the same arguments.
   ///
   /// If the entrypoint exits with any other exit code, returns it.
   ///
@@ -133,11 +134,14 @@ class Bootstrapper {
       buildProcessState.deserializeAndSet(result.message);
       final exitCode = result.exitCode;
 
-      if (exitCode != ExitCode.tempFail.code) {
+      if (exitCode != ChildProcess.recompileBuildersExitCode &&
+          exitCode != ChildProcess.assetDeletedExitCode) {
         return exitCode;
       }
 
-      buildLog.nextBuild(recompilingBuilders: true);
+      buildLog.nextBuild(
+        recompilingBuilders: exitCode == ChildProcess.recompileBuildersExitCode,
+      );
     }
   }
 

build_runner/lib/src/bootstrap/processes.dart

@@ -6,11 +6,14 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:build/build.dart';
+import 'package:io/io.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 
 import '../build_plan/builder_factories.dart';
 import '../build_runner.dart';
+import '../logging/build_log.dart';
 import 'build_process_state.dart';
 
 /// Methods for causing a child process to run and do work.
@@ -266,6 +269,24 @@ class ChildProcess {
     await stdout.close();
     exit(exitCode);
   }
+
+  /// Exits indicating that a file that should exist was deleted during the
+  /// build and the build cannot complete.
+  ///
+  /// The parent process will retry the whole command.
+  static Future<Never> exitDueToAssetDeleted(AssetId id) async {
+    buildLog.error('$id was unexpectedly deleted, restarting the build.');
+    await ChildProcess.exitWithMessage(
+      exitCode: assetDeletedExitCode,
+      message: buildProcessState.serialize(),
+    );
+  }
+
+  /// The exit code used to indicate "rebuild the builders".
+  static int recompileBuildersExitCode = ExitCode.tempFail.code;
+
+  /// The exit code used to indicate "try again, an asset was deleted".
+  static int assetDeletedExitCode = ExitCode.data.code;
 }
 
 // A code in the "private use" Unicode area, so it should not be in any log

build_runner/lib/src/build/asset_graph/graph.dart

@@ -21,7 +21,6 @@ import '../../build_plan/build_phases.dart';
 import '../../build_plan/phase.dart';
 import '../../constants.dart';
 import '../../io/generated_asset_hider.dart';
-import '../../io/reader_writer.dart';
 import '../library_cycle_graph/phased_asset_deps.dart';
 import 'exceptions.dart';
 import 'node.dart';
@@ -183,8 +182,7 @@ class AssetGraph implements GeneratedAssetHider {
   static Future<AssetGraph> build(
     BuildPhases buildPhases,
     Set<AssetId> sources,
-    BuildPackages buildPackages,
-    ReaderWriter readerWriter, {
+    BuildPackages buildPackages, {
     String? kernelDigest,
   }) async {
     final graph = AssetGraph._(
@@ -201,11 +199,6 @@ class AssetGraph implements GeneratedAssetHider {
       sources,
       placeholders: placeholders,
     );
-    // Pre-emptively compute digests for the nodes we know have outputs.
-    await graph._setDigests(
-      sources.where((id) => graph.get(id)?.primaryOutputs.isNotEmpty == true),
-      readerWriter,
-    );
     return graph;
   }
 
@@ -247,20 +240,6 @@ class AssetGraph implements GeneratedAssetHider {
     }
   }
 
-  /// Uses [readerWriter] to compute the [Digest] for nodes with [ids] and set
-  /// the `lastKnownDigest` field.
-  Future<void> _setDigests(
-    Iterable<AssetId> ids,
-    ReaderWriter readerWriter,
-  ) async {
-    for (final id in ids) {
-      final digest = await readerWriter.digest(id);
-      updateNode(id, (nodeBuilder) {
-        nodeBuilder.digest = digest;
-      });
-    }
-  }
-
   /// Changes [id] and its transitive`primaryOutput`s to `missingSource` nodes.
   ///
   /// Removes post build applications with removed assets as inputs.
@@ -344,12 +323,10 @@ class AssetGraph implements GeneratedAssetHider {
   /// Outputs that are deleted from the filesystem are retained in the graph as
   /// `missingSource` nodes.
   ///
-  /// Returns the set of [AssetId]s that were deleted.
+  /// Returns the set of [AssetId]s to delete.
   Future<Set<AssetId>> updateAndInvalidate(
     BuildPhases buildPhases,
     Map<AssetId, ChangeType> updates,
-    Future Function(AssetId id) delete,
-    ReaderWriter readerWriter,
   ) async {
     final newIds = <AssetId>{};
     final modifyIds = <AssetId>{};
@@ -378,22 +355,6 @@ class AssetGraph implements GeneratedAssetHider {
 
     _addSources(newIds);
 
-    final newAndModifiedNodes = [
-      for (final id in modifyIds.followedBy(newIds)) get(id)!,
-    ];
-    // Pre-emptively compute digests for the new and modified nodes we know have
-    // outputs.
-    await _setDigests(
-      newAndModifiedNodes
-          .where(
-            (node) =>
-                node.isTrackedInput &&
-                (node.primaryOutputs.isNotEmpty || node.digest != null),
-          )
-          .map((node) => node.id),
-      readerWriter,
-    );
-
     // Compute generated nodes that will no longer be output because their
     // primary input was deleted. Delete them.
     final transitiveRemovedIds = <AssetId>{};
@@ -409,9 +370,6 @@ class AssetGraph implements GeneratedAssetHider {
         addTransitivePrimaryOutputs(id);
       }
     }
-    final idsToDelete = Set<AssetId>.from(transitiveRemovedIds)
-      ..removeAll(removeIds);
-    await Future.wait(idsToDelete.map(delete));
 
     // Change deleted source assets and their transitive primary outputs to
     // `missingSource` nodes, rather than deleting them. This allows them to
@@ -426,7 +384,8 @@ class AssetGraph implements GeneratedAssetHider {
     _addOutputsForSources(buildPhases, newIds);
 
     _nodes.clearComputationResults();
-    return idsToDelete;
+    transitiveRemovedIds.removeAll(removeIds);
+    return transitiveRemovedIds;
   }
 
   /// Crawl up primary inputs to see if the original Source file matches the

build_runner/lib/src/build/build.dart

@@ -156,7 +156,7 @@ class Build {
         processedOutputs: processedOutputs,
       );
 
-  Future<BuildResult> run(Map<AssetId, ChangeType> updates) async {
+  Future<BuildResult> run(Set<AssetId> updates) async {
     buildLog.configuration = buildLog.configuration.rebuild(
       (b) => b..singleOutputPackage = buildPackages.singleOutputPackage,
     );
@@ -220,37 +220,85 @@ class Build {
     return result;
   }
 
-  Future<void> _updateAssetGraph(Map<AssetId, ChangeType> updates) async {
+  Future<void> _updateAssetGraph(Set<AssetId> updates) async {
     changedInputs.clear();
     deletedAssets.clear();
-    for (final update in updates.entries) {
-      if (update.value == ChangeType.REMOVE) {
-        deletedAssets.add(update.key);
-      } else {
-        changedInputs.add(update.key);
-        if (update.value == ChangeType.ADD) {
-          newPrimaryInputs.add(update.key);
+
+    // Check what actually changed for each asset in `updates`.
+    readerWriter.cache.invalidate(updates);
+    final resolvedUpdates = <AssetId, ChangeType>{};
+    final newDigests = <AssetId, Digest>{};
+    for (final id in updates) {
+      final oldNode = assetGraph.get(id);
+      final oldExisted =
+          oldNode != null && oldNode.type != NodeType.missingSource;
+      final oldDigest = oldNode?.digest;
+      var exists = false;
+      Digest? newDigest;
+      if (await readerWriter.canRead(id)) {
+        exists = true;
+        // Assets are only eagerly read if they were an input in the previous
+        // build. In that case, they have a digest. Compute the new digest to
+        // check if the file changed.
+        if (oldDigest != null) {
+          try {
+            newDigest = await readerWriter.digest(id);
+            newDigests[id] = newDigest;
+          } catch (_) {
+            // Was deleted after `canRead`.
+            exists = false;
+          }
         }
       }
+
+      if (oldExisted && !exists) {
+        resolvedUpdates[id] = ChangeType.REMOVE;
+        deletedAssets.add(id);
+      } else if (!oldExisted && exists) {
+        changedInputs.add(id);
+        newPrimaryInputs.add(id);
+        resolvedUpdates[id] = ChangeType.ADD;
+      } else if (oldExisted &&
+          oldDigest != null &&
+          exists &&
+          oldDigest != newDigest) {
+        changedInputs.add(id);
+        resolvedUpdates[id] = ChangeType.MODIFY;
+      }
     }
-    readerWriter.cache.invalidate(changedInputs);
+
     final deleted = await assetGraph.updateAndInvalidate(
       buildPhases,
-      updates,
-      _delete,
-      readerWriter,
+      resolvedUpdates,
     );
+    for (final id in deleted) {
+      await readerWriter.delete(id);
+    }
     deletedAssets.addAll(deleted);
+    for (final entry in newDigests.entries) {
+      assetGraph.updateNode(entry.key, (nodeBuilder) {
+        nodeBuilder.digest = entry.value;
+      });
+    }
   }
 
   /// Runs a build inside a zone with an error handler and stack chain
   /// capturing.
-  Future<BuildResult> _safeBuild(Map<AssetId, ChangeType> updates) {
+  Future<BuildResult> _safeBuild(Set<AssetId> idsToCheck) {
     final done = Completer<BuildResult>();
     runZonedGuarded(
       () async {
         if (!assetGraph.cleanBuild) {
-          await _updateAssetGraph(updates);
+          await _updateAssetGraph(idsToCheck);
+        }
+        for (final id in assetGraph.sources) {
+          final node = assetGraph.get(id)!;
+          if (node.digest == null && node.primaryOutputs.isNotEmpty) {
+            final digest = await readerWriter.digest(id);
+            assetGraph.updateNode(id, (nodeBuilder) {
+              nodeBuilder.digest = digest;
+            });
+          }
         }
         await resolversImpl?.takeLockAndStartBuild(assetGraph);
         final result = await _runPhases();

build_runner/lib/src/build/build_series.dart

@@ -42,12 +42,12 @@ class BuildSeries {
 
   final ResourceManager _resourceManager = ResourceManager();
 
-  /// For the first build only, updates from the previous serialized build
-  /// state.
+  /// For the first build only, assets that were updated since 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;
+  BuiltSet<AssetId>? _updatesFromLoad;
 
   final StreamController<BuildResult> _buildResultsController =
       StreamController.broadcast();
@@ -61,7 +61,7 @@ class BuildSeries {
     required BuildPlan buildPlan,
     required AssetGraph assetGraph,
     required ReaderWriter readerWriter,
-    required BuiltMap<AssetId, ChangeType>? updatesFromLoad,
+    required BuiltSet<AssetId>? updatesFromLoad,
   }) : _buildPlan = buildPlan,
        _assetGraph = assetGraph,
        _readerWriter = readerWriter,
@@ -160,16 +160,6 @@ class BuildSeries {
         continue;
       }
 
-      // For modifications, confirm that the content actually changed.
-      if (change.type == ChangeType.MODIFY) {
-        // Use `_buildPlan.readerWriter` which has no cache to do a real read.
-        final newDigest = await _buildPlan.readerWriter.digest(id);
-        if (node.digest != newDigest) {
-          result.add(change);
-        }
-        continue;
-      }
-
       // It's an add of "missing source" node or a deletion of an input.
       result.add(change);
     }
@@ -215,7 +205,7 @@ class BuildSeries {
   /// Set [recentlyBootstrapped] to skip doing checks that are done during
   /// bootstrapping. If [recentlyBootstrapped] then [updates] must be empty.
   Future<BuildResult> run(
-    Map<AssetId, ChangeType> updates, {
+    Set<AssetId> updates, {
     required bool recentlyBootstrapped,
     BuiltSet<BuildDirectory>? buildDirs,
     BuiltSet<BuildFilter>? buildFilters,
@@ -239,7 +229,7 @@ class BuildSeries {
       }
     }
 
-    if (updates.keys.any(_isBuildConfiguration)) {
+    if (updates.any(_isBuildConfiguration)) {
       _buildPlan = await _buildPlan.reload();
       await _buildPlan.deleteFilesAndFolders();
       // A config change might have caused new builders to be needed, which
@@ -267,7 +257,7 @@ class BuildSeries {
     buildFilters ??= _buildPlan.buildOptions.buildFilters;
     if (firstBuild) {
       if (_updatesFromLoad != null) {
-        updates = _updatesFromLoad!.toMap()..addAll(updates);
+        updates = _updatesFromLoad!.toSet()..addAll(updates);
         _updatesFromLoad = null;
       }
     } else {