Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

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

Merged
merged 3 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 4 additions & 19 deletions tools/engine_tool/lib/src/build_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
];

Expand Down Expand Up @@ -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,
];

Expand Down
11 changes: 8 additions & 3 deletions tools/engine_tool/lib/src/commands/build_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 =
Expand All @@ -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
Expand All @@ -93,6 +97,7 @@ et build //flutter/fml:fml_benchmarks # Build a specific target in `//flutter/f
build,
extraGnArgs: extraGnArgs,
targets: ninjaTargets,
enableRbe: useRbe,
);
}
}
11 changes: 11 additions & 0 deletions tools/engine_tool/lib/src/commands/query_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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
Expand Down
23 changes: 18 additions & 5 deletions tools/engine_tool/lib/src/commands/run_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
);
}

Expand Down Expand Up @@ -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;
}
Expand Down
19 changes: 17 additions & 2 deletions tools/engine_tool/lib/src/commands/test_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
19 changes: 19 additions & 0 deletions tools/engine_tool/lib/src/environment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
}
}
3 changes: 2 additions & 1 deletion tools/engine_tool/lib/src/gn_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 '
Expand Down
28 changes: 28 additions & 0 deletions tools/engine_tool/test/build_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 9 additions & 2 deletions tools/engine_tool/test/gn_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down