diff --git a/build_daemon/CHANGELOG.md b/build_daemon/CHANGELOG.md index d5b600b7c..a42590368 100644 --- a/build_daemon/CHANGELOG.md +++ b/build_daemon/CHANGELOG.md @@ -1,5 +1,7 @@ -## 3.0.3-dev +## 3.1.0-dev +- Add `BuildResults.changedAssets` containing asset URIs changed during a + build. - Updated the example to use `dart pub` instead of `pub`. ## 3.0.2 diff --git a/build_daemon/lib/data/build_status.dart b/build_daemon/lib/data/build_status.dart index fbbf8391b..35ab6e663 100644 --- a/build_daemon/lib/data/build_status.dart +++ b/build_daemon/lib/data/build_status.dart @@ -6,6 +6,8 @@ import 'package:built_collection/built_collection.dart'; import 'package:built_value/built_value.dart'; import 'package:built_value/serializer.dart'; +import 'build_target.dart'; + part 'build_status.g.dart'; class BuildStatus extends EnumClass { @@ -59,4 +61,16 @@ abstract class BuildResults BuildResults._(); BuiltList get results; + + /// A list of asset URIs that were modified since the last build. + /// + /// This includes both sources that were updated and affected generated assets + /// that were rebuilt. + /// + /// To avoid communication overhead for clients not interested in this field, + /// it is not set by default. To enable it, register at least one target with + /// [DefaultBuildTarget.reportChangedAssets]. + /// However, build implementations can unconditionally set this field as it + /// is stripped out in the daemon server implementation. + BuiltList? get changedAssets; } diff --git a/build_daemon/lib/data/build_status.g.dart b/build_daemon/lib/data/build_status.g.dart index efe08e3f6..6245cd03a 100644 --- a/build_daemon/lib/data/build_status.g.dart +++ b/build_daemon/lib/data/build_status.g.dart @@ -151,7 +151,15 @@ class _$BuildResultsSerializer implements StructuredSerializer { specifiedType: const FullType(BuiltList, const [const FullType(BuildResult)])), ]; - + Object? value; + value = object.changedAssets; + if (value != null) { + result + ..add('changedAssets') + ..add(serializers.serialize(value, + specifiedType: + const FullType(BuiltList, const [const FullType(Uri)]))); + } return result; } @@ -173,6 +181,12 @@ class _$BuildResultsSerializer implements StructuredSerializer { BuiltList, const [const FullType(BuildResult)]))! as BuiltList); break; + case 'changedAssets': + result.changedAssets.replace(serializers.deserialize(value, + specifiedType: + const FullType(BuiltList, const [const FullType(Uri)]))! + as BuiltList); + break; } } @@ -320,11 +334,13 @@ class DefaultBuildResultBuilder class _$BuildResults extends BuildResults { @override final BuiltList results; + @override + final BuiltList? changedAssets; factory _$BuildResults([void Function(BuildResultsBuilder)? updates]) => (new BuildResultsBuilder()..update(updates)).build(); - _$BuildResults._({required this.results}) : super._() { + _$BuildResults._({required this.results, this.changedAssets}) : super._() { BuiltValueNullFieldError.checkNotNull(results, 'BuildResults', 'results'); } @@ -338,18 +354,21 @@ class _$BuildResults extends BuildResults { @override bool operator ==(Object other) { if (identical(other, this)) return true; - return other is BuildResults && results == other.results; + return other is BuildResults && + results == other.results && + changedAssets == other.changedAssets; } @override int get hashCode { - return $jf($jc(0, results.hashCode)); + return $jf($jc($jc(0, results.hashCode), changedAssets.hashCode)); } @override String toString() { return (newBuiltValueToStringHelper('BuildResults') - ..add('results', results)) + ..add('results', results) + ..add('changedAssets', changedAssets)) .toString(); } } @@ -363,12 +382,19 @@ class BuildResultsBuilder _$this._results ??= new ListBuilder(); set results(ListBuilder? results) => _$this._results = results; + ListBuilder? _changedAssets; + ListBuilder get changedAssets => + _$this._changedAssets ??= new ListBuilder(); + set changedAssets(ListBuilder? changedAssets) => + _$this._changedAssets = changedAssets; + BuildResultsBuilder(); BuildResultsBuilder get _$this { final $v = _$v; if ($v != null) { _results = $v.results.toBuilder(); + _changedAssets = $v.changedAssets?.toBuilder(); _$v = null; } return this; @@ -389,12 +415,16 @@ class BuildResultsBuilder _$BuildResults build() { _$BuildResults _$result; try { - _$result = _$v ?? new _$BuildResults._(results: results.build()); + _$result = _$v ?? + new _$BuildResults._( + results: results.build(), changedAssets: _changedAssets?.build()); } catch (_) { late String _$failedField; try { _$failedField = 'results'; results.build(); + _$failedField = 'changedAssets'; + _changedAssets?.build(); } catch (e) { throw new BuiltValueNestedFieldError( 'BuildResults', _$failedField, e.toString()); diff --git a/build_daemon/lib/data/build_target.dart b/build_daemon/lib/data/build_target.dart index 779b84787..7544e9242 100644 --- a/build_daemon/lib/data/build_target.dart +++ b/build_daemon/lib/data/build_target.dart @@ -2,6 +2,8 @@ import 'package:built_collection/built_collection.dart'; import 'package:built_value/built_value.dart'; import 'package:built_value/serializer.dart'; +import 'build_status.dart'; + part 'build_target.g.dart'; /// The string representation of a build target, e.g. folder path. @@ -16,6 +18,10 @@ abstract class DefaultBuildTarget static Serializer get serializer => _$defaultBuildTargetSerializer; + @BuiltValueHook(initializeBuilder: true) + static void _setDefaults(DefaultBuildTargetBuilder b) => + b.reportChangedAssets = false; + factory DefaultBuildTarget([void Function(DefaultBuildTargetBuilder) b]) = _$DefaultBuildTarget; @@ -38,6 +44,13 @@ abstract class DefaultBuildTarget /// - package:*/** /// - $target/** BuiltSet? get buildFilters; + + /// Whether the [BuildResults] events emitted for this target should report a + /// list of assets invalidated in a build. + /// + /// This defaults to `false` to reduce the serialization overhead when this + /// information is not required. + bool get reportChangedAssets; } /// The location to write the build outputs. diff --git a/build_daemon/lib/data/build_target.g.dart b/build_daemon/lib/data/build_target.g.dart index c60f4248a..10cec22ed 100644 --- a/build_daemon/lib/data/build_target.g.dart +++ b/build_daemon/lib/data/build_target.g.dart @@ -27,6 +27,9 @@ class _$DefaultBuildTargetSerializer serializers.serialize(object.blackListPatterns, specifiedType: const FullType(BuiltSet, const [const FullType(RegExp)])), + 'reportChangedAssets', + serializers.serialize(object.reportChangedAssets, + specifiedType: const FullType(bool)), 'target', serializers.serialize(object.target, specifiedType: const FullType(String)), @@ -79,6 +82,10 @@ class _$DefaultBuildTargetSerializer const FullType(BuiltSet, const [const FullType(String)]))! as BuiltSet); break; + case 'reportChangedAssets': + result.reportChangedAssets = serializers.deserialize(value, + specifiedType: const FullType(bool)) as bool; + break; case 'target': result.target = serializers.deserialize(value, specifiedType: const FullType(String)) as String; @@ -153,6 +160,8 @@ class _$DefaultBuildTarget extends DefaultBuildTarget { @override final BuiltSet? buildFilters; @override + final bool reportChangedAssets; + @override final String target; factory _$DefaultBuildTarget( @@ -163,10 +172,13 @@ class _$DefaultBuildTarget extends DefaultBuildTarget { {required this.blackListPatterns, this.outputLocation, this.buildFilters, + required this.reportChangedAssets, required this.target}) : super._() { BuiltValueNullFieldError.checkNotNull( blackListPatterns, 'DefaultBuildTarget', 'blackListPatterns'); + BuiltValueNullFieldError.checkNotNull( + reportChangedAssets, 'DefaultBuildTarget', 'reportChangedAssets'); BuiltValueNullFieldError.checkNotNull( target, 'DefaultBuildTarget', 'target'); } @@ -187,14 +199,19 @@ class _$DefaultBuildTarget extends DefaultBuildTarget { blackListPatterns == other.blackListPatterns && outputLocation == other.outputLocation && buildFilters == other.buildFilters && + reportChangedAssets == other.reportChangedAssets && target == other.target; } @override int get hashCode { return $jf($jc( - $jc($jc($jc(0, blackListPatterns.hashCode), outputLocation.hashCode), - buildFilters.hashCode), + $jc( + $jc( + $jc($jc(0, blackListPatterns.hashCode), + outputLocation.hashCode), + buildFilters.hashCode), + reportChangedAssets.hashCode), target.hashCode)); } @@ -204,6 +221,7 @@ class _$DefaultBuildTarget extends DefaultBuildTarget { ..add('blackListPatterns', blackListPatterns) ..add('outputLocation', outputLocation) ..add('buildFilters', buildFilters) + ..add('reportChangedAssets', reportChangedAssets) ..add('target', target)) .toString(); } @@ -231,11 +249,18 @@ class DefaultBuildTargetBuilder set buildFilters(SetBuilder? buildFilters) => _$this._buildFilters = buildFilters; + bool? _reportChangedAssets; + bool? get reportChangedAssets => _$this._reportChangedAssets; + set reportChangedAssets(bool? reportChangedAssets) => + _$this._reportChangedAssets = reportChangedAssets; + String? _target; String? get target => _$this._target; set target(String? target) => _$this._target = target; - DefaultBuildTargetBuilder(); + DefaultBuildTargetBuilder() { + DefaultBuildTarget._setDefaults(this); + } DefaultBuildTargetBuilder get _$this { final $v = _$v; @@ -243,6 +268,7 @@ class DefaultBuildTargetBuilder _blackListPatterns = $v.blackListPatterns.toBuilder(); _outputLocation = $v.outputLocation?.toBuilder(); _buildFilters = $v.buildFilters?.toBuilder(); + _reportChangedAssets = $v.reportChangedAssets; _target = $v.target; _$v = null; } @@ -269,6 +295,10 @@ class DefaultBuildTargetBuilder blackListPatterns: blackListPatterns.build(), outputLocation: _outputLocation?.build(), buildFilters: _buildFilters?.build(), + reportChangedAssets: BuiltValueNullFieldError.checkNotNull( + reportChangedAssets, + 'DefaultBuildTarget', + 'reportChangedAssets'), target: BuiltValueNullFieldError.checkNotNull( target, 'DefaultBuildTarget', 'target')); } catch (_) { diff --git a/build_daemon/lib/data/serializers.g.dart b/build_daemon/lib/data/serializers.g.dart index e2eb74a32..9bc79ae17 100644 --- a/build_daemon/lib/data/serializers.g.dart +++ b/build_daemon/lib/data/serializers.g.dart @@ -20,6 +20,9 @@ Serializers _$serializers = (new Serializers().toBuilder() ..addBuilderFactory( const FullType(BuiltList, const [const FullType(BuildResult)]), () => new ListBuilder()) + ..addBuilderFactory( + const FullType(BuiltList, const [const FullType(Uri)]), + () => new ListBuilder()) ..addBuilderFactory( const FullType(BuiltSet, const [const FullType(RegExp)]), () => new SetBuilder()) diff --git a/build_daemon/lib/src/fakes/fake_test_builder.dart b/build_daemon/lib/src/fakes/fake_test_builder.dart index f386b31fa..e4d79cc4a 100644 --- a/build_daemon/lib/src/fakes/fake_test_builder.dart +++ b/build_daemon/lib/src/fakes/fake_test_builder.dart @@ -15,6 +15,8 @@ class FakeTestDaemonBuilder implements DaemonBuilder { static final buildCompletedMessage = 'Build Completed'; final _outputStreamController = StreamController(); + final _buildsController = StreamController.broadcast(); + late final Stream _logs; FakeTestDaemonBuilder() { @@ -22,7 +24,7 @@ class FakeTestDaemonBuilder implements DaemonBuilder { } @override - Stream get builds => Stream.empty(); + Stream get builds => _buildsController.stream; @override Stream get logs => _logs; @@ -34,8 +36,16 @@ class FakeTestDaemonBuilder implements DaemonBuilder { ..loggerName = loggerName ..level = Level.INFO ..message = buildCompletedMessage)); + + _buildsController.add(BuildResults( + (b) => b.changedAssets.add(Uri.parse('package:foo/bar.dart')))); } @override - Future stop() => _outputStreamController.close(); + Future stop() { + return Future.wait([ + _outputStreamController.close(), + _buildsController.close(), + ]); + } } diff --git a/build_daemon/lib/src/managers/build_target_manager.dart b/build_daemon/lib/src/managers/build_target_manager.dart index 34cf72c26..94d51727f 100644 --- a/build_daemon/lib/src/managers/build_target_manager.dart +++ b/build_daemon/lib/src/managers/build_target_manager.dart @@ -19,6 +19,7 @@ bool _shouldBuild(BuildTarget target, Iterable changes) => /// the Dart Build Daemon. class BuildTargetManager { var _buildTargets = >{}; + final _channelSubscriptions = >{}; bool Function(BuildTarget, Iterable) shouldBuild; @@ -37,16 +38,24 @@ class BuildTargetManager { /// Adds a tracked build target with corresponding interested channel. void addBuildTarget(BuildTarget target, WebSocketChannel channel) { _buildTargets.putIfAbsent(target, () => {}).add(channel); + _channelSubscriptions.putIfAbsent(channel, () => {}).add(target); } /// Returns channels that are interested in the provided target. Set channels(BuildTarget target) => _buildTargets[target] ?? {}; - void removeChannel(WebSocketChannel channel) => - _buildTargets = Map.fromEntries(_buildTargets.entries - .map((e) => MapEntry(e.key, e.value..remove(channel))) - .where((e) => e.value.isNotEmpty)); + /// Returns build targets that the [channel] has added. + Iterable targetsFor(WebSocketChannel channel) { + return _channelSubscriptions[channel] ?? const Iterable.empty(); + } + + void removeChannel(WebSocketChannel channel) { + _buildTargets = Map.fromEntries(_buildTargets.entries + .map((e) => MapEntry(e.key, e.value..remove(channel))) + .where((e) => e.value.isNotEmpty)); + _channelSubscriptions.remove(channel); + } Set targetsForChanges(List changes) => targets.where((target) => shouldBuild(target, changes)).toSet(); diff --git a/build_daemon/lib/src/server.dart b/build_daemon/lib/src/server.dart index 26241efb1..12c27d6ec 100644 --- a/build_daemon/lib/src/server.dart +++ b/build_daemon/lib/src/server.dart @@ -144,9 +144,27 @@ class Server { } })) ..add(_builder.builds.listen((status) { - var message = jsonEncode(_serializers.serialize(status)); + // Don't serialize or send changed assets if the client isn't interested + // in them. + String? message, messageWithoutChangedAssets; + for (var channel in _interestedChannels) { - channel.sink.add(message); + var targets = _buildTargetManager.targetsFor(channel); + var wantsChangedAssets = targets + .any((e) => e is DefaultBuildTarget && e.reportChangedAssets); + + String messageForChannel; + + if (wantsChangedAssets) { + messageForChannel = + message ??= jsonEncode(_serializers.serialize(status)); + } else { + messageForChannel = messageWithoutChangedAssets ??= jsonEncode( + _serializers + .serialize(status.rebuild((b) => b.changedAssets = null))); + } + + channel.sink.add(messageForChannel); } })) ..add(_logs.listen((log) { diff --git a/build_daemon/pubspec.yaml b/build_daemon/pubspec.yaml index 7da37211d..6af4fca18 100644 --- a/build_daemon/pubspec.yaml +++ b/build_daemon/pubspec.yaml @@ -1,5 +1,5 @@ name: build_daemon -version: 3.0.3-dev +version: 3.1.0-dev description: A daemon for running Dart builds. repository: https://github.com/dart-lang/build/tree/master/build_daemon @@ -8,7 +8,7 @@ environment: dependencies: built_collection: ^5.0.0 - built_value: ^8.0.0 + built_value: ^8.1.0 http_multi_server: ^3.0.0 logging: ^1.0.0 path: ^1.8.0 @@ -21,8 +21,9 @@ dependencies: dev_dependencies: build_runner: ^2.0.0 + analyzer: '<3.4.0' # TODO: untangle analyzer dependency - built_value_generator: ^8.0.0 + built_value_generator: ^8.1.0 lints: '>=1.0.0 <3.0.0' mockito: ^5.0.0 test: ^1.16.0 diff --git a/build_daemon/test/server_test.dart b/build_daemon/test/server_test.dart index 2ce26740f..f82b43ccd 100644 --- a/build_daemon/test/server_test.dart +++ b/build_daemon/test/server_test.dart @@ -9,6 +9,7 @@ import 'dart:async'; import 'dart:convert'; import 'package:build_daemon/data/build_request.dart'; +import 'package:build_daemon/data/build_status.dart'; import 'package:build_daemon/data/build_target.dart'; import 'package:build_daemon/data/build_target_request.dart'; import 'package:build_daemon/data/serializers.dart'; @@ -16,37 +17,39 @@ import 'package:build_daemon/data/server_log.dart'; import 'package:build_daemon/src/fakes/fake_change_provider.dart'; import 'package:build_daemon/src/fakes/fake_test_builder.dart'; import 'package:build_daemon/src/server.dart'; +import 'package:stream_transform/stream_transform.dart'; import 'package:test/test.dart'; import 'package:web_socket_channel/io.dart'; void main() { group('Server', () { final webTarget = DefaultBuildTarget((b) => b.target = 'web'); - late final StreamController logController; - late final Stream logs; - late final IOWebSocketChannel client; - late final Server server; + late StreamController controller; + late IOWebSocketChannel client; + late Server server; + late int port; setUp(() async { - logController = StreamController(); - logs = logController.stream.asBroadcastStream(); + controller = StreamController(); // Start the server. server = _createServer(); - final port = await server.listen(); + port = await server.listen(); // Connect the client and redirect its logs. - client = _createClient(logController, port); + client = _createClient(controller, port); }); tearDown(() async { await server.stop(); await client.sink.close(); - await logController.close(); + await controller.close(); await expectLater(server.onDone, completes); }); test('can forward logs to the client', () async { + final logs = controller.stream.whereType().asBroadcastStream(); + // Setup listening for a completed build. final buildCompleted = expectLater( logs, @@ -77,6 +80,41 @@ void main() { // Await for logs to arrive to the client. await logsReceived; }); + + test('forwards changed assets to interested clients', () async { + final interestedEvents = StreamController(); + final interstedClient = _createClient(interestedEvents, port); + addTearDown(interstedClient.sink.close); + addTearDown(interestedEvents.close); + + // Register default client, not intersted in changes + client.sink.add(jsonEncode(serializers + .serialize(BuildTargetRequest((b) => b.target = webTarget)))); + + // Register second client which is interested in changes. + final targetWithChanges = DefaultBuildTarget((b) => b + ..target = '' + ..reportChangedAssets = true); + interstedClient.sink.add(jsonEncode(serializers + .serialize(BuildTargetRequest((b) => b.target = targetWithChanges)))); + + // Request a build. As there is no ordering guarantee between the two + // sockets, wait a bit first + await Future.delayed(const Duration(milliseconds: 500)); + client.sink.add(jsonEncode(serializers.serialize(BuildRequest()))); + + expect( + interestedEvents.stream, + emitsThrough(isA() + .having((e) => e.changedAssets, 'changedAssets', isNotEmpty)), + ); + + await expectLater( + controller.stream, + emitsThrough(isA() + .having((e) => e.changedAssets, 'changedAssets', isNull)), + ); + }); }); } @@ -89,15 +127,13 @@ Server _createServer() { } IOWebSocketChannel _createClient( - StreamController controller, + StreamController controller, int port, ) { final client = IOWebSocketChannel.connect('ws://localhost:$port'); client.stream.listen((data) { final message = serializers.deserialize(jsonDecode(data as String)); - if (message is ServerLog) { - controller.add(message); - } + controller.add(message); }); return client; } diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index 4c3c23b38..9a420c7af 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -4,6 +4,7 @@ errors from the directory watcher. - Ignore the no_leading_underscores_for_library_prefixes lint in the generated build script. +- Report changed assets when running as a deamon. ## 2.1.8 diff --git a/build_runner/lib/src/daemon/daemon_builder.dart b/build_runner/lib/src/daemon/daemon_builder.dart index 8b512f905..4af1c4610 100644 --- a/build_runner/lib/src/daemon/daemon_builder.dart +++ b/build_runner/lib/src/daemon/daemon_builder.dart @@ -111,10 +111,22 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder { '${target.target}/**', _buildOptions.packageGraph.root.name)); } } + Iterable? outputs; + try { var mergedChanges = collectChanges([changes]); var result = await _builder.run(mergedChanges, buildDirs: buildDirs, buildFilters: buildFilters); + var interestedInOutputs = + targets.any((e) => e is DefaultBuildTarget && e.reportChangedAssets); + + if (interestedInOutputs) { + outputs = { + for (var change in changes) change.id, + ...result.outputs, + }; + } + for (var target in targets) { if (result.status == core.BuildStatus.success) { // TODO(grouma) - Can we notify if a target was cached? @@ -140,7 +152,7 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder { } _logMessage(Level.SEVERE, 'Build Failed:\n${e.toString()}'); } - _signalEnd(results); + _signalEnd(results, outputs?.map((e) => e.uri)); } @override @@ -156,9 +168,16 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder { ..level = level, )); - void _signalEnd(Iterable results) { + void _signalEnd(Iterable results, + [Iterable? changedAssets]) { _buildingCompleter!.complete(); - _buildResults.add(BuildResults((b) => b..results.addAll(results))); + _buildResults.add(BuildResults((b) { + b.results.addAll(results); + + if (changedAssets != null) { + b.changedAssets.addAll(changedAssets); + } + })); } void _signalStart(Iterable targets) { diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index b526ee73e..ccb11d1fb 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -12,7 +12,7 @@ dependencies: analyzer: ">=1.4.0 <4.0.0" build: ">=2.1.0 <2.3.0" build_config: ">=1.0.0 <1.1.0" - build_daemon: ^3.0.0 + build_daemon: ^3.1.0 build_resolvers: ^2.0.0 build_runner_core: ^7.2.0 code_builder: ^4.0.0 @@ -52,3 +52,7 @@ dev_dependencies: test_process: ^2.0.0 _test_common: path: ../_test_common + +dependency_overrides: + build_daemon: + path: ../build_daemon diff --git a/build_runner/test/daemon/daemon_test.dart b/build_runner/test/daemon/daemon_test.dart index 1bc1dc082..2701f62a9 100644 --- a/build_runner/test/daemon/daemon_test.dart +++ b/build_runner/test/daemon/daemon_test.dart @@ -24,7 +24,9 @@ void main() { Process? daemonProcess; Stream? stdoutLines; String workspace() => p.join(d.sandbox, 'a'); - final webTarget = DefaultBuildTarget((b) => b..target = 'web'); + final webTarget = DefaultBuildTarget((b) => b + ..target = 'web' + ..reportChangedAssets = true); final testTarget = DefaultBuildTarget((b) => b..target = 'test'); var clients = []; @@ -310,10 +312,17 @@ main() { ..registerBuildTarget(webTarget) ..startBuild(); clients.add(client); + expect( - client.buildResults, - emitsThrough((BuildResults b) => - b.results.first.status == BuildStatus.succeeded)); + client.buildResults, + emitsThrough( + isA() + .having((e) => e.results.first.status, 'results.first.status', + BuildStatus.succeeded) + .having((e) => e.changedAssets, 'changedAsssets', + contains(Uri.parse('asset:a/web/main.dart.js'))), + ), + ); }); test('allows multiple clients to connect and build', () async {