Skip to content

Commit de4e077

Browse files
dcharkesCommit Queue
authored andcommitted
[dartdev] Progress updates for running hooks
Shows a progress update on the interactive terminal for running hooks. The progress update visually aligns with progress updates from pub `Building package executable... (1.3s)` as `Running build hooks... (0.7s)`. Closes: dart-lang/native#2439 Progress updates visibility: * `dart run`, only if there are hooks to avoid cluttering stdout more. #61696 * `dart test`, also only if there are any hooks. * `dart build`, always show progress update lines. * `dart install`, always show progress updates lines. * `dart compile`, changed the check to check for the existence of hooks but don't ever run them. This CL also renames some "native assets" to "build hooks". This CL also fixes an issue with `dart test` run without `pub get` in a Flutter project. Closes: #61697 Change-Id: I88f6e07dff1d4f5c0733f83f073640b75cc54e79 Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-arm64-try,pkg-win-release-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/453920 Reviewed-by: Michael Goderbauer <[email protected]> Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Daco Harkes <[email protected]>
1 parent ad5a1bb commit de4e077

File tree

12 files changed

+204
-61
lines changed

12 files changed

+204
-61
lines changed

pkg/dartdev/lib/src/commands/build.dart

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'package:dartdev/src/commands/compile.dart';
1111
import 'package:dartdev/src/experiments.dart';
1212
import 'package:dartdev/src/native_assets_bundling.dart';
1313
import 'package:dartdev/src/native_assets_macos.dart';
14+
import 'package:dartdev/src/progress.dart';
1415
import 'package:dartdev/src/sdk.dart';
1516
import 'package:front_end/src/api_prototype/compiler_options.dart'
1617
show Verbosity;
@@ -32,7 +33,7 @@ class BuildCommand extends DartdevCommand {
3233
{bool verbose = false,
3334
required this.recordUseEnabled,
3435
required this.dataAssetsExperimentEnabled})
35-
: super(cmdName, 'Build a Dart application including native assets.',
36+
: super(cmdName, 'Build a Dart application including code assets.',
3637
verbose) {
3738
addSubcommand(BuildCliSubcommand(
3839
verbose: verbose,
@@ -232,8 +233,6 @@ See documentation on https://dart.dev/interop/c-interop#native-assets.
232233
final binDirectory = Directory.fromUri(bundleDirectory.uri.resolve('bin/'));
233234
await binDirectory.create(recursive: true);
234235

235-
stdout.writeln('Building native assets.');
236-
237236
final packageConfig =
238237
await DartNativeAssetsBuilder.loadPackageConfig(packageConfigUri);
239238
if (packageConfig == null) {
@@ -253,9 +252,12 @@ See documentation on https://dart.dev/interop/c-interop#native-assets.
253252
verbose: verbose,
254253
dataAssetsExperimentEnabled: dataAssetsExperimentEnabled,
255254
);
256-
final buildResult = await builder.buildNativeAssetsAOT();
255+
final buildResult = await progress(
256+
'Running build hooks',
257+
builder.buildNativeAssetsAOT,
258+
);
257259
if (buildResult == null) {
258-
stderr.writeln('Native assets build failed.');
260+
stderr.writeln('Running build hooks failed.');
259261
return 255;
260262
}
261263

@@ -294,13 +296,16 @@ See documentation on https://dart.dev/interop/c-interop#native-assets.
294296
if (first) {
295297
// Multiple executables are only supported with recorded uses
296298
// disabled, so don't re-invoke link hooks.
297-
linkResult = await builder.linkNativeAssetsAOT(
298-
recordedUsagesPath: recordedUsagesPath,
299-
buildResult: buildResult,
299+
linkResult = await progress(
300+
'Running link hooks',
301+
() => builder.linkNativeAssetsAOT(
302+
recordedUsagesPath: recordedUsagesPath,
303+
buildResult: buildResult,
304+
),
300305
);
301306
}
302307
if (linkResult == null) {
303-
stderr.writeln('Native assets link failed.');
308+
stderr.writeln('Running link hooks failed.');
304309
return 255;
305310
}
306311

pkg/dartdev/lib/src/commands/compile.dart

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -643,17 +643,10 @@ Remove debugging information from the output and save it separately to the speci
643643
if (await builder.warnOnNativeAssets()) {
644644
return 255;
645645
}
646-
} else {
647-
final assets = await builder.compileNativeAssetsJit();
648-
if (assets == null) {
649-
stderr.writeln('Native assets build failed.');
650-
return 255;
651-
}
652-
if (assets.isNotEmpty) {
653-
stderr.writeln(
654-
"'dart compile' does currently not support native assets.");
655-
return 255;
656-
}
646+
} else if (await builder.hasHooks()) {
647+
stderr.writeln(
648+
"'dart compile' does not support build hooks, use 'dart build' instead.");
649+
return 255;
657650
}
658651
}
659652
}

pkg/dartdev/lib/src/commands/run.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:io';
77

88
import 'package:args/args.dart';
99
import 'package:dartdev/src/commands/compile.dart';
10+
import 'package:dartdev/src/progress.dart';
1011
import 'package:front_end/src/api_prototype/compiler_options.dart'
1112
show Verbosity;
1213
import 'package:frontend_server/resident_frontend_server_utils.dart'
@@ -420,11 +421,13 @@ class RunCommand extends DartdevCommand {
420421
if (await builder.warnOnNativeAssets()) {
421422
return errorExitCode;
422423
}
423-
} else {
424-
final assetsYamlFileUri =
425-
await builder.compileNativeAssetsJitYamlFile();
424+
} else if (await builder.hasHooks()) {
425+
final assetsYamlFileUri = await progress(
426+
'Running build hooks',
427+
builder.compileNativeAssetsJitYamlFile,
428+
);
426429
if (assetsYamlFileUri == null) {
427-
log.stderr('Error: Compiling native assets failed.');
430+
log.stderr('Error: Running build hooks failed.');
428431
return errorExitCode;
429432
}
430433
nativeAssets = assetsYamlFileUri.toFilePath();

pkg/dartdev/lib/src/commands/test.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:io';
77

88
import 'package:args/args.dart';
99
import 'package:dartdev/src/experiments.dart';
10+
import 'package:dartdev/src/progress.dart';
1011
import 'package:pub/pub.dart';
1112

1213
import '../core.dart';
@@ -81,11 +82,13 @@ Run "${runner!.executableName} help" to see global options.''');
8182
if (await builder.warnOnNativeAssets()) {
8283
return DartdevCommand.errorExitCode;
8384
}
84-
} else {
85-
final assetsYamlFileUri =
86-
await builder.compileNativeAssetsJitYamlFile();
85+
} else if (await builder.hasHooks()) {
86+
final assetsYamlFileUri = await progress(
87+
'Running build hooks',
88+
builder.compileNativeAssetsJitYamlFile,
89+
);
8790
if (assetsYamlFileUri == null) {
88-
log.stderr('Error: Compiling native assets failed.');
91+
log.stderr('Error: Running build hooks failed.');
8992
return DartdevCommand.errorExitCode;
9093
}
9194
// TODO(https://github.com/dart-lang/sdk/issues/60489): Add a way to

pkg/dartdev/lib/src/native_assets.dart

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import 'package:hooks/hooks.dart';
1515
import 'package:hooks_runner/hooks_runner.dart';
1616
import 'package:logging/logging.dart';
1717
import 'package:package_config/package_config.dart' as package_config;
18+
import 'package:pub/pub.dart';
1819
import 'package:yaml/yaml.dart' show loadYaml;
1920

2021
import 'core.dart';
@@ -118,6 +119,16 @@ class DartNativeAssetsBuilder {
118119
);
119120
}
120121

122+
/// Check whether there are build hooks in the project.
123+
///
124+
/// Fine to run together with the other methods, this is stateful, no work is
125+
/// duplicated.
126+
Future<bool> hasHooks() async {
127+
final builder = await _nativeAssetsBuildRunner;
128+
final packageNames = await builder.packagesWithBuildHooks();
129+
return packageNames.isNotEmpty;
130+
}
131+
121132
Future<bool> warnOnNativeAssets() async {
122133
final builder = await _nativeAssetsBuildRunner;
123134
final packageNames = await builder.packagesWithBuildHooks();
@@ -225,8 +236,9 @@ class DartNativeAssetsBuilder {
225236
if (pubspecMaybe != null) {
226237
// Silently run `pub get`, this is what would happen in
227238
// `getExecutableForCommand` later.
228-
final result = await Process.run(sdk.dart, ['pub', 'get']);
229-
if (result.exitCode != 0) {
239+
try {
240+
await ensurePubspecResolved(uri.toFilePath());
241+
} on ResolutionFailedException {
230242
return null;
231243
}
232244
packageConfig = await _findPackageConfigUri(uri);

pkg/dartdev/lib/src/progress.dart

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Adapted from package:pub lib/src/progress.dart to align progress updates
6+
// visually with pub. It would be better to let pub stream updates to dartdev
7+
// and have dartdev display progress in a consistent way for all kinds of
8+
// progress.
9+
// TODO(https://dartbug.com/61539): Standardize.
10+
11+
import 'dart:async';
12+
import 'dart:io';
13+
14+
/// Runs [callback] while displaying a live-updating progress indicator.
15+
///
16+
/// The [message] is shown to the user, followed by "..." and a timer.
17+
/// The progress indicator is only animated if output is going to a terminal.
18+
/// When the [callback] completes, the progress indicator is stopped and the
19+
/// final time is shown.
20+
Future<T> progress<T>(
21+
String message,
22+
Future<T> Function() callback,
23+
) async {
24+
final progress = _Progress(message);
25+
return callback().whenComplete(progress._stop);
26+
}
27+
28+
/// A live-updating progress indicator for long-running log entries.
29+
class _Progress {
30+
/// The timer used to write "..." during a progress log.
31+
late final Timer _timer;
32+
33+
/// The [Stopwatch] used to track how long a progress log has been running.
34+
final _stopwatch = Stopwatch();
35+
36+
/// The progress message as it's being incrementally appended.
37+
///
38+
/// When the progress is done, a single entry will be added to the log for it.
39+
final String _message;
40+
41+
/// Gets the current progress time as a parenthesized, formatted string.
42+
String get _time => '(${_niceDuration(_stopwatch.elapsed)})';
43+
44+
/// The length of the most recently-printed [_time] string.
45+
var _timeLength = 0;
46+
47+
/// Creates a new progress indicator.
48+
_Progress(this._message) {
49+
_stopwatch.start();
50+
51+
// The animation is only shown when it would be meaningful to a human.
52+
// That means we're writing a visible message to a TTY at normal log levels
53+
// with non-JSON output.
54+
if (!_terminalOutputForStdout) {
55+
// Not animating, so just log the start and wait until the task is
56+
// completed.
57+
stdout.write('$_message...');
58+
return;
59+
}
60+
61+
_timer = Timer.periodic(const Duration(milliseconds: 100), (_) {
62+
_update();
63+
});
64+
65+
stdout.write('$_message... ');
66+
}
67+
68+
/// Stops the progress indicator.
69+
void _stop() {
70+
if (!_terminalOutputForStdout) {
71+
// Not animating, so just log the start and wait until the task is
72+
// completed.
73+
stdout.write('$_message...');
74+
return;
75+
}
76+
77+
_stopwatch.stop();
78+
_timer.cancel();
79+
// print one final update to show the user the final time.
80+
_update();
81+
stdout.writeln();
82+
}
83+
84+
/// Refreshes the progress line.
85+
void _update() {
86+
// Show the time only once it gets noticeably long.
87+
if (_stopwatch.elapsed.inSeconds == 0) return;
88+
89+
// Erase the last time that was printed. Erasing just the time using `\b`
90+
// rather than using `\r` to erase the entire line ensures that we don't
91+
// spam progress lines if they're wider than the terminal width.
92+
stdout.write('\b' * _timeLength);
93+
final time = _time;
94+
_timeLength = time.length;
95+
stdout.write(gray(time));
96+
}
97+
}
98+
99+
/// Returns `true` if [stdout] should be treated as a terminal.
100+
bool get _terminalOutputForStdout => stdout.hasTerminal;
101+
102+
/// Returns a human-friendly representation of [duration].
103+
String _niceDuration(Duration duration) {
104+
final hasMinutes = duration.inMinutes > 0;
105+
final result = hasMinutes ? '${duration.inMinutes}:' : '';
106+
107+
final s = duration.inSeconds % 60;
108+
final ms = duration.inMilliseconds % 1000;
109+
110+
final msString = (ms ~/ 100).toString();
111+
112+
return "$result${hasMinutes ? _padLeft(s.toString(), 2, '0') : s}"
113+
'.${msString}s';
114+
}
115+
116+
/// Pads [source] to [length] by adding [char]s at the beginning.
117+
///
118+
/// If [char] is `null`, it defaults to a space.
119+
String _padLeft(String source, int length, [String char = ' ']) {
120+
if (source.length >= length) return source;
121+
122+
return char * (length - source.length) + source;
123+
}
124+
125+
/// Wraps [text] in the ANSI escape codes to make it gray when on a platform
126+
/// that supports that.
127+
///
128+
/// Use this for text that's less important than the text around it.
129+
String gray(String text) => '$_gray$text$_none';
130+
131+
final _none = _getAnsi('\u001b[0m');
132+
final _gray = _getAnsi('\u001b[38;5;245m');
133+
134+
String _getAnsi(String ansiCode) => canUseAnsiCodes ? ansiCode : '';
135+
136+
/// Whether ansi codes such as color escapes are safe to use.
137+
///
138+
/// On a terminal we can use ansi codes also on Windows.
139+
///
140+
/// Tests should make sure to run the subprocess with or without an attached
141+
/// terminal to decide if colors will be provided.
142+
bool get canUseAnsiCodes {
143+
return (!Platform.environment.containsKey('NO_COLOR')) &&
144+
_terminalOutputForStdout &&
145+
stdout.supportsAnsiEscapes;
146+
}

pkg/dartdev/test/commands/help_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ Global
8585
uninstall Remove a globally installed Dart CLI tool.
8686
8787
Project
88-
build Build a Dart application including native assets.
88+
build Build a Dart application including code assets.
8989
compile Compile Dart to various formats.
9090
create Create a new Dart project.
9191
pub Work with packages.

pkg/dartdev/test/native_assets/build_test.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ void main([List<String> args = const []]) async {
4848
workingDirectory: dartAppUri,
4949
logger: logger,
5050
);
51+
expect(result.stdout, contains('Running build hooks'));
52+
expect(result.stdout, contains('Running link hooks'));
5153
if (verbose) {
5254
expect(result.stdout, contains(usingTargetOSMessage));
5355
expect(result.stdout, contains('build.dart'));
@@ -110,7 +112,7 @@ void main(List<String> args) {
110112
expect(
111113
result.stderr,
112114
contains(
113-
'Native assets build failed.',
115+
'Running build hooks failed.',
114116
),
115117
);
116118
expect(result.exitCode, 255);

pkg/dartdev/test/native_assets/compile_test.dart

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
// @dart=2.18
66

7-
import 'dart:io';
8-
97
import 'package:test/test.dart';
108

119
import '../utils.dart';
@@ -31,35 +29,10 @@ void main() async {
3129
expect(
3230
result.stderr,
3331
contains(
34-
"'dart compile' does currently not support native assets.",
32+
"'dart compile' does not support build hooks, use 'dart build' instead.",
3533
),
3634
);
3735
expect(result.exitCode, 255);
3836
});
3937
});
40-
41-
test('dart compile native assets build failure', timeout: longTimeout,
42-
() async {
43-
await nativeAssetsTest('dart_app', (dartAppUri) async {
44-
final buildDotDart = dartAppUri.resolve('../native_add/hook/build.dart');
45-
await File.fromUri(buildDotDart).writeAsString('''
46-
void main(List<String> args) {
47-
throw UnimplementedError();
48-
}
49-
''');
50-
final result = await runDart(
51-
arguments: [
52-
'compile',
53-
'exe',
54-
'bin/dart_app.dart',
55-
],
56-
workingDirectory: dartAppUri,
57-
logger: logger,
58-
expectExitCodeZero: false,
59-
);
60-
expect(result.stderr,
61-
contains('Building assets for package:native_add failed.'));
62-
expect(result.exitCode, 255);
63-
});
64-
});
6538
}

0 commit comments

Comments
 (0)