From 930dcbd5721fc13b68604e80b9353b87db67c0ab Mon Sep 17 00:00:00 2001 From: David Morgan Date: Fri, 19 Sep 2025 17:46:54 +0200 Subject: [PATCH] Encapsulate graph and build script updates in BuildSeries. --- build_runner/lib/src/build/build_series.dart | 169 +++++++++++++----- .../lib/src/build_plan/build_options.dart | 4 + .../lib/src/commands/build_command.dart | 6 +- .../src/commands/daemon/change_providers.dart | 13 +- .../src/commands/daemon/daemon_builder.dart | 34 +--- .../lib/src/commands/serve/server.dart | 18 -- .../lib/src/commands/watch/change_filter.dart | 51 ------ .../lib/src/commands/watch/watcher.dart | 51 +----- .../lib/src/commands/watch_command.dart | 3 - .../commands/serve/serve_handler_test.dart | 7 +- build_runner/test/common/test_phases.dart | 2 +- build_test/lib/src/test_builder.dart | 2 +- 12 files changed, 160 insertions(+), 200 deletions(-) delete mode 100644 build_runner/lib/src/commands/watch/change_filter.dart diff --git a/build_runner/lib/src/build/build_series.dart b/build_runner/lib/src/build/build_series.dart index 696e311149..7dd5756ea6 100644 --- a/build_runner/lib/src/build/build_series.dart +++ b/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,20 +35,19 @@ 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? updatesFromLoad; + BuiltMap? _updatesFromLoad; final StreamController _buildResultsController = StreamController.broadcast(); @@ -52,25 +55,106 @@ class BuildSeries { /// Whether the next build is the first build. bool firstBuild = true; - Future 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? 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 get buildResults => _buildResultsController.stream; Future? _currentBuildResult; + bool _hasBuildScriptChanged(Set 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 shouldProcess( + AssetChange change, + Set 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> 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? buildDirs, BuiltSet? 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 create({required BuildPlan buildPlan}) async { - final assetGraph = buildPlan.takeAssetGraph(); - final build = BuildSeries._( - buildPlan, - assetGraph, - buildPlan.buildScriptUpdates, - buildPlan.updates, - ); - return build; - } + Future beforeExit() => _resourceManager.beforeExit(); } diff --git a/build_runner/lib/src/build_plan/build_options.dart b/build_runner/lib/src/build_plan/build_options.dart index a527953571..fd50554869 100644 --- a/build_runner/lib/src/build_plan/build_options.dart +++ b/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, diff --git a/build_runner/lib/src/commands/build_command.dart b/build_runner/lib/src/commands/build_command.dart index cb335e23b8..09ae3bf3b1 100644 --- a/build_runner/lib/src/commands/build_command.dart +++ b/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; } } diff --git a/build_runner/lib/src/commands/daemon/change_providers.dart b/build_runner/lib/src/commands/daemon/change_providers.dart index 179e9bc2c8..99e4877865 100644 --- a/build_runner/lib/src/commands/daemon/change_providers.dart +++ b/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> Function() _function; - ManualChangeProviderImpl(this._assetTracker, this._assetGraph); + ManualChangeProviderImpl(this._function); @override Future> collectChanges() async { - final updates = await _assetTracker.collectChanges(_assetGraph); - return List.of( - updates.entries.map((entry) => WatchEvent(entry.value, '${entry.key}')), - ); + return _function(); } } diff --git a/build_runner/lib/src/commands/daemon/daemon_builder.dart b/build_runner/lib/src/commands/daemon/daemon_builder.dart index d39fcb9b73..ff64bf5220 100644 --- a/build_runner/lib/src/commands/daemon/daemon_builder.dart +++ b/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((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> 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, diff --git a/build_runner/lib/src/commands/serve/server.dart b/build_runner/lib/src/commands/serve/server.dart index 8d4cd7c8a0..3b90409429 100644 --- a/build_runner/lib/src/commands/serve/server.dart +++ b/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 diff --git a/build_runner/lib/src/commands/watch/change_filter.dart b/build_runner/lib/src/commands/watch/change_filter.dart deleted file mode 100644 index ca5b915eca..0000000000 --- a/build_runner/lib/src/commands/watch/change_filter.dart +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) 2019, 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:watcher/watcher.dart'; - -import '../../build/asset_graph/graph.dart'; -import '../../build/asset_graph/node.dart'; -import '../../build_plan/target_graph.dart'; -import '../../constants.dart'; -import '../../io/reader_writer.dart'; -import 'asset_change.dart'; - -/// Returns if a given asset change should be considered for building. -FutureOr shouldProcess( - AssetChange change, - AssetGraph assetGraph, - TargetGraph targetGraph, - bool willCreateOutputDir, - Set expectedDeletes, - ReaderWriter readerWriter, -) { - if (_isCacheFile(change) && !assetGraph.contains(change.id)) return false; - final node = assetGraph.get(change.id); - if (node != null) { - if (!willCreateOutputDir && !node.changesRequireRebuild) return false; - if (_isAddOrEditOnGeneratedFile(node, change.type)) return false; - if (change.type == ChangeType.MODIFY) { - // Was it really modified or just touched? - readerWriter.cache.invalidate([change.id]); - return readerWriter - .digest(change.id) - .then((newDigest) => node.digest != newDigest); - } - } else { - if (change.type != ChangeType.ADD) return false; - if (!targetGraph.anyMatchesAsset(change.id)) return false; - } - if (_isExpectedDelete(change, expectedDeletes)) return false; - return true; -} - -bool _isAddOrEditOnGeneratedFile(AssetNode node, ChangeType changeType) => - node.type == NodeType.generated && changeType != ChangeType.REMOVE; - -bool _isCacheFile(AssetChange change) => change.id.path.startsWith(cacheDir); - -bool _isExpectedDelete(AssetChange change, Set expectedDeletes) => - expectedDeletes.remove(change.id); diff --git a/build_runner/lib/src/commands/watch/watcher.dart b/build_runner/lib/src/commands/watch/watcher.dart index d9b2c5fc1f..39dc46661c 100644 --- a/build_runner/lib/src/commands/watch/watcher.dart +++ b/build_runner/lib/src/commands/watch/watcher.dart @@ -8,7 +8,6 @@ import 'package:build/build.dart'; import 'package:crypto/crypto.dart'; import 'package:stream_transform/stream_transform.dart'; -import '../../build/asset_graph/graph.dart'; import '../../build/build_result.dart'; import '../../build/build_series.dart'; import '../../build_plan/build_options.dart'; @@ -19,7 +18,6 @@ import '../../io/build_output_reader.dart'; import '../../io/reader_writer.dart'; import '../../logging/build_log.dart'; import 'asset_change.dart'; -import 'change_filter.dart'; import 'collect_changes.dart'; import 'graph_watcher.dart'; import 'node_watcher.dart'; @@ -29,13 +27,6 @@ class Watcher { BuildSeries? _buildSeries; - AssetGraph? get assetGraph => _buildSeries?.assetGraph; - - /// Whether or not we will be creating any output directories. - /// - /// If not, then we don't care about source edits that don't have outputs. - final bool willCreateOutputDirs; - /// Should complete when we need to kill the build. final _terminateCompleter = Completer(); @@ -44,11 +35,7 @@ class Watcher { /// Pending expected delete events from the build. final Set _expectedDeletes = {}; - Watcher({ - required BuildPlan buildPlan, - required Future until, - required this.willCreateOutputDirs, - }) { + Watcher({required BuildPlan buildPlan, required Future until}) { this.buildPlan = buildPlan.copyWith( readerWriter: buildPlan.readerWriter.copyWith( onDelete: _expectedDeletes.add, @@ -76,28 +63,11 @@ class Watcher { Future doBuild(List> changes) async { buildLog.nextBuild(); - final build = _buildSeries!; + final buildSeries = _buildSeries!; final mergedChanges = collectChanges(changes); _expectedDeletes.clear(); - if (!buildOptions.skipBuildScriptCheck) { - if (build.buildScriptUpdates!.hasBeenUpdated( - mergedChanges.keys.toSet(), - )) { - _terminateCompleter.complete(); - buildLog.error('Terminating builds due to build script update.'); - return BuildResult( - status: BuildStatus.failure, - failureType: FailureType.buildScriptChanged, - buildOutputReader: BuildOutputReader( - buildPlan: buildPlan, - readerWriter: readerWriter, - assetGraph: assetGraph!, - ), - ); - } - } - return build.run(mergedChanges); + return buildSeries.run(mergedChanges); } final terminate = Future.any([until, _terminateCompleter.future]).then((_) { @@ -163,14 +133,7 @@ class Watcher { return change; }) .asyncWhere((change) { - return shouldProcess( - change, - assetGraph!, - buildPlan.targetGraph, - willCreateOutputDirs, - _expectedDeletes, - readerWriter, - ); + return _buildSeries!.shouldProcess(change, _expectedDeletes); }) .debounceBuffer( testingOverrides.debounceDelay ?? const Duration(milliseconds: 250), @@ -178,6 +141,10 @@ class Watcher { .takeUntil(terminate) .asyncMapBuffer((changes) => currentBuildResult = doBuild(changes)) .listen((BuildResult result) { + if (result.failureType == FailureType.buildScriptChanged) { + _terminateCompleter.complete(); + buildLog.error('Terminating builds due to build script update.'); + } if (controller.isClosed) return; controller.add(result); }) @@ -206,7 +173,7 @@ class Watcher { BuildResult firstBuild; BuildSeries? build; - build = _buildSeries = await BuildSeries.create(buildPlan: buildPlan); + build = _buildSeries = BuildSeries(buildPlan); firstBuild = await build.run({}); // It is possible this is already closed if the user kills the process // early, which results in an exception without this check. diff --git a/build_runner/lib/src/commands/watch_command.dart b/build_runner/lib/src/commands/watch_command.dart index 162a335e88..f08bcc331b 100644 --- a/build_runner/lib/src/commands/watch_command.dart +++ b/build_runner/lib/src/commands/watch_command.dart @@ -63,9 +63,6 @@ class WatchCommand implements BuildRunnerCommand { final watcher = Watcher( buildPlan: buildPlan, until: terminator.shouldTerminate, - willCreateOutputDirs: buildOptions.buildDirs.any( - (target) => target.outputLocation?.path.isNotEmpty ?? false, - ), ); unawaited( diff --git a/build_runner/test/commands/serve/serve_handler_test.dart b/build_runner/test/commands/serve/serve_handler_test.dart index a74cf393c8..0fcf7ff809 100644 --- a/build_runner/test/commands/serve/serve_handler_test.dart +++ b/build_runner/test/commands/serve/serve_handler_test.dart @@ -120,7 +120,7 @@ void main() { packageGraph, readerWriter, ); - watchImpl = MockWatchImpl(packageGraph, assetGraph); + watchImpl = MockWatchImpl(packageGraph); serveHandler = ServeHandler(watchImpl); finalizedReader = BuildOutputReader.graphOnly( readerWriter: readerWriter, @@ -639,9 +639,6 @@ void main() { } class MockWatchImpl implements Watcher { - @override - final AssetGraph assetGraph; - Future? _currentBuild; @override @@ -667,7 +664,7 @@ class MockWatchImpl implements Watcher { _futureBuildResultsController.add(result); } - MockWatchImpl(this.packageGraph, this.assetGraph) { + MockWatchImpl(this.packageGraph) { final firstBuild = Completer(); _currentBuild = firstBuild.future; _futureBuildResultsController.stream.listen((futureBuildResult) { diff --git a/build_runner/test/common/test_phases.dart b/build_runner/test/common/test_phases.dart index 0877d28819..3f5efb0302 100644 --- a/build_runner/test/common/test_phases.dart +++ b/build_runner/test/common/test_phases.dart @@ -154,7 +154,7 @@ Future testPhases( await buildPlan.deleteFilesAndFolders(); BuildResult result; - final build = await BuildSeries.create(buildPlan: buildPlan); + final build = BuildSeries(buildPlan); result = await build.run({}); await build.beforeExit(); diff --git a/build_test/lib/src/test_builder.dart b/build_test/lib/src/test_builder.dart index ede1dfcbcc..001cd3efa3 100644 --- a/build_test/lib/src/test_builder.dart +++ b/build_test/lib/src/test_builder.dart @@ -467,7 +467,7 @@ Future testBuilderFactories( ); await buildPlan.deleteFilesAndFolders(); - final buildSeries = await BuildSeries.create(buildPlan: buildPlan); + final buildSeries = BuildSeries(buildPlan); // Run the build. final buildResult = await buildSeries.run({});