From 03da551853607da87b142c7a3491d8e4bb4e1b97 Mon Sep 17 00:00:00 2001 From: Zach Anderson Date: Sat, 10 Feb 2024 18:55:50 -0800 Subject: [PATCH] Initializes RBE in the build config runner --- tools/gn | 32 +-- tools/pkg/engine_build_configs/bin/run.dart | 9 + .../lib/src/build_config_runner.dart | 205 +++++++++++++----- .../test/build_config_runner_test.dart | 192 ++++++++++++++-- .../pkg/process_fakes/lib/process_fakes.dart | 13 +- 5 files changed, 341 insertions(+), 110 deletions(-) diff --git a/tools/gn b/tools/gn index 18be4bd6d240f..05db32eeef80f 100755 --- a/tools/gn +++ b/tools/gn @@ -219,36 +219,6 @@ def setup_rbe(args): # care of starting and stopping the compiler proxy. running_on_luci = os.environ.get('LUCI_CONTEXT') is not None - # Bootstrap reproxy if not running in CI. - if not running_on_luci: - cipd_reclient_dir = os.path.join( - SRC_ROOT, - 'buildtools', - buildtools_dir(), - 'reclient', - ) - bootstrap_path = os.path.join(cipd_reclient_dir, 'bootstrap') - bootstrap_path = bootstrap_path + '.exe' if get_host_os() == 'win' else bootstrap_path - reproxy_path = os.path.join(cipd_reclient_dir, 'reproxy') - rbe_cfg_path = os.path.join( - SRC_ROOT, - 'flutter', - 'build', - 'rbe', - 'reclient-' + get_host_os() + '.cfg', - ) - bootstrap_cmd = [ - bootstrap_path, - '--re_proxy=' + reproxy_path, - '--automatic_auth=true', - '--cfg=' + rbe_cfg_path, - ] - try: - subprocess.call(bootstrap_cmd, cwd=SRC_ROOT) - except subprocess.CalledProcessError as exc: - print('Failed to boostrap reproxy: ', exc.returncode, exc.output) - return {} - if args.rbe_server_address: rbe_gn_args['rbe_server_address'] = args.rbe_server_address if args.rbe_exec_strategy: @@ -1188,7 +1158,7 @@ def parse_args(args): parser.add_argument( '--trace-gn', - default=False, + default=True, action='store_true', help='Write a GN trace log (gn_trace.json) in the Chromium tracing ' 'format in the build directory.' diff --git a/tools/pkg/engine_build_configs/bin/run.dart b/tools/pkg/engine_build_configs/bin/run.dart index a06fd1b6fcca4..91e43a15ea0da 100644 --- a/tools/pkg/engine_build_configs/bin/run.dart +++ b/tools/pkg/engine_build_configs/bin/run.dart @@ -108,11 +108,20 @@ The build names are the "name" fields of the maps in the list of "builds". return; } + // If RBE config files aren't in the tree, then disable RBE. + final String rbeConfigPath = p.join( + engine.srcDir.path, 'flutter', 'build', 'rbe', + ); + final List extraGnArgs = [ + if (!io.Directory(rbeConfigPath).existsSync()) '--no-rbe', + ]; + final GlobalBuildRunner buildRunner = GlobalBuildRunner( platform: const LocalPlatform(), processRunner: ProcessRunner(), engineSrcDir: engine.srcDir, build: targetBuild, + extraGnArgs: extraGnArgs, runGenerators: false, runTests: false, ); 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 3c64e1b4afff5..191993545c293 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 @@ -4,7 +4,7 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:io' as io show Directory, Process; +import 'dart:io' as io show Directory, Process, ProcessResult; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; @@ -41,7 +41,7 @@ final class RunnerProgress extends RunnerEvent { RunnerProgress( super.name, super.command, super.timestamp, this.what, this.completed, this.total, this.done, - ) : percent = (completed * 1000) / total; + ) : percent = (completed * 100) / total; /// What a command is currently working on, for example a build target or /// the name of a test. @@ -263,7 +263,6 @@ final class GlobalBuildRunner extends Runner { gnArgs.remove(arg.$1); } } - return gnArgs; }(); @@ -289,67 +288,157 @@ final class GlobalBuildRunner extends Runner { return result.ok; } - // TODO(zanderso): This should start and stop RBE when it is an --rbe build. - Future _runNinja(RunnerEventHandler eventHandler) async { - final String ninjaPath = p.join( - engineSrcDir.path, 'flutter', 'third_party', 'ninja', 'ninja', + late final String _hostCpu = (){ + if (platform.isWindows) { + return platform.environment['PROCESSOR_ARCHITECTURE'] ?? 'x64'; + } + final List unameCommand = ['uname', '-m']; + final io.ProcessResult unameResult = processRunner.processManager.runSync( + unameCommand, ); - final String outDir = p.join(engineSrcDir.path, 'out', build.ninja.config); - final List command = [ - ninjaPath, - '-C', outDir, - if (_isGomaOrRbe) ...['-j', '200'], - ...extraNinjaArgs, - ...build.ninja.targets, - ]; - eventHandler(RunnerStart('${build.name}: ninja', command, DateTime.now())); + return unameResult.exitCode == 0 ? (unameResult.stdout as String).trim() : 'x64'; + }(); - final ProcessRunnerResult processResult; + late final String _buildtoolsPath = (){ + final String platformDir = switch (platform.operatingSystem) { + Platform.linux => 'linux-$_hostCpu', + Platform.macOS => 'mac-$_hostCpu', + Platform.windows => 'windows-$_hostCpu', + _ => '', + }; + return p.join(engineSrcDir.path, 'buildtools', platformDir); + }(); + + Future _bootstrapRbe( + RunnerEventHandler eventHandler, { + bool shutdown = false, + }) async { + final String reclientPath = p.join(_buildtoolsPath, 'reclient'); + final String exe = platform.isWindows ? '.exe' : ''; + final String bootstrapPath = p.join(reclientPath, 'bootstrap$exe'); + final String reproxyPath = p.join(reclientPath, 'reproxy$exe'); + final String reclientConfigFile = switch (platform.operatingSystem) { + Platform.linux => 'reclient-linux.cfg', + Platform.macOS => 'reclient-mac.cfg', + Platform.windows => 'reclient-win.cfg', + _ => '', + }; + final String reclientConfigPath = p.join( + engineSrcDir.path, 'flutter', 'build', 'rbe', reclientConfigFile, + ); + final List bootstrapCommand = [ + bootstrapPath, + '--re_proxy=$reproxyPath', + '--automatic_auth=true', + if (shutdown) + '--shutdown' + else ...[ + '--cfg=$reclientConfigPath' + ], + ]; + if (!processRunner.processManager.canRun(bootstrapPath)) { + eventHandler(RunnerError( + build.name, [], DateTime.now(), '"$bootstrapPath" not found.', + )); + return false; + } + eventHandler(RunnerStart( + '${build.name}: RBE ${shutdown ? 'shutdown' : 'startup'}', + bootstrapCommand, + DateTime.now(), + )); + final ProcessRunnerResult bootstrapResult; if (dryRun) { - processResult = _dryRunResult; + bootstrapResult = _dryRunResult; } else { - final io.Process process = await processRunner.processManager.start( - command, - workingDirectory: engineSrcDir.path, + bootstrapResult = await processRunner.runProcess( + bootstrapCommand, failOk: true, ); - final List stderrOutput = []; - final Completer stdoutComplete = Completer(); - final Completer stderrComplete = Completer(); - - process.stdout - .transform(const Utf8Decoder()) - .transform(const LineSplitter()) - .listen( - (String line) { - _ninjaProgress(eventHandler, command, line); - }, - onDone: () async => stdoutComplete.complete(), - ); + } + eventHandler(RunnerResult( + '${build.name}: RBE ${shutdown ? 'shutdown' : 'startup'}', + bootstrapCommand, + DateTime.now(), + bootstrapResult, + )); + return bootstrapResult.exitCode == 0; + } - process.stderr.listen( - stderrOutput.addAll, - onDone: () async => stderrComplete.complete(), + Future _runNinja(RunnerEventHandler eventHandler) async { + if (_isRbe) { + if (!await _bootstrapRbe(eventHandler)) { + return false; + } + } + bool success = false; + try { + final String ninjaPath = p.join( + engineSrcDir.path, 'flutter', 'third_party', 'ninja', 'ninja', ); - - await Future.wait(>[ - stdoutComplete.future, stderrComplete.future, - ]); - final int exitCode = await process.exitCode; - - processResult = ProcessRunnerResult( - exitCode, - [], // stdout. - stderrOutput, // stderr. - stderrOutput, // combined, - pid: process.pid, // pid, + final String outDir = p.join( + engineSrcDir.path, 'out', build.ninja.config, ); - } + final List command = [ + ninjaPath, + '-C', outDir, + if (_isGomaOrRbe) ...['-j', '200'], + ...extraNinjaArgs, + ...build.ninja.targets, + ]; + eventHandler(RunnerStart( + '${build.name}: ninja', command, DateTime.now()), + ); + final ProcessRunnerResult processResult; + if (dryRun) { + processResult = _dryRunResult; + } else { + final io.Process process = await processRunner.processManager.start( + command, + workingDirectory: engineSrcDir.path, + ); + final List stderrOutput = []; + final Completer stdoutComplete = Completer(); + final Completer stderrComplete = Completer(); + + process.stdout + .transform(const Utf8Decoder()) + .transform(const LineSplitter()) + .listen( + (String line) { + _ninjaProgress(eventHandler, command, line); + }, + onDone: () async => stdoutComplete.complete(), + ); + + process.stderr.listen( + stderrOutput.addAll, + onDone: () async => stderrComplete.complete(), + ); - final RunnerResult result = RunnerResult( - '${build.name}: ninja', command, DateTime.now(), processResult, - ); - eventHandler(result); - return result.ok; + await Future.wait(>[ + stdoutComplete.future, stderrComplete.future, + ]); + final int exitCode = await process.exitCode; + + processResult = ProcessRunnerResult( + exitCode, + [], // stdout. + stderrOutput, // stderr. + stderrOutput, // combined, + pid: process.pid, // pid, + ); + } + eventHandler(RunnerResult( + '${build.name}: ninja', command, DateTime.now(), processResult, + )); + success = processResult.exitCode == 0; + } finally { + if (_isRbe) { + // Ignore failures to shutdown. + await _bootstrapRbe(eventHandler, shutdown: true); + } + } + return success; } // Parse lines of the form '[6232/6269] LINK ./accessibility_unittests'. @@ -389,10 +478,8 @@ final class GlobalBuildRunner extends Runner { )); } - late final bool _isGoma = build.gn.contains('--goma') || - extraGnArgs.contains('--goma'); - late final bool _isRbe = build.gn.contains('--rbe') || - extraGnArgs.contains('--rbe'); + late final bool _isGoma = _mergedGnArgs.contains('--goma'); + late final bool _isRbe = _mergedGnArgs.contains('--rbe'); late final bool _isGomaOrRbe = _isGoma || _isRbe; Future _runGenerators(RunnerEventHandler eventHandler) async { diff --git a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart index 04f400d05c399..fa49ba23a9f51 100644 --- a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart +++ b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart @@ -37,7 +37,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, task: generator, @@ -63,7 +63,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, test: test, @@ -91,7 +91,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -152,7 +152,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -191,7 +191,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -218,8 +218,9 @@ void main() { final GlobalBuildRunner buildRunner = GlobalBuildRunner( platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( - // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager( + unameResult: io.ProcessResult(1, 0, 'arm64', ''), + ), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -234,11 +235,24 @@ void main() { expect(runResult, isTrue); - // Check that the events for the Ninja command are correct. + // Check that the events for the RBE bootstrap command are correct. expect(events[2] is RunnerStart, isTrue); - expect(events[2].name, equals('$buildName: ninja')); - expect(events[2].command.contains('-j'), isTrue); - expect(events[2].command.contains('200'), isTrue); + expect(events[2].name, equals('$buildName: RBE startup')); + expect(events[3] is RunnerResult, isTrue); + expect(events[3].name, equals('$buildName: RBE startup')); + + // Check that the events for the Ninja command are correct. + expect(events[4] is RunnerStart, isTrue); + expect(events[4].name, equals('$buildName: ninja')); + expect(events[4].command.contains('-j'), isTrue); + expect(events[4].command.contains('200'), isTrue); + expect(events[5] is RunnerResult, isTrue); + expect(events[5].name, equals('$buildName: ninja')); + + expect(events[6] is RunnerStart, isTrue); + expect(events[6].name, equals('$buildName: RBE shutdown')); + expect(events[7] is RunnerResult, isTrue); + expect(events[7].name, equals('$buildName: RBE shutdown')); }); test('GlobalBuildRunner skips GN when runGn is false', () async { @@ -247,7 +261,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -281,7 +295,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -322,7 +336,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -365,7 +379,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -395,8 +409,9 @@ void main() { final GlobalBuildRunner buildRunner = GlobalBuildRunner( platform: FakePlatform(operatingSystem: Platform.linux), processRunner: ProcessRunner( - // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager( + unameResult: io.ProcessResult(1, 0, 'arm64', ''), + ), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -431,7 +446,7 @@ void main() { platform: FakePlatform(operatingSystem: Platform.macOS), processRunner: ProcessRunner( // dryRun should not try to spawn any processes. - processManager: FakeProcessManager(), + processManager: _fakeProcessManager(), ), engineSrcDir: engine.srcDir, build: targetBuild, @@ -444,4 +459,145 @@ void main() { expect(runResult, isFalse); expect(events[0] is RunnerError, isTrue); }); + + test('GlobalBuildRunner fails when gn fails', () async { + final GlobalBuild targetBuild = buildConfig.builds[0]; + final GlobalBuildRunner buildRunner = GlobalBuildRunner( + platform: FakePlatform(operatingSystem: Platform.linux), + processRunner: ProcessRunner( + processManager: _fakeProcessManager( + gnResult: io.ProcessResult(1, 1, '', ''), + ), + ), + engineSrcDir: engine.srcDir, + build: targetBuild, + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + final String buildName = targetBuild.name; + + expect(runResult, isFalse); + + expect(events[0] is RunnerStart, isTrue); + expect(events[0].name, equals('$buildName: GN')); + expect(events[1] is RunnerResult, isTrue); + expect((events[1] as RunnerResult).ok, isFalse); + }); + + test('GlobalBuildRunner fails when ninja fails', () async { + final GlobalBuild targetBuild = buildConfig.builds[0]; + final GlobalBuildRunner buildRunner = GlobalBuildRunner( + platform: FakePlatform(operatingSystem: Platform.linux), + processRunner: ProcessRunner( + processManager: _fakeProcessManager( + ninjaResult: io.ProcessResult(1, 1, '', ''), + ), + ), + engineSrcDir: engine.srcDir, + build: targetBuild, + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + final String buildName = targetBuild.name; + + expect(runResult, isFalse); + + expect(events[2] is RunnerStart, isTrue); + expect(events[2].name, equals('$buildName: ninja')); + expect(events[3] is RunnerResult, isTrue); + expect((events[3] as RunnerResult).ok, isFalse); + }); + + test('GlobalBuildRunner fails an RBE build when bootstrap fails', () async { + final GlobalBuild targetBuild = buildConfig.builds[0]; + final GlobalBuildRunner buildRunner = GlobalBuildRunner( + platform: FakePlatform(operatingSystem: Platform.linux), + processRunner: ProcessRunner( + processManager: _fakeProcessManager( + unameResult: io.ProcessResult(1, 0, 'arm64', ''), + bootstrapResult: io.ProcessResult(1, 1, '', ''), + ), + ), + engineSrcDir: engine.srcDir, + build: targetBuild, + extraGnArgs: ['--rbe'], + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + final String buildName = targetBuild.name; + + expect(runResult, isFalse); + + expect(events[2] is RunnerStart, isTrue); + expect(events[2].name, equals('$buildName: RBE startup')); + expect(events[3] is RunnerResult, isTrue); + expect(events[3].name, equals('$buildName: RBE startup')); + expect((events[3] as RunnerResult).ok, isFalse); + }); + + test('GlobalBuildRunner fails an RBE build when bootstrap does not exist', () async { + final GlobalBuild targetBuild = buildConfig.builds[0]; + final GlobalBuildRunner buildRunner = GlobalBuildRunner( + platform: FakePlatform(operatingSystem: Platform.linux), + processRunner: ProcessRunner( + processManager: _fakeProcessManager( + unameResult: io.ProcessResult(1, 0, 'arm64', ''), + canRun: (Object? exe, {String? workingDirectory}) { + if (exe is String? && exe != null && exe.endsWith('bootstrap')) { + return false; + } + return true; + }, + ), + ), + engineSrcDir: engine.srcDir, + build: targetBuild, + extraGnArgs: ['--rbe'], + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + expect(runResult, isFalse); + + expect(events[2] is RunnerError, isTrue); + }); +} + +FakeProcessManager _fakeProcessManager({ + io.ProcessResult? unameResult, + io.ProcessResult? bootstrapResult, + io.ProcessResult? gnResult, + io.ProcessResult? ninjaResult, + bool Function(Object?, {String? workingDirectory})? canRun, + bool failUnknown = true, +}) { + final io.ProcessResult success = io.ProcessResult(1, 0, '', ''); + FakeProcess fakeProcess(io.ProcessResult? result) => FakeProcess( + exitCode: result?.exitCode ?? 0, + stdout: result?.stdout as String? ?? '', + stderr: result?.stderr as String? ?? '', + ); + return FakeProcessManager( + canRun: canRun ?? (Object? exe, {String? workingDirectory}) => true, + onRun: (List cmd) => switch (cmd) { + ['uname', ...] => unameResult ?? success, + _ => failUnknown ? io.ProcessResult(1, 1, '', '') : success, + }, + onStart: (List cmd) => switch (cmd) { + [final String exe, ...] when exe.endsWith('gn') => + fakeProcess(gnResult), + [final String exe, ...] when exe.endsWith('bootstrap') => + fakeProcess(bootstrapResult), + [final String exe, ...] when exe.endsWith('ninja') => + fakeProcess(ninjaResult), + _ => failUnknown ? FakeProcess(exitCode: 1) : FakeProcess(), + }, + ); } diff --git a/tools/pkg/process_fakes/lib/process_fakes.dart b/tools/pkg/process_fakes/lib/process_fakes.dart index 356f1d391f33d..bab281110be55 100644 --- a/tools/pkg/process_fakes/lib/process_fakes.dart +++ b/tools/pkg/process_fakes/lib/process_fakes.dart @@ -15,7 +15,8 @@ final class FakeProcessManager implements ProcessManager { FakeProcessManager({ io.ProcessResult Function(List command) onRun = unhandledRun, io.Process Function(List command) onStart = unhandledStart, - }) : _onRun = onRun, _onStart = onStart; + bool Function(Object?, {String? workingDirectory}) canRun = unhandledCanRun, + }) : _onRun = onRun, _onStart = onStart, _canRun = canRun; /// A default implementation of [onRun] that throws an [UnsupportedError]. static io.ProcessResult unhandledRun(List command) { @@ -27,11 +28,19 @@ final class FakeProcessManager implements ProcessManager { throw UnsupportedError('Unhandled start: ${command.join(' ')}'); } + /// A default implementation of [canRun] that returns `true`. + static bool unhandledCanRun(Object? executable, {String? workingDirectory}) { + return true; + } + final io.ProcessResult Function(List command) _onRun; final io.Process Function(List command) _onStart; + final bool Function(Object?, {String? workingDirectory}) _canRun; @override - bool canRun(Object? executable, {String? workingDirectory}) => true; + bool canRun(Object? executable, {String? workingDirectory}) { + return _canRun(executable, workingDirectory: workingDirectory); + } @override bool killPid(int pid, [io.ProcessSignal signal = io.ProcessSignal.sigterm]) => true;