Refactor multi-file build parsing into a single BuildPlan class. (flutter/engine#55720)

Closes https://github.com/flutter/flutter/issues/148444 (code de-duplicated).
Closes https://github.com/flutter/flutter/issues/150877 (`--build-strategy=local`).
Closes https://github.com/flutter/flutter/issues/150884 (`--build-strategy=remote`).

Replaces duplicate code across ~3 commands (query, test, build) with a class called `BuildPlan`, which encapsulates (a) concurrency, (b) build configuration, (c) RBE, (d) LTO, and (e) build strategy. I also moved all of the validation of the build plan into `build_plan_test`, which gives us better coverage at a lower cost (less things to think about in that suite of tests).

I know the diff looks scary, but 1K of the 1.4K is tests.

/cc @gaaclarke @flar

engine/src/flutter/tools/engine_tool/lib/src/build_plan.dart

@@ -0,0 +1,279 @@
+// 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:args/args.dart';
+import 'package:collection/collection.dart';
+import 'package:engine_build_configs/engine_build_configs.dart';
+import 'package:meta/meta.dart';
+
+import 'build_utils.dart';
+import 'environment.dart';
+import 'logger.dart';
+
+const _flagConfig = 'config';
+const _flagConcurrency = 'concurrency';
+const _flagStrategy = 'build-strategy';
+const _flagRbe = 'rbe';
+const _flagLto = 'lto';
+
+/// Describes what (platform, targets) and how (strategy, options) to build.
+///
+/// Multiple commands in `et` are used to indirectly run builds, which often
+/// means running some combination of `gn`, `ninja`, and RBE bootstrap scripts;
+/// this class encapsulates the information needed to do so.
+@immutable
+final class BuildPlan {
+  /// Creates a new build plan from parsed command-line arguments.
+  ///
+  /// The [ArgParser] that produced [args] must have been configured with
+  /// [configureArgParser].
+  factory BuildPlan.fromArgResults(
+    ArgResults args,
+    Environment environment, {
+    required List<Build> builds,
+    String? Function() defaultBuild = _defaultHostDebug,
+  }) {
+    final build = () {
+      final name = args.option(_flagConfig) ?? defaultBuild();
+      final config = builds.firstWhereOrNull(
+        (b) => mangleConfigName(environment, b.name) == name,
+      );
+      if (config == null) {
+        if (name == null) {
+          throw FatalError('No build configuration specified.');
+        }
+        throw FatalError('Unknown build configuration: $name');
+      }
+      return config;
+    }();
+    return BuildPlan._(
+      build: build,
+      strategy: BuildStrategy.values.byName(args.option(_flagStrategy)!),
+      useRbe: () {
+        final useRbe = args.flag(_flagRbe);
+        if (useRbe && !environment.hasRbeConfigInTree()) {
+          throw FatalError(
+            'RBE requested but configuration not found.\n\n$_rbeInstructions',
+          );
+        }
+        return useRbe;
+      }(),
+      useLto: () {
+        if (args.wasParsed(_flagLto)) {
+          return args.flag(_flagLto);
+        }
+        return !build.gn.contains('--no-lto');
+      }(),
+      concurrency: () {
+        final value = args.option(_flagConcurrency);
+        if (value == null) {
+          return null;
+        }
+        if (int.tryParse(value) case final value? when value >= 0) {
+          return value;
+        }
+        throw FatalError('Invalid value for --$_flagConcurrency: $value');
+      }(),
+    );
+  }
+
+  BuildPlan._({
+    required this.build,
+    required this.strategy,
+    required this.useRbe,
+    required this.useLto,
+    required this.concurrency,
+  }) {
+    if (!useRbe && strategy == BuildStrategy.remote) {
+      throw FatalError(
+        'Cannot use remote builds without RBE enabled.\n\n$_rbeInstructions',
+      );
+    }
+  }
+
+  static String _defaultHostDebug() {
+    return 'host_debug';
+  }
+
+  /// Adds options to [parser] for configuring a [BuildPlan].
+  ///
+  /// Returns the list of builds that can be configured.
+  @useResult
+  static List<Build> configureArgParser(
+    ArgParser parser,
+    Environment environment, {
+    required bool help,
+    required Map<String, BuilderConfig> configs,
+  }) {
+    // Add --config.
+    final builds = runnableBuilds(
+      environment,
+      configs,
+      environment.verbose || !help,
+    );
+    debugCheckBuilds(builds);
+    parser.addOption(
+      _flagConfig,
+      abbr: 'c',
+      defaultsTo: () {
+        if (builds.any((b) => b.name == 'host_debug')) {
+          return 'host_debug';
+        }
+        return null;
+      }(),
+      allowed: [
+        for (final config in builds) mangleConfigName(environment, config.name),
+      ],
+      allowedHelp: {
+        for (final config in builds)
+          mangleConfigName(environment, config.name): config.description,
+      },
+    );
+
+    // Add --lto.
+    parser.addFlag(
+      _flagLto,
+      help: ''
+          'Whether LTO should be enabled for a build.\n'
+          "If omitted, defaults to the configuration's specified value, "
+          'which is typically (but not always) --no-lto.',
+      defaultsTo: null,
+      hide: !environment.verbose,
+    );
+
+    // Add --rbe.
+    final hasRbeConfigInTree = environment.hasRbeConfigInTree();
+    parser.addFlag(
+      _flagRbe,
+      defaultsTo: hasRbeConfigInTree,
+      help: () {
+        var rbeHelp = 'Enable pre-configured remote build execution.';
+        if (!hasRbeConfigInTree || environment.verbose) {
+          rbeHelp += '\n\n$_rbeInstructions';
+        }
+        return rbeHelp;
+      }(),
+    );
+
+    // Add --build-strategy.
+    parser.addOption(
+      _flagStrategy,
+      defaultsTo: _defaultStrategy.name,
+      allowed: BuildStrategy.values.map((e) => e.name),
+      allowedHelp: {
+        for (final e in BuildStrategy.values) e.name: e._help,
+      },
+      help: 'How to prefer remote or local builds.',
+      hide: !hasRbeConfigInTree && !environment.verbose,
+    );
+
+    // Add --concurrency.
+    parser.addOption(
+      _flagConcurrency,
+      abbr: 'j',
+      help: 'How many jobs to run in parallel.',
+    );
+
+    return builds;
+  }
+
+  /// The build configuration to use.
+  final Build build;
+
+  /// How to prefer remote or local builds.
+  final BuildStrategy strategy;
+  static const _defaultStrategy = BuildStrategy.auto;
+
+  /// Whether to configure the build plan to use RBE (remote build execution).
+  final bool useRbe;
+  static const _rbeInstructions = ''
+      'Google employees can follow the instructions at '
+      'https://flutter.dev/to/engine-rbe to enable RBE, which can '
+      'parallelize builds and reduce build times on faster internet '
+      'connections.';
+
+  /// How many jobs to run in parallel.
+  ///
+  /// If `null`, the build system will use the default number of jobs.
+  final int? concurrency;
+
+  /// Whether to build with LTO (link-time optimization).
+  final bool useLto;
+
+  @override
+  bool operator ==(Object other) {
+    return other is BuildPlan &&
+        build.name == other.build.name &&
+        strategy == other.strategy &&
+        useRbe == other.useRbe &&
+        useLto == other.useLto &&
+        concurrency == other.concurrency;
+  }
+
+  @override
+  int get hashCode {
+    return Object.hash(build.name, strategy, useRbe, useLto, concurrency);
+  }
+
+  /// Converts this build plan to its equivalent [RbeConfig].
+  RbeConfig toRbeConfig() {
+    switch (strategy) {
+      case BuildStrategy.auto:
+        return const RbeConfig();
+      case BuildStrategy.local:
+        return const RbeConfig(
+          execStrategy: RbeExecStrategy.local,
+          remoteDisabled: true,
+        );
+      case BuildStrategy.remote:
+        return const RbeConfig(execStrategy: RbeExecStrategy.remote);
+    }
+  }
+
+  /// Converts this build plan into extra GN arguments to pass to the build.
+  List<String> toGnArgs() {
+    return [
+      if (!useRbe) '--no-rbe',
+      if (useLto) '--lto' else '--no-lto',
+    ];
+  }
+
+  @override
+  String toString() {
+    final buffer = StringBuffer('BuildPlan <');
+    buffer.writeln();
+    buffer.writeln('  build: ${build.name}');
+    buffer.writeln('  useLto: $useLto');
+    buffer.writeln('  useRbe: $useRbe');
+    buffer.writeln('  strategy: $strategy');
+    buffer.writeln('  concurrency: $concurrency');
+    buffer.write('>');
+    return buffer.toString();
+  }
+}
+
+/// User-specified strategy for executing a build.
+enum BuildStrategy {
+  /// Automatically determine the best build strategy.
+  auto(
+    'Prefer remote builds and fallback silently to local builds.',
+  ),
+
+  /// Build locally.
+  local(
+    'Use local builds.'
+    '\n'
+    'No internet connection is required.',
+  ),
+
+  /// Build remotely.
+  remote(
+    'Use remote builds.'
+    '\n'
+    'If --$_flagStrategy is not specified, the build will fail.',
+  );
+
+  const BuildStrategy(this._help);
+  final String _help;
+}

engine/src/flutter/tools/engine_tool/lib/src/build_utils.dart

@@ -7,7 +7,6 @@ import 'dart:io' as io;
 import 'package:engine_build_configs/engine_build_configs.dart';
 import 'package:path/path.dart' as p;
 
-import 'commands/flags.dart';
 import 'environment.dart';
 import 'label.dart';
 import 'logger.dart';
@@ -120,20 +119,6 @@ String demangleConfigName(Environment env, String name) {
   return _doNotMangle(env, name) ? name : '${_osPrefix(env)}$name';
 }
 
-/// Make an RbeConfig.
-RbeConfig makeRbeConfig(String execStrategy) {
-  switch (execStrategy) {
-    case buildStrategyFlagValueAuto:
-      return const RbeConfig();
-    case buildStrategyFlagValueLocal:
-      return const RbeConfig(execStrategy: RbeExecStrategy.local);
-    case buildStrategyFlagValueRemote:
-      return const RbeConfig(execStrategy: RbeExecStrategy.remote);
-    default:
-      throw FatalError('Unknown RBE execution strategy "$execStrategy"');
-  }
-}
-
 /// Build the build target in the environment.
 Future<int> runBuild(
   Environment environment,

engine/src/flutter/tools/engine_tool/lib/src/commands/build_command.dart

@@ -4,11 +4,11 @@
 
 import 'package:engine_build_configs/engine_build_configs.dart';
 
+import '../build_plan.dart';
 import '../build_utils.dart';
 import '../gn.dart';
 import '../label.dart';
 import 'command.dart';
-import 'flags.dart';
 
 /// The root 'build' command.
 final class BuildCommand extends CommandBase {
@@ -19,21 +19,11 @@ final class BuildCommand extends CommandBase {
     super.help = false,
     super.usageLineLength,
   }) {
-    // When printing the help/usage for this command, only list all builds
+    builds = BuildPlan.configureArgParser(
-    // when the --verbose flag is supplied.
-    final bool includeCiBuilds = environment.verbose || !help;
-    builds = runnableBuilds(environment, configs, includeCiBuilds);
-    debugCheckBuilds(builds);
-    addConfigOption(
-      environment,
       argParser,
-      builds,
+      environment,
-    );
+      configs: configs,
-    addConcurrencyOption(argParser);
+      help: help,
-    addRbeOptions(argParser, environment);
-    argParser.addFlag(
-      ltoFlag,
-      help: 'Whether LTO should be enabled for a build. Default is disabled',
     );
   }
 
@@ -53,60 +43,43 @@ et build //flutter/fml:fml_benchmarks  # Build a specific target in `//flutter/f
 
   @override
   Future<int> run() async {
-    final String configName = argResults![configFlag] as String;
+    final plan = BuildPlan.fromArgResults(
-    final bool useRbe = argResults![rbeFlag] as bool;
+      argResults!,
-    if (useRbe && !environment.hasRbeConfigInTree()) {
+      environment,
-      environment.logger.error('RBE was requested but no RBE config was found');
+      builds: builds,
-      return 1;
+    );
-    }
-    final bool useLto = argResults![ltoFlag] as bool;
-    final String demangledName = demangleConfigName(environment, configName);
-    final Build? build =
-        builds.where((Build build) => build.name == demangledName).firstOrNull;
-    if (build == null) {
-      environment.logger.error('Could not find config $configName');
-      return 1;
-    }
 
-    final String dashJ = argResults![concurrencyFlag] as String;
+    final commandLineTargets = argResults!.rest;
-    final int? concurrency = int.tryParse(dashJ);
-    if (concurrency == null || concurrency < 0) {
-      environment.logger.error('-j must specify a positive integer.');
-      return 1;
-    }
-
-    final List<String> extraGnArgs = <String>[
-      if (!useRbe) '--no-rbe',
-      if (useLto) '--lto' else '--no-lto',
-    ];
-
-    final List<String> commandLineTargets = argResults!.rest;
     if (commandLineTargets.isNotEmpty &&
-        !await ensureBuildDir(environment, build, enableRbe: useRbe)) {
+        !await ensureBuildDir(
+          environment,
+          plan.build,
+          enableRbe: plan.useRbe,
+        )) {
       return 1;
     }
 
     // Builds only accept labels as arguments, so convert patterns to labels.
     // TODO(matanlurey): Can be optimized in cases where wildcards are not used.
-    final Gn gn = Gn.fromEnvironment(environment);
+    final gn = Gn.fromEnvironment(environment);
-    final Set<Label> allTargets = <Label>{};
+    final allTargets = <Label>{};
-    for (final String pattern in commandLineTargets) {
+    for (final pattern in commandLineTargets) {
-      final TargetPattern target = TargetPattern.parse(pattern);
+      final target = TargetPattern.parse(pattern);
-      final List<BuildTarget> targets = await gn.desc(
+      final targets = await gn.desc(
-        'out/${build.ninja.config}',
+        'out/${plan.build.ninja.config}',
         target,
       );
-      allTargets.addAll(targets.map((BuildTarget target) => target.label));
+      allTargets.addAll(targets.map((target) => target.label));
     }
 
     return runBuild(
       environment,
-      build,
+      plan.build,
-      concurrency: concurrency,
+      concurrency: plan.concurrency ?? 0,
-      extraGnArgs: extraGnArgs,
+      extraGnArgs: plan.toGnArgs(),
       targets: allTargets.toList(),
-      enableRbe: useRbe,
+      enableRbe: plan.useRbe,
-      rbeConfig: makeRbeConfig(argResults![buildStrategyFlag] as String),
+      rbeConfig: plan.toRbeConfig(),
     );
   }
 }

engine/src/flutter/tools/engine_tool/lib/src/commands/command.dart

@@ -4,11 +4,8 @@
 
 import 'package:args/args.dart';
 import 'package:args/command_runner.dart';
-import 'package:engine_build_configs/engine_build_configs.dart';
 
-import '../build_utils.dart';
 import '../environment.dart';
-import 'flags.dart';
 
 /// The base class that all commands and subcommands should inherit from.
 abstract base class CommandBase extends Command<int> {
@@ -34,63 +31,3 @@ abstract base class CommandBase extends Command<int> {
     environment.logger.status(usage);
   }
 }
-
-/// Adds the -c (--config) option to the parser.
-void addConfigOption(
-  Environment environment,
-  ArgParser parser,
-  List<Build> builds, {
-  String defaultsTo = 'host_debug',
-}) {
-  parser.addOption(
-    configFlag,
-    abbr: 'c',
-    defaultsTo: defaultsTo,
-    help: 'Specify the build config to use. Run "et help build --verbose" to '
-        'see the full list of runnable configurations.',
-    allowed: <String>[
-      for (final Build config in builds)
-        mangleConfigName(environment, config.name),
-    ],
-    allowedHelp: <String, String>{
-      for (final Build config in builds)
-        mangleConfigName(environment, config.name): config.description,
-    },
-  );
-}
-
-/// Adds the -j option to the parser.
-void addConcurrencyOption(ArgParser parser) {
-  parser.addOption(
-    concurrencyFlag,
-    abbr: 'j',
-    defaultsTo: '0',
-    help: 'Specify the concurrency level to use for the ninja build.',
-  );
-}
-
-/// Adds the --rbe and --exec-stragey flags.
-void addRbeOptions(ArgParser parser, Environment environment) {
-  parser.addFlag(
-    rbeFlag,
-    defaultsTo: environment.hasRbeConfigInTree(),
-    help: 'RBE is enabled by default when available.',
-  );
-  parser.addOption(
-    buildStrategyFlag,
-    allowed: [
-      buildStrategyFlagValueAuto,
-      buildStrategyFlagValueLocal,
-      buildStrategyFlagValueRemote,
-    ],
-    allowedHelp: {
-      buildStrategyFlagValueAuto:
-          'If RBE is enabled, use remote builds, otherwise fallback to local builds.',
-      buildStrategyFlagValueLocal:
-          'Use local builds regardless of RBE being enabled.',
-      buildStrategyFlagValueRemote:
-          'Use RBE builds, and fail if they are not available instead of falling back.'
-    },
-    defaultsTo: buildStrategyFlagValueAuto,
-  );
-}

engine/src/flutter/tools/engine_tool/lib/src/commands/command_runner.dart

@@ -80,7 +80,8 @@ 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.\n\nThis is a community supported project, file '
+      'a bug or feature request: https://flutter.dev/to/engine-tool-bug.';
 
   /// The host system environment.
   final Environment environment;

engine/src/flutter/tools/engine_tool/lib/src/commands/flags.dart

@@ -11,18 +11,10 @@
 // an --everything flag.
 
 // Keep this list alphabetized.
-const String allFlag = 'all';
+const allFlag = 'all';
-const String builderFlag = 'builder';
+const builderFlag = 'builder';
-const String concurrencyFlag = 'concurrency';
+const dryRunFlag = 'dry-run';
-const String configFlag = 'config';
+const quietFlag = 'quiet';
-const String dryRunFlag = 'dry-run';
+const runTestsFlag = 'run-tests';
-const String buildStrategyFlag = 'build-strategy';
+const verboseFlag = 'verbose';
-const String buildStrategyFlagValueAuto = 'auto';
+const testOnlyFlag = 'testonly';
-const String buildStrategyFlagValueLocal = 'local';
-const String buildStrategyFlagValueRemote = 'remote';
-const String ltoFlag = 'lto';
-const String quietFlag = 'quiet';
-const String rbeFlag = 'rbe';
-const String runTestsFlag = 'run-tests';
-const String verboseFlag = 'verbose';
-const String testOnlyFlag = 'testonly';

engine/src/flutter/tools/engine_tool/lib/src/commands/query_command.dart

@@ -4,6 +4,7 @@
 
 import 'package:engine_build_configs/engine_build_configs.dart';
 
+import '../build_plan.dart';
 import '../build_utils.dart';
 import '../gn.dart';
 import '../label.dart';
@@ -139,15 +140,11 @@ final class QueryTargetsCommand extends CommandBase {
     required this.configs,
     super.help = false,
   }) {
-    // When printing the help/usage for this command, only list all builds
+    builds = BuildPlan.configureArgParser(
-    // when the --verbose flag is supplied.
-    final bool includeCiBuilds = environment.verbose || !help;
-    builds = runnableBuilds(environment, configs, includeCiBuilds);
-    debugCheckBuilds(builds);
-    addConfigOption(
-      environment,
       argParser,
-      builds,
+      environment,
+      configs: configs,
+      help: help,
     );
     argParser.addFlag(
       testOnlyFlag,
@@ -155,11 +152,6 @@ final class QueryTargetsCommand extends CommandBase {
       help: 'Filter build targets to only include tests',
       negatable: false,
     );
-    argParser.addFlag(
-      rbeFlag,
-      defaultsTo: environment.hasRbeConfigInTree(),
-      help: 'RBE is enabled by default when available.',
-    );
   }
 
   /// Build configurations loaded from the engine from under ci/builders.
@@ -180,41 +172,36 @@ et query targets //flutter/fml/...  # List all targets under `//flutter/fml`
 
   @override
   Future<int> run() async {
-    final String configName = argResults![configFlag] as String;
+    final plan = BuildPlan.fromArgResults(
-    final bool testOnly = argResults![testOnlyFlag] as bool;
+      argResults!,
-    final bool useRbe = argResults![rbeFlag] as bool;
+      environment,
-    if (useRbe && !environment.hasRbeConfigInTree()) {
+      builds: builds,
-      environment.logger.error('RBE was requested but no RBE config was found');
+    );
-      return 1;
-    }
-    final String demangledName = demangleConfigName(environment, configName);
-    final Build? build =
-        builds.where((Build build) => build.name == demangledName).firstOrNull;
-    if (build == null) {
-      environment.logger.error('Could not find config $configName');
-      return 1;
-    }
 
-    if (!await ensureBuildDir(environment, build, enableRbe: useRbe)) {
+    if (!await ensureBuildDir(
+      environment,
+      plan.build,
+      enableRbe: plan.useRbe,
+    )) {
       return 1;
     }
 
     // Builds only accept labels as arguments, so convert patterns to labels.
     // TODO(matanlurey): Can be optimized in cases where wildcards are not used.
-    final Gn gn = Gn.fromEnvironment(environment);
+    final gn = Gn.fromEnvironment(environment);
 
     // TODO(matanlurey): Discuss if we want to just require '//...'.
     // For now this retains the existing behavior.
-    List<String> patterns = argResults!.rest;
+    var patterns = argResults!.rest;
     if (patterns.isEmpty) {
-      patterns = <String>['//...'];
+      patterns = ['//...'];
     }
 
-    final Set<BuildTarget> allTargets = <BuildTarget>{};
+    final allTargets = <BuildTarget>{};
-    for (final String pattern in patterns) {
+    for (final pattern in patterns) {
-      final TargetPattern target = TargetPattern.parse(pattern);
+      final target = TargetPattern.parse(pattern);
-      final List<BuildTarget> targets = await gn.desc(
+      final targets = await gn.desc(
-        'out/${build.ninja.config}',
+        'out/${plan.build.ninja.config}',
         target,
       );
       allTargets.addAll(targets);
@@ -225,7 +212,8 @@ et query targets //flutter/fml/...  # List all targets under `//flutter/fml`
       return 1;
     }
 
-    for (final BuildTarget target in allTargets) {
+    final testOnly = argResults!.flag(testOnlyFlag);
+    for (final target in allTargets) {
       if (testOnly && (!target.testOnly || target is! ExecutableBuildTarget)) {
         continue;
       }

engine/src/flutter/tools/engine_tool/lib/src/commands/run_command.dart

@@ -4,10 +4,11 @@
 
 import 'dart:io' show ProcessStartMode;
 
+import 'package:collection/collection.dart';
 import 'package:engine_build_configs/engine_build_configs.dart';
 import 'package:meta/meta.dart';
-import 'package:process_runner/process_runner.dart';
 
+import '../build_plan.dart';
 import '../build_utils.dart';
 import '../flutter_tool_interop/device.dart';
 import '../flutter_tool_interop/flutter_tool.dart';
@@ -15,7 +16,6 @@ import '../flutter_tool_interop/target_platform.dart';
 import '../label.dart';
 import '../logger.dart';
 import 'command.dart';
-import 'flags.dart';
 
 /// The root 'run' command.
 final class RunCommand extends CommandBase {
@@ -27,21 +27,12 @@ final class RunCommand extends CommandBase {
     super.usageLineLength,
     @visibleForTesting FlutterTool? flutterTool,
   }) {
-    // When printing the help/usage for this command, only list all builds
+    builds = BuildPlan.configureArgParser(
-    // when the --verbose flag is supplied.
-    final bool includeCiBuilds = environment.verbose || !help;
-    builds = runnableBuilds(environment, configs, includeCiBuilds);
-    debugCheckBuilds(builds);
-    // We default to nothing in order to automatically detect attached devices
-    // and select an appropriate target from them.
-    addConfigOption(
-      environment,
       argParser,
-      builds,
+      environment,
-      defaultsTo: '',
+      configs: configs,
+      help: help,
     );
-    addConcurrencyOption(argParser);
-    addRbeOptions(argParser, environment);
     _flutterTool = flutterTool ?? FlutterTool.fromEnvironment(environment);
   }
 
@@ -64,17 +55,15 @@ See `flutter run --help` for a listing
 ''';
 
   Build? _findTargetBuild(String configName) {
-    final String demangledName = demangleConfigName(environment, configName);
+    final demangledName = demangleConfigName(environment, configName);
-    return builds
+    return builds.firstWhereOrNull((build) => build.name == demangledName);
-        .where((Build build) => build.name == demangledName)
-        .firstOrNull;
   }
 
   Build? _findHostBuild(Build? targetBuild) {
     if (targetBuild == null) {
       return null;
     }
-    final String mangledName = mangleConfigName(environment, targetBuild.name);
+    final mangledName = mangleConfigName(environment, targetBuild.name);
     if (mangledName.contains('host_')) {
       return targetBuild;
     }
@@ -94,13 +83,13 @@ See `flutter run --help` for a listing
 
   String _getDeviceId() {
     if (argResults!.rest.contains('-d')) {
-      final int index = argResults!.rest.indexOf('-d') + 1;
+      final index = argResults!.rest.indexOf('-d') + 1;
       if (index < argResults!.rest.length) {
         return argResults!.rest[index];
       }
     }
     if (argResults!.rest.contains('--device-id')) {
-      final int index = argResults!.rest.indexOf('--device-id') + 1;
+      final index = argResults!.rest.indexOf('--device-id') + 1;
       if (index < argResults!.rest.length) {
         return argResults!.rest[index];
       }
@@ -110,7 +99,7 @@ See `flutter run --help` for a listing
 
   String _getMode() {
     // Sniff the build mode from the args that will be passed to flutter run.
-    String mode = 'debug';
+    var mode = 'debug';
     if (argResults!.rest.contains('--profile')) {
       mode = 'profile';
     } else if (argResults!.rest.contains('--release')) {
@@ -124,92 +113,59 @@ See `flutter run --help` for a listing
     return RunTarget.detectAndSelect(devices, idPrefix: _getDeviceId());
   })();
 
-  Future<String> _selectTargetConfig() async {
-    final configName = argResults![configFlag] as String;
-    if (configName.isNotEmpty) {
-      return configName;
-    }
-    final target = await _runTarget;
-    if (target == null) {
-      return 'host_debug';
-    }
-
-    final result = target.buildConfigFor(_getMode());
-    environment.logger.status('Building to run on $result');
-    return result;
-  }
-
   @override
   Future<int> run() async {
     if (!environment.processRunner.processManager.canRun('flutter')) {
       throw FatalError('Cannot find the "flutter" command in your PATH');
     }
 
-    final configName = await _selectTargetConfig();
-    final targetBuild = _findTargetBuild(configName);
-    if (targetBuild == null) {
-      throw FatalError('Could not find build $configName');
-    }
-
-    final hostBuild = _findHostBuild(targetBuild);
-    if (hostBuild == null) {
-      throw FatalError('Could not find host build for $configName');
-    }
-
-    final useRbe = argResults!.flag(rbeFlag);
-    if (useRbe && !environment.hasRbeConfigInTree()) {
-      throw FatalError('RBE was requested but no RBE config was found');
-    }
-
-    final extraGnArgs = [
-      if (!useRbe) '--no-rbe',
-    ];
     final target = await _runTarget;
+    final plan = BuildPlan.fromArgResults(
+      argResults!,
+      environment,
+      builds: builds,
+      defaultBuild: () => target?.buildConfigFor(_getMode()),
+    );
+
+    final hostBuild = _findHostBuild(plan.build);
+    if (hostBuild == null) {
+      throw FatalError('Could not find host build for ${plan.build.name}');
+    }
+
     final buildTargetsForShell = target?.buildTargetsForShell ?? [];
 
-    final concurrency = int.tryParse(argResults![concurrencyFlag] as String);
-    if (concurrency == null || concurrency < 0) {
-      throw FatalError(
-        '--$concurrencyFlag (-j) must specify a positive integer.',
-      );
-    }
-
     // First build the host.
-    int r = await runBuild(
+    var r = await runBuild(
       environment,
       hostBuild,
-      concurrency: concurrency,
+      concurrency: plan.concurrency ?? 0,
-      extraGnArgs: extraGnArgs,
+      extraGnArgs: plan.toGnArgs(),
-      enableRbe: useRbe,
+      enableRbe: plan.useRbe,
-      rbeConfig: makeRbeConfig(argResults![buildStrategyFlag] as String),
+      rbeConfig: plan.toRbeConfig(),
     );
     if (r != 0) {
       throw FatalError('Failed to build host (${hostBuild.name})');
     }
 
     // Now build the target if it isn't the same.
-    if (hostBuild.name != targetBuild.name) {
+    if (hostBuild.name != plan.build.name) {
       r = await runBuild(
         environment,
-        targetBuild,
+        plan.build,
-        concurrency: concurrency,
+        concurrency: plan.concurrency ?? 0,
-        extraGnArgs: extraGnArgs,
+        extraGnArgs: plan.toGnArgs(),
-        enableRbe: useRbe,
+        enableRbe: plan.useRbe,
         targets: buildTargetsForShell,
-        rbeConfig: makeRbeConfig(argResults![buildStrategyFlag] as String),
+        rbeConfig: plan.toRbeConfig(),
       );
       if (r != 0) {
-        throw FatalError('Failed to build target (${targetBuild.name})');
+        throw FatalError('Failed to build target (${plan.build.name})');
       }
     }
 
-    final String mangledBuildName =
+    final mangledBuildName = mangleConfigName(environment, plan.build.name);
-        mangleConfigName(environment, targetBuild.name);
+    final mangledHostBuildName = mangleConfigName(environment, hostBuild.name);
-
+    final command = <String>[
-    final String mangledHostBuildName =
-        mangleConfigName(environment, hostBuild.name);
-
-    final List<String> command = <String>[
       'flutter',
       'run',
       '--local-engine-src-path',
@@ -223,8 +179,7 @@ See `flutter run --help` for a listing
 
     // TODO(johnmccutchan): Be smart and if the user requested a profile
     // config, add the '--profile' flag when invoking flutter run.
-    final ProcessRunnerResult result =
+    final result = await environment.processRunner.runProcess(
-        await environment.processRunner.runProcess(
       command,
       runInShell: true,
       startMode: ProcessStartMode.inheritStdio,

engine/src/flutter/tools/engine_tool/lib/src/commands/test_command.dart

@@ -4,13 +4,13 @@
 
 import 'package:engine_build_configs/engine_build_configs.dart';
 
+import '../build_plan.dart';
 import '../build_utils.dart';
 import '../gn.dart';
 import '../label.dart';
 import '../proc_utils.dart';
 import '../worker_pool.dart';
 import 'command.dart';
-import 'flags.dart';
 
 /// The root 'test' command.
 final class TestCommand extends CommandBase {
@@ -21,18 +21,12 @@ final class TestCommand extends CommandBase {
     super.help = false,
     super.usageLineLength,
   }) {
-    // When printing the help/usage for this command, only list all builds
+    builds = BuildPlan.configureArgParser(
-    // when the --verbose flag is supplied.
-    final bool includeCiBuilds = environment.verbose || !help;
-    builds = runnableBuilds(environment, configs, includeCiBuilds);
-    debugCheckBuilds(builds);
-    addConfigOption(
-      environment,
       argParser,
-      builds,
+      environment,
+      configs: configs,
+      help: help,
     );
-    addConcurrencyOption(argParser);
-    addRbeOptions(argParser, environment);
   }
 
   /// List of compatible builds.
@@ -51,40 +45,29 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
 
   @override
   Future<int> run() async {
-    final String configName = argResults![configFlag] as String;
+    final plan = BuildPlan.fromArgResults(
-    final bool useRbe = argResults![rbeFlag] as bool;
+      argResults!,
-    if (useRbe && !environment.hasRbeConfigInTree()) {
+      environment,
-      environment.logger.error('RBE was requested but no RBE config was found');
+      builds: builds,
-      return 1;
+    );
-    }
-    final String demangledName = demangleConfigName(environment, configName);
-    final Build? build =
-        builds.where((Build build) => build.name == demangledName).firstOrNull;
-    if (build == null) {
-      environment.logger.error('Could not find config $configName');
-      return 1;
-    }
 
-    final String dashJ = argResults![concurrencyFlag] as String;
+    if (!await ensureBuildDir(
-    final int? concurrency = int.tryParse(dashJ);
+      environment,
-    if (concurrency == null || concurrency < 0) {
+      plan.build,
-      environment.logger.error('-j must specify a positive integer.');
+      enableRbe: plan.useRbe,
-      return 1;
+    )) {
-    }
-
-    if (!await ensureBuildDir(environment, build, enableRbe: useRbe)) {
       return 1;
     }
 
     // Builds only accept labels as arguments, so convert patterns to labels.
-    final Gn gn = Gn.fromEnvironment(environment);
+    final gn = Gn.fromEnvironment(environment);
 
     // Figure out what targets the user wants to build.
-    final Set<BuildTarget> buildTargets = <BuildTarget>{};
+    final buildTargets = <BuildTarget>{};
-    for (final String pattern in argResults!.rest) {
+    for (final pattern in argResults!.rest) {
-      final TargetPattern target = TargetPattern.parse(pattern);
+      final target = TargetPattern.parse(pattern);
-      final List<BuildTarget> found = await gn.desc(
+      final found = await gn.desc(
-        'out/${build.ninja.config}',
+        'out/${plan.build.ninja.config}',
         target,
       );
       buildTargets.addAll(found);
@@ -96,7 +79,7 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
     }
 
     // Make sure there is at least one test target.
-    final List<ExecutableBuildTarget> testTargets = buildTargets
+    final testTargets = buildTargets
         .whereType<ExecutableBuildTarget>()
         .where((ExecutableBuildTarget t) => t.testOnly)
         .toList();
@@ -106,24 +89,24 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
       return 1;
     }
 
-    final int buildExitCode = await runBuild(
+    final buildExitCode = await runBuild(
       environment,
-      build,
+      plan.build,
-      concurrency: concurrency,
+      concurrency: plan.concurrency ?? 0,
-      targets: testTargets.map((BuildTarget target) => target.label).toList(),
+      targets: testTargets.map((target) => target.label).toList(),
-      enableRbe: useRbe,
+      enableRbe: plan.useRbe,
-      rbeConfig: makeRbeConfig(argResults![buildStrategyFlag] as String),
+      rbeConfig: plan.toRbeConfig(),
     );
     if (buildExitCode != 0) {
       return buildExitCode;
     }
-    final WorkerPool workerPool = WorkerPool(
+    final workerPool = WorkerPool(
       environment,
       ProcessTaskProgressReporter(environment),
     );
-    final Set<ProcessTask> tasks = <ProcessTask>{};
+    final tasks = <ProcessTask>{};
-    for (final ExecutableBuildTarget target in testTargets) {
+    for (final target in testTargets) {
-      final List<String> commandLine = <String>[target.executable];
+      final commandLine = <String>[target.executable];
       tasks.add(ProcessTask(
         target.label.toString(),
         environment,

engine/src/flutter/tools/engine_tool/pubspec.yaml

@@ -14,6 +14,7 @@ resolution: workspace
 
 dependencies:
   args: any
+  collection: any
   engine_build_configs: any
   engine_repo_tools: any
   file: any

engine/src/flutter/tools/engine_tool/test/build_command_test.dart

@@ -281,41 +281,6 @@ void main() {
     );
   });
 
-  test('build command fails when rbe is enabled but not supported', () async {
-    final testEnv = TestEnvironment.withTestEngine(
-      abi: Abi.macosArm64,
-      // Intentionally omit withRbe: true.
-      // That means the //flutter/build/rbe directory will not be created.
-    );
-    addTearDown(testEnv.cleanup);
-
-    final builder = TestBuilderConfig();
-    builder.addBuild(
-      name: 'ci/android_debug_rbe_arm64',
-      dimension: TestDroneDimension.mac,
-      enableRbe: true,
-    );
-    final runner = ToolCommandRunner(
-      environment: testEnv.environment,
-      configs: {
-        'mac_test_config': builder.buildConfig(
-          path: 'ci/builders/mac_test_config.json',
-        ),
-      },
-    );
-    final result = await runner.run([
-      'build',
-      '--config',
-      'ci/android_debug_rbe_arm64',
-      '--rbe',
-    ]);
-    expect(result, equals(1));
-    expect(
-      testEnv.testLogs.map((LogRecord r) => r.message).join(),
-      contains('RBE was requested but no RBE config was found'),
-    );
-  });
-
   test('build command does not run rbe when disabled', () async {
     final testEnv = TestEnvironment.withTestEngine(
       abi: Abi.macosArm64,

engine/src/flutter/tools/engine_tool/test/build_utils_test.dart

@@ -1,25 +0,0 @@
-// 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 'package:engine_tool/src/build_utils.dart';
-import 'package:engine_tool/src/commands/flags.dart';
-import 'package:test/test.dart';
-
-void main() {
-  test('makeRbeConfig local selects local strategy', () {
-    expect(makeRbeConfig(buildStrategyFlagValueLocal),
-        equals(const RbeConfig(execStrategy: RbeExecStrategy.local)));
-  });
-
-  test('makeRbeConfig remote selects remote strategy', () {
-    expect(makeRbeConfig(buildStrategyFlagValueRemote),
-        equals(const RbeConfig(execStrategy: RbeExecStrategy.remote)));
-  });
-
-  test('makeRbeConfig auto selects racing strategy', () {
-    expect(makeRbeConfig(buildStrategyFlagValueAuto).execStrategy,
-        equals(RbeExecStrategy.racing));
-  });
-}

engine/src/flutter/tools/engine_tool/test/run_command_test.dart

@@ -128,84 +128,6 @@ void main() {
         ),
       );
     });
-
-    test('fails if RBE was requested but no RBE config was found', () async {
-      final builders = TestBuilderConfig();
-      builders.addBuild(
-        name: 'linux/android_debug_arm64',
-        dimension: TestDroneDimension.linux,
-        targetDir: 'android_debug_arm64',
-      );
-      builders.addBuild(
-        name: 'linux/host_debug',
-        dimension: TestDroneDimension.linux,
-        targetDir: 'host_debug',
-      );
-
-      final et = _engineTool(RunCommand(
-        environment: testEnvironment,
-        configs: {
-          'linux_test_config': builders.buildConfig(
-            path: 'ci/builders/linux_test_config.json',
-          ),
-        },
-        flutterTool: flutterTool,
-      ));
-
-      expect(
-        () => et.run(['run', '--rbe', '--config=android_debug_arm64']),
-        throwsA(
-          isA<FatalError>().having(
-            (a) => a.toString(),
-            'toString()',
-            contains('RBE was requested but no RBE config was found'),
-          ),
-        ),
-      );
-    });
-
-    group('fails if -j is not a positive integer', () {
-      for (final arg in ['-1', 'foo']) {
-        test('fails if -j is $arg', () async {
-          final builders = TestBuilderConfig();
-          builders.addBuild(
-            name: 'linux/android_debug_arm64',
-            dimension: TestDroneDimension.linux,
-            targetDir: 'android_debug_arm64',
-          );
-          builders.addBuild(
-            name: 'linux/host_debug',
-            dimension: TestDroneDimension.linux,
-            targetDir: 'host_debug',
-          );
-
-          final et = _engineTool(RunCommand(
-            environment: testEnvironment,
-            configs: {
-              'linux_test_config': builders.buildConfig(
-                path: 'ci/builders/linux_test_config.json',
-              ),
-            },
-            flutterTool: flutterTool,
-          ));
-
-          expect(
-            () => et.run([
-              'run',
-              '--config=android_debug_arm64',
-              '--concurrency=$arg',
-            ]),
-            throwsA(
-              isA<FatalError>().having(
-                (a) => a.toString(),
-                'toString()',
-                contains('concurrency (-j) must specify a positive integer'),
-              ),
-            ),
-          );
-        });
-      }
-    });
   });
 
   group('builds and executes `flutter run`', () {

engine/src/flutter/tools/engine_tool/test/src/test_build_configs.dart

@@ -26,6 +26,7 @@ final class TestBuilderConfig {
     required String name,
     required TestDroneDimension dimension,
     bool enableRbe = false,
+    bool? enableLto,
     String description = 'A default description.',
     String? targetDir,
     (String, List<String>)? generatorTask,
@@ -39,6 +40,7 @@ final class TestBuilderConfig {
       'gclient_variables': <String, Object?>{},
       'gn': [
         if (enableRbe) '--rbe',
+        if (enableLto == false) '--no-lto',
       ],
       'name': name,
       'description': description,