Add an initial 'build' command to engine_tool (#50681)

johnmccutchan · web-flow · commit 0f21eba104d2 · 2024-02-23T10:02:34.000-08:00
* Adds a 'build' command to `et`. * Renamed `query builds` to `query builders` because that seemed to be more precise(?) Some extra requests during review: 1. I've left some questions I'd like answers to at the top of `build_command.dart` I suspect @zanderso and @loic-sharma can give me (some?) answers. 2. I suspect I'm holding the FakeProcessManager wrong or there is a better way to write the tests in `build_command_test.dart`. Pointers to good examples are appreciated.
diff --git a/tools/engine_tool/lib/src/build_utils.dart b/tools/engine_tool/lib/src/build_utils.dart
@@ -0,0 +1,52 @@
+// 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 'package:engine_build_configs/engine_build_configs.dart';
+
+import 'environment.dart';
+
+/// A function that returns true or false when given a [BuildConfig] and its
+/// name.
+typedef ConfigFilter = bool Function(String name, BuildConfig config);
+
+/// A function that returns true or false when given a [BuildConfig] name
+/// and a [GlobalBuild].
+typedef BuildFilter = bool Function(String configName, GlobalBuild build);
+
+/// Returns a filtered copy of [input] filtering out configs where test
+/// returns false.
+Map<String, BuildConfig> filterBuildConfigs(
+    Map<String, BuildConfig> input, ConfigFilter test) {
+  return <String, BuildConfig>{
+    for (final MapEntry<String, BuildConfig> entry in input.entries)
+      if (test(entry.key, entry.value)) entry.key: entry.value,
+  };
+}
+
+/// Returns a copy of [input] filtering out configs that are not runnable
+/// on the current platform.
+Map<String, BuildConfig> runnableBuildConfigs(
+    Environment env, Map<String, BuildConfig> input) {
+  return filterBuildConfigs(input, (String name, BuildConfig config) {
+    return config.canRunOn(env.platform);
+  });
+}
+
+/// Returns a List of [GlobalBuild] that match test.
+List<GlobalBuild> filterBuilds(
+    Map<String, BuildConfig> input, BuildFilter test) {
+  return <GlobalBuild>[
+    for (final MapEntry<String, BuildConfig> entry in input.entries)
+      for (final GlobalBuild build in entry.value.builds)
+        if (test(entry.key, build)) build,
+  ];
+}
+
+/// Returns a list of runnable builds.
+List<GlobalBuild> runnableBuilds(
+    Environment env, Map<String, BuildConfig> input) {
+  return filterBuilds(input, (String configName, GlobalBuild build) {
+    return build.canRunOn(env.platform);
+  });
+}
diff --git a/tools/engine_tool/lib/src/commands/build_command.dart b/tools/engine_tool/lib/src/commands/build_command.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 'package:engine_build_configs/engine_build_configs.dart';
+
+import '../build_utils.dart';
+
+import 'command.dart';
+
+const String _configFlag = 'config';
+
+// TODO(johnmccutchan): Should BuildConfig be BuilderConfig and GlobalBuild be BuildConfig?
+// TODO(johnmccutchan): List all available build targets and allow the user
+// to specify which one(s) we should build on the cli.
+// TODO(johnmccutchan): Can we show a progress indicator like 'running gn...'?
+
+/// The root 'build' command.
+final class BuildCommand extends CommandBase {
+  /// Constructs the 'build' command.
+  BuildCommand({
+    required super.environment,
+    required Map<String, BuildConfig> configs,
+  }) {
+    builds = runnableBuilds(environment, configs);
+    // Add options here that are common to all queries.
+    argParser.addOption(
+      _configFlag,
+      abbr: 'c',
+      defaultsTo: 'host_debug',
+      help: 'Specify the build config to use',
+      allowed: <String>[
+        for (final GlobalBuild config in runnableBuilds(environment, configs))
+          config.name,
+      ],
+      allowedHelp: <String, String>{
+        for (final GlobalBuild config in runnableBuilds(environment, configs))
+          config.name: config.gn.join(' '),
+      },
+    );
+  }
+
+  /// List of compatible builds.
+  late final List<GlobalBuild> builds;
+
+  @override
+  String get name => 'build';
+
+  @override
+  String get description => 'Builds the engine';
+
+  @override
+  Future<int> run() async {
+    final String configName = argResults![_configFlag] as String;
+    final GlobalBuild? build = builds
+        .where((GlobalBuild build) => build.name == configName)
+        .firstOrNull;
+    if (build == null) {
+      environment.logger.error('Could not find config $configName');
+      return 1;
+    }
+    final GlobalBuildRunner buildRunner = GlobalBuildRunner(
+        platform: environment.platform,
+        processRunner: environment.processRunner,
+        abi: environment.abi,
+        engineSrcDir: environment.engine.srcDir,
+        build: build);
+    void handler(RunnerEvent event) {
+      switch (event) {
+        case RunnerStart():
+          environment.logger.info('$event: ${event.command.join(' ')}');
+        case RunnerProgress(done: true):
+          environment.logger.clearLine();
+          environment.logger.status(event);
+        case RunnerProgress(done: false):
+          {
+            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');
+          }
+        default:
+          environment.logger.status(event);
+      }
+    }
+
+    final bool buildResult = await buildRunner.run(handler);
+    return buildResult ? 0 : 1;
+  }
+}
diff --git a/tools/engine_tool/lib/src/commands/command_runner.dart b/tools/engine_tool/lib/src/commands/command_runner.dart
@@ -3,11 +3,10 @@
 // found in the LICENSE file.
 
 import 'package:args/command_runner.dart';
-
 import 'package:engine_build_configs/engine_build_configs.dart';
 
 import '../environment.dart';
-import 'command.dart';
+import 'build_command.dart';
 import 'format_command.dart';
 import 'query_command.dart';
 
@@ -19,14 +18,12 @@ final class ToolCommandRunner extends CommandRunner<int> {
     required this.environment,
     required this.configs,
   }) : super(toolName, toolDescription) {
-    final List<CommandBase> commands = <CommandBase>[
+    final List<Command<int>> commands = <Command<int>>[
       FormatCommand(
         environment: environment,
       ),
-      QueryCommand(
-        environment: environment,
-        configs: configs,
-      ),
+      QueryCommand(environment: environment, configs: configs),
+      BuildCommand(environment: environment, configs: configs),
     ];
     commands.forEach(addCommand);
   }
@@ -38,7 +35,7 @@ final class ToolCommandRunner extends CommandRunner<int> {
   /// The description of the tool reported in the tool's usage and help
   /// messages.
   static const String toolDescription = 'A command line tool for working on '
-                                        'the Flutter Engine.';
+      'the Flutter Engine.';
 
   /// The host system environment.
   final Environment environment;
@@ -48,7 +45,7 @@ final class ToolCommandRunner extends CommandRunner<int> {
 
   @override
   Future<int> run(Iterable<String> args) async {
-    try{
+    try {
       return await runCommand(parse(args)) ?? 0;
     } on FormatException catch (e) {
       environment.logger.error(e);
diff --git a/tools/engine_tool/lib/src/commands/query_command.dart b/tools/engine_tool/lib/src/commands/query_command.dart
@@ -28,8 +28,7 @@ final class QueryCommand extends CommandBase {
         help: 'Restrict the query to a single builder.',
         allowed: <String>[
           for (final MapEntry<String, BuildConfig> entry in configs.entries)
-            if (entry.value.canRunOn(environment.platform))
-              entry.key,
+            if (entry.value.canRunOn(environment.platform)) entry.key,
         ],
         allowedHelp: <String, String>{
           // TODO(zanderso): Add human readable descriptions to the json files.
@@ -45,7 +44,7 @@ final class QueryCommand extends CommandBase {
         negatable: false,
       );
 
-    addSubcommand(QueryBuildsCommand(
+    addSubcommand(QueryBuildersCommand(
       environment: environment,
       configs: configs,
     ));
@@ -59,13 +58,13 @@ final class QueryCommand extends CommandBase {
 
   @override
   String get description => 'Provides information about build configurations '
-                            'and tests.';
+      'and tests.';
 }
 
-// ignore: public_member_api_docs
-final class QueryBuildsCommand extends CommandBase {
-  // ignore: public_member_api_docs
-  QueryBuildsCommand({
+/// The 'query builds' command.
+final class QueryBuildersCommand extends CommandBase {
+  /// Constructs the 'query build' command.
+  QueryBuildersCommand({
     required super.environment,
     required this.configs,
   });
@@ -74,11 +73,11 @@ final class QueryBuildsCommand extends CommandBase {
   final Map<String, BuildConfig> configs;
 
   @override
-  String get name => 'builds';
+  String get name => 'builders';
 
   @override
-  String get description => 'Provides information about CI build '
-                            'configurations';
+  String get description => 'Provides information about CI builder '
+      'configurations';
 
   @override
   Future<int> run() async {
diff --git a/tools/engine_tool/test/build_command_test.dart b/tools/engine_tool/test/build_command_test.dart
@@ -0,0 +1,122 @@
+// 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/build_utils.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 BuildConfig linuxTestConfig = BuildConfig.fromJson(
+    path: 'ci/builders/linux_test_config.json',
+    map: convert.jsonDecode(fixtures.testConfig('Linux'))
+        as Map<String, Object?>,
+  );
+
+  final BuildConfig macTestConfig = BuildConfig.fromJson(
+    path: 'ci/builders/mac_test_config.json',
+    map: convert.jsonDecode(fixtures.testConfig('Mac-12'))
+        as Map<String, Object?>,
+  );
+
+  final BuildConfig winTestConfig = BuildConfig.fromJson(
+    path: 'ci/builders/win_test_config.json',
+    map: convert.jsonDecode(fixtures.testConfig('Windows-11'))
+        as Map<String, Object?>,
+  );
+
+  final Map<String, BuildConfig> configs = <String, BuildConfig>{
+    'linux_test_config': linuxTestConfig,
+    'linux_test_config2': linuxTestConfig,
+    'mac_test_config': macTestConfig,
+    'win_test_config': winTestConfig,
+  };
+
+  (Environment, List<List<String>>) linuxEnv(Logger logger) {
+    final List<List<String>> runHistory = <List<String>>[];
+    return (
+      Environment(
+        abi: ffi.Abi.linuxX64,
+        engine: engine,
+        platform: FakePlatform(operatingSystem: Platform.linux),
+        processRunner: ProcessRunner(
+          processManager: FakeProcessManager(onStart: (List<String> command) {
+            runHistory.add(command);
+            return FakeProcess();
+          }, onRun: (List<String> command) {
+            runHistory.add(command);
+            return io.ProcessResult(81, 0, '', '');
+          }),
+        ),
+        logger: logger,
+      ),
+      runHistory
+    );
+  }
+
+  test('can find host runnable build', () async {
+    final Logger logger = Logger.test();
+    final (Environment env, _) = linuxEnv(logger);
+    final List<GlobalBuild> result = runnableBuilds(env, configs);
+    expect(result.length, equals(2));
+    expect(result[0].name, equals('build_name'));
+  });
+
+  test('build command invokes gn', () async {
+    final Logger logger = Logger.test();
+    final (Environment env, List<List<String>> runHistory) = linuxEnv(logger);
+    final ToolCommandRunner runner = ToolCommandRunner(
+      environment: env,
+      configs: configs,
+    );
+    final int result = await runner.run(<String>[
+      'build',
+      '--config',
+      'build_name',
+    ]);
+    expect(result, equals(0));
+    expect(runHistory.length, greaterThanOrEqualTo(1));
+    expect(runHistory[0].length, greaterThanOrEqualTo(1));
+    expect(runHistory[0][0], contains('gn'));
+  });
+
+  test('build command invokes ninja', () async {
+    final Logger logger = Logger.test();
+    final (Environment env, List<List<String>> runHistory) = linuxEnv(logger);
+    final ToolCommandRunner runner = ToolCommandRunner(
+      environment: env,
+      configs: configs,
+    );
+    final int result = await runner.run(<String>[
+      'build',
+      '--config',
+      'build_name',
+    ]);
+    expect(result, equals(0));
+    expect(runHistory.length, greaterThanOrEqualTo(2));
+    expect(runHistory[1].length, greaterThanOrEqualTo(1));
+    expect(runHistory[1][0], contains('ninja'));
+  });
+}
diff --git a/tools/engine_tool/test/query_command_test.dart b/tools/engine_tool/test/query_command_test.dart
