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

[flutter_plugin_tools] Fix build-examples for packages #4248

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
66 changes: 53 additions & 13 deletions script/tool/lib/src/build_examples_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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>{};
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<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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Instead of using continue do you think an else clause would be preferable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally don't like to nest non-trivial primary logic in an else clause.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, not my preference but not in violation of our style. Academics have argued about it for decades: https://en.wikipedia.org/wiki/Structured_programming#Early_exit

}

builtSomething = true;

String buildPlatform = platform.label;
if (platform.label.toLowerCase() != platform.flutterBuildType) {
buildPlatform += ' (${platform.flutterBuildType})';
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
9 changes: 8 additions & 1 deletion script/tool/lib/src/common/package_looping_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(<String>['Unhandled exception']);
}
if (result.state == RunState.skipped) {
final String message =
'${indentation}SKIPPING: ${result.details.first}';
Expand Down
137 changes: 70 additions & 67 deletions script/tool/lib/src/common/plugin_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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<String, String> defaultVariants = <String, String>{
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<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
Expand All @@ -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
Expand All @@ -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?;
}
2 changes: 1 addition & 1 deletion script/tool/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading