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

Commit 219ec84

Browse files
authored
[et] Lookup output filesystem path, not label (#52248)
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: * `_runGnDesc`: returns the JSON output of `gn desc buildDir target` * `_runGnOutputs`: returns the output files listed by `gn outputs buildDir target` Noticed while working on: flutter/flutter#147071 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent c49b7b4 commit 219ec84

File tree

4 files changed

+67
-35
lines changed

4 files changed

+67
-35
lines changed

tools/engine_tool/lib/src/gn_utils.dart

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -77,30 +77,9 @@ final class BuildTarget {
7777
Future<Map<String, BuildTarget>> findTargets(
7878
Environment environment, Directory buildDir) async {
7979
final Map<String, BuildTarget> r = <String, BuildTarget>{};
80-
final List<String> getBuildInfoCommandLine = <String>[
81-
gnBinPath(environment),
82-
'desc',
83-
buildDir.path,
84-
'*',
85-
'--format=json',
86-
];
87-
88-
final ProcessRunnerResult result = await environment.processRunner.runProcess(
89-
getBuildInfoCommandLine,
90-
workingDirectory: environment.engine.srcDir,
91-
failOk: true);
92-
93-
// Handle any process failures.
94-
fatalIfFailed(environment, getBuildInfoCommandLine, result);
95-
96-
late final Map<String, Object?> jsonResult;
97-
try {
98-
jsonResult = jsonDecode(result.stdout) as Map<String, Object?>;
99-
} catch (e) {
100-
environment.logger.fatal(
101-
'gn desc output could not be parsed:\nE=$e\nIN=${result.stdout}\n');
102-
}
10380

81+
final Map<String, Object?> jsonResult =
82+
await _runGnDesc(buildDir.path, '*', environment);
10483
for (final MapEntry<String, Object?> targetEntry in jsonResult.entries) {
10584
final String label = targetEntry.key;
10685
if (targetEntry.value == null) {
@@ -120,7 +99,7 @@ Future<Map<String, BuildTarget>> findTargets(
12099
}
121100
final bool testOnly = getBool(properties, 'testonly');
122101
final List<String> outputs =
123-
getListOfString(properties, 'outputs') ?? <String>[];
102+
await _runGnOutputs(buildDir.path, label, environment);
124103
File? executable;
125104
if (type == BuildTargetType.executable) {
126105
if (outputs.isEmpty) {
@@ -135,6 +114,55 @@ Future<Map<String, BuildTarget>> findTargets(
135114
return r;
136115
}
137116

117+
/// Returns the JSON output of running `gn desc buildDir label`.
118+
Future<Map<String, Object?>> _runGnDesc(
119+
String buildDir, String label, Environment environment) async {
120+
final List<String> commandline = <String>[
121+
gnBinPath(environment),
122+
'desc',
123+
buildDir,
124+
label,
125+
'--format=json',
126+
];
127+
128+
final ProcessRunnerResult result = await environment.processRunner.runProcess(
129+
commandline,
130+
workingDirectory: environment.engine.srcDir,
131+
failOk: true);
132+
133+
// Handle any process failures.
134+
fatalIfFailed(environment, commandline, result);
135+
136+
late final Map<String, Object?> jsonResult;
137+
try {
138+
jsonResult = jsonDecode(result.stdout) as Map<String, Object?>;
139+
} catch (e) {
140+
environment.logger.fatal(
141+
'gn desc output could not be parsed:\nE=$e\nIN=${result.stdout}\n');
142+
}
143+
return jsonResult;
144+
}
145+
146+
/// Returns the output paths returned by `gn outputs buildDir label`.
147+
Future<List<String>> _runGnOutputs(
148+
String buildDir, String label, Environment environment) async {
149+
final List<String> commandline = <String>[
150+
gnBinPath(environment),
151+
'outputs',
152+
buildDir,
153+
label,
154+
];
155+
final ProcessRunnerResult result = await environment.processRunner.runProcess(
156+
commandline,
157+
workingDirectory: environment.engine.srcDir,
158+
failOk: true);
159+
160+
// Handle any process failures.
161+
fatalIfFailed(environment, commandline, result);
162+
163+
return result.stdout.split('\n');
164+
}
165+
138166
/// Process selectors and filter allTargets for matches.
139167
///
140168
/// We support:

tools/engine_tool/test/build_command_test.dart

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -360,11 +360,11 @@ void main() {
360360
'//flutter/fml:fml_arc_unittests',
361361
]);
362362
expect(result, equals(0));
363-
expect(testEnv.processHistory.length, greaterThanOrEqualTo(2));
364-
expect(testEnv.processHistory[3].command[0], contains('ninja'));
365-
expect(testEnv.processHistory[3].command[2], endsWith('/host_debug'));
363+
expect(testEnv.processHistory.length, greaterThan(6));
364+
expect(testEnv.processHistory[6].command[0], contains('ninja'));
365+
expect(testEnv.processHistory[6].command[2], endsWith('/host_debug'));
366366
expect(
367-
testEnv.processHistory[3].command[5],
367+
testEnv.processHistory[6].command[5],
368368
equals('flutter/fml:fml_arc_unittests'),
369369
);
370370
} finally {
@@ -389,19 +389,19 @@ void main() {
389389
'//flutter/...',
390390
]);
391391
expect(result, equals(0));
392-
expect(testEnv.processHistory.length, greaterThanOrEqualTo(2));
393-
expect(testEnv.processHistory[3].command[0], contains('ninja'));
394-
expect(testEnv.processHistory[3].command[2], endsWith('/host_debug'));
392+
expect(testEnv.processHistory.length, greaterThan(6));
393+
expect(testEnv.processHistory[6].command[0], contains('ninja'));
394+
expect(testEnv.processHistory[6].command[2], endsWith('/host_debug'));
395395
expect(
396-
testEnv.processHistory[3].command[5],
396+
testEnv.processHistory[6].command[5],
397397
equals('flutter/display_list:display_list_unittests'),
398398
);
399399
expect(
400-
testEnv.processHistory[3].command[6],
400+
testEnv.processHistory[6].command[6],
401401
equals('flutter/flow:flow_unittests'),
402402
);
403403
expect(
404-
testEnv.processHistory[3].command[7],
404+
testEnv.processHistory[6].command[7],
405405
equals('flutter/fml:fml_arc_unittests'),
406406
);
407407
} finally {

tools/engine_tool/test/gn_utils_test.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ void main() {
3939
final List<CannedProcess> cannedProcesses = <CannedProcess>[
4040
CannedProcess((List<String> command) => command.contains('desc'),
4141
stdout: fixtures.gnDescOutput()),
42+
CannedProcess((List<String> command) => command.contains('outputs'),
43+
stdout: 'display_list_unittests'),
4244
];
4345

4446
test('find test targets', () async {

tools/engine_tool/test/test_command_test.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ void main() {
4242
final List<CannedProcess> cannedProcesses = <CannedProcess>[
4343
CannedProcess((List<String> command) => command.contains('desc'),
4444
stdout: fixtures.gnDescOutput()),
45+
CannedProcess((List<String> command) => command.contains('outputs'),
46+
stdout: 'display_list_unittests'),
4547
];
4648

4749
test('test command executes test', () async {
@@ -83,7 +85,7 @@ void main() {
8385
'//third_party/protobuf:protoc',
8486
]);
8587
expect(result, equals(1));
86-
expect(testEnvironment.processHistory.length, lessThan(3));
88+
expect(testEnvironment.processHistory.length, lessThan(6));
8789
expect(testEnvironment.processHistory.where((ExecutedProcess process) {
8890
return process.command[0].contains('protoc');
8991
}), isEmpty);

0 commit comments

Comments
 (0)