From ca3c65433c5ab1141c30be204bfcb3eee659ed3b Mon Sep 17 00:00:00 2001
From: Matan Lurey <matan@lurey.org>
Date: Wed, 9 Oct 2024 15:30:10 -0700
Subject: [PATCH 1/4] Recognize groups, and warn when we silently discard
 targets.

---
 .../lib/src/commands/build_command.dart       |  9 +++
 tools/engine_tool/lib/src/gn.dart             | 42 +++++++++++
 .../engine_tool/test/build_command_test.dart  | 69 +++++++++++++++++++
 .../test/external_tools/gn_test.dart          | 34 +++++++++
 4 files changed, 154 insertions(+)

diff --git a/tools/engine_tool/lib/src/commands/build_command.dart b/tools/engine_tool/lib/src/commands/build_command.dart
index 70fa5b8feb4af..5c5aefa660307 100644
--- a/tools/engine_tool/lib/src/commands/build_command.dart
+++ b/tools/engine_tool/lib/src/commands/build_command.dart
@@ -72,6 +72,15 @@ et build //flutter/fml:fml_benchmarks  # Build a specific target in `//flutter/f
       allTargets.addAll(targets.map((target) => target.label));
     }
 
+    // Warn that we've discarded some targets.
+    // Other warnings should have been emitted above, so if this ends up being
+    // unneccesarily noisy, we can remove it or limit it to verbose mode.
+    if (allTargets.length < commandLineTargets.length) {
+      environment.logger.warning(
+        'One or more targets specified did not match any build targets.',
+      );
+    }
+
     return runBuild(
       environment,
       plan.build,
diff --git a/tools/engine_tool/lib/src/gn.dart b/tools/engine_tool/lib/src/gn.dart
index c2e6b71af5af9..e9cdd489b7260 100644
--- a/tools/engine_tool/lib/src/gn.dart
+++ b/tools/engine_tool/lib/src/gn.dart
@@ -1,3 +1,4 @@
+import 'package:collection/collection.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 
@@ -106,6 +107,9 @@ interface class Gn {
             JsonObject(properties),
           );
           if (target == null) {
+            _environment.logger.warning(
+              'Unknown target type for $label: type=${properties['type']}',
+            );
             return null;
           }
           return target;
@@ -180,6 +184,11 @@ sealed class BuildTarget {
           testOnly: testOnly,
           json: json,
         ),
+      'group' => GroupBuildTarget(
+          label: Label.parseGn(label),
+          testOnly: testOnly,
+          deps: json.stringList('deps').map(Label.parseGn).toList(),
+        ),
       _ => null,
     };
   }
@@ -281,3 +290,36 @@ final class ExecutableBuildTarget extends BuildTarget {
   String toString() =>
       'ExecutableBuildTarget($label, testOnly=$testOnly, executable=$executable)';
 }
+
+/// A build target that [group]s a meta-target of other build targets.
+///
+/// [group]: https://gn.googlesource.com/gn/+/main/docs/reference.md#func_group
+final class GroupBuildTarget extends BuildTarget {
+  /// Construct a group build target.
+  const GroupBuildTarget({
+    required super.label,
+    required super.testOnly,
+    required this.deps,
+  });
+
+  /// The list of dependencies for this group target.
+  final List<Label> deps;
+
+  @override
+  bool operator ==(Object other) {
+    if (other is! GroupBuildTarget) {
+      return false;
+    }
+    if (label != other.label || testOnly != other.testOnly) {
+      return false;
+    }
+    return const ListEquality<Object>().equals(deps, other.deps);
+  }
+
+  @override
+  int get hashCode => Object.hash(label, testOnly, Object.hashAll(deps));
+
+  @override
+  String toString() =>
+      'GroupBuildTarget($label, testOnly=$testOnly, deps=$deps)';
+}
diff --git a/tools/engine_tool/test/build_command_test.dart b/tools/engine_tool/test/build_command_test.dart
index 12401b95bde13..1a09172f9642b 100644
--- a/tools/engine_tool/test/build_command_test.dart
+++ b/tools/engine_tool/test/build_command_test.dart
@@ -13,6 +13,7 @@ import 'package:path/path.dart' as path;
 import 'package:test/test.dart';
 
 import 'fixtures.dart' as fixtures;
+import 'src/matchers.dart';
 import 'src/test_build_configs.dart';
 import 'utils.dart';
 
@@ -646,6 +647,74 @@ The input testing/scenario_app:sceario_app matches no targets, configs or files.
     );
   });
 
+  test('build command warns on an unrecognized action', () async {
+    final testEnv = TestEnvironment.withTestEngine(
+      abi: Abi.macosArm64,
+      cannedProcesses: [
+        CannedProcess(
+          (command) => command.contains('desc'),
+          stdout: convert.jsonEncode({
+            '//flutter/tools/unrecognized:action': {
+              'outputs': [
+                '//out/host_debug/unrecognized_action',
+              ],
+              'testonly': true,
+              'type': 'unrecognized',
+            },
+          }),
+        ),
+      ],
+    );
+    addTearDown(testEnv.cleanup);
+
+    final builder = TestBuilderConfig();
+    builder.addBuild(
+      name: 'ci/host_debug',
+      targetDir: 'host_debug',
+      dimension: TestDroneDimension.mac,
+    );
+    final runner = ToolCommandRunner(
+      environment: testEnv.environment,
+      configs: {
+        'mac_test_config': builder.buildConfig(
+          path: 'ci/builders/mac_test_config.json',
+        ),
+      },
+    );
+
+    final result = await runner.run([
+      'build',
+      '--config',
+      'ci/host_debug',
+      '//flutter/tools/unrecognized:action',
+    ]);
+
+    printOnFailure(testEnv.testLogs.map((r) => r.message).join('\n'));
+    expect(result, equals(0));
+
+    expect(
+      testEnv.testLogs,
+      contains(
+        logRecord(
+          stringContainsInOrder([
+            'Unknown target type',
+            '//flutter/tools/unrecognized:action',
+            'type=unrecognized',
+          ]),
+          level: Logger.warningLevel,
+        ),
+      ),
+    );
+
+    expect(
+      testEnv.testLogs,
+      contains(logRecord(
+        contains('One or more targets specified did not match'),
+        level: Logger.warningLevel,
+      )),
+    );
+  });
+
   test('et help build line length is not too big', () async {
     final testEnv = TestEnvironment.withTestEngine(
       verbose: true,
diff --git a/tools/engine_tool/test/external_tools/gn_test.dart b/tools/engine_tool/test/external_tools/gn_test.dart
index efd1a8c463ffd..d61cfffffd41a 100644
--- a/tools/engine_tool/test/external_tools/gn_test.dart
+++ b/tools/engine_tool/test/external_tools/gn_test.dart
@@ -127,6 +127,40 @@ void main() {
     );
   });
 
+  test('parses a group', () async {
+    final testEnv = TestEnvironment.withTestEngine(
+      cannedProcesses: [
+        CannedProcess(
+          (List<String> command) => command.contains('desc'),
+          stdout: '''
+            {
+              "//foo/bar:baz_group": {
+                "deps": ["//foo/bar:baz_shared_library"],
+                "testonly": true,
+                "type": "group"
+              }
+            }
+          ''',
+        ),
+      ],
+    );
+    addTearDown(testEnv.cleanup);
+
+    final gn = Gn.fromEnvironment(testEnv.environment);
+    final targets = await gn.desc('out/Release', TargetPattern('//foo', 'bar'));
+    expect(targets, hasLength(1));
+
+    final groupTarget = targets.single;
+    expect(
+      groupTarget,
+      GroupBuildTarget(
+        label: Label('//foo/bar', 'baz_group'),
+        testOnly: true,
+        deps: [Label('//foo/bar', 'baz_shared_library')],
+      ),
+    );
+  });
+
   test('parses a dart_test action as an executable', () async {
     final testEnv = TestEnvironment.withTestEngine(
       cannedProcesses: [

From b12b35e81c2385743875abc75e1f01cf5a28acdc Mon Sep 17 00:00:00 2001
From: Matan Lurey <matan@lurey.org>
Date: Thu, 10 Oct 2024 10:41:58 -0700
Subject: [PATCH 2/4] Add license header.

---
 tools/engine_tool/lib/src/gn.dart | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/engine_tool/lib/src/gn.dart b/tools/engine_tool/lib/src/gn.dart
index e9cdd489b7260..9ad883f09255e 100644
--- a/tools/engine_tool/lib/src/gn.dart
+++ b/tools/engine_tool/lib/src/gn.dart
@@ -1,3 +1,7 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
 import 'package:collection/collection.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;

From 95a7fb2a9cdc587d4453d9fd7c3eac873185dfec Mon Sep 17 00:00:00 2001
From: Matan Lurey <matan@lurey.org>
Date: Thu, 10 Oct 2024 10:57:03 -0700
Subject: [PATCH 3/4] ++

---
 tools/engine_tool/lib/src/commands/build_command.dart | 7 ++++++-
 tools/engine_tool/test/build_command_test.dart        | 5 ++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/engine_tool/lib/src/commands/build_command.dart b/tools/engine_tool/lib/src/commands/build_command.dart
index 5c5aefa660307..6094ffd2db0a1 100644
--- a/tools/engine_tool/lib/src/commands/build_command.dart
+++ b/tools/engine_tool/lib/src/commands/build_command.dart
@@ -76,8 +76,13 @@ et build //flutter/fml:fml_benchmarks  # Build a specific target in `//flutter/f
     // Other warnings should have been emitted above, so if this ends up being
     // unneccesarily noisy, we can remove it or limit it to verbose mode.
     if (allTargets.length < commandLineTargets.length) {
+      // Report which targets were not found.
+      final notFound = commandLineTargets.where(
+        (target) => !allTargets.contains(Label.parse(target)),
+      );
       environment.logger.warning(
-        'One or more targets specified did not match any build targets.',
+        'One or more targets specified did not match any build targets:\n\n'
+        '${notFound.join('\n')}',
       );
     }
 
diff --git a/tools/engine_tool/test/build_command_test.dart b/tools/engine_tool/test/build_command_test.dart
index 1a09172f9642b..2c50bb01c73e8 100644
--- a/tools/engine_tool/test/build_command_test.dart
+++ b/tools/engine_tool/test/build_command_test.dart
@@ -709,7 +709,10 @@ The input testing/scenario_app:sceario_app matches no targets, configs or files.
     expect(
       testEnv.testLogs,
       contains(logRecord(
-        contains('One or more targets specified did not match'),
+        stringContainsInOrder([
+          'One or more targets specified did not match',
+          '//flutter/tools/unrecognized:action',
+        ]),
         level: Logger.warningLevel,
       )),
     );

From c3f0d384cde45d5e1fc8fd7259f9cc172fe2d50a Mon Sep 17 00:00:00 2001
From: Matan Lurey <matan@lurey.org>
Date: Thu, 10 Oct 2024 10:59:07 -0700
Subject: [PATCH 4/4] ++

---
 tools/engine_tool/test/{ => commands}/run_command_test.dart | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename tools/engine_tool/test/{ => commands}/run_command_test.dart (99%)

diff --git a/tools/engine_tool/test/run_command_test.dart b/tools/engine_tool/test/commands/run_command_test.dart
similarity index 99%
rename from tools/engine_tool/test/run_command_test.dart
rename to tools/engine_tool/test/commands/run_command_test.dart
index d293e5a20afde..925d800053195 100644
--- a/tools/engine_tool/test/run_command_test.dart
+++ b/tools/engine_tool/test/commands/run_command_test.dart
@@ -19,7 +19,7 @@ import 'package:process_fakes/process_fakes.dart';
 import 'package:process_runner/process_runner.dart';
 import 'package:test/test.dart';
 
-import 'src/test_build_configs.dart';
+import '../src/test_build_configs.dart';
 
 void main() {
   late io.Directory tempRoot;