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

Commit fa4d97e

Browse files
Expand understanding of build targets in et (#51868)
- s/TestTarget/BuildTarget. - Use a more informative way of querying for build targets from gn - Port existing code to use new interfaces - Replace 'query tests' with 'query targets' and a --testonly flag - Extend 'et build' with support for build target selectors. - Extend 'et query targets' with support for build target selectors.
1 parent 0b28ef2 commit fa4d97e

File tree

12 files changed

+417
-249
lines changed

12 files changed

+417
-249
lines changed

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:io';
6+
57
import 'package:engine_build_configs/engine_build_configs.dart';
8+
import 'package:path/path.dart' as p;
69

710
import '../build_utils.dart';
11+
import '../gn_utils.dart';
812
import 'command.dart';
913
import 'flags.dart';
1014

@@ -18,7 +22,9 @@ final class BuildCommand extends CommandBase {
1822
builds = runnableBuilds(environment, configs);
1923
debugCheckBuilds(builds);
2024
addConfigOption(
21-
environment, argParser, runnableBuilds(environment, configs),
25+
environment,
26+
argParser,
27+
runnableBuilds(environment, configs),
2228
);
2329
argParser.addFlag(
2430
rbeFlag,
@@ -35,7 +41,9 @@ final class BuildCommand extends CommandBase {
3541
String get name => 'build';
3642

3743
@override
38-
String get description => 'Builds the engine';
44+
String get description => 'Builds the engine'
45+
'et build //flutter/fml/... # Build all targets in `//flutter/fml/`'
46+
'et build //flutter/fml:fml_benchmarks # Build a specific target in `//flutter/fml/`';
3947

4048
@override
4149
Future<int> run() async {
@@ -53,6 +61,22 @@ final class BuildCommand extends CommandBase {
5361
if (!useRbe) '--no-rbe',
5462
];
5563

56-
return runBuild(environment, build, extraGnArgs: extraGnArgs);
64+
final Map<String, BuildTarget> allTargets = await findTargets(environment,
65+
Directory(p.join(environment.engine.outDir.path, build.ninja.config)));
66+
final Set<BuildTarget> selectedTargets =
67+
selectTargets(argResults!.rest, allTargets);
68+
if (selectedTargets.isEmpty) {
69+
environment.logger.error(
70+
'No build targets matched ${argResults!.rest}\nRun `et query targets` to see list of targets.');
71+
return 1;
72+
}
73+
74+
// Chop off the '//' prefix.
75+
final List<String> buildTargets = selectedTargets
76+
.map<String>(
77+
(BuildTarget target) => target.label.substring('//'.length))
78+
.toList();
79+
return runBuild(environment, build,
80+
extraGnArgs: extraGnArgs, targets: buildTargets);
5781
}
5882
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ const String quietFlag = 'quiet';
1919
const String rbeFlag = 'rbe';
2020
const String runTestsFlag = 'run-tests';
2121
const String verboseFlag = 'verbose';
22+
const String testOnlyFlag = 'testonly';

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

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ final class QueryCommand extends CommandBase {
4848
environment: environment,
4949
configs: configs,
5050
));
51-
addSubcommand(QueryTestsCommand(
51+
addSubcommand(QueryTargetsCommand(
5252
environment: environment,
5353
configs: configs,
5454
));
@@ -131,17 +131,25 @@ final class QueryBuildersCommand extends CommandBase {
131131
}
132132
}
133133

134-
/// The query tests command.
135-
final class QueryTestsCommand extends CommandBase {
136-
/// Constructs the 'query tests' command.
137-
QueryTestsCommand({
134+
/// The query targets command.
135+
final class QueryTargetsCommand extends CommandBase {
136+
/// Constructs the 'query targets' command.
137+
QueryTargetsCommand({
138138
required super.environment,
139139
required this.configs,
140140
}) {
141141
builds = runnableBuilds(environment, configs);
142142
debugCheckBuilds(builds);
143143
addConfigOption(
144-
environment, argParser, runnableBuilds(environment, configs),
144+
environment,
145+
argParser,
146+
runnableBuilds(environment, configs),
147+
);
148+
argParser.addFlag(
149+
testOnlyFlag,
150+
abbr: 't',
151+
help: 'Filter build targets to only include tests',
152+
negatable: false,
145153
);
146154
}
147155

@@ -152,24 +160,38 @@ final class QueryTestsCommand extends CommandBase {
152160
late final List<Build> builds;
153161

154162
@override
155-
String get name => 'tests';
163+
String get name => 'targets';
156164

157165
@override
158-
String get description => 'Provides information about test targets';
166+
String get description => 'Provides information about build targets'
167+
'et query targets --testonly # List only test targets'
168+
'et query targets //flutter/fml/... # List all targets under `//flutter/fml`';
159169

160170
@override
161171
Future<int> run() async {
162172
final String configName = argResults![configFlag] as String;
173+
final bool testOnly = argResults![testOnlyFlag] as bool;
163174
final String demangledName = demangleConfigName(environment, configName);
164175
final Build? build =
165176
builds.where((Build build) => build.name == demangledName).firstOrNull;
166177
if (build == null) {
167178
environment.logger.error('Could not find config $configName');
168179
return 1;
169180
}
170-
final Map<String, TestTarget> targets = await findTestTargets(environment,
181+
final Map<String, BuildTarget> allTargets = await findTargets(environment,
171182
Directory(p.join(environment.engine.outDir.path, build.ninja.config)));
172-
for (final TestTarget target in targets.values) {
183+
final Set<BuildTarget> selectedTargets =
184+
selectTargets(argResults!.rest, allTargets);
185+
if (selectedTargets.isEmpty) {
186+
environment.logger.error(
187+
'No build targets matched ${argResults!.rest}\nRun `et query targets` to see list of targets.');
188+
return 1;
189+
}
190+
for (final BuildTarget target in selectedTargets) {
191+
if (testOnly &&
192+
(!target.testOnly || target.type != BuildTargetType.executable)) {
193+
continue;
194+
}
173195
environment.logger.status(target.label);
174196
}
175197
return 0;

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ final class TestCommand extends CommandBase {
2424
builds = runnableBuilds(environment, configs);
2525
debugCheckBuilds(builds);
2626
addConfigOption(
27-
environment, argParser, runnableBuilds(environment, configs),
27+
environment,
28+
argParser,
29+
runnableBuilds(environment, configs),
2830
);
2931
}
3032

@@ -50,19 +52,29 @@ final class TestCommand extends CommandBase {
5052
return 1;
5153
}
5254

53-
final Map<String, TestTarget> allTargets = await findTestTargets(
54-
environment,
55+
final Map<String, BuildTarget> allTargets = await findTargets(environment,
5556
Directory(p.join(environment.engine.outDir.path, build.ninja.config)));
56-
final Set<TestTarget> selectedTargets =
57+
final Set<BuildTarget> selectedTargets =
5758
selectTargets(argResults!.rest, allTargets);
5859
if (selectedTargets.isEmpty) {
5960
environment.logger.error(
60-
'No build targets matched ${argResults!.rest}\nRun `et query tests` to see list of targets.');
61+
'No build targets matched ${argResults!.rest}\nRun `et query targets --testonly` to see list of targets.');
6162
return 1;
6263
}
64+
for (final BuildTarget target in selectedTargets) {
65+
if (!target.testOnly || target.type != BuildTargetType.executable) {
66+
// Remove any targets that aren't testOnly and aren't executable.
67+
selectedTargets.remove(target);
68+
}
69+
if (target.executable == null) {
70+
environment.logger.fatal(
71+
'$target is an executable but is missing the executable path');
72+
}
73+
}
6374
// Chop off the '//' prefix.
6475
final List<String> buildTargets = selectedTargets
65-
.map<String>((TestTarget target) => target.label.substring('//'.length))
76+
.map<String>(
77+
(BuildTarget target) => target.label.substring('//'.length))
6678
.toList();
6779
// TODO(johnmccutchan): runBuild manipulates buildTargets and adds some
6880
// targets listed in Build. Fix this.
@@ -74,8 +86,8 @@ final class TestCommand extends CommandBase {
7486
final WorkerPool workerPool =
7587
WorkerPool(environment, ProcessTaskProgressReporter(environment));
7688
final Set<ProcessTask> tasks = <ProcessTask>{};
77-
for (final TestTarget target in selectedTargets) {
78-
final List<String> commandLine = <String>[target.executable.path];
89+
for (final BuildTarget target in selectedTargets) {
90+
final List<String> commandLine = <String>[target.executable!.path];
7991
tasks.add(ProcessTask(
8092
target.label, environment, environment.engine.srcDir, commandLine));
8193
}

0 commit comments

Comments
 (0)