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..f52fca1ac 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,16 @@ dependencies: test: ^1.16.0 test_descriptor: ^2.0.0 watcher: ^1.0.0 + +dev_dependencies: + dart_flutter_team_lints: ^3.1.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/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/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/pubspec.yaml b/build_runner/pubspec.yaml index 445bdb7fa..ed122de5f 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -1,8 +1,8 @@ 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 +#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: ^8.0.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/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/CHANGELOG.md b/build_runner_core/CHANGELOG.md index 830645e32..700eae87a 100644 --- a/build_runner_core/CHANGELOG.md +++ b/build_runner_core/CHANGELOG.md @@ -1,5 +1,9 @@ -## 7.3.3-wip +## 8.0.0-wip +- __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/lib/build_runner_core.dart b/build_runner_core/lib/build_runner_core.dart index 58b963efb..55fa3db70 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 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 new file mode 100644 index 000000000..7e904f37f --- /dev/null +++ b/build_runner_core/lib/src/asset/batch.dart @@ -0,0 +1,163 @@ +// 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'; + +/// 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. +final class _FileSystemWriteBatch { + final Map _pendingWrites = {}; + + _FileSystemWriteBatch._(); + + 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(id); + } + })); + + _pendingWrites.clear(); + } +} + +/// 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._(); + + return ( + BatchReader(reader, pathProvidingReader, batch), + BatchWriter(writer, batch), + ); +} + +final class _PendingFileState { + final List? content; + + const _PendingFileState(this.content); + + bool get isDeleted => content == null; +} + +@internal +final class BatchReader extends AssetReader + implements RunnerAssetReader, PathProvidingAssetReader { + final RunnerAssetReader _inner; + final PathProvidingAssetReader _innerPathProviding; + final _FileSystemWriteBatch _batch; + + BatchReader(this._inner, this._innerPathProviding, this._batch); + + _PendingFileState? _stateFor(AssetId id) { + return _batch._pendingWrites[id]; + } + + @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 { + 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 BatchWriter extends RunnerAssetWriter { + final RunnerAssetWriter _inner; + final _FileSystemWriteBatch _batch; + + BatchWriter(this._inner, this._batch); + + @override + Future delete(AssetId id) async { + _batch._pendingWrites[id] = const _PendingFileState(null); + } + + @override + Future writeAsBytes(AssetId id, List bytes) async { + _batch._pendingWrites[id] = _PendingFileState(bytes); + } + + @override + Future writeAsString(AssetId id, String contents, + {Encoding encoding = utf8}) async { + _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 74f7ad1d9..4ca30ab5a 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,17 +35,33 @@ class IOEnvironment implements BuildEnvironment { final PackageGraph _packageGraph; - IOEnvironment(this._packageGraph, - {bool? assumeTty, bool outputSymlinksOnly = false}) - : _isInteractive = assumeTty == true || _canPrompt(), - _outputSymlinksOnly = outputSymlinksOnly, - reader = FileBasedAssetReader(_packageGraph), - writer = FileBasedAssetWriter(_packageGraph) { - if (_outputSymlinksOnly && Platform.isWindows) { + IOEnvironment._(this.reader, this.writer, this._isInteractive, + this._outputSymlinksOnly, this._packageGraph); + + factory IOEnvironment( + PackageGraph packageGraph, { + bool? assumeTty, + bool outputSymlinksOnly = false, + bool lowResourcesMode = false, + }) { + 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 diff --git a/build_runner_core/lib/src/generate/build_impl.dart b/build_runner_core/lib/src/generate/build_impl.dart index 71db62a16..80c8e4854 100644 --- a/build_runner_core/lib/src/generate/build_impl.dart +++ b/build_runner_core/lib/src/generate/build_impl.dart @@ -245,6 +245,7 @@ class _SingleBuild { performance: result.performance); } } + await _environment.writer.completeBuild(); await _resourceManager.disposeAll(); result = await _environment.finalizeBuild( result, diff --git a/build_runner_core/pubspec.yaml b/build_runner_core/pubspec.yaml index 8911cf0aa..f44425f1b 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: 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 @@ -49,3 +49,7 @@ dev_dependencies: topics: - build-runner + +dependency_overrides: + build_runner: + path: ../build_runner 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..b8f6e00a4 --- /dev/null +++ b/build_runner_core/test/asset/batch_test.dart @@ -0,0 +1,92 @@ +// 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: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 writer; + + setUp(() async { + packageGraph = await _createTestPackage(); + final fileReader = FileBasedAssetReader(packageGraph); + final fileWriter = FileBasedAssetWriter(packageGraph); + + (reader, writer) = wrapInBatch( + reader: fileReader, + pathProvidingReader: fileReader, + writer: fileWriter); + }); + + test('delays writes until end', () async { + 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 writer.writeAsString(_generatedAsset, 'content'); + expect(await reader.readAsString(_generatedAsset), 'content'); + }); + + test('delays deletions', () async { + 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 { + 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)); + }); +} + +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..93db16236 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,13 +14,13 @@ dev_dependencies: workspace: #- _test #- _test/pkgs/provides_builder -- _test_common +#- _test_common # - build - build_config - build_daemon - build_modules # - build_resolvers -- build_runner +#- build_runner # - build_runner_core - build_test #- build_web_compilers