diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 2ef34c184b11..1f1da3551ef8 100644 --- a/script/tool/CHANGELOG.md +++ b/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`. diff --git a/script/tool/lib/src/build_examples_command.dart b/script/tool/lib/src/build_examples_command.dart index e441f61d5644..56c2f5c7dc87 100644 --- a/script/tool/lib/src/build_examples_command.dart +++ b/script/tool/lib/src/build_examples_command.dart @@ -117,39 +117,65 @@ class BuildExamplesCommand extends PackageLoopingCommand { Future runForPackage(RepositoryPackage package) async { final List errors = []; + final bool isPlugin = isFlutterPlugin(package); final Iterable<_PlatformDetails> requestedPlatforms = _platforms.entries .where( (MapEntry entry) => getBoolArg(entry.key)) .map((MapEntry entry) => entry.value); - final Set<_PlatformDetails> buildPlatforms = <_PlatformDetails>{}; - final Set<_PlatformDetails> unsupportedPlatforms = <_PlatformDetails>{}; - for (final _PlatformDetails platform in requestedPlatforms) { - if (pluginSupportsPlatform(platform.pluginPlatform, package, - variant: platform.pluginPlatformVariant)) { - buildPlatforms.add(platform); - } else { - unsupportedPlatforms.add(platform); - } + + // Platform support is checked at the package level for plugins; there is + // no package-level platform information for non-plugin packages. + final Set<_PlatformDetails> buildPlatforms = isPlugin + ? requestedPlatforms + .where((_PlatformDetails platform) => pluginSupportsPlatform( + platform.pluginPlatform, package, + 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: ' - '${buildPlatforms.map((_PlatformDetails platform) => platform.label).join(',')}'); + print('Building for: ${platformDisplayList(buildPlatforms)}'); + + final Set<_PlatformDetails> unsupportedPlatforms = + requestedPlatforms.toSet().difference(buildPlatforms); if (unsupportedPlatforms.isNotEmpty) { + final List 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 extraBuildFlags; } diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 00caeb30ef42..c80f4bfbcd5a 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -223,7 +223,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(['Unhandled exception']); + } if (result.state == RunState.skipped) { final String message = '${indentation}SKIPPING: ${result.details.first}'; diff --git a/script/tool/lib/src/common/plugin_utils.dart b/script/tool/lib/src/common/plugin_utils.dart index 06af675e71ef..6cfe9928d689 100644 --- a/script/tool/lib/src/common/plugin_utils.dart +++ b/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 = - _readPlatformPubspecSectionForPlugin(platform, plugin); - if (platformEntry == null) { + + final YamlMap? platformEntry = + _readPlatformPubspecSectionForPlugin(platform, plugin); + 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 - // for required mode if specified. - if (requiredMode != null) { - final bool federated = platformEntry.containsKey('default_package'); - if (federated != (requiredMode == PlatformSupport.federated)) { + // 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; } - } - - // 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 defaultVariants = { - kPlatformWindows: platformVariantWin32, - }; - if (variant != defaultVariants[platform]) { - return false; - } + } else { + // Platforms with variants have a default variant when unspecified for + // backward compatibility. Must match the flutter tool logic. + const Map defaultVariants = { + 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 = - _readPlatformPubspecSectionForPlugin(platform, plugin); - 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 { + final YamlMap? platformEntry = + _readPlatformPubspecSectionForPlugin(platform, plugin); + 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'; } /// 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 File pubspecFile = plugin.pubspecFile; - 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 { + final YamlMap? pluginSection = _readPluginPubspecSection(plugin); + if (pluginSection == null) { + 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; - } on YamlException { + } + 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?; } diff --git a/script/tool/pubspec.yaml b/script/tool/pubspec.yaml index 7c2bb0b3e3c0..adf62ca35a1a 100644 --- a/script/tool/pubspec.yaml +++ b/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 diff --git a/script/tool/test/build_examples_command_test.dart b/script/tool/test/build_examples_command_test.dart index a17107c18e27..d9cbad246d28 100644 --- a/script/tool/test/build_examples_command_test.dart +++ b/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: [], + platformSupport: { + kPlatformIos: const PlatformDetails(PlatformSupport.inline) + }); + + processRunner + .mockProcessesForExecutable[getFlutterCommand(mockPlatform)] = + [ + MockProcess(exitCode: 1) // flutter packages get + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['build-examples', '--ios'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + 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: { + kPlatformAndroid: const PlatformDetails(PlatformSupport.inline), + }); + + final List output = await runCapturingPrint( + runner, ['build-examples', '--apk', '--ios', '--macos']); + + expect( + output, + containsAllInOrder([ + 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: [ + 'example/ios/Runner.xcodeproj/project.pbxproj' + ]); + + final List output = await runCapturingPrint( + runner, ['build-examples', '--ios']); + + expect( + output, + containsAllInOrder([ + contains('Running for package'), + contains('BUILDING package/example for iOS'), + ]), + ); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const [ + 'build', + 'ios', + '--no-codesign', + ], + packageDirectory.childDirectory('example').path), + ])); + }); + + test('skips non-Flutter examples', () async { + createFakePackage('package', packagesDir, isFlutter: false); + + final List output = await runCapturingPrint( + runner, ['build-examples', '--ios']); + + expect( + output, + containsAllInOrder([ + contains('Running for package'), + contains('No examples found supporting requested platform(s).'), + ]), + ); + + expect(processRunner.recordedCalls, orderedEquals([])); + }); + + test('skips when there is no example', () async { + createFakePackage('package', packagesDir, + isFlutter: true, examples: []); + + final List output = await runCapturingPrint( + runner, ['build-examples', '--ios']); + + expect( + output, + containsAllInOrder([ + contains('Running for package'), + contains('No examples found supporting requested platform(s).'), + ]), + ); + + expect(processRunner.recordedCalls, orderedEquals([])); + }); + + test('skip when example does not support requested platform', () async { + createFakePackage('package', packagesDir, + isFlutter: true, + extraFiles: ['example/linux/CMakeLists.txt']); + + final List output = await runCapturingPrint( + runner, ['build-examples', '--ios']); + + expect( + output, + containsAllInOrder([ + contains('Running for package'), + contains('Skipping iOS for package/example; not supported.'), + contains('No examples found supporting requested platform(s).'), + ]), + ); + + expect(processRunner.recordedCalls, orderedEquals([])); + }); + + test('logs skipped platforms when only some are supported', () async { + final Directory packageDirectory = createFakePackage( + 'package', packagesDir, + isFlutter: true, + extraFiles: ['example/linux/CMakeLists.txt']); + + final List output = await runCapturingPrint( + runner, ['build-examples', '--apk', '--linux']); + + expect( + output, + containsAllInOrder([ + contains('Running for package'), + contains('Building for: Android, Linux'), + contains('Skipping Android for package/example; not supported.'), + ]), + ); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const ['build', 'linux'], + packageDirectory.childDirectory('example').path), + ])); + }); + }); }); } diff --git a/script/tool/test/common/package_looping_command_test.dart b/script/tool/test/common/package_looping_command_test.dart index 721923ae9c6e..7cf03960a74d 100644 --- a/script/tool/test/common/package_looping_command_test.dart +++ b/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())); }); - 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 output = + await runCommand(command, errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + '${_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 output = + await runCommand(command, errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + '${_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(); }