[flutter_plugin_tools] Fix build-examples for packages (#4248)

The build-examples command was filtering what it attempted to build by plugin platform, which means it never does anything for non-plugin packages. flutter/packages has steps that run this command, which suggests it used to work and regressed at some point, but nobody noticed; this will re-enable those builds so that we are getting CI coverage that the examples in flutter/packages build.

Mostly fixes https://github.com/flutter/flutter/issues/88435 (needs a flutter/packages tool pin roll to pick this up)

script/tool/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.6.0+1
+
+- Fixed `build-examples` to work for non-plugin packages.
+
 ## 0.6.0
 
 - Added Android native integration test support to `native-test`.

script/tool/lib/src/build_examples_command.dart

@@ -117,39 +117,65 @@ class BuildExamplesCommand extends PackageLoopingCommand {
   Future<PackageResult> runForPackage(RepositoryPackage package) async {
     final List<String> errors = <String>[];
 
+    final bool isPlugin = isFlutterPlugin(package);
     final Iterable<_PlatformDetails> requestedPlatforms = _platforms.entries
         .where(
             (MapEntry<String, _PlatformDetails> entry) => getBoolArg(entry.key))
         .map((MapEntry<String, _PlatformDetails> entry) => entry.value);
-    final Set<_PlatformDetails> buildPlatforms = <_PlatformDetails>{};
+
-    final Set<_PlatformDetails> unsupportedPlatforms = <_PlatformDetails>{};
+    // Platform support is checked at the package level for plugins; there is
-    for (final _PlatformDetails platform in requestedPlatforms) {
+    // no package-level platform information for non-plugin packages.
-      if (pluginSupportsPlatform(platform.pluginPlatform, package,
+    final Set<_PlatformDetails> buildPlatforms = isPlugin
-          variant: platform.pluginPlatformVariant)) {
+        ? requestedPlatforms
-        buildPlatforms.add(platform);
+            .where((_PlatformDetails platform) => pluginSupportsPlatform(
-      } else {
+                platform.pluginPlatform, package,
-        unsupportedPlatforms.add(platform);
+                variant: platform.pluginPlatformVariant))
-      }
+            .toSet()
+        : requestedPlatforms.toSet();
+
+    String platformDisplayList(Iterable<_PlatformDetails> platforms) {
+      return platforms.map((_PlatformDetails p) => p.label).join(', ');
     }
+
     if (buildPlatforms.isEmpty) {
       final String unsupported = requestedPlatforms.length == 1
           ? '${requestedPlatforms.first.label} is not supported'
-          : 'None of [${requestedPlatforms.map((_PlatformDetails p) => p.label).join(',')}] are supported';
+          : 'None of [${platformDisplayList(requestedPlatforms)}] are supported';
       return PackageResult.skip('$unsupported by this plugin');
     }
-    print('Building for: '
+    print('Building for: ${platformDisplayList(buildPlatforms)}');
-        '${buildPlatforms.map((_PlatformDetails platform) => platform.label).join(',')}');
+
+    final Set<_PlatformDetails> unsupportedPlatforms =
+        requestedPlatforms.toSet().difference(buildPlatforms);
     if (unsupportedPlatforms.isNotEmpty) {
+      final List<String> skippedPlatforms = unsupportedPlatforms
+          .map((_PlatformDetails platform) => platform.label)
+          .toList();
+      skippedPlatforms.sort();
       print('Skipping unsupported platform(s): '
-          '${unsupportedPlatforms.map((_PlatformDetails platform) => platform.label).join(',')}');
+          '${skippedPlatforms.join(', ')}');
     }
     print('');
 
+    bool builtSomething = false;
     for (final RepositoryPackage example in package.getExamples()) {
       final String packageName =
           getRelativePosixPath(example.directory, from: packagesDir);
 
       for (final _PlatformDetails platform in buildPlatforms) {
+        // Repo policy is that a plugin must have examples configured for all
+        // supported platforms. For packages, just log and skip any requested
+        // platform that a package doesn't have set up.
+        if (!isPlugin &&
+            !example.directory
+                .childDirectory(platform.flutterPlatformDirectory)
+                .existsSync()) {
+          print('Skipping ${platform.label} for $packageName; not supported.');
+          continue;
+        }
+
+        builtSomething = true;
+
         String buildPlatform = platform.label;
         if (platform.label.toLowerCase() != platform.flutterBuildType) {
           buildPlatform += ' (${platform.flutterBuildType})';
@@ -162,6 +188,15 @@ class BuildExamplesCommand extends PackageLoopingCommand {
       }
     }
 
+    if (!builtSomething) {
+      if (isPlugin) {
+        errors.add('No examples found');
+      } else {
+        return PackageResult.skip(
+            'No examples found supporting requested platform(s).');
+      }
+    }
+
     return errors.isEmpty
         ? PackageResult.success()
         : PackageResult.fail(errors);
@@ -235,6 +270,11 @@ class _PlatformDetails {
   /// The `flutter build` build type.
   final String flutterBuildType;
 
+  /// The Flutter platform directory name.
+  // In practice, this is the same as the plugin platform key for all platforms.
+  // If that changes, this can be adjusted.
+  String get flutterPlatformDirectory => pluginPlatform;
+
   /// Any extra flags to pass to `flutter build`.
   final List<String> extraBuildFlags;
 }

script/tool/lib/src/common/package_looping_command.dart

@@ -237,7 +237,14 @@ abstract class PackageLoopingCommand extends PluginCommand {
         continue;
       }
 
-      final PackageResult result = await runForPackage(entry.package);
+      PackageResult result;
+      try {
+        result = await runForPackage(entry.package);
+      } catch (e, stack) {
+        printError(e.toString());
+        printError(stack.toString());
+        result = PackageResult.fail(<String>['Unhandled exception']);
+      }
       if (result.state == RunState.skipped) {
         final String message =
             '${indentation}SKIPPING: ${result.details.first}';

script/tool/lib/src/common/plugin_utils.dart

@@ -17,6 +17,11 @@ enum PlatformSupport {
   federated,
 }
 
+/// Returns true if [package] is a Flutter plugin.
+bool isFlutterPlugin(RepositoryPackage package) {
+  return _readPluginPubspecSection(package) != null;
+}
+
 /// Returns true if [package] is a Flutter [platform] plugin.
 ///
 /// It checks this by looking for the following pattern in the pubspec:
@@ -40,46 +45,43 @@ bool pluginSupportsPlatform(
       platform == kPlatformMacos ||
       platform == kPlatformWindows ||
       platform == kPlatformLinux);
-  try {
+
-    final YamlMap? platformEntry =
+  final YamlMap? platformEntry =
-        _readPlatformPubspecSectionForPlugin(platform, plugin);
+      _readPlatformPubspecSectionForPlugin(platform, plugin);
-    if (platformEntry == null) {
+  if (platformEntry == null) {
+    return false;
+  }
+
+  // If the platform entry is present, then it supports the platform. Check
+  // for required mode if specified.
+  if (requiredMode != null) {
+    final bool federated = platformEntry.containsKey('default_package');
+    if (federated != (requiredMode == PlatformSupport.federated)) {
       return false;
     }
+  }
 
-    // If the platform entry is present, then it supports the platform. Check
+  // If a variant is specified, check for that variant.
-    // for required mode if specified.
+  if (variant != null) {
-    if (requiredMode != null) {
+    const String variantsKey = 'supportedVariants';
-      final bool federated = platformEntry.containsKey('default_package');
+    if (platformEntry.containsKey(variantsKey)) {
-      if (federated != (requiredMode == PlatformSupport.federated)) {
+      if (!(platformEntry['supportedVariants']! as YamlList)
+          .contains(variant)) {
+        return false;
+      }
+    } else {
+      // Platforms with variants have a default variant when unspecified for
+      // backward compatibility. Must match the flutter tool logic.
+      const Map<String, String> defaultVariants = <String, String>{
+        kPlatformWindows: platformVariantWin32,
+      };
+      if (variant != defaultVariants[platform]) {
         return false;
       }
     }
-
-    // If a variant is specified, check for that variant.
-    if (variant != null) {
-      const String variantsKey = 'supportedVariants';
-      if (platformEntry.containsKey(variantsKey)) {
-        if (!(platformEntry['supportedVariants']! as YamlList)
-            .contains(variant)) {
-          return false;
-        }
-      } else {
-        // Platforms with variants have a default variant when unspecified for
-        // backward compatibility. Must match the flutter tool logic.
-        const Map<String, String> defaultVariants = <String, String>{
-          kPlatformWindows: platformVariantWin32,
-        };
-        if (variant != defaultVariants[platform]) {
-          return false;
-        }
-      }
-    }
-
-    return true;
-  } on YamlException {
-    return false;
   }
+
+  return true;
 }
 
 /// Returns true if [plugin] includes native code for [platform], as opposed to
@@ -89,24 +91,18 @@ bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
     // Web plugins are always Dart-only.
     return false;
   }
-  try {
+  final YamlMap? platformEntry =
-    final YamlMap? platformEntry =
+      _readPlatformPubspecSectionForPlugin(platform, plugin);
-        _readPlatformPubspecSectionForPlugin(platform, plugin);
+  if (platformEntry == null) {
-    if (platformEntry == null) {
-      return false;
-    }
-    // All other platforms currently use pluginClass for indicating the native
-    // code in the plugin.
-    final String? pluginClass = platformEntry['pluginClass'] as String?;
-    // TODO(stuartmorgan): Remove the check for 'none' once none of the plugins
-    // in the repository use that workaround. See
-    // https://github.com/flutter/flutter/issues/57497 for context.
-    return pluginClass != null && pluginClass != 'none';
-  } on FileSystemException {
-    return false;
-  } on YamlException {
     return false;
   }
+  // All other platforms currently use pluginClass for indicating the native
+  // code in the plugin.
+  final String? pluginClass = platformEntry['pluginClass'] as String?;
+  // TODO(stuartmorgan): Remove the check for 'none' once none of the plugins
+  // in the repository use that workaround. See
+  // https://github.com/flutter/flutter/issues/57497 for context.
+  return pluginClass != null && pluginClass != 'none';
 }
 
 /// Returns the
@@ -118,26 +114,33 @@ bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
 /// or the pubspec couldn't be read.
 YamlMap? _readPlatformPubspecSectionForPlugin(
     String platform, RepositoryPackage plugin) {
-  try {
+  final YamlMap? pluginSection = _readPluginPubspecSection(plugin);
-    final File pubspecFile = plugin.pubspecFile;
+  if (pluginSection == null) {
-    final YamlMap pubspecYaml =
-        loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
-    final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
-    if (flutterSection == null) {
-      return null;
-    }
-    final YamlMap? pluginSection = flutterSection['plugin'] as YamlMap?;
-    if (pluginSection == null) {
-      return null;
-    }
-    final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
-    if (platforms == null) {
-      return null;
-    }
-    return platforms[platform] as YamlMap?;
-  } on FileSystemException {
-    return null;
-  } on YamlException {
     return null;
   }
+  final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
+  if (platforms == null) {
+    return null;
+  }
+  return platforms[platform] as YamlMap?;
+}
+
+/// Returns the
+///   flutter:
+///     plugin:
+///       platforms:
+/// section from [plugin]'s pubspec.yaml, or null if either it is not present,
+/// or the pubspec couldn't be read.
+YamlMap? _readPluginPubspecSection(RepositoryPackage package) {
+  final File pubspecFile = package.pubspecFile;
+  if (!pubspecFile.existsSync()) {
+    return null;
+  }
+  final YamlMap pubspecYaml =
+      loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
+  final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
+  if (flutterSection == null) {
+    return null;
+  }
+  return flutterSection['plugin'] as YamlMap?;
 }

script/tool/pubspec.yaml

@@ -1,7 +1,7 @@
 name: flutter_plugin_tools
 description: Productivity utils for flutter/plugins and flutter/packages
 repository: https://github.com/flutter/plugins/tree/master/script/tool
-version: 0.6.0
+version: 0.6.0+1
 
 dependencies:
   args: ^2.1.0

script/tool/test/build_examples_command_test.dart

@@ -82,6 +82,35 @@ void main() {
           ]));
     });
 
+    test('fails if a plugin has no examples', () async {
+      createFakePlugin('plugin', packagesDir,
+          examples: <String>[],
+          platformSupport: <String, PlatformDetails>{
+            kPlatformIos: const PlatformDetails(PlatformSupport.inline)
+          });
+
+      processRunner
+              .mockProcessesForExecutable[getFlutterCommand(mockPlatform)] =
+          <io.Process>[
+        MockProcess(exitCode: 1) // flutter packages get
+      ];
+
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['build-examples', '--ios'], errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('The following packages had errors:'),
+            contains('  plugin:\n'
+                '    No examples found'),
+          ]));
+    });
+
     test('building for iOS when plugin is not set up for iOS results in no-op',
         () async {
       mockPlatform.isMacOS = true;
@@ -517,5 +546,138 @@ void main() {
                 pluginExampleDirectory.path),
           ]));
     });
+
+    test('logs skipped platforms', () async {
+      createFakePlugin('plugin', packagesDir,
+          platformSupport: <String, PlatformDetails>{
+            kPlatformAndroid: const PlatformDetails(PlatformSupport.inline),
+          });
+
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['build-examples', '--apk', '--ios', '--macos']);
+
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Skipping unsupported platform(s): iOS, macOS'),
+        ]),
+      );
+    });
+
+    group('packages', () {
+      test('builds when requested platform is supported by example', () async {
+        final Directory packageDirectory = createFakePackage(
+            'package', packagesDir, isFlutter: true, extraFiles: <String>[
+          'example/ios/Runner.xcodeproj/project.pbxproj'
+        ]);
+
+        final List<String> output = await runCapturingPrint(
+            runner, <String>['build-examples', '--ios']);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running for package'),
+            contains('BUILDING package/example for iOS'),
+          ]),
+        );
+
+        expect(
+            processRunner.recordedCalls,
+            orderedEquals(<ProcessCall>[
+              ProcessCall(
+                  getFlutterCommand(mockPlatform),
+                  const <String>[
+                    'build',
+                    'ios',
+                    '--no-codesign',
+                  ],
+                  packageDirectory.childDirectory('example').path),
+            ]));
+      });
+
+      test('skips non-Flutter examples', () async {
+        createFakePackage('package', packagesDir, isFlutter: false);
+
+        final List<String> output = await runCapturingPrint(
+            runner, <String>['build-examples', '--ios']);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running for package'),
+            contains('No examples found supporting requested platform(s).'),
+          ]),
+        );
+
+        expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
+      });
+
+      test('skips when there is no example', () async {
+        createFakePackage('package', packagesDir,
+            isFlutter: true, examples: <String>[]);
+
+        final List<String> output = await runCapturingPrint(
+            runner, <String>['build-examples', '--ios']);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running for package'),
+            contains('No examples found supporting requested platform(s).'),
+          ]),
+        );
+
+        expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
+      });
+
+      test('skip when example does not support requested platform', () async {
+        createFakePackage('package', packagesDir,
+            isFlutter: true,
+            extraFiles: <String>['example/linux/CMakeLists.txt']);
+
+        final List<String> output = await runCapturingPrint(
+            runner, <String>['build-examples', '--ios']);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running for package'),
+            contains('Skipping iOS for package/example; not supported.'),
+            contains('No examples found supporting requested platform(s).'),
+          ]),
+        );
+
+        expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
+      });
+
+      test('logs skipped platforms when only some are supported', () async {
+        final Directory packageDirectory = createFakePackage(
+            'package', packagesDir,
+            isFlutter: true,
+            extraFiles: <String>['example/linux/CMakeLists.txt']);
+
+        final List<String> output = await runCapturingPrint(
+            runner, <String>['build-examples', '--apk', '--linux']);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running for package'),
+            contains('Building for: Android, Linux'),
+            contains('Skipping Android for package/example; not supported.'),
+          ]),
+        );
+
+        expect(
+            processRunner.recordedCalls,
+            orderedEquals(<ProcessCall>[
+              ProcessCall(
+                  getFlutterCommand(mockPlatform),
+                  const <String>['build', 'linux'],
+                  packageDirectory.childDirectory('example').path),
+            ]));
+      });
+    });
   });
 }

script/tool/test/common/package_looping_command_test.dart

@@ -36,6 +36,8 @@ const String _errorFile = 'errors';
 const String _skipFile = 'skip';
 // The filename within a package containing warnings to log during runForPackage.
 const String _warningFile = 'warnings';
+// The filename within a package indicating that it should throw.
+const String _throwFile = 'throw';
 
 void main() {
   late FileSystem fileSystem;
@@ -117,7 +119,7 @@ void main() {
       expect(() => runCommand(command), throwsA(isA<ToolExit>()));
     });
 
-    test('does not stop looping', () async {
+    test('does not stop looping on error', () async {
       createFakePackage('package_a', packagesDir);
       final Directory failingPackage =
           createFakePlugin('package_b', packagesDir);
@@ -141,6 +143,31 @@ void main() {
             '${_startHeadingColor}Running for package_c...$_endColor',
           ]));
     });
+
+    test('does not stop looping on exceptions', () async {
+      createFakePackage('package_a', packagesDir);
+      final Directory failingPackage =
+          createFakePlugin('package_b', packagesDir);
+      createFakePackage('package_c', packagesDir);
+      failingPackage.childFile(_throwFile).createSync();
+
+      final TestPackageLoopingCommand command =
+          createTestCommand(hasLongOutput: false);
+      Error? commandError;
+      final List<String> output =
+          await runCommand(command, errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+          output,
+          containsAllInOrder(<String>[
+            '${_startHeadingColor}Running for package_a...$_endColor',
+            '${_startHeadingColor}Running for package_b...$_endColor',
+            '${_startHeadingColor}Running for package_c...$_endColor',
+          ]));
+    });
   });
 
   group('package iteration', () {
@@ -437,6 +464,31 @@ void main() {
           ]));
     });
 
+    test('logs unhandled exceptions as errors', () async {
+      createFakePackage('package_a', packagesDir);
+      final Directory failingPackage =
+          createFakePlugin('package_b', packagesDir);
+      createFakePackage('package_c', packagesDir);
+      failingPackage.childFile(_throwFile).createSync();
+
+      final TestPackageLoopingCommand command =
+          createTestCommand(hasLongOutput: false);
+      Error? commandError;
+      final List<String> output =
+          await runCommand(command, errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+          output,
+          containsAllInOrder(<String>[
+            '${_startErrorColor}Exception: Uh-oh$_endColor',
+            '${_startErrorColor}The following packages had errors:$_endColor',
+            '$_startErrorColor  package_b:\n    Unhandled exception$_endColor',
+          ]));
+    });
+
     test('prints run summary on success', () async {
       final Directory warnPackage1 =
           createFakePackage('package_a', packagesDir);
@@ -657,6 +709,10 @@ class TestPackageLoopingCommand extends PackageLoopingCommand {
     if (errorFile.existsSync()) {
       return PackageResult.fail(errorFile.readAsLinesSync());
     }
+    final File throwFile = package.directory.childFile(_throwFile);
+    if (throwFile.existsSync()) {
+      throw Exception('Uh-oh');
+    }
     return PackageResult.success();
   }