Handle build config changes without restarting. (#4224)

Author: davidmorgan

build_runner/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 2.8.1-wip
 
+- Watch mode: handle `build.yaml` changes without restarting.
+- Remove log output about `build_runner` internals.
 - Print the port that gets picked if you pass 0 for a port number, for example
   with `dart run build_runner serve web:0`.
 - Improved warnings when an option is specified for an unknown builder.

build_runner/lib/src/bootstrap/bootstrap.dart

@@ -64,7 +64,6 @@ Future<int> generateAndRun(
         if (kernelFile.existsSync()) {
           kernelFile.deleteSync();
         }
-        buildLog.fullBuildBecause(FullBuildReason.incompatibleScript);
       }
     } on CannotBuildException {
       return ExitCode.config.code;
@@ -109,8 +108,6 @@ Future<int> generateAndRun(
         );
         messagePort.sendPort.send(ExitCode.config.code);
         exitPort.sendPort.send(null);
-      } else {
-        buildLog.fullBuildBecause(FullBuildReason.incompatibleScript);
       }
       File(scriptKernelLocation).renameSync(scriptKernelCachedLocation);
     }
@@ -157,10 +154,8 @@ Future<bool> _createKernelIfNeeded(Iterable<String> experiments) async {
       // If we failed to serialize an asset graph for the snapshot, then we
       // don't want to re-use it because we can't check if it is up to date.
       kernelFile.renameSync(scriptKernelCachedLocation);
-      buildLog.fullBuildBecause(FullBuildReason.incompatibleAssetGraph);
     } else if (!await _checkImportantPackageDepsAndExperiments(experiments)) {
       kernelFile.renameSync(scriptKernelCachedLocation);
-      buildLog.fullBuildBecause(FullBuildReason.incompatibleScript);
     }
   }
 
@@ -175,7 +170,6 @@ Future<bool> _createKernelIfNeeded(Iterable<String> experiments) async {
     );
 
     var hadErrors = false;
-    buildLog.doing('Compiling the build script.');
     try {
       final result = await client.compile();
       hadErrors = result.errorCount > 0 || !kernelCacheFile.existsSync();

build_runner/lib/src/bootstrap/build_process_state.dart

@@ -33,14 +33,6 @@ class BuildProcessState {
   int get displayedLines => (_state['displayedLines'] as int?) ?? 0;
   set displayedLines(int? value) => _state['displayedLines'] = value;
 
-  /// For `buildLog`, the reason why a full build was needed.
-  FullBuildReason get fullBuildReason => FullBuildReason.values.singleWhere(
-    (v) => v.name == _state['fullBuildReason'],
-    orElse: () => FullBuildReason.clean,
-  );
-  set fullBuildReason(FullBuildReason buildType) =>
-      _state['fullBuildReason'] = buildType.name;
-
   /// For `buildLog`, the elapsed time since the process started.
   int get elapsedMillis => _state['elapsedMillis'] as int? ?? 0;
   set elapsedMillis(int elapsedMillis) =>
@@ -104,20 +96,6 @@ class BuildProcessState {
   }
 }
 
-/// Reason why `build_runner` will do a full build; or `none` for an
-/// incremental build.
-enum FullBuildReason {
-  clean('full build'),
-  incompatibleScript('full build because builders changed'),
-  incompatibleAssetGraph('full build because there is no valid asset graph'),
-  incompatibleBuild('full build because target changed'),
-  none('incremental build');
-
-  const FullBuildReason(this.message);
-
-  final String message;
-}
-
 /// The `BuildLog` mode for the process.
 enum BuildLogMode {
   /// Line by line logging.

build_runner/lib/src/bootstrap/build_script_generate.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:io';
 
 import 'package:build_config/build_config.dart';
 import 'package:code_builder/code_builder.dart';
@@ -27,8 +28,13 @@ const scriptKernelCachedSuffix = '.cached';
 
 final _lastShortFormatDartVersion = Version(3, 6, 0);
 
+Future<bool> hasGeneratedBuildScriptChanged() async {
+  final script = await generateBuildScript();
+  final file = File(scriptLocation);
+  return !file.existsSync() || file.readAsStringSync() != script;
+}
+
 Future<String> generateBuildScript() async {
-  buildLog.doing('Generating the build script.');
   final builderFactories = await loadBuilderFactories();
   final library = Library(
     (b) => b.body.addAll([

build_runner/lib/src/build/build.dart

@@ -14,7 +14,6 @@ import 'package:glob/glob.dart';
 import 'package:path/path.dart' as p;
 import 'package:watcher/watcher.dart';
 
-import '../bootstrap/build_process_state.dart';
 import '../build_plan/build_options.dart';
 import '../build_plan/build_phases.dart';
 import '../build_plan/build_plan.dart';
@@ -137,9 +136,6 @@ class Build {
   BuildPhases get buildPhases => buildPlan.buildPhases;
 
   Future<BuildResult> run(Map<AssetId, ChangeType> updates) async {
-    if (!assetGraph.cleanBuild) {
-      buildLog.fullBuildBecause(FullBuildReason.none);
-    }
     buildLog.configuration = buildLog.configuration.rebuild(
       (b) => b..rootPackageName = packageGraph.root.name,
     );
@@ -234,14 +230,11 @@ class Build {
     final done = Completer<BuildResult>();
     runZonedGuarded(
       () async {
-        buildLog.doing('Updating the asset graph.');
         if (!assetGraph.cleanBuild) {
           await _updateAssetGraph(updates);
         }
 
-        buildLog.startBuild();
         final result = await _runPhases();
-        buildLog.doing('Writing the asset graph.');
 
         assetGraph.previousBuildTriggersDigest =
             targetGraph.buildTriggers.digest;
@@ -269,7 +262,6 @@ class Build {
 
         // Log performance information if requested
         if (buildOptions.logPerformanceDir != null) {
-          buildLog.doing('Writing the performance log.');
           assert(result.performance != null);
           final now = DateTime.now();
           final logPath = p.join(
@@ -365,7 +357,6 @@ class Build {
       }
 
       // Post build phase.
-      buildLog.doing('Running the post build.');
       if (buildPhases.postBuildPhase.builderActions.isNotEmpty) {
         outputs.addAll(
           await performanceTracker.trackBuildPhase(

build_runner/lib/src/build/build_result.dart

@@ -78,7 +78,6 @@ enum BuildStatus { success, failure }
 class FailureType {
   static final general = FailureType._(1);
   static final cantCreate = FailureType._(73);
-  static final buildConfigChanged = FailureType._(75);
   static final buildScriptChanged = FailureType._(75);
   final int exitCode;
   FailureType._(this.exitCode);

build_runner/lib/src/build/build_series.dart

@@ -8,15 +8,16 @@ import 'package:build/build.dart';
 import 'package:built_collection/built_collection.dart';
 import 'package:watcher/watcher.dart';
 
+import '../bootstrap/build_script_generate.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 '../logging/build_log.dart';
 import 'asset_graph/graph.dart';
 import 'asset_graph/node.dart';
 import 'build.dart';
@@ -35,11 +36,10 @@ 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;
+  BuildPlan _buildPlan;
+  AssetGraph _assetGraph;
+  ReaderWriter _readerWriter;
 
-  final AssetGraph _assetGraph;
-
-  final ReaderWriter _readerWriter;
   final ResourceManager _resourceManager = ResourceManager();
 
   /// For the first build only, updates from the previous serialized build
@@ -105,19 +105,21 @@ class BuildSeries {
       return false;
     }
 
-    final node =
-        _assetGraph.contains(change.id) ? _assetGraph.get(change.id) : null;
+    final id = change.id;
+    if (_isBuildConfiguration(id)) return true;
+
+    final node = _assetGraph.contains(id) ? _assetGraph.get(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;
+      if (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);
+      return _buildPlan.targetGraph.anyMatchesAsset(id);
     }
 
     // Changes to files that are part of the build.
@@ -136,15 +138,21 @@ class BuildSeries {
 
     // 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);
+      _readerWriter.cache.invalidate([id]);
+      final newDigest = await _readerWriter.digest(id);
       return node.digest != newDigest;
     }
 
     // It's an add of "missing source" node or a deletion of an input.
     return true;
   }
 
+  bool _isBuildConfiguration(AssetId id) =>
+      id.path == 'build.yaml' ||
+      id.path.endsWith('.build.yaml') ||
+      (id.package == _buildPlan.packageGraph.root.name &&
+          id.path == 'build.${_buildPlan.buildOptions.configKey}.yaml');
+
   Future<List<WatchEvent>> checkForChanges() async {
     final updates = await AssetTracker(
       _buildPlan.readerWriter,
@@ -178,14 +186,29 @@ class BuildSeries {
     BuiltSet<BuildFilter>? buildFilters,
   }) async {
     if (_hasBuildScriptChanged(updates.keys.toSet())) {
-      return BuildResult(
-        status: BuildStatus.failure,
-        failureType: FailureType.buildScriptChanged,
-        buildOutputReader: BuildOutputReader(
-          buildPlan: _buildPlan,
-          readerWriter: _readerWriter,
-          assetGraph: _assetGraph,
-        ),
+      return BuildResult.buildScriptChanged();
+    }
+
+    if (updates.keys.any(_isBuildConfiguration)) {
+      _buildPlan = await _buildPlan.reload();
+      await _buildPlan.deleteFilesAndFolders();
+      // A config change might have caused new builders to be needed, which
+      // needs a restart to change the build script.
+      if (_buildPlan.restartIsNeeded) {
+        return BuildResult.buildScriptChanged();
+      }
+      // A config change might have changed builder factories, which needs a
+      // restart to change the build script.
+      if (await hasGeneratedBuildScriptChanged()) {
+        return BuildResult.buildScriptChanged();
+      }
+      _assetGraph = _buildPlan.takeAssetGraph();
+      _readerWriter = _buildPlan.readerWriter.copyWith(
+        generatedAssetHider: _assetGraph,
+        cache:
+            _buildPlan.buildOptions.enableLowResourcesMode
+                ? const PassthroughFilesystemCache()
+                : InMemoryFilesystemCache(),
       );
     }
 
@@ -202,6 +225,7 @@ class BuildSeries {
       }
     }
 
+    if (!firstBuild) buildLog.nextBuild();
     final build = Build(
       buildPlan: _buildPlan.copyWith(
         buildDirs: buildDirs,

build_runner/lib/src/build_plan/build_plan.dart

@@ -9,7 +9,6 @@ import 'package:build/experiments.dart';
 import 'package:built_collection/built_collection.dart';
 import 'package:watcher/watcher.dart';
 
-import '../bootstrap/build_process_state.dart';
 import '../bootstrap/build_script_updates.dart';
 import '../build/asset_graph/exceptions.dart';
 import '../build/asset_graph/graph.dart';
@@ -125,7 +124,6 @@ class BuildPlan {
     var restartIsNeeded = false;
     if (builderApplications == null) {
       restartIsNeeded = true;
-      buildLog.fullBuildBecause(FullBuildReason.incompatibleScript);
       builderApplications = BuiltList();
     }
 
@@ -139,11 +137,8 @@ class BuildPlan {
         );
     buildPhases.checkOutputLocations(packageGraph.root.name);
     if (buildPhases.inBuildPhases.isEmpty &&
-        buildPhases.postBuildPhase.builderActions.isEmpty) {
-      buildLog.warning('Nothing to build.');
-    }
+        buildPhases.postBuildPhase.builderActions.isEmpty) {}
 
-    buildLog.doing('Reading the asset graph.');
     AssetGraph? previousAssetGraph;
     final filesToDelete = <AssetId>{};
     final foldersToDelete = <AssetId>{};
@@ -158,9 +153,7 @@ class BuildPlan {
       previousAssetGraph = AssetGraph.deserialize(
         await readerWriter.readAsBytes(assetGraphId),
       );
-      if (previousAssetGraph == null) {
-        buildLog.fullBuildBecause(FullBuildReason.incompatibleAssetGraph);
-      } else {
+      if (previousAssetGraph != null) {
         final buildPhasesChanged =
             buildPhases.digest != previousAssetGraph.buildPhasesDigest;
         final pkgVersionsChanged =
@@ -175,7 +168,6 @@ class BuildPlan {
               previousAssetGraph.dartVersion,
               Platform.version,
             )) {
-          buildLog.fullBuildBecause(FullBuildReason.incompatibleBuild);
           // Mark old outputs for deletion.
           filesToDelete.addAll(
             previousAssetGraph.outputsToDelete(packageGraph),
@@ -211,7 +203,6 @@ class BuildPlan {
     BuildScriptUpdates? buildScriptUpdates;
     Map<AssetId, ChangeType>? updates;
     if (previousAssetGraph != null) {
-      buildLog.doing('Checking for updates.');
       updates = await assetTracker.computeSourceUpdates(
         inputSources,
         cacheDirSources,
@@ -229,7 +220,6 @@ class BuildPlan {
           !buildOptions.skipBuildScriptCheck &&
           buildScriptUpdates.hasBeenUpdated(updates.keys.toSet());
       if (buildScriptUpdated) {
-        buildLog.fullBuildBecause(FullBuildReason.incompatibleScript);
         // Mark old outputs for deletion.
         filesToDelete.addAll(previousAssetGraph.outputsToDelete(packageGraph));
         foldersToDelete.add(generatedOutputDirectoryId);
@@ -255,8 +245,6 @@ class BuildPlan {
     }
 
     if (assetGraph == null) {
-      buildLog.doing('Creating the asset graph.');
-
       // Files marked for deletion are not inputs.
       inputSources.removeAll(filesToDelete);
 
@@ -369,8 +357,6 @@ class BuildPlan {
   }
 
   Future<void> deleteFilesAndFolders() async {
-    buildLog.doing('Doing initial build cleanup.');
-
     // Hidden outputs are deleted if needed by deleting the entire folder. So,
     // only outputs in the source folder need to be deleted explicitly. Use a
     // `ReaderWriter` that only acts on the source folder.
@@ -386,6 +372,19 @@ class BuildPlan {
       await cleanupReaderWriter.deleteDirectory(id);
     }
   }
+
+  /// Reloads the build plan.
+  ///
+  /// Works just like a new load of the build plan, but supresses the usual log
+  /// output.
+  ///
+  /// The caller must call [deleteFilesAndFolders] on the result and check
+  /// [restartIsNeeded].
+  Future<BuildPlan> reload() => BuildPlan.load(
+    builderFactories: builderFactories,
+    buildOptions: buildOptions,
+    testingOverrides: testingOverrides,
+  );
 }
 
 bool isSameSdkVersion(String? thisVersion, String? thatVersion) =>