Cleanup.

Author: davidmorgan

build_runner_core/lib/src/asset_graph/graph.dart

@@ -74,6 +74,8 @@ class AssetGraph implements GeneratedAssetHider {
   /// Digests from the current build's [BuildPhases].
   BuiltList<Digest> postBuildActionsOptionsDigests;
 
+  /// Imports of resolved assets in the previous build, or `null` if this is a
+  /// clean build.
   PhasedAssetDeps? previousPhasedAssetDeps;
 
   AssetGraph._(

build_runner_core/lib/src/asset_graph/node.dart

@@ -274,8 +274,8 @@ abstract class GeneratedNodeState
   /// to generate it.
   BuiltSet<AssetId> get inputs;
 
-  /// Dependency graphs used in analysis.
-  BuiltSet<AssetId> get inputGraphs;
+  /// Entrypoints used for resolution with the analyzer.
+  BuiltSet<AssetId> get resolverEntrypoints;
 
   /// Whether the generation succeded, or `null` if it did not run.
   ///

build_runner_core/lib/src/asset_graph/node.g.dart

@@ -114,7 +114,9 @@ class Build {
   /// checked against the digest from the previous build.
   final Set<AssetId> changedOutputs = {};
 
-  final Map<LibraryCycleGraph, bool> hasGraphChanged = Map.identity();
+  /// Whether a graph from [previousLibraryCycleGraphLoader] has any changed
+  /// transitive source.
+  final Map<LibraryCycleGraph, bool> changedGraphs = Map.identity();
 
   Build({
     required this.environment,
@@ -904,7 +906,7 @@ class Build {
       }
     }
 
-    for (final graphId in firstOutputState.inputGraphs) {
+    for (final graphId in firstOutputState.resolverEntrypoints) {
       if (await _hasInputGraphChanged(
         phaseNumber: phaseNumber,
         entrypointId: graphId,
@@ -927,19 +929,24 @@ class Build {
     return false;
   }
 
-  /// Returns whether any source in the _previous build_ transitive import graph
-  /// of [entrypointId] has changed.
+  /// Whether any source in the _previous build_ transitive import graph
+  /// of [entrypointId] has a change visible at [phaseNumber].
+  ///
+  /// There is a tradeoff between returning early when a first change is
+  /// encountered and continuing to process the graph to produce results that
+  /// might be useful later. This implementation is eager, it computes whether
+  /// every subgraph reachable from [entrypointId] has changed.
   Future<bool> _hasInputGraphChanged({
-    required int phaseNumber,
     required AssetId entrypointId,
+    required int phaseNumber,
   }) async {
     // If the result has already been calculated, return it.
     final entrypointGraph = (await previousLibraryCycleGraphLoader
         .libraryCycleGraphOf(
           previousDepsLoader!,
           entrypointId,
         )).valueAt(phase: phaseNumber);
-    final maybeResult = hasGraphChanged[entrypointGraph];
+    final maybeResult = changedGraphs[entrypointGraph];
     if (maybeResult != null) {
       return maybeResult;
     }
@@ -951,7 +958,7 @@ class Build {
 
       // If there are multiple paths to a node, it might have been calculated
       // for another path.
-      if (hasGraphChanged.containsKey(nextGraph)) {
+      if (changedGraphs.containsKey(nextGraph)) {
         graphsToCheckStack.removeLast();
         continue;
       }
@@ -962,7 +969,7 @@ class Build {
       // returning to this graph.
       final childGraphsWithWorkToDo = <LibraryCycleGraph>[];
       for (final childGraph in nextGraph.children) {
-        final maybeChildResult = hasGraphChanged[childGraph];
+        final maybeChildResult = changedGraphs[childGraph];
         if (maybeChildResult == null) {
           childGraphsWithWorkToDo.add(childGraph);
         }
@@ -983,29 +990,30 @@ class Build {
         }
       }
       if (rootLibraryCycleHasChanged) {
-        hasGraphChanged[nextGraph] = true;
+        changedGraphs[nextGraph] = true;
       } else {
         var anyChildHasChanged = false;
         for (final childGraph in nextGraph.children) {
-          final childResult = hasGraphChanged[childGraph];
+          final childResult = changedGraphs[childGraph];
           if (childResult == null) {
             throw StateError('Child graphs should have been checked.');
           } else if (childResult) {
             anyChildHasChanged = true;
             break;
           }
         }
-        hasGraphChanged[nextGraph] = anyChildHasChanged;
+        changedGraphs[nextGraph] = anyChildHasChanged;
       }
       graphsToCheckStack.removeLast();
     }
 
-    return hasGraphChanged[entrypointGraph]!;
+    return changedGraphs[entrypointGraph]!;
   }
 
+  /// Whether [input] has a change visible at [phaseNumber].
   Future<bool> _hasInputChanged({
-    required int phaseNumber,
     required AssetId input,
+    required int phaseNumber,
   }) async {
     final inputNode = assetGraph.get(input)!;
     if (inputNode.type == NodeType.generated) {
@@ -1223,7 +1231,7 @@ class Build {
       outputNode = assetGraph.updateNode(output, (nodeBuilder) {
         nodeBuilder.generatedNodeState
           ..inputs.replace(usedInputs)
-          ..inputGraphs.replace(inputTracker.resolverEntrypoints)
+          ..resolverEntrypoints.replace(inputTracker.resolverEntrypoints)
           ..result = result;
         nodeBuilder.digest = digest;
       });

build_runner_core/lib/src/generate/build.dart

@@ -222,6 +222,7 @@ class SingleStepReaderWriter extends AssetReader
   Future<bool> _isReadable(
     AssetId id, {
     bool catchInvalidInputs = false,
+    bool track = true,
   }) async {
     try {
       _checkInvalidInput(id);
@@ -234,13 +235,13 @@ class SingleStepReaderWriter extends AssetReader
     }
 
     if (_runningBuild == null) {
-      inputTracker.add(id);
+      if (track) inputTracker.add(id);
       return _delegate.canRead(id);
     }
 
     final node = _runningBuild.assetGraph.get(id);
     if (node == null) {
-      inputTracker.add(id);
+      if (track) inputTracker.add(id);
       _runningBuild.assetGraph.add(AssetNode.missingSource(id));
       return false;
     }
@@ -252,15 +253,19 @@ class SingleStepReaderWriter extends AssetReader
     // it's an output of a generator running in parallel, which means it's
     // hidden and can't be an input.
     if (!readability.inSamePhase) {
-      inputTracker.add(id);
+      if (track) inputTracker.add(id);
     }
 
     return readability.canRead;
   }
 
   @override
-  Future<bool> canRead(AssetId id) async {
-    final isReadable = await _isReadable(id, catchInvalidInputs: true);
+  Future<bool> canRead(AssetId id, {bool track = true}) async {
+    final isReadable = await _isReadable(
+      id,
+      catchInvalidInputs: true,
+      track: track,
+    );
     if (!isReadable) return false;
     if (_runningBuild == null) return true;
 
@@ -299,8 +304,12 @@ class SingleStepReaderWriter extends AssetReader
   }
 
   @override
-  Future<String> readAsString(AssetId id, {Encoding encoding = utf8}) async {
-    final isReadable = await _isReadable(id);
+  Future<String> readAsString(
+    AssetId id, {
+    Encoding encoding = utf8,
+    bool track = true,
+  }) async {
+    final isReadable = await _isReadable(id, track: track);
     if (!isReadable) {
       throw AssetNotFoundException(id);
     }
@@ -442,8 +451,6 @@ class SingleStepReaderWriter extends AssetReader
 
     var node = _runningBuild.assetGraph.get(id);
     if (node == null) {
-      // TODO(davidmorgan): test coverage.
-      _runningBuild.assetGraph.add(AssetNode.missingSource(id));
       return PhasedValue.fixed('');
     }
 
@@ -467,55 +474,16 @@ class SingleStepReaderWriter extends AssetReader
       }
     }
 
-    final canRead = await _delegate.canRead(id);
-    if (canRead && node.digest == null) {
-      await _ensureDigest(id);
-    }
-
-    return PhasedValue.fixed(canRead ? await _delegate.readAsString(id) : '');
+    return PhasedValue.fixed(
+      await _delegate.canRead(id) ? await _delegate.readAsString(id) : '',
+    );
   }
 
   @override
   Future<String?> readAtPhase(AssetId id) async {
-    // TODO(davidmorgan): dedupe.
-    if (_runningBuild == null) {
-      final exists = await _delegate.canRead(id);
-      if (exists) {
-        return await _delegate.readAsString(id);
-      } else {
-        return null;
-      }
-    }
-
-    var node = _runningBuild.assetGraph.get(id);
-    if (node == null) {
-      // TODO(davidmorgan): test coverage.
-      _runningBuild.assetGraph.add(AssetNode.missingSource(id));
-      return null;
-    }
-
-    if (node.type == NodeType.generated) {
-      final nodePhase = node.generatedNodeConfiguration!.phaseNumber;
-      if (nodePhase >= phase) {
-        return null;
-      } else {
-        // If needed, trigger a build at an earlier phase.
-        if (!_runningBuild.assetIsProcessedOutput(id)) {
-          await _runningBuild.nodeBuilder(id);
-          node = _runningBuild.assetGraph.get(id)!;
-        }
-        return (node.wasOutput && node.generatedNodeState!.result == true)
-            ? await _delegate.readAsString(id)
-            : null;
-      }
-    }
-
-    final canRead = await _delegate.canRead(id);
-    if (canRead && node.digest == null) {
-      await _ensureDigest(id);
-    }
-
-    return canRead ? await _delegate.readAsString(id) : null;
+    return await canRead(id, track: false)
+        ? await readAsString(id, track: false)
+        : null;
   }
 
   @override

build_runner_core/lib/src/generate/single_step_reader_writer.dart

@@ -17,6 +17,8 @@ void main() {
     tester = InvalidationTester();
   });
 
+  // This was sufficient to cause a stack overflow when `Build` used a recursive
+  // algorithm to check for changes to graphs.
   final size = 1500;
 
   group('a.1 <== a.2, a.2 resolves z1 --> ... --> z$size', () {