Infer --rbe based on the existence of //flutter/build/rbe (#52700)

Closes flutter/flutter#148006.

Author: matanlurey

tools/engine_tool/lib/src/build_utils.dart

@@ -2,10 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:io' as io show Directory;
-
 import 'package:engine_build_configs/engine_build_configs.dart';
-import 'package:path/path.dart' as p;
 
 import 'environment.dart';
 import 'logger.dart';
@@ -122,18 +119,12 @@ String demangleConfigName(Environment env, String name) {
 Future<int> runBuild(
   Environment environment,
   Build build, {
+  required bool enableRbe,
   List<String> extraGnArgs = const <String>[],
   List<String> targets = const <String>[],
 }) async {
-  // If RBE config files aren't in the tree, then disable RBE.
-  final String rbeConfigPath = p.join(
-    environment.engine.srcDir.path,
-    'flutter',
-    'build',
-    'rbe',
-  );
   final List<String> gnArgs = <String>[
-    if (!io.Directory(rbeConfigPath).existsSync()) '--no-rbe',
+    if (!enableRbe) '--no-rbe',
     ...extraGnArgs,
   ];
 
@@ -192,16 +183,10 @@ Future<int> runGn(
   Environment environment,
   Build build, {
   List<String> extraGnArgs = const <String>[],
+  required bool enableRbe,
 }) async {
-  // If RBE config files aren't in the tree, then disable RBE.
-  final String rbeConfigPath = p.join(
-    environment.engine.srcDir.path,
-    'flutter',
-    'build',
-    'rbe',
-  );
   final List<String> gnArgs = <String>[
-    if (!io.Directory(rbeConfigPath).existsSync()) '--no-rbe',
+    if (!enableRbe) '--no-rbe',
     ...extraGnArgs,
   ];
 

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

@@ -30,9 +30,8 @@ final class BuildCommand extends CommandBase {
     );
     argParser.addFlag(
       rbeFlag,
-      defaultsTo: true,
-      help: 'RBE is enabled by default when available. Use --no-rbe to '
-          'disable it.',
+      defaultsTo: environment.hasRbeConfigInTree(),
+      help: 'RBE is enabled by default when available.',
     );
     argParser.addFlag(
       ltoFlag,
@@ -57,6 +56,10 @@ et build //flutter/fml:fml_benchmarks  # Build a specific target in `//flutter/f
   Future<int> run() async {
     final String configName = argResults![configFlag] as String;
     final bool useRbe = argResults![rbeFlag] as bool;
+    if (useRbe && !environment.hasRbeConfigInTree()) {
+      environment.logger.error('RBE was requested but no RBE config was found');
+      return 1;
+    }
     final bool useLto = argResults![ltoFlag] as bool;
     final String demangledName = demangleConfigName(environment, configName);
     final Build? build =
@@ -76,6 +79,7 @@ et build //flutter/fml:fml_benchmarks  # Build a specific target in `//flutter/f
       environment,
       build,
       argResults!.rest,
+      enableRbe: useRbe,
     );
     if (selectedTargets == null) {
       // The user typed something wrong and targetsFromCommandLine has already
@@ -93,6 +97,7 @@ et build //flutter/fml:fml_benchmarks  # Build a specific target in `//flutter/f
       build,
       extraGnArgs: extraGnArgs,
       targets: ninjaTargets,
+      enableRbe: useRbe,
     );
   }
 }

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

@@ -154,6 +154,11 @@ 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.
@@ -176,6 +181,11 @@ et query targets //flutter/fml/...  # List all targets under `//flutter/fml`
   Future<int> run() async {
     final String configName = argResults![configFlag] as String;
     final bool testOnly = argResults![testOnlyFlag] as bool;
+    final bool useRbe = argResults![rbeFlag] as bool;
+    if (useRbe && !environment.hasRbeConfigInTree()) {
+      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;
@@ -189,6 +199,7 @@ et query targets //flutter/fml/...  # List all targets under `//flutter/fml`
       build,
       argResults!.rest,
       defaultToAll: true,
+      enableRbe: useRbe,
     );
     if (selectedTargets == null) {
       // The user typed something wrong and targetsFromCommandLine has already

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

@@ -36,9 +36,8 @@ final class RunCommand extends CommandBase {
     );
     argParser.addFlag(
       rbeFlag,
-      defaultsTo: true,
-      help: 'RBE is enabled by default when available. Use --no-rbe to '
-          'disable it.',
+      defaultsTo: environment.hasRbeConfigInTree(),
+      help: 'RBE is enabled by default when available.',
     );
   }
 
@@ -152,19 +151,33 @@ See `flutter run --help` for a listing
     }
 
     final bool useRbe = argResults![rbeFlag] as bool;
+    if (useRbe && !environment.hasRbeConfigInTree()) {
+      environment.logger.error('RBE was requested but no RBE config was found');
+      return 1;
+    }
     final List<String> extraGnArgs = <String>[
       if (!useRbe) '--no-rbe',
     ];
 
     // First build the host.
-    int r = await runBuild(environment, hostBuild, extraGnArgs: extraGnArgs);
+    int r = await runBuild(
+      environment,
+      hostBuild,
+      extraGnArgs: extraGnArgs,
+      enableRbe: useRbe,
+    );
     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, extraGnArgs: extraGnArgs);
+      r = await runBuild(
+        environment,
+        build,
+        extraGnArgs: extraGnArgs,
+        enableRbe: useRbe,
+      );
       if (r != 0) {
         return r;
       }

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

@@ -30,6 +30,11 @@ final class TestCommand extends CommandBase {
       argParser,
       builds,
     );
+    argParser.addFlag(
+      rbeFlag,
+      defaultsTo: environment.hasRbeConfigInTree(),
+      help: 'RBE is enabled by default when available.',
+    );
   }
 
   /// List of compatible builds.
@@ -48,6 +53,11 @@ 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 bool useRbe = argResults![rbeFlag] as bool;
+    if (useRbe && !environment.hasRbeConfigInTree()) {
+      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;
@@ -61,6 +71,7 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
       build,
       argResults!.rest,
       defaultToAll: true,
+      enableRbe: useRbe,
     );
     if (selectedTargets == null) {
       // The user typed something wrong and targetsFromCommandLine has already
@@ -90,8 +101,12 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
         .toList();
     // TODO(johnmccutchan): runBuild manipulates buildTargets and adds some
     // targets listed in Build. Fix this.
-    final int buildExitCode =
-        await runBuild(environment, build, targets: buildTargets);
+    final int buildExitCode = await runBuild(
+      environment,
+      build,
+      targets: buildTargets,
+      enableRbe: useRbe,
+    );
     if (buildExitCode != 0) {
       return buildExitCode;
     }

tools/engine_tool/lib/src/environment.dart

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 import 'dart:ffi' as ffi show Abi;
+import 'dart:io' as io show Directory;
 
 import 'package:engine_repo_tools/engine_repo_tools.dart';
+import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:process_runner/process_runner.dart';
 
@@ -44,4 +46,21 @@ final class Environment {
 
   /// Facility for commands to run subprocesses.
   final ProcessRunner processRunner;
+
+  /// Whether it appears that the current environment supports remote builds.
+  ///
+  /// This is a heuristic based on the presence of certain directories in the
+  /// engine repo; it is not a guarantee that remote builds will work (due to
+  /// authentication, network, or other issues).
+  ///
+  /// **Note**: This calls does synchronous I/O.
+  bool hasRbeConfigInTree() {
+    final String rbeConfigPath = p.join(
+      engine.srcDir.path,
+      'flutter',
+      'build',
+      'rbe',
+    );
+    return io.Directory(rbeConfigPath).existsSync();
+  }
 }

tools/engine_tool/lib/src/gn_utils.dart

@@ -222,6 +222,7 @@ Future<List<BuildTarget>?> targetsFromCommandLine(
   Build build,
   List<String> commandLineTargets, {
   bool defaultToAll = false,
+  required bool enableRbe,
 }) async {
   // If there are no targets specified on the command line, then delegate to
   // the default targets specified in the Build object unless directed
@@ -240,7 +241,7 @@ Future<List<BuildTarget>?> targetsFromCommandLine(
     environment.logger.status(
       'Build output directory at ${buildDir.path} not found. Running GN.',
     );
-    final int gnResult = await runGn(environment, build);
+    final int gnResult = await runGn(environment, build, enableRbe: enableRbe);
     if (gnResult != 0 || !buildDir.existsSync()) {
       environment.logger.error(
         'The specified build did not produce the expected build '

tools/engine_tool/test/build_command_test.dart

@@ -9,6 +9,7 @@ import 'package:engine_build_configs/engine_build_configs.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:path/path.dart' as path;
 import 'package:platform/platform.dart';
@@ -176,6 +177,33 @@ void main() {
     }
   });
 
+  test('build command fails when rbe is enabled but not supported', () async {
+    final TestEnvironment testEnv = TestEnvironment.withTestEngine(
+      cannedProcesses: cannedProcesses,
+      // Intentionally omit withRbe: true.
+      // That means the //flutter/build/rbe directory will not be created.
+    );
+    try {
+      final ToolCommandRunner runner = ToolCommandRunner(
+        environment: testEnv.environment,
+        configs: configs,
+      );
+      final int result = await runner.run(<String>[
+        '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'),
+      );
+    } finally {
+      testEnv.cleanup();
+    }
+  });
+
   test('build command does not run rbe when disabled', () async {
     final TestEnvironment testEnv = TestEnvironment.withTestEngine(
       withRbe: true,

tools/engine_tool/test/gn_utils_test.dart

@@ -91,7 +91,10 @@ void main() {
         (Build build) => build.name == 'linux/host_debug',
       ).firstOrNull;
       final List<BuildTarget>? selectedTargets = await targetsFromCommandLine(
-        env, build!, <String>[],
+        env,
+        build!,
+        <String>[],
+        enableRbe: false,
       );
       expect(selectedTargets, isNotNull);
       expect(selectedTargets, isEmpty);
@@ -111,7 +114,11 @@ void main() {
         (Build build) => build.name == 'linux/host_debug',
       ).firstOrNull;
       final List<BuildTarget>? selectedTargets = await targetsFromCommandLine(
-        env, build!, <String>[], defaultToAll: true,
+        env,
+        build!,
+        <String>[],
+        defaultToAll: true,
+        enableRbe: false,
       );
       expect(selectedTargets, isNotNull);
       expect(selectedTargets, isNotEmpty);