From 8d0f4dd8ccb1bb2f4aadf8faae0c7829eeb17d6b Mon Sep 17 00:00:00 2001 From: John McCutchan Date: Tue, 27 Feb 2024 12:17:33 -0800 Subject: [PATCH] Add et run command The `run` command builds both the host and target engines and then invokes `flutter run` so that it runs with the custom engine builds. It is expected that 'et run' be used inside the directory of a flutter application. Command line flags passed after `--` will be forwarded to `flutter run`. Some examples: `et run` Build the debug variant and runs the app in that mode. `et run -- --profile` Build the profile variant and runs the app in that mode. `et run -- --release` Build the release variant and runs the app in that mode. --- bin/et | 6 +- ci/builders/local_engine.json | 118 ++++++++++++++ testing/litetest/lib/src/matchers.dart | 78 +++++---- tools/engine_tool/lib/main.dart | 20 +-- tools/engine_tool/lib/src/build_utils.dart | 57 +++++++ .../lib/src/commands/build_command.dart | 43 +---- .../lib/src/commands/command_runner.dart | 6 +- .../lib/src/commands/run_command.dart | 153 ++++++++++++++++++ .../engine_tool/test/build_command_test.dart | 2 +- tools/engine_tool/test/fixtures.dart | 24 +++ .../engine_tool/test/query_command_test.dart | 8 +- tools/engine_tool/test/run_command_test.dart | 92 +++++++++++ tools/pkg/engine_build_configs/bin/check.dart | 42 ++--- .../lib/src/build_config_runner.dart | 21 ++- 14 files changed, 546 insertions(+), 124 deletions(-) create mode 100644 ci/builders/local_engine.json create mode 100644 tools/engine_tool/lib/src/commands/run_command.dart create mode 100644 tools/engine_tool/test/run_command_test.dart diff --git a/bin/et b/bin/et index 54da8248cf034..a3c30df0241d9 100755 --- a/bin/et +++ b/bin/et @@ -58,11 +58,9 @@ PLATFORM="${OS}-${CPU}" DART_SDK_DIR="${ENGINE_DIR}/prebuilts/${PLATFORM}/dart-sdk" DART="${DART_SDK_DIR}/bin/dart" -cd "${ENGINE_DIR}/tools/engine_tool" - -if [ ! -d ".dart_tool" ]; then +if [ ! -d "${ENGINE_DIR}/tools/engine_tool/.dart_tool" ]; then echo "You must run 'gclient sync -D' before using this tool." exit 1 fi -"${DART}" --disable-dart-dev bin/et.dart "$@" +"${DART}" --disable-dart-dev "${ENGINE_DIR}/tools/engine_tool/bin/et.dart" "$@" diff --git a/ci/builders/local_engine.json b/ci/builders/local_engine.json new file mode 100644 index 0000000000000..078bebb55d81a --- /dev/null +++ b/ci/builders/local_engine.json @@ -0,0 +1,118 @@ +{ + "builds": [ + { + "drone_dimensions": [ + "os=Mac-13", + "os=Linux" + ], + "gn": [ + "--runtime-mode", + "debug", + "--android", + "--android-cpu=arm64", + "--no-stripped", + "--enable-impeller-vulkan", + "--no-lto" + ], + "name": "android_debug_arm64", + "ninja": { + "config": "android_debug_arm64", + "targets": [] + } + }, + { + "drone_dimensions": [ + "os=Mac-13", + "os=Linux" + ], + "gn": [ + "--runtime-mode", + "profile", + "--android", + "--android-cpu=arm64", + "--no-stripped", + "--enable-impeller-vulkan", + "--no-lto" + ], + "name": "android_profile_arm64", + "ninja": { + "config": "android_profile_arm64", + "targets": [] + } + }, + { + "drone_dimensions": [ + "os=Mac-13", + "os=Linux" + ], + "gn": [ + "--runtime-mode", + "release", + "--android", + "--android-cpu=arm64", + "--no-stripped", + "--enable-impeller-vulkan", + "--no-lto" + ], + "name": "android_release_arm64", + "ninja": { + "config": "android_release_arm64", + "targets": [] + } + }, + { + "drone_dimensions": [ + "os=Mac-13", + "os=Linux" + ], + "gn": [ + "--runtime-mode", + "debug", + "--no-stripped", + "--enable-impeller-vulkan", + "--no-lto" + ], + "name": "host_debug", + "ninja": { + "config": "host_debug", + "targets": [] + } + }, + { + "drone_dimensions": [ + "os=Mac-13", + "os=Linux" + ], + "gn": [ + "--runtime-mode", + "profile", + "--no-stripped", + "--enable-impeller-vulkan", + "--no-lto" + ], + "name": "host_profile", + "ninja": { + "config": "host_profile", + "targets": [] + } + }, + { + "drone_dimensions": [ + "os=Mac-13", + "os=Linux" + ], + "gn": [ + "--runtime-mode", + "release", + "--no-stripped", + "--enable-impeller-vulkan", + "--no-lto" + ], + "name": "host_release", + "ninja": { + "config": "host_release", + "targets": [] + } + } + ] +} \ No newline at end of file diff --git a/testing/litetest/lib/src/matchers.dart b/testing/litetest/lib/src/matchers.dart index e41f09ad57c9f..eb5419a3f4826 100644 --- a/testing/litetest/lib/src/matchers.dart +++ b/testing/litetest/lib/src/matchers.dart @@ -62,8 +62,8 @@ void isNotEmpty(dynamic d) { /// Gives a [Matcher] that asserts that the value being matched is within /// `tolerance` of `value`. Matcher closeTo(num value, num tolerance) => (dynamic actual) { - Expect.approxEquals(value, actual as num, tolerance); -}; + Expect.approxEquals(value, actual as num, tolerance); + }; /// A [Matcher] that matches NaN. void isNaN(dynamic v) { @@ -74,8 +74,8 @@ void isNaN(dynamic v) { /// Gives a [Matcher] that asserts that the value being matched is not equal to /// `unexpected`. Matcher notEquals(dynamic unexpected) => (dynamic actual) { - Expect.notEquals(unexpected, actual); -}; + Expect.notEquals(unexpected, actual); + }; /// A [Matcher] that matches non-zero values. void isNonZero(dynamic d) { @@ -90,44 +90,64 @@ void throwsRangeError(dynamic d) { /// Gives a [Matcher] that asserts that the value being matched is a [String] /// that contains `s` as a substring. Matcher contains(String s) => (dynamic d) { - expect(d, isInstanceOf()); - Expect.contains(s, d as String); -}; + expect(d, isInstanceOf()); + Expect.contains(s, d as String); + }; /// Gives a [Matcher] that asserts that the value being matched is an [Iterable] /// of length `d`. Matcher hasLength(int l) => (dynamic d) { - expect(d, isInstanceOf>()); - expect((d as Iterable).length, equals(l)); -}; + expect(d, isInstanceOf>()); + expect((d as Iterable).length, equals(l)); + }; /// Gives a matcher that asserts that the value being matched is a [String] that /// starts with `s`. Matcher startsWith(String s) => (dynamic d) { - expect(d, isInstanceOf()); - final String h = d as String; - if (!h.startsWith(s)) { - Expect.fail('Expected "$h" to start with "$s"'); - } -}; + expect(d, isInstanceOf()); + final String h = d as String; + if (!h.startsWith(s)) { + Expect.fail('Expected "$h" to start with "$s"'); + } + }; /// Gives a matcher that asserts that the value being matched is a [String] that /// ends with `s`. Matcher endsWith(String s) => (dynamic d) { - expect(d, isInstanceOf()); - final String h = d as String; - if (!h.endsWith(s)) { - Expect.fail('Expected "$h" to end with "$s"'); - } -}; + expect(d, isInstanceOf()); + final String h = d as String; + if (!h.endsWith(s)) { + Expect.fail('Expected "$h" to end with "$s"'); + } + }; /// Gives a matcher that asserts that the value being matched is a [String] that /// regexp matches with `pattern`. Matcher hasMatch(String pattern) => (dynamic d) { - expect(d, isInstanceOf()); - final String h = d as String; - final RegExp regExp = RegExp(pattern); - if (!regExp.hasMatch(h)) { - Expect.fail('Expected "$h" to match with "$pattern"'); - } -}; + expect(d, isInstanceOf()); + final String h = d as String; + final RegExp regExp = RegExp(pattern); + if (!regExp.hasMatch(h)) { + Expect.fail('Expected "$h" to match with "$pattern"'); + } + }; + +/// Gives a matcher that asserts that the value being matched is a List +/// that contains the entries in `pattern` in order. There may be values +/// that are not in the pattern interleaved. +Matcher containsStringsInOrder(List pattern) => (dynamic d) { + expect(d, isInstanceOf>()); + final List input = d as List; + int cursor = 0; + for (final String el in input) { + if (cursor == pattern.length) { + break; + } + if (el == pattern[cursor]) { + cursor++; + } + } + if (cursor < pattern.length) { + Expect.fail('Did not find ${pattern[cursor]} in $d}'); + } + }; diff --git a/tools/engine_tool/lib/main.dart b/tools/engine_tool/lib/main.dart index dba9ac0ca2a9f..e3354bc291b3c 100644 --- a/tools/engine_tool/lib/main.dart +++ b/tools/engine_tool/lib/main.dart @@ -3,7 +3,7 @@ // found in the LICENSE file. import 'dart:ffi' as ffi show Abi; -import 'dart:io' as io show Directory, exitCode, stderr; +import 'dart:io' as io show Directory, Platform, exitCode, stderr; import 'package:engine_build_configs/engine_build_configs.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; @@ -19,7 +19,7 @@ void main(List args) async { // Find the engine repo. final Engine engine; try { - engine = Engine.findWithin(); + engine = Engine.findWithin(p.dirname(io.Platform.script.toFilePath())); } catch (e) { io.stderr.writeln(e); io.exitCode = 1; @@ -51,15 +51,17 @@ void main(List args) async { io.exitCode = 1; } + final Environment environment = Environment( + abi: ffi.Abi.current(), + engine: engine, + platform: const LocalPlatform(), + processRunner: ProcessRunner(), + logger: Logger(), + ); + // Use the Engine and BuildConfig collection to build the CommandRunner. final ToolCommandRunner runner = ToolCommandRunner( - environment: Environment( - abi: ffi.Abi.current(), - engine: engine, - platform: const LocalPlatform(), - processRunner: ProcessRunner(), - logger: Logger(), - ), + environment: environment, configs: configs, ); diff --git a/tools/engine_tool/lib/src/build_utils.dart b/tools/engine_tool/lib/src/build_utils.dart index 715ee2afa2701..6925f47de40c7 100644 --- a/tools/engine_tool/lib/src/build_utils.dart +++ b/tools/engine_tool/lib/src/build_utils.dart @@ -5,6 +5,7 @@ import 'package:engine_build_configs/engine_build_configs.dart'; import 'environment.dart'; +import 'logger.dart'; /// A function that returns true or false when given a [BuilderConfig] and its /// name. @@ -48,3 +49,59 @@ List runnableBuilds(Environment env, Map input) { return build.canRunOn(env.platform); }); } + +/// Validates the list of builds. +/// Calls assert. +void debugCheckBuilds(List builds) { + final Set names = {}; + + for (final Build build in builds) { + assert(!names.contains(build.name), + 'More than one build has the name ${build.name}'); + names.add(build.name); + } +} + +/// Build the build target in the environment. +Future runBuild(Environment environment, Build build) async { + final BuildRunner buildRunner = BuildRunner( + platform: environment.platform, + processRunner: environment.processRunner, + abi: environment.abi, + engineSrcDir: environment.engine.srcDir, + build: build, + runTests: false, + ); + + Spinner? spinner; + void handler(RunnerEvent event) { + switch (event) { + case RunnerStart(): + environment.logger.status('$event ', newline: false); + spinner = environment.logger.startSpinner(); + case RunnerProgress(done: true): + spinner?.finish(); + spinner = null; + environment.logger.clearLine(); + environment.logger.status(event); + case RunnerProgress(done: false): + { + spinner?.finish(); + spinner = null; + final String percent = '${event.percent.toStringAsFixed(1)}%'; + final String fraction = '(${event.completed}/${event.total})'; + final String prefix = '[${event.name}] $percent $fraction '; + final String what = event.what; + environment.logger.clearLine(); + environment.logger.status('$prefix$what', newline: false, fit: true); + } + default: + spinner?.finish(); + spinner = null; + environment.logger.status(event); + } + } + + final bool buildResult = await buildRunner.run(handler); + return buildResult ? 0 : 1; +} diff --git a/tools/engine_tool/lib/src/commands/build_command.dart b/tools/engine_tool/lib/src/commands/build_command.dart index c70a261abce66..3c77f780bc5d3 100644 --- a/tools/engine_tool/lib/src/commands/build_command.dart +++ b/tools/engine_tool/lib/src/commands/build_command.dart @@ -5,7 +5,6 @@ import 'package:engine_build_configs/engine_build_configs.dart'; import '../build_utils.dart'; -import '../logger.dart'; import 'command.dart'; import 'flags.dart'; @@ -17,6 +16,7 @@ final class BuildCommand extends CommandBase { required Map configs, }) { builds = runnableBuilds(environment, configs); + debugCheckBuilds(builds); // Add options here that are common to all queries. argParser.addOption( configFlag, @@ -52,46 +52,7 @@ final class BuildCommand extends CommandBase { environment.logger.error('Could not find config $configName'); return 1; } - final BuildRunner buildRunner = BuildRunner( - platform: environment.platform, - processRunner: environment.processRunner, - abi: environment.abi, - engineSrcDir: environment.engine.srcDir, - build: build, - runTests: false, - ); - - Spinner? spinner; - void handler(RunnerEvent event) { - switch (event) { - case RunnerStart(): - environment.logger.status('$event ', newline: false); - spinner = environment.logger.startSpinner(); - case RunnerProgress(done: true): - spinner?.finish(); - spinner = null; - environment.logger.clearLine(); - environment.logger.status(event); - case RunnerProgress(done: false): - { - spinner?.finish(); - spinner = null; - final String percent = '${event.percent.toStringAsFixed(1)}%'; - final String fraction = '(${event.completed}/${event.total})'; - final String prefix = '[${event.name}] $percent $fraction '; - final String what = event.what; - environment.logger.clearLine(); - environment.logger - .status('$prefix$what', newline: false, fit: true); - } - default: - spinner?.finish(); - spinner = null; - environment.logger.status(event); - } - } - final bool buildResult = await buildRunner.run(handler); - return buildResult ? 0 : 1; + return runBuild(environment, build); } } diff --git a/tools/engine_tool/lib/src/commands/command_runner.dart b/tools/engine_tool/lib/src/commands/command_runner.dart index 2b5da2cea8d8c..4633fd216916a 100644 --- a/tools/engine_tool/lib/src/commands/command_runner.dart +++ b/tools/engine_tool/lib/src/commands/command_runner.dart @@ -9,6 +9,9 @@ import '../environment.dart'; import 'build_command.dart'; import 'format_command.dart'; import 'query_command.dart'; +import 'run_command.dart'; + +const int _usageLineLength = 80; /// The root command runner. final class ToolCommandRunner extends CommandRunner { @@ -17,13 +20,14 @@ final class ToolCommandRunner extends CommandRunner { ToolCommandRunner({ required this.environment, required this.configs, - }) : super(toolName, toolDescription) { + }) : super(toolName, toolDescription, usageLineLength: _usageLineLength) { final List> commands = >[ FormatCommand( environment: environment, ), QueryCommand(environment: environment, configs: configs), BuildCommand(environment: environment, configs: configs), + RunCommand(environment: environment, configs: configs), ]; commands.forEach(addCommand); } diff --git a/tools/engine_tool/lib/src/commands/run_command.dart b/tools/engine_tool/lib/src/commands/run_command.dart new file mode 100644 index 0000000000000..52c61ed481ea6 --- /dev/null +++ b/tools/engine_tool/lib/src/commands/run_command.dart @@ -0,0 +1,153 @@ +// Copyright 2013 The Flutter Authors. 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:io' show ProcessStartMode; + +import 'package:engine_build_configs/engine_build_configs.dart'; +import 'package:process_runner/process_runner.dart'; + +import '../build_utils.dart'; +import 'command.dart'; +import 'flags.dart'; + +/// The root 'run' command. +final class RunCommand extends CommandBase { + /// Constructs the 'run' command. + RunCommand({ + required super.environment, + required Map configs, + }) { + builds = runnableBuilds(environment, configs); + debugCheckBuilds(builds); + + argParser.addOption( + configFlag, + abbr: 'c', + defaultsTo: '', + help: + 'Specify the build config to use for the target build (usually auto detected)', + allowed: [ + for (final Build build in runnableBuilds(environment, configs)) + build.name, + ], + allowedHelp: { + for (final Build build in runnableBuilds(environment, configs)) + build.name: build.gn.join(' '), + }, + ); + } + + /// List of compatible builds. + late final List builds; + + @override + String get name => 'run'; + + @override + String get description => 'Run a flutter app with a local engine build' + 'All arguments after -- are forwarded to flutter run, e.g.: ' + 'et run -- --profile' + 'et run -- -d chrome' + 'See `flutter run --help` for a listing'; + + Build? _lookup(String configName) { + return builds.where((Build build) => build.name == configName).firstOrNull; + } + + Build? _findHostBuild(Build? targetBuild) { + if (targetBuild == null) { + return null; + } + + final String name = targetBuild.name; + if (name.startsWith('host_')) { + return targetBuild; + } + // TODO(johnmccutchan): This is brittle, it would be better if we encoded + // the host config name in the target config. + if (name.contains('_debug')) { + return _lookup('host_debug'); + } else if (name.contains('_profile')) { + return _lookup('host_profile'); + } else if (name.contains('_release')) { + return _lookup('host_release'); + } + return null; + } + + String _selectTargetConfig() { + final String configName = argResults![configFlag] as String; + if (configName.isNotEmpty) { + return configName; + } + // TODO(johnmccutchan): We need a way to invoke flutter tool and be told + // which OS and CPU architecture the selected device requires, for now + // use some hard coded values: + const String targetOS = 'android'; + const String cpuArch = 'arm64'; + + // Sniff the build mode from the args that will be passed to flutter run. + String mode = 'debug'; + if (argResults!.rest.contains('--profile')) { + mode = 'profile'; + } else if (argResults!.rest.contains('--release')) { + mode = 'release'; + } + + return '${targetOS}_${mode}_$cpuArch'; + } + + @override + Future run() async { + if (!environment.processRunner.processManager.canRun('flutter')) { + environment.logger.error('Cannot find flutter command in your path'); + return 1; + } + final String configName = _selectTargetConfig(); + final Build? build = _lookup(configName); + final Build? hostBuild = _findHostBuild(build); + if (build == null) { + environment.logger.error('Could not find build $configName'); + return 1; + } + if (hostBuild == null) { + environment.logger.error('Could not find host build for $configName'); + return 1; + } + + // First build the host. + int r = await runBuild(environment, hostBuild); + if (r != 0) { + return r; + } + + // Now build the target if it isn't the same. + if (hostBuild.name != build.name) { + r = await runBuild(environment, build); + if (r != 0) { + return r; + } + } + + // TODO(johnmccutchan): Be smart and if the user requested a profile + // config, add the '--profile' flag when invoking flutter run. + final ProcessRunnerResult result = + await environment.processRunner.runProcess( + [ + 'flutter', + 'run', + '--local-engine-src-path', + environment.engine.srcDir.path, + '--local-engine', + build.name, + '--local-engine-host', + hostBuild.name, + ...argResults!.rest, + ], + runInShell: true, + startMode: ProcessStartMode.inheritStdio, + ); + return result.exitCode; + } +} diff --git a/tools/engine_tool/test/build_command_test.dart b/tools/engine_tool/test/build_command_test.dart index 58b59882620df..cae302a59759a 100644 --- a/tools/engine_tool/test/build_command_test.dart +++ b/tools/engine_tool/test/build_command_test.dart @@ -80,7 +80,7 @@ void main() { final Logger logger = Logger.test(); final (Environment env, _) = linuxEnv(logger); final List result = runnableBuilds(env, configs); - expect(result.length, equals(2)); + expect(result.length, equals(6)); expect(result[0].name, equals('build_name')); }); diff --git a/tools/engine_tool/test/fixtures.dart b/tools/engine_tool/test/fixtures.dart index 596609a1db977..89d7a7da1c30b 100644 --- a/tools/engine_tool/test/fixtures.dart +++ b/tools/engine_tool/test/fixtures.dart @@ -46,6 +46,30 @@ String testConfig(String os) => ''' } ] } + }, + {}, + {}, + { + "drone_dimensions": [ + "os=$os" + ], + "gn": ["--gn-arg", "--lto", "--goma", "--no-rbe"], + "name": "host_debug", + "ninja": { + "config": "host_debug", + "targets": ["ninja_target"] + } + }, + { + "drone_dimensions": [ + "os=$os" + ], + "gn": ["--gn-arg", "--lto", "--goma", "--no-rbe"], + "name": "android_debug_arm64", + "ninja": { + "config": "android_debug_arm64", + "targets": ["ninja_target"] + } } ], "generators": { diff --git a/tools/engine_tool/test/query_command_test.dart b/tools/engine_tool/test/query_command_test.dart index 3b47b93f4b38a..32d157c196e11 100644 --- a/tools/engine_tool/test/query_command_test.dart +++ b/tools/engine_tool/test/query_command_test.dart @@ -89,8 +89,12 @@ void main() { '\n', '"linux_test_config" builder:\n', ' "build_name" config\n', + ' "host_debug" config\n', + ' "android_debug_arm64" config\n', '"linux_test_config2" builder:\n', ' "build_name" config\n', + ' "host_debug" config\n', + ' "android_debug_arm64" config\n', ]), ); }); @@ -117,6 +121,8 @@ void main() { '\n', '"linux_test_config" builder:\n', ' "build_name" config\n', + ' "host_debug" config\n', + ' "android_debug_arm64" config\n', ])); }); @@ -135,7 +141,7 @@ void main() { expect(result, equals(0)); expect( logger.testLogs.length, - equals(10), + equals(26), ); }); } diff --git a/tools/engine_tool/test/run_command_test.dart b/tools/engine_tool/test/run_command_test.dart new file mode 100644 index 0000000000000..26c6db01cd416 --- /dev/null +++ b/tools/engine_tool/test/run_command_test.dart @@ -0,0 +1,92 @@ +// Copyright 2013 The Flutter Authors. 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:convert' as convert; +import 'dart:ffi' as ffi show Abi; +import 'dart:io' as io; + +import 'package:engine_build_configs/engine_build_configs.dart'; +import 'package:engine_repo_tools/engine_repo_tools.dart'; +import 'package:engine_tool/src/commands/command_runner.dart'; +import 'package:engine_tool/src/environment.dart'; +import 'package:engine_tool/src/logger.dart'; +import 'package:litetest/litetest.dart'; +import 'package:platform/platform.dart'; +import 'package:process_fakes/process_fakes.dart'; +import 'package:process_runner/process_runner.dart'; + +import 'fixtures.dart' as fixtures; + +void main() { + final Engine engine; + try { + engine = Engine.findWithin(); + } catch (e) { + io.stderr.writeln(e); + io.exitCode = 1; + return; + } + + final BuilderConfig linuxTestConfig = BuilderConfig.fromJson( + path: 'ci/builders/linux_test_config.json', + map: convert.jsonDecode(fixtures.testConfig('Linux')) + as Map, + ); + + final BuilderConfig macTestConfig = BuilderConfig.fromJson( + path: 'ci/builders/mac_test_config.json', + map: convert.jsonDecode(fixtures.testConfig('Mac-12')) + as Map, + ); + + final BuilderConfig winTestConfig = BuilderConfig.fromJson( + path: 'ci/builders/win_test_config.json', + map: convert.jsonDecode(fixtures.testConfig('Windows-11')) + as Map, + ); + + final Map configs = { + 'linux_test_config': linuxTestConfig, + 'linux_test_config2': linuxTestConfig, + 'mac_test_config': macTestConfig, + 'win_test_config': winTestConfig, + }; + + (Environment, List>) linuxEnv(Logger logger) { + final List> runHistory = >[]; + return ( + Environment( + abi: ffi.Abi.linuxX64, + engine: engine, + platform: FakePlatform(operatingSystem: Platform.linux), + processRunner: ProcessRunner( + processManager: FakeProcessManager(onStart: (List command) { + runHistory.add(command); + return FakeProcess(); + }, onRun: (List command) { + runHistory.add(command); + return io.ProcessResult(81, 0, '', ''); + }), + ), + logger: logger, + ), + runHistory + ); + } + + test('run command invokes flutter run', () async { + final Logger logger = Logger.test(); + final (Environment env, List> runHistory) = linuxEnv(logger); + final ToolCommandRunner runner = ToolCommandRunner( + environment: env, + configs: configs, + ); + final int result = + await runner.run(['run', '--', '--weird_argument']); + expect(result, equals(0)); + expect(runHistory.length, greaterThanOrEqualTo(5)); + expect(runHistory[4], + containsStringsInOrder(['flutter', 'run', '--weird_argument'])); + }); +} diff --git a/tools/pkg/engine_build_configs/bin/check.dart b/tools/pkg/engine_build_configs/bin/check.dart index c74d7203c9fce..7c3e759311a71 100644 --- a/tools/pkg/engine_build_configs/bin/check.dart +++ b/tools/pkg/engine_build_configs/bin/check.dart @@ -7,7 +7,6 @@ import 'dart:io' as io; import 'package:engine_build_configs/engine_build_configs.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:path/path.dart' as p; -import 'package:platform/platform.dart'; // Usage: // $ dart bin/check.dart [/path/to/engine/src] @@ -65,39 +64,20 @@ void main(List args) { } for (final String error in buildConfigErrors) { io.stderr.writeln(' $error'); + io.exitCode = 1; } } - // Check that global builds for the same platform are uniquely named. - final List hostPlatforms = [ - Platform.linux, - Platform.macOS, - Platform.windows, - ]; - - // For each host platform, a mapping from GlobalBuild.name to GlobalBuild. - final Map> builds = - >{}; - for (final String platform in hostPlatforms) { - builds[platform] = {}; - } - for (final String name in configs.keys) { - final BuilderConfig buildConfig = configs[name]!; - for (final Build build in buildConfig.builds) { - for (final String platform in hostPlatforms) { - if (!build.canRunOn(FakePlatform(operatingSystem: platform))) { - continue; - } - if (builds[platform]!.containsKey(build.name)) { - final (BuilderConfig oldConfig, _) = builds[platform]![build.name]!; - io.stderr.writeln( - '${build.name} is duplicated.\n' - '\tFound definition in ${buildConfig.path}.\n' - '\tPrevious definition was in ${oldConfig.path}.', - ); - io.exitCode = 1; - } - builds[platform]![build.name] = (buildConfig, build); + // We require all builds within a builder config to be uniquely named. + final Map> builderBuildSet = >{}; + for (final String builderName in configs.keys) { + final BuilderConfig builderConfig = configs[builderName]!; + final Set builds = + builderBuildSet.putIfAbsent(builderName, () => {}); + for (final Build build in builderConfig.builds) { + if (builds.contains(build.name)) { + io.stderr.writeln('${build.name} is duplicated in $builderName\n'); + io.exitCode = 1; } } } diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index ec1f94d41fbf9..41fcd319af8e6 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -432,6 +432,7 @@ final class BuildRunner extends Runner { workingDirectory: engineSrcDir.path, ); final List stderrOutput = []; + final List stdoutOutput = []; final Completer stdoutComplete = Completer(); final Completer stderrComplete = Completer(); @@ -440,7 +441,11 @@ final class BuildRunner extends Runner { .transform(const LineSplitter()) .listen( (String line) { - _ninjaProgress(eventHandler, command, line); + if (_ninjaProgress(eventHandler, command, line)) { + return; + } + final List bytes = utf8.encode('$line\n'); + stdoutOutput.addAll(bytes); }, onDone: () async => stdoutComplete.complete(), ); @@ -458,9 +463,9 @@ final class BuildRunner extends Runner { processResult = ProcessRunnerResult( exitCode, - [], // stdout. + stdoutOutput, // stdout. stderrOutput, // stderr. - stderrOutput, // combined, + [], // combined, pid: process.pid, // pid, ); } @@ -481,7 +486,8 @@ final class BuildRunner extends Runner { } // Parse lines of the form '[6232/6269] LINK ./accessibility_unittests'. - void _ninjaProgress( + // Returns false if the line is not a ninja progress line. + bool _ninjaProgress( RunnerEventHandler eventHandler, List command, String line, @@ -491,19 +497,19 @@ final class BuildRunner extends Runner { if (maybeProgress.length < 3 || maybeProgress[0] != '[' || maybeProgress[maybeProgress.length - 1] != ']') { - return; + return false; } // Extract the two numbers by stripping the '[' and ']' and splitting on // the '/'. final List progress = maybeProgress.substring(1, maybeProgress.length - 1).split('/'); if (progress.length < 2) { - return; + return false; } final int? completed = int.tryParse(progress[0]); final int? total = int.tryParse(progress[1]); if (completed == null || total == null) { - return; + return false; } eventHandler(RunnerProgress( '${build.name}: ninja', @@ -514,6 +520,7 @@ final class BuildRunner extends Runner { total, completed == total, // True when done. )); + return true; } late final bool _isGoma = _mergedGnArgs.contains('--goma');