From 4a1d0207df514e337f03e9a0d52e78e26c947f16 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Fri, 15 Nov 2024 22:36:52 +0100 Subject: [PATCH 1/7] Use batches for builds --- build_runner/lib/src/generate/build.dart | 24 ++- build_runner/lib/src/generate/watch_impl.dart | 6 +- build_runner/pubspec.yaml | 9 +- build_runner_core/CHANGELOG.md | 4 +- build_runner_core/lib/build_runner_core.dart | 1 + build_runner_core/lib/src/asset/batch.dart | 191 ++++++++++++++++++ .../lib/src/environment/io_environment.dart | 26 ++- build_runner_core/pubspec.yaml | 2 +- build_runner_core/test/asset/batch_test.dart | 103 ++++++++++ pubspec.yaml | 2 +- 10 files changed, 345 insertions(+), 23 deletions(-) create mode 100644 build_runner_core/lib/src/asset/batch.dart create mode 100644 build_runner_core/test/asset/batch_test.dart diff --git a/build_runner/lib/src/generate/build.dart b/build_runner/lib/src/generate/build.dart index 4c827d8af..30de63b96 100644 --- a/build_runner/lib/src/generate/build.dart +++ b/build_runner/lib/src/generate/build.dart @@ -106,17 +106,19 @@ Future build(List builders, ); var terminator = Terminator(terminateEventStream); try { - var build = await BuildRunner.create( - options, - environment, - builders, - builderConfigOverrides, - isReleaseBuild: isReleaseBuild, - ); - var result = - await build.run({}, buildDirs: buildDirs, buildFilters: buildFilters); - await build.beforeExit(); - return result; + return await runWithFileSystemBatch(() async { + var build = await BuildRunner.create( + options, + environment, + builders, + builderConfigOverrides!, + isReleaseBuild: isReleaseBuild!, + ); + var result = await build + .run({}, buildDirs: buildDirs!, buildFilters: buildFilters!); + await build.beforeExit(); + return result; + }, environment.writer); } finally { await terminator.cancel(); await options.logListener.cancel(); diff --git a/build_runner/lib/src/generate/watch_impl.dart b/build_runner/lib/src/generate/watch_impl.dart index 68726c0f8..85b866799 100644 --- a/build_runner/lib/src/generate/watch_impl.dart +++ b/build_runner/lib/src/generate/watch_impl.dart @@ -247,8 +247,10 @@ class WatchImpl implements BuildState { failureType: FailureType.buildScriptChanged); } } - return build.run(mergedChanges, - buildDirs: _buildDirs, buildFilters: _buildFilters); + return runWithFileSystemBatch( + () => build.run(mergedChanges, + buildDirs: _buildDirs, buildFilters: _buildFilters), + environment.writer); } var terminate = Future.any([until, _terminateCompleter.future]).then((_) { diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index 445bdb7fa..f073463c1 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -2,7 +2,7 @@ name: build_runner version: 2.4.13 description: A build system for Dart code generation and modular compilation. repository: https://github.com/dart-lang/build/tree/master/build_runner -resolution: workspace +#resolution: workspace environment: sdk: ^3.5.0 @@ -20,7 +20,7 @@ dependencies: build_config: ">=1.1.0 <1.2.0" build_daemon: ^4.0.0 build_resolvers: ^2.0.0 - build_runner_core: ^7.2.0 + build_runner_core: ^7.4.0-wip code_builder: ^4.2.0 collection: ^1.15.0 crypto: ^3.0.0 @@ -53,10 +53,15 @@ dev_dependencies: path: ../_test_common build_test: ^2.0.0 build_web_compilers: ^4.0.0 + dart_flutter_team_lints: ^3.1.0 stream_channel: ^2.0.0 test: ^1.25.5 test_descriptor: ^2.0.0 test_process: ^2.0.0 +dependency_overrides: + build_runner_core: + path: ../build_runner_core + topics: - build-runner diff --git a/build_runner_core/CHANGELOG.md b/build_runner_core/CHANGELOG.md index 830645e32..6328f0f43 100644 --- a/build_runner_core/CHANGELOG.md +++ b/build_runner_core/CHANGELOG.md @@ -1,5 +1,7 @@ -## 7.3.3-wip +## 7.4.0-wip +- Add `runWithFileSystemBatch` to batch asset writes before flushing them at + once. - Bump the min sdk to 3.6.0-dev.228. - Require analyzer ^6.9.0. - Fix analyzer deprecations. diff --git a/build_runner_core/lib/build_runner_core.dart b/build_runner_core/lib/build_runner_core.dart index 58b963efb..06f1f8aeb 100644 --- a/build_runner_core/lib/build_runner_core.dart +++ b/build_runner_core/lib/build_runner_core.dart @@ -4,6 +4,7 @@ export 'package:build/build.dart' show PostProcessBuildStep, PostProcessBuilder; +export 'src/asset/batch.dart' show runWithFileSystemBatch; export 'src/asset/file_based.dart'; export 'src/asset/finalized_reader.dart'; export 'src/asset/reader.dart' show RunnerAssetReader; diff --git a/build_runner_core/lib/src/asset/batch.dart b/build_runner_core/lib/src/asset/batch.dart new file mode 100644 index 000000000..045d2b14f --- /dev/null +++ b/build_runner_core/lib/src/asset/batch.dart @@ -0,0 +1,191 @@ +// Copyright (c) 2024, 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 'dart:convert'; + +import 'package:build/build.dart'; +import 'package:glob/glob.dart'; +import 'package:meta/meta.dart'; + +import '../environment/io_environment.dart'; +import 'reader.dart'; +import 'writer.dart'; + +/// Runs [inner] in a zone that indicates to compatible [RunnerAssetWriter]s +/// that written assets should not be written to the underlying file system +/// directly, but rather in a batch after [inner] has completed. +/// +/// This solves a common issue when `build_runner` and other tools such as an +/// analysis server are running concurrently: During a build, generated assets +/// are written one-by-one. The analyzer will discover these changes while the +/// build is running and report lints for a partial state of the workspace where +/// not all generated files are present, leading to a subpar experience. +/// +/// [runWithFileSystemBatch] is intended to wrap a single build run. Instead of +/// writing assets directly, they are first cached in memory and then written at +/// once after the build has completed. This causes fewer invalidations for +/// external tools. The [writer] will be used to complete the writes collected +/// while running [inner]. +/// +/// The underlying asset readers and writes both need to support file system +/// batches (reader support is crucial to report changed assets that have not +/// been flushed to the file system yet). The default [IOEnvironment] will use +/// batch-aware readers and writers when not in low-resources mode. +Future runWithFileSystemBatch( + Future Function() inner, + RunnerAssetWriter writer, +) async { + final batch = FileSystemWriteBatch._(); + final result = + await runZoned(inner, zoneValues: {FileSystemWriteBatch._key: batch}); + await batch._completeWrites(writer); + return result; +} + +/// A batch of file system writes that should be committed at once instead of +/// when [AssetWriter.writeAsBytes] or [AssetWriter.writeAsString] is called. +/// +/// During a typical build run emitting generated files one-by-one, it's +/// possible that other running tools such as an analysis server will have to +/// re-analyze incomplete states multiple times. +/// By storing pending outputs in memory first and then committing them at the +/// end of the build, we have a better view over that needs to happen. +/// +/// The default [IOEnvironment] uses readers and writes that are batch-aware +/// outside of low-memory mode. +@internal +final class FileSystemWriteBatch { + final Map _pendingWrites = {}; + + FileSystemWriteBatch._(); + + Future _completeWrites(RunnerAssetWriter writer) async { + await Future.wait(_pendingWrites.entries.map((e) async { + if (e.value.content case final content?) { + await writer.writeAsBytes(e.key, content); + } else { + await writer.delete(e.key); + } + })); + + _pendingWrites.clear(); + } + + static FileSystemWriteBatch? get current { + return Zone.current[_key] as FileSystemWriteBatch?; + } + + static final _key = Object(); +} + +final class _PendingFileState { + final List? content; + + const _PendingFileState(this.content); + + bool get isDeleted => content == null; +} + +@internal +final class BatchAwareReader extends AssetReader + implements RunnerAssetReader, PathProvidingAssetReader { + final RunnerAssetReader _inner; + final PathProvidingAssetReader _innerPathProviding; + + BatchAwareReader(this._inner, this._innerPathProviding); + + _PendingFileState? _stateFor(AssetId id) { + if (FileSystemWriteBatch.current case final batch?) { + return batch._pendingWrites[id]; + } else { + return null; + } + } + + @override + Future canRead(AssetId id) async { + if (_stateFor(id) case final state?) { + return !state.isDeleted; + } else { + return await _inner.canRead(id); + } + } + + @override + Stream findAssets(Glob glob, {String? package}) { + return _inner + .findAssets(glob, package: package) + .where((asset) => _stateFor(asset)?.isDeleted != true); + } + + @override + String pathTo(AssetId id) { + return _innerPathProviding.pathTo(id); + } + + @override + Future> readAsBytes(AssetId id) async { + if (_stateFor(id) case final state?) { + if (state.isDeleted) { + throw AssetNotFoundException(id); + } else { + return state.content!; + } + } else { + return await _inner.readAsBytes(id); + } + } + + @override + Future readAsString(AssetId id, {Encoding encoding = utf8}) async { + final bytes = await readAsBytes(id); + return encoding.decode(bytes); + } +} + +@internal +final class BatchAwareWriter extends RunnerAssetWriter { + final RunnerAssetWriter _inner; + + BatchAwareWriter(this._inner); + + FileSystemWriteBatch? _batchFor(AssetId id) { + // Don't batch writes for hidden files unlikely to be consumed by other + // tools. + if (id.pathSegments case ['.dart_tool', ...]) { + return null; + } + + return FileSystemWriteBatch.current; + } + + @override + Future delete(AssetId id) async { + if (_batchFor(id) case final batch?) { + batch._pendingWrites[id] = const _PendingFileState(null); + } else { + await _inner.delete(id); + } + } + + @override + Future writeAsBytes(AssetId id, List bytes) async { + if (_batchFor(id) case final batch?) { + batch._pendingWrites[id] = _PendingFileState(bytes); + } else { + await _inner.writeAsBytes(id, bytes); + } + } + + @override + Future writeAsString(AssetId id, String contents, + {Encoding encoding = utf8}) async { + if (_batchFor(id) case final batch?) { + batch._pendingWrites[id] = _PendingFileState(encoding.encode(contents)); + } else { + await _inner.writeAsString(id, contents, encoding: encoding); + } + } +} diff --git a/build_runner_core/lib/src/environment/io_environment.dart b/build_runner_core/lib/src/environment/io_environment.dart index 74f7ad1d9..8d2285c97 100644 --- a/build_runner_core/lib/src/environment/io_environment.dart +++ b/build_runner_core/lib/src/environment/io_environment.dart @@ -8,6 +8,7 @@ import 'dart:io'; import 'package:build/build.dart'; import 'package:logging/logging.dart'; +import '../asset/batch.dart'; import '../asset/file_based.dart'; import '../asset/reader.dart'; import '../asset/writer.dart'; @@ -34,12 +35,15 @@ class IOEnvironment implements BuildEnvironment { final PackageGraph _packageGraph; - IOEnvironment(this._packageGraph, - {bool? assumeTty, bool outputSymlinksOnly = false}) - : _isInteractive = assumeTty == true || _canPrompt(), + IOEnvironment( + this._packageGraph, { + bool? assumeTty, + bool outputSymlinksOnly = false, + bool lowResourcesMode = false, + }) : _isInteractive = assumeTty == true || _canPrompt(), _outputSymlinksOnly = outputSymlinksOnly, - reader = FileBasedAssetReader(_packageGraph), - writer = FileBasedAssetWriter(_packageGraph) { + reader = _createReader(_packageGraph, lowResourcesMode), + writer = _createWriter(_packageGraph, lowResourcesMode) { if (_outputSymlinksOnly && Platform.isWindows) { _logger.warning('Symlinks to files are not yet working on Windows, you ' 'may experience issues using this mode. Follow ' @@ -89,6 +93,18 @@ class IOEnvironment implements BuildEnvironment { } return buildResult; } + + static RunnerAssetReader _createReader( + PackageGraph packageGraph, bool lowResources) { + var reader = FileBasedAssetReader(packageGraph); + return lowResources ? reader : BatchAwareReader(reader, reader); + } + + static RunnerAssetWriter _createWriter( + PackageGraph packageGraph, bool lowResources) { + var writer = FileBasedAssetWriter(packageGraph); + return lowResources ? writer : BatchAwareWriter(writer); + } } bool _canPrompt() => diff --git a/build_runner_core/pubspec.yaml b/build_runner_core/pubspec.yaml index 8911cf0aa..6a2dfaffc 100644 --- a/build_runner_core/pubspec.yaml +++ b/build_runner_core/pubspec.yaml @@ -1,5 +1,5 @@ name: build_runner_core -version: 7.3.3-wip +version: 7.4.0-wip description: Core tools to organize the structure of a build and run Builders. repository: https://github.com/dart-lang/build/tree/master/build_runner_core diff --git a/build_runner_core/test/asset/batch_test.dart b/build_runner_core/test/asset/batch_test.dart new file mode 100644 index 000000000..b4bffe501 --- /dev/null +++ b/build_runner_core/test/asset/batch_test.dart @@ -0,0 +1,103 @@ +// Copyright (c) 2024, 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. +@TestOn('vm') +library; + +import 'dart:io'; + +import 'package:build/build.dart'; +import 'package:build_runner_core/build_runner_core.dart'; +import 'package:build_runner_core/src/asset/batch.dart'; +import 'package:glob/glob.dart'; +import 'package:package_config/package_config_types.dart'; +import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +void main() { + late PackageGraph packageGraph; + late RunnerAssetReader reader; + late RunnerAssetWriter underlyingWriter, writer; + + setUp(() async { + packageGraph = await _createTestPackage(); + final fileReader = FileBasedAssetReader(packageGraph); + final fileWriter = underlyingWriter = FileBasedAssetWriter(packageGraph); + + reader = BatchAwareReader(fileReader, fileReader); + writer = BatchAwareWriter(fileWriter); + }); + + test('delays writes until end', () async { + await runWithFileSystemBatch(() async { + await writer.writeAsString(_generatedAsset, ''); + expect(await File(d.path('pkg/lib/generated.dart')).exists(), isFalse); + }, underlyingWriter); + + expect(await File(d.path('pkg/lib/generated.dart')).exists(), isTrue); + }); + + test('can read pending writes', () async { + await runWithFileSystemBatch(() async { + await writer.writeAsString(_generatedAsset, 'content'); + expect(await reader.readAsString(_generatedAsset), 'content'); + }, underlyingWriter); + }); + + test('delays deletions', () async { + await runWithFileSystemBatch(() async { + await writer.delete(AssetId.parse('root|lib/source.dart')); + expect(await File(d.path('pkg/lib/source.dart')).exists(), isTrue); + }, underlyingWriter); + + expect(await File(d.path('pkg/lib/source.dart')).exists(), isFalse); + }); + + test('deletions are consistent with reader', () async { + await runWithFileSystemBatch(() async { + final glob = Glob('lib/**'); + await expectLater(reader.readAsString(_sourceAsset), completes); + await expectLater( + reader.findAssets(glob, package: 'root'), emits(_sourceAsset)); + + await writer.delete(_sourceAsset); + + await expectLater(() => reader.readAsString(_sourceAsset), + throwsA(isA())); + await expectLater( + reader.findAssets(glob, package: 'root'), neverEmits(_sourceAsset)); + }, underlyingWriter); + }); + + test('does not delay writes in .dart_tool', () async { + await runWithFileSystemBatch(() async { + await writer.writeAsString(AssetId.parse('root|.dart_tool/cache'), ''); + expect(await File(d.path('pkg/.dart_tool/cache')).exists(), isTrue); + }, underlyingWriter); + }); +} + +AssetId _sourceAsset = AssetId.parse('root|lib/source.dart'); +AssetId _generatedAsset = AssetId.parse('root|lib/generated.dart'); + +Future _createTestPackage() async { + await d.dir('pkg', [ + d.file('pubspec.yaml', ''' +name: root + +environment: + sdk: ^3.5.0 +'''), + d.dir('lib', [ + d.file('source.dart'), + ]), + ]).create(); + + return PackageGraph.fromRoot(PackageNode( + 'root', + d.path('pkg'), + DependencyType.path, + LanguageVersion.parse('3.5'), + isRoot: true, + )); +} diff --git a/pubspec.yaml b/pubspec.yaml index 66d17e67c..61c4510c3 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -20,7 +20,7 @@ workspace: - build_daemon - build_modules # - build_resolvers -- build_runner +#- build_runner # - build_runner_core - build_test #- build_web_compilers From e1259f8f8eb8b9f8c6fad97f07ac1d2b3cc77af1 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Sat, 16 Nov 2024 01:23:08 +0100 Subject: [PATCH 2/7] Add changelog entry to build runner --- build_runner/CHANGELOG.md | 5 +++++ build_runner/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index 512b38f81..285d8aeaa 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.4.14-wip + +- Write generated assets at the end of a build to avoid invalidating other + tools with a file watcher multiple times. + ## 2.4.13 - Bump the min sdk to 3.5.0. diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index f073463c1..e052d6bf8 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -1,5 +1,5 @@ name: build_runner -version: 2.4.13 +version: 2.4.14-wip description: A build system for Dart code generation and modular compilation. repository: https://github.com/dart-lang/build/tree/master/build_runner #resolution: workspace From d7c262379c3fcc63e304d935257029c53493067f Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Mon, 18 Nov 2024 23:30:40 +0100 Subject: [PATCH 3/7] Review feedback --- build_runner/lib/src/generate/build.dart | 24 ++-- build_runner/lib/src/generate/watch_impl.dart | 6 +- .../lib/src/watcher/delete_writer.dart | 5 + build_runner_core/lib/build_runner_core.dart | 2 +- build_runner_core/lib/src/asset/batch.dart | 125 +++++++----------- .../lib/src/asset/build_cache.dart | 3 + .../lib/src/asset/file_based.dart | 3 + build_runner_core/lib/src/asset/writer.dart | 6 + .../lib/src/environment/io_environment.dart | 39 +++--- .../lib/src/generate/build_impl.dart | 7 +- build_runner_core/test/asset/batch_test.dart | 61 ++++----- 11 files changed, 131 insertions(+), 150 deletions(-) diff --git a/build_runner/lib/src/generate/build.dart b/build_runner/lib/src/generate/build.dart index 30de63b96..4c827d8af 100644 --- a/build_runner/lib/src/generate/build.dart +++ b/build_runner/lib/src/generate/build.dart @@ -106,19 +106,17 @@ Future build(List builders, ); var terminator = Terminator(terminateEventStream); try { - return await runWithFileSystemBatch(() async { - var build = await BuildRunner.create( - options, - environment, - builders, - builderConfigOverrides!, - isReleaseBuild: isReleaseBuild!, - ); - var result = await build - .run({}, buildDirs: buildDirs!, buildFilters: buildFilters!); - await build.beforeExit(); - return result; - }, environment.writer); + var build = await BuildRunner.create( + options, + environment, + builders, + builderConfigOverrides, + isReleaseBuild: isReleaseBuild, + ); + var result = + await build.run({}, buildDirs: buildDirs, buildFilters: buildFilters); + await build.beforeExit(); + return result; } finally { await terminator.cancel(); await options.logListener.cancel(); diff --git a/build_runner/lib/src/generate/watch_impl.dart b/build_runner/lib/src/generate/watch_impl.dart index 85b866799..68726c0f8 100644 --- a/build_runner/lib/src/generate/watch_impl.dart +++ b/build_runner/lib/src/generate/watch_impl.dart @@ -247,10 +247,8 @@ class WatchImpl implements BuildState { failureType: FailureType.buildScriptChanged); } } - return runWithFileSystemBatch( - () => build.run(mergedChanges, - buildDirs: _buildDirs, buildFilters: _buildFilters), - environment.writer); + return build.run(mergedChanges, + buildDirs: _buildDirs, buildFilters: _buildFilters); } var terminate = Future.any([until, _terminateCompleter.future]).then((_) { diff --git a/build_runner/lib/src/watcher/delete_writer.dart b/build_runner/lib/src/watcher/delete_writer.dart index e8092f5dc..daee6ce3c 100644 --- a/build_runner/lib/src/watcher/delete_writer.dart +++ b/build_runner/lib/src/watcher/delete_writer.dart @@ -29,4 +29,9 @@ class OnDeleteWriter implements RunnerAssetWriter { Future writeAsString(AssetId id, String contents, {Encoding encoding = utf8}) => _writer.writeAsString(id, contents, encoding: encoding); + + @override + Future completeBuild() async { + await _writer.completeBuild(); + } } diff --git a/build_runner_core/lib/build_runner_core.dart b/build_runner_core/lib/build_runner_core.dart index 06f1f8aeb..55fa3db70 100644 --- a/build_runner_core/lib/build_runner_core.dart +++ b/build_runner_core/lib/build_runner_core.dart @@ -4,7 +4,7 @@ export 'package:build/build.dart' show PostProcessBuildStep, PostProcessBuilder; -export 'src/asset/batch.dart' show runWithFileSystemBatch; +export 'src/asset/batch.dart' show wrapInBatch; export 'src/asset/file_based.dart'; export 'src/asset/finalized_reader.dart'; export 'src/asset/reader.dart' show RunnerAssetReader; diff --git a/build_runner_core/lib/src/asset/batch.dart b/build_runner_core/lib/src/asset/batch.dart index 045d2b14f..d788cfa6e 100644 --- a/build_runner_core/lib/src/asset/batch.dart +++ b/build_runner_core/lib/src/asset/batch.dart @@ -13,37 +13,6 @@ import '../environment/io_environment.dart'; import 'reader.dart'; import 'writer.dart'; -/// Runs [inner] in a zone that indicates to compatible [RunnerAssetWriter]s -/// that written assets should not be written to the underlying file system -/// directly, but rather in a batch after [inner] has completed. -/// -/// This solves a common issue when `build_runner` and other tools such as an -/// analysis server are running concurrently: During a build, generated assets -/// are written one-by-one. The analyzer will discover these changes while the -/// build is running and report lints for a partial state of the workspace where -/// not all generated files are present, leading to a subpar experience. -/// -/// [runWithFileSystemBatch] is intended to wrap a single build run. Instead of -/// writing assets directly, they are first cached in memory and then written at -/// once after the build has completed. This causes fewer invalidations for -/// external tools. The [writer] will be used to complete the writes collected -/// while running [inner]. -/// -/// The underlying asset readers and writes both need to support file system -/// batches (reader support is crucial to report changed assets that have not -/// been flushed to the file system yet). The default [IOEnvironment] will use -/// batch-aware readers and writers when not in low-resources mode. -Future runWithFileSystemBatch( - Future Function() inner, - RunnerAssetWriter writer, -) async { - final batch = FileSystemWriteBatch._(); - final result = - await runZoned(inner, zoneValues: {FileSystemWriteBatch._key: batch}); - await batch._completeWrites(writer); - return result; -} - /// A batch of file system writes that should be committed at once instead of /// when [AssetWriter.writeAsBytes] or [AssetWriter.writeAsString] is called. /// @@ -61,23 +30,39 @@ final class FileSystemWriteBatch { FileSystemWriteBatch._(); - Future _completeWrites(RunnerAssetWriter writer) async { - await Future.wait(_pendingWrites.entries.map((e) async { - if (e.value.content case final content?) { - await writer.writeAsBytes(e.key, content); + Future completeWrites(RunnerAssetWriter writer) async { + await Future.wait(_pendingWrites.keys.map((id) async { + final pending = _pendingWrites[id]!; + + if (pending.content case final content?) { + await writer.writeAsBytes(id, content); } else { - await writer.delete(e.key); + await writer.delete(id); } })); _pendingWrites.clear(); } +} - static FileSystemWriteBatch? get current { - return Zone.current[_key] as FileSystemWriteBatch?; - } +/// Wraps a pair of a [RunnerAssetReader] with path-prividing capabilities and +/// a [RunnerAssetWriter] into a pair of readers and writers that will +/// internally buffer writes and only flush them in +/// [RunnerAssetWriter.completeBuild]. +/// +/// The returned reader will see pending writes by the returned writer before +/// they are flushed to the file system. +(RunnerAssetReader, RunnerAssetWriter) wrapInBatch({ + required RunnerAssetReader reader, + required PathProvidingAssetReader pathProvidingReader, + required RunnerAssetWriter writer, +}) { + final batch = FileSystemWriteBatch._(); - static final _key = Object(); + return ( + BatchReader(reader, pathProvidingReader, batch), + BatchWriter(writer, batch), + ); } final class _PendingFileState { @@ -89,19 +74,16 @@ final class _PendingFileState { } @internal -final class BatchAwareReader extends AssetReader +final class BatchReader extends AssetReader implements RunnerAssetReader, PathProvidingAssetReader { final RunnerAssetReader _inner; final PathProvidingAssetReader _innerPathProviding; + final FileSystemWriteBatch _batch; - BatchAwareReader(this._inner, this._innerPathProviding); + BatchReader(this._inner, this._innerPathProviding, this._batch); _PendingFileState? _stateFor(AssetId id) { - if (FileSystemWriteBatch.current case final batch?) { - return batch._pendingWrites[id]; - } else { - return null; - } + return _batch._pendingWrites[id]; } @override @@ -140,52 +122,43 @@ final class BatchAwareReader extends AssetReader @override Future readAsString(AssetId id, {Encoding encoding = utf8}) async { - final bytes = await readAsBytes(id); - return encoding.decode(bytes); + if (_stateFor(id) case final state?) { + if (state.isDeleted) { + throw AssetNotFoundException(id); + } else { + return encoding.decode(state.content!); + } + } else { + return await _inner.readAsString(id, encoding: encoding); + } } } @internal -final class BatchAwareWriter extends RunnerAssetWriter { +final class BatchWriter extends RunnerAssetWriter { final RunnerAssetWriter _inner; + final FileSystemWriteBatch _batch; - BatchAwareWriter(this._inner); - - FileSystemWriteBatch? _batchFor(AssetId id) { - // Don't batch writes for hidden files unlikely to be consumed by other - // tools. - if (id.pathSegments case ['.dart_tool', ...]) { - return null; - } - - return FileSystemWriteBatch.current; - } + BatchWriter(this._inner, this._batch); @override Future delete(AssetId id) async { - if (_batchFor(id) case final batch?) { - batch._pendingWrites[id] = const _PendingFileState(null); - } else { - await _inner.delete(id); - } + _batch._pendingWrites[id] = const _PendingFileState(null); } @override Future writeAsBytes(AssetId id, List bytes) async { - if (_batchFor(id) case final batch?) { - batch._pendingWrites[id] = _PendingFileState(bytes); - } else { - await _inner.writeAsBytes(id, bytes); - } + _batch._pendingWrites[id] = _PendingFileState(bytes); } @override Future writeAsString(AssetId id, String contents, {Encoding encoding = utf8}) async { - if (_batchFor(id) case final batch?) { - batch._pendingWrites[id] = _PendingFileState(encoding.encode(contents)); - } else { - await _inner.writeAsString(id, contents, encoding: encoding); - } + _batch._pendingWrites[id] = _PendingFileState(encoding.encode(contents)); + } + + @override + Future completeBuild() async { + await _batch.completeWrites(_inner); } } diff --git a/build_runner_core/lib/src/asset/build_cache.dart b/build_runner_core/lib/src/asset/build_cache.dart index 56b0ae9fa..c9156fc1b 100644 --- a/build_runner_core/lib/src/asset/build_cache.dart +++ b/build_runner_core/lib/src/asset/build_cache.dart @@ -84,6 +84,9 @@ class BuildCacheWriter implements RunnerAssetWriter { @override Future delete(AssetId id) => _delegate.delete(_cacheLocation(id, _assetGraph, _rootPackage)); + + @override + Future completeBuild() async {} } AssetId _cacheLocation(AssetId id, AssetGraph assetGraph, String rootPackage) { diff --git a/build_runner_core/lib/src/asset/file_based.dart b/build_runner_core/lib/src/asset/file_based.dart index 4b74ee9f2..e3e239dee 100644 --- a/build_runner_core/lib/src/asset/file_based.dart +++ b/build_runner_core/lib/src/asset/file_based.dart @@ -107,6 +107,9 @@ class FileBasedAssetWriter implements RunnerAssetWriter { } }); } + + @override + Future completeBuild() async {} } /// Returns the path to [id] for a given [packageGraph]. diff --git a/build_runner_core/lib/src/asset/writer.dart b/build_runner_core/lib/src/asset/writer.dart index f4769eada..9fd01c714 100644 --- a/build_runner_core/lib/src/asset/writer.dart +++ b/build_runner_core/lib/src/asset/writer.dart @@ -11,4 +11,10 @@ typedef OnDelete = void Function(AssetId id); abstract class RunnerAssetWriter implements AssetWriter { Future delete(AssetId id); + + /// Called after each completed build. + /// + /// Some [RunnerAssetWriter] implementations may buffer completed writes + /// internally and flush them in [completeBuild]. + Future completeBuild(); } diff --git a/build_runner_core/lib/src/environment/io_environment.dart b/build_runner_core/lib/src/environment/io_environment.dart index 8d2285c97..4ca30ab5a 100644 --- a/build_runner_core/lib/src/environment/io_environment.dart +++ b/build_runner_core/lib/src/environment/io_environment.dart @@ -35,20 +35,33 @@ class IOEnvironment implements BuildEnvironment { final PackageGraph _packageGraph; - IOEnvironment( - this._packageGraph, { + IOEnvironment._(this.reader, this.writer, this._isInteractive, + this._outputSymlinksOnly, this._packageGraph); + + factory IOEnvironment( + PackageGraph packageGraph, { bool? assumeTty, bool outputSymlinksOnly = false, bool lowResourcesMode = false, - }) : _isInteractive = assumeTty == true || _canPrompt(), - _outputSymlinksOnly = outputSymlinksOnly, - reader = _createReader(_packageGraph, lowResourcesMode), - writer = _createWriter(_packageGraph, lowResourcesMode) { - if (_outputSymlinksOnly && Platform.isWindows) { + }) { + if (outputSymlinksOnly && Platform.isWindows) { _logger.warning('Symlinks to files are not yet working on Windows, you ' 'may experience issues using this mode. Follow ' 'https://github.com/dart-lang/sdk/issues/33966 for updates.'); } + + var fileReader = FileBasedAssetReader(packageGraph); + var fileWriter = FileBasedAssetWriter(packageGraph); + + var (reader, writer) = lowResourcesMode + ? (fileReader, fileWriter) + : wrapInBatch( + reader: fileReader, + pathProvidingReader: fileReader, + writer: fileWriter); + + return IOEnvironment._(reader, writer, assumeTty == true || _canPrompt(), + outputSymlinksOnly, packageGraph); } @override @@ -93,18 +106,6 @@ class IOEnvironment implements BuildEnvironment { } return buildResult; } - - static RunnerAssetReader _createReader( - PackageGraph packageGraph, bool lowResources) { - var reader = FileBasedAssetReader(packageGraph); - return lowResources ? reader : BatchAwareReader(reader, reader); - } - - static RunnerAssetWriter _createWriter( - PackageGraph packageGraph, bool lowResources) { - var writer = FileBasedAssetWriter(packageGraph); - return lowResources ? writer : BatchAwareWriter(writer); - } } bool _canPrompt() => diff --git a/build_runner_core/lib/src/generate/build_impl.dart b/build_runner_core/lib/src/generate/build_impl.dart index 71db62a16..5fffd8e85 100644 --- a/build_runner_core/lib/src/generate/build_impl.dart +++ b/build_runner_core/lib/src/generate/build_impl.dart @@ -93,7 +93,12 @@ class BuildImpl { Set buildFilters = const {}}) { finalizedReader.reset(_buildPaths(buildDirs), buildFilters); return _SingleBuild(this, buildDirs, buildFilters).run(updates) - ..whenComplete(_resolvers.reset); + ..whenComplete(_handleCompletedBuild); + } + + Future _handleCompletedBuild() async { + await _environment.writer.completeBuild(); + _resolvers.reset(); } static Future create( diff --git a/build_runner_core/test/asset/batch_test.dart b/build_runner_core/test/asset/batch_test.dart index b4bffe501..b8f6e00a4 100644 --- a/build_runner_core/test/asset/batch_test.dart +++ b/build_runner_core/test/asset/batch_test.dart @@ -8,7 +8,7 @@ import 'dart:io'; import 'package:build/build.dart'; import 'package:build_runner_core/build_runner_core.dart'; -import 'package:build_runner_core/src/asset/batch.dart'; + import 'package:glob/glob.dart'; import 'package:package_config/package_config_types.dart'; import 'package:test/test.dart'; @@ -17,63 +17,52 @@ import 'package:test_descriptor/test_descriptor.dart' as d; void main() { late PackageGraph packageGraph; late RunnerAssetReader reader; - late RunnerAssetWriter underlyingWriter, writer; + late RunnerAssetWriter writer; setUp(() async { packageGraph = await _createTestPackage(); final fileReader = FileBasedAssetReader(packageGraph); - final fileWriter = underlyingWriter = FileBasedAssetWriter(packageGraph); + final fileWriter = FileBasedAssetWriter(packageGraph); - reader = BatchAwareReader(fileReader, fileReader); - writer = BatchAwareWriter(fileWriter); + (reader, writer) = wrapInBatch( + reader: fileReader, + pathProvidingReader: fileReader, + writer: fileWriter); }); test('delays writes until end', () async { - await runWithFileSystemBatch(() async { - await writer.writeAsString(_generatedAsset, ''); - expect(await File(d.path('pkg/lib/generated.dart')).exists(), isFalse); - }, underlyingWriter); + await writer.writeAsString(_generatedAsset, ''); + expect(await File(d.path('pkg/lib/generated.dart')).exists(), isFalse); + await writer.completeBuild(); expect(await File(d.path('pkg/lib/generated.dart')).exists(), isTrue); }); test('can read pending writes', () async { - await runWithFileSystemBatch(() async { - await writer.writeAsString(_generatedAsset, 'content'); - expect(await reader.readAsString(_generatedAsset), 'content'); - }, underlyingWriter); + await writer.writeAsString(_generatedAsset, 'content'); + expect(await reader.readAsString(_generatedAsset), 'content'); }); test('delays deletions', () async { - await runWithFileSystemBatch(() async { - await writer.delete(AssetId.parse('root|lib/source.dart')); - expect(await File(d.path('pkg/lib/source.dart')).exists(), isTrue); - }, underlyingWriter); + await writer.delete(AssetId.parse('root|lib/source.dart')); + expect(await File(d.path('pkg/lib/source.dart')).exists(), isTrue); + await writer.completeBuild(); expect(await File(d.path('pkg/lib/source.dart')).exists(), isFalse); }); test('deletions are consistent with reader', () async { - await runWithFileSystemBatch(() async { - final glob = Glob('lib/**'); - await expectLater(reader.readAsString(_sourceAsset), completes); - await expectLater( - reader.findAssets(glob, package: 'root'), emits(_sourceAsset)); - - await writer.delete(_sourceAsset); - - await expectLater(() => reader.readAsString(_sourceAsset), - throwsA(isA())); - await expectLater( - reader.findAssets(glob, package: 'root'), neverEmits(_sourceAsset)); - }, underlyingWriter); - }); + final glob = Glob('lib/**'); + await expectLater(reader.readAsString(_sourceAsset), completes); + await expectLater( + reader.findAssets(glob, package: 'root'), emits(_sourceAsset)); + + await writer.delete(_sourceAsset); - test('does not delay writes in .dart_tool', () async { - await runWithFileSystemBatch(() async { - await writer.writeAsString(AssetId.parse('root|.dart_tool/cache'), ''); - expect(await File(d.path('pkg/.dart_tool/cache')).exists(), isTrue); - }, underlyingWriter); + await expectLater(() => reader.readAsString(_sourceAsset), + throwsA(isA())); + await expectLater( + reader.findAssets(glob, package: 'root'), neverEmits(_sourceAsset)); }); } From dc9a81b6309373c79fb679f88926cb502abd0629 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Mon, 18 Nov 2024 23:35:42 +0100 Subject: [PATCH 4/7] Bump version of build_runner_core --- build_runner/pubspec.yaml | 2 +- build_runner_core/CHANGELOG.md | 8 +++++--- build_runner_core/pubspec.yaml | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index e052d6bf8..ed122de5f 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -20,7 +20,7 @@ dependencies: build_config: ">=1.1.0 <1.2.0" build_daemon: ^4.0.0 build_resolvers: ^2.0.0 - build_runner_core: ^7.4.0-wip + build_runner_core: ^8.0.0-wip code_builder: ^4.2.0 collection: ^1.15.0 crypto: ^3.0.0 diff --git a/build_runner_core/CHANGELOG.md b/build_runner_core/CHANGELOG.md index 6328f0f43..700eae87a 100644 --- a/build_runner_core/CHANGELOG.md +++ b/build_runner_core/CHANGELOG.md @@ -1,7 +1,9 @@ -## 7.4.0-wip +## 8.0.0-wip -- Add `runWithFileSystemBatch` to batch asset writes before flushing them at - once. +- __Breaking__: Add `completeBuild` to `RunnerAssetWriter`, a method expected + to be called by the build system at the end of a completed build. +- Add `wrapInBatch` to obtain a reader/writer pair that will batch writes + before flushing them at the end of a build. - Bump the min sdk to 3.6.0-dev.228. - Require analyzer ^6.9.0. - Fix analyzer deprecations. diff --git a/build_runner_core/pubspec.yaml b/build_runner_core/pubspec.yaml index 6a2dfaffc..50963d155 100644 --- a/build_runner_core/pubspec.yaml +++ b/build_runner_core/pubspec.yaml @@ -1,5 +1,5 @@ name: build_runner_core -version: 7.4.0-wip +version: 8.0.0-wip description: Core tools to organize the structure of a build and run Builders. repository: https://github.com/dart-lang/build/tree/master/build_runner_core From 3f7a5e3fcbefc91f0569e764279ed4181aef525c Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Tue, 19 Nov 2024 18:46:37 +0100 Subject: [PATCH 5/7] Make batch private, fix dependency resolution --- build_runner_core/lib/src/asset/batch.dart | 11 +++++------ build_runner_core/pubspec.yaml | 4 ++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/build_runner_core/lib/src/asset/batch.dart b/build_runner_core/lib/src/asset/batch.dart index d788cfa6e..7e904f37f 100644 --- a/build_runner_core/lib/src/asset/batch.dart +++ b/build_runner_core/lib/src/asset/batch.dart @@ -24,11 +24,10 @@ import 'writer.dart'; /// /// The default [IOEnvironment] uses readers and writes that are batch-aware /// outside of low-memory mode. -@internal -final class FileSystemWriteBatch { +final class _FileSystemWriteBatch { final Map _pendingWrites = {}; - FileSystemWriteBatch._(); + _FileSystemWriteBatch._(); Future completeWrites(RunnerAssetWriter writer) async { await Future.wait(_pendingWrites.keys.map((id) async { @@ -57,7 +56,7 @@ final class FileSystemWriteBatch { required PathProvidingAssetReader pathProvidingReader, required RunnerAssetWriter writer, }) { - final batch = FileSystemWriteBatch._(); + final batch = _FileSystemWriteBatch._(); return ( BatchReader(reader, pathProvidingReader, batch), @@ -78,7 +77,7 @@ final class BatchReader extends AssetReader implements RunnerAssetReader, PathProvidingAssetReader { final RunnerAssetReader _inner; final PathProvidingAssetReader _innerPathProviding; - final FileSystemWriteBatch _batch; + final _FileSystemWriteBatch _batch; BatchReader(this._inner, this._innerPathProviding, this._batch); @@ -137,7 +136,7 @@ final class BatchReader extends AssetReader @internal final class BatchWriter extends RunnerAssetWriter { final RunnerAssetWriter _inner; - final FileSystemWriteBatch _batch; + final _FileSystemWriteBatch _batch; BatchWriter(this._inner, this._batch); diff --git a/build_runner_core/pubspec.yaml b/build_runner_core/pubspec.yaml index 50963d155..f44425f1b 100644 --- a/build_runner_core/pubspec.yaml +++ b/build_runner_core/pubspec.yaml @@ -49,3 +49,7 @@ dev_dependencies: topics: - build-runner + +dependency_overrides: + build_runner: + path: ../build_runner From 43f26d37a63d3f12a6797f7cedd5573cf782de3c Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Tue, 19 Nov 2024 19:43:51 +0100 Subject: [PATCH 6/7] Start fixing tests --- _test_common/lib/in_memory_writer.dart | 3 +++ _test_common/lib/runner_asset_writer_spy.dart | 3 +++ _test_common/lib/sdk.dart | 6 ++---- _test_common/pubspec.yaml | 12 +++++++++++- .../integration_tests/utils/build_descriptor.dart | 5 +++-- build_runner_core/lib/src/generate/build_impl.dart | 8 ++------ pubspec.yaml | 2 +- 7 files changed, 25 insertions(+), 14 deletions(-) diff --git a/_test_common/lib/in_memory_writer.dart b/_test_common/lib/in_memory_writer.dart index 194650917..2435845ff 100644 --- a/_test_common/lib/in_memory_writer.dart +++ b/_test_common/lib/in_memory_writer.dart @@ -38,4 +38,7 @@ class InMemoryRunnerAssetWriter extends InMemoryAssetWriter FakeWatcher.notifyWatchers(WatchEvent( ChangeType.REMOVE, p.absolute(id.package, p.fromUri(id.path)))); } + + @override + Future completeBuild() async {} } diff --git a/_test_common/lib/runner_asset_writer_spy.dart b/_test_common/lib/runner_asset_writer_spy.dart index 33ad297ba..17ace2422 100644 --- a/_test_common/lib/runner_asset_writer_spy.dart +++ b/_test_common/lib/runner_asset_writer_spy.dart @@ -20,4 +20,7 @@ class RunnerAssetWriterSpy extends AssetWriterSpy implements RunnerAssetWriter { _assetsDeleted.add(id); return _delegate.delete(id); } + + @override + Future completeBuild() async {} } diff --git a/_test_common/lib/sdk.dart b/_test_common/lib/sdk.dart index fcd13a5c9..d67d08dba 100644 --- a/_test_common/lib/sdk.dart +++ b/_test_common/lib/sdk.dart @@ -12,8 +12,6 @@ import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; -String _dartBinary = p.join(sdkBin, 'dart'); - final bool supportsUnsoundNullSafety = Version.parse(Platform.version.split(' ').first).major == 2; @@ -54,7 +52,7 @@ Future startPub(String package, String command, /// The [script] should be a relative path under [package]. Future runDart(String package, String script, {Iterable? args}) => - Process.run(_dartBinary, [script, ...?args], + Process.run(dartBinary, [script, ...?args], workingDirectory: p.join(d.sandbox, package)); /// Starts the `dart` script [script] in [package] with [args]. @@ -62,5 +60,5 @@ Future runDart(String package, String script, /// The [script] should be a relative path under [package]. Future startDart(String package, String script, {Iterable? args}) => - Process.start(_dartBinary, [script, ...?args], + Process.start(dartBinary, [script, ...?args], workingDirectory: p.join(d.sandbox, package)); diff --git a/_test_common/pubspec.yaml b/_test_common/pubspec.yaml index 3e37eaca1..6145bab9a 100644 --- a/_test_common/pubspec.yaml +++ b/_test_common/pubspec.yaml @@ -1,7 +1,7 @@ name: _test_common publish_to: none description: Test infra for writing build tests. Is not published. -resolution: workspace +#resolution: workspace environment: sdk: ^3.5.0 @@ -19,3 +19,13 @@ dependencies: test: ^1.16.0 test_descriptor: ^2.0.0 watcher: ^1.0.0 + +dependency_overrides: + build: + path: ../build + build_config: + path: ../build_config + build_runner_core: + path: ../build_runner_core + build_test: + path: ../build_test diff --git a/build_runner/test/integration_tests/utils/build_descriptor.dart b/build_runner/test/integration_tests/utils/build_descriptor.dart index f3afdefc5..f91e8530e 100644 --- a/build_runner/test/integration_tests/utils/build_descriptor.dart +++ b/build_runner/test/integration_tests/utils/build_descriptor.dart @@ -10,6 +10,7 @@ import 'dart:isolate'; import 'package:_test_common/sdk.dart'; import 'package:async/async.dart'; import 'package:build/build.dart'; +import 'package:build_runner_core/build_runner_core.dart'; import 'package:package_config/package_config.dart'; import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart'; @@ -106,7 +107,7 @@ Future package(Iterable otherPackages, ]).create(); await Future.wait(otherPackages.map((d) => d.create())); await pubGet('a'); - return BuildTool._('dart', ['run', 'build_runner']); + return BuildTool._(dartBinary, ['run', 'build_runner']); } /// Create a package in [d.sandbox] with a `tool/build.dart` script using @@ -145,7 +146,7 @@ Future packageWithBuildScript( ...contents ]).create(); await pubGet('a'); - return BuildTool._('dart', [p.join('tool', 'build.dart')]); + return BuildTool._(dartBinary, [p.join('tool', 'build.dart')]); } String _buildersFile( diff --git a/build_runner_core/lib/src/generate/build_impl.dart b/build_runner_core/lib/src/generate/build_impl.dart index 5fffd8e85..80c8e4854 100644 --- a/build_runner_core/lib/src/generate/build_impl.dart +++ b/build_runner_core/lib/src/generate/build_impl.dart @@ -93,12 +93,7 @@ class BuildImpl { Set buildFilters = const {}}) { finalizedReader.reset(_buildPaths(buildDirs), buildFilters); return _SingleBuild(this, buildDirs, buildFilters).run(updates) - ..whenComplete(_handleCompletedBuild); - } - - Future _handleCompletedBuild() async { - await _environment.writer.completeBuild(); - _resolvers.reset(); + ..whenComplete(_resolvers.reset); } static Future create( @@ -250,6 +245,7 @@ class _SingleBuild { performance: result.performance); } } + await _environment.writer.completeBuild(); await _resourceManager.disposeAll(); result = await _environment.finalizeBuild( result, diff --git a/pubspec.yaml b/pubspec.yaml index 61c4510c3..93db16236 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,7 +14,7 @@ dev_dependencies: workspace: #- _test #- _test/pkgs/provides_builder -- _test_common +#- _test_common # - build - build_config - build_daemon From baf05050eae9875838a40fd5aa2daf196bb1bccd Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Wed, 20 Nov 2024 22:22:39 +0100 Subject: [PATCH 7/7] Add lints dependency for out-of-workspace pkg --- _test_common/pubspec.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/_test_common/pubspec.yaml b/_test_common/pubspec.yaml index 6145bab9a..f52fca1ac 100644 --- a/_test_common/pubspec.yaml +++ b/_test_common/pubspec.yaml @@ -20,6 +20,9 @@ dependencies: test_descriptor: ^2.0.0 watcher: ^1.0.0 +dev_dependencies: + dart_flutter_team_lints: ^3.1.0 + dependency_overrides: build: path: ../build