From 51f761270899e576f5678a378bc4450ca800945e Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Sat, 20 Apr 2024 09:58:32 -0700 Subject: [PATCH] [et] Lookup output filesystem path, not label Sets BuildTarget.executable to the `root_out_dir`-relative path of the executable (e.g. `displaylist_unittests`) instead of its label (e.g. `//out/host_debug/displaylist_unittests`). This is required since, in the lines following the output lookup, we assume it to be a path relative to the build output directory. Also breaks out functions for: * `_gnDesc`: returns the JSON output of `gn desc buildDir target` * `_gnOutputs`: returns the output files listed by `gn outputs buildDir target` Noticed while working on: https://github.com/flutter/flutter/issues/147071 --- tools/engine_tool/lib/src/gn_utils.dart | 76 +++++++++++++------ .../engine_tool/test/build_command_test.dart | 20 ++--- tools/engine_tool/test/gn_utils_test.dart | 2 + tools/engine_tool/test/test_command_test.dart | 4 +- 4 files changed, 67 insertions(+), 35 deletions(-) diff --git a/tools/engine_tool/lib/src/gn_utils.dart b/tools/engine_tool/lib/src/gn_utils.dart index dc6887f74afed..8d67843ff5380 100644 --- a/tools/engine_tool/lib/src/gn_utils.dart +++ b/tools/engine_tool/lib/src/gn_utils.dart @@ -77,30 +77,9 @@ final class BuildTarget { Future> findTargets( Environment environment, Directory buildDir) async { final Map r = {}; - final List getBuildInfoCommandLine = [ - gnBinPath(environment), - 'desc', - buildDir.path, - '*', - '--format=json', - ]; - - final ProcessRunnerResult result = await environment.processRunner.runProcess( - getBuildInfoCommandLine, - workingDirectory: environment.engine.srcDir, - failOk: true); - - // Handle any process failures. - fatalIfFailed(environment, getBuildInfoCommandLine, result); - - late final Map jsonResult; - try { - jsonResult = jsonDecode(result.stdout) as Map; - } catch (e) { - environment.logger.fatal( - 'gn desc output could not be parsed:\nE=$e\nIN=${result.stdout}\n'); - } + final Map jsonResult = + await _runGnDesc(buildDir.path, '*', environment); for (final MapEntry targetEntry in jsonResult.entries) { final String label = targetEntry.key; if (targetEntry.value == null) { @@ -120,7 +99,7 @@ Future> findTargets( } final bool testOnly = getBool(properties, 'testonly'); final List outputs = - getListOfString(properties, 'outputs') ?? []; + await _runGnOutputs(buildDir.path, label, environment); File? executable; if (type == BuildTargetType.executable) { if (outputs.isEmpty) { @@ -135,6 +114,55 @@ Future> findTargets( return r; } +/// Returns the JSON output of running `gn desc buildDir label`. +Future> _runGnDesc( + String buildDir, String label, Environment environment) async { + final List commandline = [ + gnBinPath(environment), + 'desc', + buildDir, + label, + '--format=json', + ]; + + final ProcessRunnerResult result = await environment.processRunner.runProcess( + commandline, + workingDirectory: environment.engine.srcDir, + failOk: true); + + // Handle any process failures. + fatalIfFailed(environment, commandline, result); + + late final Map jsonResult; + try { + jsonResult = jsonDecode(result.stdout) as Map; + } catch (e) { + environment.logger.fatal( + 'gn desc output could not be parsed:\nE=$e\nIN=${result.stdout}\n'); + } + return jsonResult; +} + +/// Returns the output paths returned by `gn outputs buildDir label`. +Future> _runGnOutputs( + String buildDir, String label, Environment environment) async { + final List commandline = [ + gnBinPath(environment), + 'outputs', + buildDir, + label, + ]; + final ProcessRunnerResult result = await environment.processRunner.runProcess( + commandline, + workingDirectory: environment.engine.srcDir, + failOk: true); + + // Handle any process failures. + fatalIfFailed(environment, commandline, result); + + return result.stdout.split('\n'); +} + /// Process selectors and filter allTargets for matches. /// /// We support: diff --git a/tools/engine_tool/test/build_command_test.dart b/tools/engine_tool/test/build_command_test.dart index fe1c65f8e1cf5..055767abd3972 100644 --- a/tools/engine_tool/test/build_command_test.dart +++ b/tools/engine_tool/test/build_command_test.dart @@ -360,11 +360,11 @@ void main() { '//flutter/fml:fml_arc_unittests', ]); expect(result, equals(0)); - expect(testEnv.processHistory.length, greaterThanOrEqualTo(2)); - expect(testEnv.processHistory[3].command[0], contains('ninja')); - expect(testEnv.processHistory[3].command[2], endsWith('/host_debug')); + expect(testEnv.processHistory.length, greaterThan(6)); + expect(testEnv.processHistory[6].command[0], contains('ninja')); + expect(testEnv.processHistory[6].command[2], endsWith('/host_debug')); expect( - testEnv.processHistory[3].command[5], + testEnv.processHistory[6].command[5], equals('flutter/fml:fml_arc_unittests'), ); } finally { @@ -389,19 +389,19 @@ void main() { '//flutter/...', ]); expect(result, equals(0)); - expect(testEnv.processHistory.length, greaterThanOrEqualTo(2)); - expect(testEnv.processHistory[3].command[0], contains('ninja')); - expect(testEnv.processHistory[3].command[2], endsWith('/host_debug')); + expect(testEnv.processHistory.length, greaterThan(6)); + expect(testEnv.processHistory[6].command[0], contains('ninja')); + expect(testEnv.processHistory[6].command[2], endsWith('/host_debug')); expect( - testEnv.processHistory[3].command[5], + testEnv.processHistory[6].command[5], equals('flutter/display_list:display_list_unittests'), ); expect( - testEnv.processHistory[3].command[6], + testEnv.processHistory[6].command[6], equals('flutter/flow:flow_unittests'), ); expect( - testEnv.processHistory[3].command[7], + testEnv.processHistory[6].command[7], equals('flutter/fml:fml_arc_unittests'), ); } finally { diff --git a/tools/engine_tool/test/gn_utils_test.dart b/tools/engine_tool/test/gn_utils_test.dart index 462c90703bb17..a24239abb225e 100644 --- a/tools/engine_tool/test/gn_utils_test.dart +++ b/tools/engine_tool/test/gn_utils_test.dart @@ -39,6 +39,8 @@ void main() { final List cannedProcesses = [ CannedProcess((List command) => command.contains('desc'), stdout: fixtures.gnDescOutput()), + CannedProcess((List command) => command.contains('outputs'), + stdout: 'display_list_unittests'), ]; test('find test targets', () async { diff --git a/tools/engine_tool/test/test_command_test.dart b/tools/engine_tool/test/test_command_test.dart index ecc86d3f8b28d..11885a480b8ec 100644 --- a/tools/engine_tool/test/test_command_test.dart +++ b/tools/engine_tool/test/test_command_test.dart @@ -42,6 +42,8 @@ void main() { final List cannedProcesses = [ CannedProcess((List command) => command.contains('desc'), stdout: fixtures.gnDescOutput()), + CannedProcess((List command) => command.contains('outputs'), + stdout: 'display_list_unittests'), ]; test('test command executes test', () async { @@ -83,7 +85,7 @@ void main() { '//third_party/protobuf:protoc', ]); expect(result, equals(1)); - expect(testEnvironment.processHistory.length, lessThan(3)); + expect(testEnvironment.processHistory.length, lessThan(6)); expect(testEnvironment.processHistory.where((ExecutedProcess process) { return process.command[0].contains('protoc'); }), isEmpty);