Refactor file outputs to BuildOutputReader. (#4222)

Author: davidmorgan

build_runner/lib/src/build/build.dart

@@ -15,7 +15,6 @@ import 'package:path/path.dart' as p;
 import 'package:watcher/watcher.dart';
 
 import '../bootstrap/build_process_state.dart';
-import '../build_plan/build_directory.dart';
 import '../build_plan/build_options.dart';
 import '../build_plan/build_phases.dart';
 import '../build_plan/build_plan.dart';
@@ -24,6 +23,7 @@ import '../build_plan/phase.dart';
 import '../build_plan/target_graph.dart';
 import '../build_plan/testing_overrides.dart';
 import '../constants.dart';
+import '../io/build_output_reader.dart';
 import '../io/create_merged_dir.dart';
 import '../io/reader_writer.dart';
 import '../logging/build_log.dart';
@@ -33,12 +33,10 @@ import 'asset_graph/node.dart';
 import 'asset_graph/post_process_build_step_id.dart';
 import 'build_dirs.dart';
 import 'build_result.dart';
-import 'finalized_assets_view.dart';
 import 'input_tracker.dart';
 import 'library_cycle_graph/asset_deps_loader.dart';
 import 'library_cycle_graph/library_cycle_graph.dart';
 import 'library_cycle_graph/library_cycle_graph_loader.dart';
-import 'optional_output_tracker.dart';
 import 'performance_tracker.dart';
 import 'performance_tracking_resolvers.dart';
 import 'resolver/analysis_driver_model.dart';
@@ -110,6 +108,9 @@ class Build {
   /// transitive source.
   final Map<LibraryCycleGraph, bool> changedGraphs = Map.identity();
 
+  /// The build output.
+  final BuildOutputReader buildOutputReader;
+
   Build({
     required this.buildPlan,
     required this.readerWriter,
@@ -122,7 +123,12 @@ class Build {
        previousDepsLoader =
            assetGraph.previousPhasedAssetDeps == null
                ? null
-               : AssetDepsLoader.fromDeps(assetGraph.previousPhasedAssetDeps!);
+               : AssetDepsLoader.fromDeps(assetGraph.previousPhasedAssetDeps!),
+       buildOutputReader = BuildOutputReader(
+         buildPlan: buildPlan,
+         readerWriter: readerWriter,
+         assetGraph: assetGraph,
+       );
 
   BuildOptions get buildOptions => buildPlan.buildOptions;
   TestingOverrides get testingOverrides => buildPlan.testingOverrides;
@@ -138,13 +144,6 @@ class Build {
       (b) => b..rootPackageName = packageGraph.root.name,
     );
     var result = await _safeBuild(updates);
-    final optionalOutputTracker = OptionalOutputTracker(
-      assetGraph,
-      targetGraph,
-      BuildDirectory.buildPaths(buildPlan.buildOptions.buildDirs),
-      buildPlan.buildOptions.buildFilters,
-      buildPhases,
-    );
     if (result.status == BuildStatus.success) {
       final failures = <AssetNode>[];
       for (final output in processedOutputs) {
@@ -169,21 +168,31 @@ class Build {
             logger.severe(error);
           }
         }
-        result = BuildResult(
-          BuildStatus.failure,
-          result.outputs,
-          performance: result.performance,
-        );
+        result = result.copyWith(status: BuildStatus.failure);
       }
     }
     readerWriter.cache.flush();
     await resourceManager.disposeAll();
-    result = await _finalizeBuild(
-      result,
-      FinalizedAssetsView(assetGraph, packageGraph, optionalOutputTracker),
-      readerWriter,
-      buildPlan.buildOptions.buildDirs,
-    );
+
+    // If requested, create output directories. If that fails, fail the build.
+    if (buildPlan.buildOptions.buildDirs.any(
+          (target) => target.outputLocation?.path.isNotEmpty ?? false,
+        ) &&
+        result.status == BuildStatus.success) {
+      if (!await createMergedOutputDirectories(
+        packageGraph: packageGraph,
+        outputSymlinksOnly: buildOptions.outputSymlinksOnly,
+        buildDirs: buildOptions.buildDirs,
+        buildOutputReader: buildOutputReader,
+        readerWriter: readerWriter,
+      )) {
+        result = result.copyWith(
+          status: BuildStatus.failure,
+          failureType: FailureType.cantCreate,
+        );
+      }
+    }
+
     _resolvers.reset();
     buildLog.finishBuild(
       result: result.status == BuildStatus.success,
@@ -282,7 +291,13 @@ class Build {
           buildLog.error(
             buildLog.renderThrowable('Unhandled build failure!', e, st),
           );
-          done.complete(BuildResult(BuildStatus.failure, []));
+          done.complete(
+            BuildResult(
+              status: BuildStatus.failure,
+              outputs: BuiltList(),
+              buildOutputReader: buildOutputReader,
+            ),
+          );
         }
       },
     );
@@ -372,9 +387,10 @@ class Build {
       );
       // Assume success, `_assetGraph.failedOutputs` will be checked later.
       return BuildResult(
-        BuildStatus.success,
-        outputs,
+        status: BuildStatus.success,
+        outputs: outputs.build(),
         performance: performanceTracker,
+        buildOutputReader: buildOutputReader,
       );
     });
   }
@@ -394,7 +410,7 @@ class Build {
         .toList(growable: false)) {
       if (!shouldBuildForDirs(
         node.id,
-        buildDirs: BuildDirectory.buildPaths(buildPlan.buildOptions.buildDirs),
+        buildDirs: buildPlan.buildOptions.buildDirs,
         buildFilters: buildPlan.buildOptions.buildFilters,
         phase: phase,
         targetGraph: targetGraph,
@@ -1211,60 +1227,6 @@ class Build {
   }
 
   Future _delete(AssetId id) => readerWriter.delete(id);
-
-  /// Invoked after each build, can modify the [BuildResult] in any way, even
-  /// converting it to a failure.
-  ///
-  /// The [finalizedAssetsView] can only be used until the returned [Future]
-  /// completes, it will expire afterwords since it can no longer guarantee a
-  /// consistent state.
-  ///
-  /// By default this returns the original result.
-  ///
-  /// Any operation may be performed, as determined by environment.
-  Future<BuildResult> _finalizeBuild(
-    BuildResult buildResult,
-    FinalizedAssetsView finalizedAssetsView,
-    ReaderWriter readerWriter,
-    BuiltSet<BuildDirectory> buildDirs,
-  ) async {
-    if (testingOverrides.finalizeBuild != null) {
-      return testingOverrides.finalizeBuild!(
-        buildResult,
-        finalizedAssetsView,
-        readerWriter,
-        buildDirs,
-      );
-    }
-    if (buildDirs.any(
-          (target) => target.outputLocation?.path.isNotEmpty ?? false,
-        ) &&
-        buildResult.status == BuildStatus.success) {
-      if (!await createMergedOutputDirectories(
-        buildDirs,
-        packageGraph,
-        readerWriter,
-        finalizedAssetsView,
-        buildOptions.outputSymlinksOnly,
-      )) {
-        return _convertToFailure(
-          buildResult,
-          failureType: FailureType.cantCreate,
-        );
-      }
-    }
-    return buildResult;
-  }
 }
 
 String _twoDigits(int n) => '$n'.padLeft(2, '0');
-
-BuildResult _convertToFailure(
-  BuildResult previous, {
-  FailureType? failureType,
-}) => BuildResult(
-  BuildStatus.failure,
-  previous.outputs,
-  performance: previous.performance,
-  failureType: failureType,
-);

build_runner/lib/src/build/build_dirs.dart

@@ -5,6 +5,7 @@
 import 'package:build/build.dart';
 import 'package:built_collection/built_collection.dart';
 
+import '../build_plan/build_directory.dart';
 import '../build_plan/build_filter.dart';
 import '../build_plan/phase.dart';
 import '../build_plan/target_graph.dart';
@@ -23,18 +24,20 @@ import '../build_plan/target_graph.dart';
 ///   `id.path` must start with one of the specified directory names.
 bool shouldBuildForDirs(
   AssetId id, {
-  required BuiltSet<String> buildDirs,
+  required BuiltSet<BuildDirectory> buildDirs,
   required BuildPhase phase,
   required TargetGraph targetGraph,
   BuiltSet<BuildFilter>? buildFilters,
 }) {
+  // Empty paths means "build everything".
+  final paths = BuildDirectory.buildPaths(buildDirs);
   buildFilters ??= BuiltSet();
   if (buildFilters.isEmpty) {
     // Build asset if: It's built to source, it's public or if it's matched by
     // a build directory.
     return !phase.hideOutput ||
-        buildDirs.isEmpty ||
-        buildDirs.any(id.path.startsWith) ||
+        paths.isEmpty ||
+        paths.any(id.path.startsWith) ||
         targetGraph.isPublicAsset(id);
   } else {
     // Don't build assets not matched by build filters
@@ -44,8 +47,8 @@ bool shouldBuildForDirs(
 
     // In filtered assets, build the public ones or those inside a build
     // directory.
-    return buildDirs.isEmpty ||
-        buildDirs.any(id.path.startsWith) ||
+    return paths.isEmpty ||
+        paths.any(id.path.startsWith) ||
         targetGraph.isPublicAsset(id);
   }
 }

build_runner/lib/src/build/build_result.dart

@@ -1,11 +1,12 @@
 // Copyright (c) 2016, 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/build.dart';
+import 'package:built_collection/built_collection.dart';
 import 'package:meta/meta.dart';
 
+import '../io/build_output_reader.dart';
 import 'performance_tracker.dart';
 
 /// The result of an individual build, this may be an incremental build or
@@ -18,22 +19,36 @@ class BuildResult {
   final FailureType? failureType;
 
   /// All outputs created/updated during this build.
-  final List<AssetId> outputs;
+  final BuiltList<AssetId> outputs;
 
-  /// The [BuildPerformance] broken out by build action, may be `null`.
+  // The build output.
+  final BuildOutputReader buildOutputReader;
+
+  /// The [BuildPerformance] broken out by build action.
   @experimental
   final BuildPerformance? performance;
 
-  BuildResult(
-    this.status,
-    List<AssetId> outputs, {
+  BuildResult({
+    required this.status,
+    BuiltList<AssetId>? outputs,
+    required this.buildOutputReader,
     this.performance,
     FailureType? failureType,
-  }) : outputs = List.unmodifiable(outputs),
-       failureType =
+  }) : failureType =
            failureType == null && status == BuildStatus.failure
                ? FailureType.general
-               : failureType;
+               : failureType,
+       outputs = outputs ?? BuiltList();
+
+  BuildResult copyWith({BuildStatus? status, FailureType? failureType}) =>
+      BuildResult(
+        status: status ?? this.status,
+        outputs: outputs,
+        buildOutputReader: buildOutputReader,
+        performance: performance,
+        failureType: failureType ?? this.failureType,
+      );
+
   @override
   String toString() {
     if (status == BuildStatus.success) {
@@ -50,9 +65,9 @@ Build Failed :(
   }
 
   factory BuildResult.buildScriptChanged() => BuildResult(
-    BuildStatus.failure,
-    const [],
+    status: BuildStatus.failure,
     failureType: FailureType.buildScriptChanged,
+    buildOutputReader: BuildOutputReader.empty(),
   );
 }
 
@@ -68,8 +83,3 @@ class FailureType {
   final int exitCode;
   FailureType._(this.exitCode);
 }
-
-abstract class BuildState {
-  Future<BuildResult>? get currentBuild;
-  Stream<BuildResult> get buildResults;
-}