Skip to content

Commit 6056abe

Browse files
[flutter_plugin_tools] Fix build-examples for packages (flutter#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 flutter#88435 (needs a flutter/packages tool pin roll to pick this up)
1 parent 5afbfe9 commit 6056abe

7 files changed

+355
-83
lines changed

script/tool/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.6.0+1
2+
3+
- Fixed `build-examples` to work for non-plugin packages.
4+
15
## 0.6.0
26

37
- Added Android native integration test support to `native-test`.

script/tool/lib/src/build_examples_command.dart

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -117,39 +117,65 @@ class BuildExamplesCommand extends PackageLoopingCommand {
117117
Future<PackageResult> runForPackage(RepositoryPackage package) async {
118118
final List<String> errors = <String>[];
119119

120+
final bool isPlugin = isFlutterPlugin(package);
120121
final Iterable<_PlatformDetails> requestedPlatforms = _platforms.entries
121122
.where(
122123
(MapEntry<String, _PlatformDetails> entry) => getBoolArg(entry.key))
123124
.map((MapEntry<String, _PlatformDetails> entry) => entry.value);
124-
final Set<_PlatformDetails> buildPlatforms = <_PlatformDetails>{};
125-
final Set<_PlatformDetails> unsupportedPlatforms = <_PlatformDetails>{};
126-
for (final _PlatformDetails platform in requestedPlatforms) {
127-
if (pluginSupportsPlatform(platform.pluginPlatform, package,
128-
variant: platform.pluginPlatformVariant)) {
129-
buildPlatforms.add(platform);
130-
} else {
131-
unsupportedPlatforms.add(platform);
132-
}
125+
126+
// Platform support is checked at the package level for plugins; there is
127+
// no package-level platform information for non-plugin packages.
128+
final Set<_PlatformDetails> buildPlatforms = isPlugin
129+
? requestedPlatforms
130+
.where((_PlatformDetails platform) => pluginSupportsPlatform(
131+
platform.pluginPlatform, package,
132+
variant: platform.pluginPlatformVariant))
133+
.toSet()
134+
: requestedPlatforms.toSet();
135+
136+
String platformDisplayList(Iterable<_PlatformDetails> platforms) {
137+
return platforms.map((_PlatformDetails p) => p.label).join(', ');
133138
}
139+
134140
if (buildPlatforms.isEmpty) {
135141
final String unsupported = requestedPlatforms.length == 1
136142
? '${requestedPlatforms.first.label} is not supported'
137-
: 'None of [${requestedPlatforms.map((_PlatformDetails p) => p.label).join(',')}] are supported';
143+
: 'None of [${platformDisplayList(requestedPlatforms)}] are supported';
138144
return PackageResult.skip('$unsupported by this plugin');
139145
}
140-
print('Building for: '
141-
'${buildPlatforms.map((_PlatformDetails platform) => platform.label).join(',')}');
146+
print('Building for: ${platformDisplayList(buildPlatforms)}');
147+
148+
final Set<_PlatformDetails> unsupportedPlatforms =
149+
requestedPlatforms.toSet().difference(buildPlatforms);
142150
if (unsupportedPlatforms.isNotEmpty) {
151+
final List<String> skippedPlatforms = unsupportedPlatforms
152+
.map((_PlatformDetails platform) => platform.label)
153+
.toList();
154+
skippedPlatforms.sort();
143155
print('Skipping unsupported platform(s): '
144-
'${unsupportedPlatforms.map((_PlatformDetails platform) => platform.label).join(',')}');
156+
'${skippedPlatforms.join(', ')}');
145157
}
146158
print('');
147159

160+
bool builtSomething = false;
148161
for (final RepositoryPackage example in package.getExamples()) {
149162
final String packageName =
150163
getRelativePosixPath(example.directory, from: packagesDir);
151164

152165
for (final _PlatformDetails platform in buildPlatforms) {
166+
// Repo policy is that a plugin must have examples configured for all
167+
// supported platforms. For packages, just log and skip any requested
168+
// platform that a package doesn't have set up.
169+
if (!isPlugin &&
170+
!example.directory
171+
.childDirectory(platform.flutterPlatformDirectory)
172+
.existsSync()) {
173+
print('Skipping ${platform.label} for $packageName; not supported.');
174+
continue;
175+
}
176+
177+
builtSomething = true;
178+
153179
String buildPlatform = platform.label;
154180
if (platform.label.toLowerCase() != platform.flutterBuildType) {
155181
buildPlatform += ' (${platform.flutterBuildType})';
@@ -162,6 +188,15 @@ class BuildExamplesCommand extends PackageLoopingCommand {
162188
}
163189
}
164190

191+
if (!builtSomething) {
192+
if (isPlugin) {
193+
errors.add('No examples found');
194+
} else {
195+
return PackageResult.skip(
196+
'No examples found supporting requested platform(s).');
197+
}
198+
}
199+
165200
return errors.isEmpty
166201
? PackageResult.success()
167202
: PackageResult.fail(errors);
@@ -235,6 +270,11 @@ class _PlatformDetails {
235270
/// The `flutter build` build type.
236271
final String flutterBuildType;
237272

273+
/// The Flutter platform directory name.
274+
// In practice, this is the same as the plugin platform key for all platforms.
275+
// If that changes, this can be adjusted.
276+
String get flutterPlatformDirectory => pluginPlatform;
277+
238278
/// Any extra flags to pass to `flutter build`.
239279
final List<String> extraBuildFlags;
240280
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,14 @@ abstract class PackageLoopingCommand extends PluginCommand {
237237
continue;
238238
}
239239

240-
final PackageResult result = await runForPackage(entry.package);
240+
PackageResult result;
241+
try {
242+
result = await runForPackage(entry.package);
243+
} catch (e, stack) {
244+
printError(e.toString());
245+
printError(stack.toString());
246+
result = PackageResult.fail(<String>['Unhandled exception']);
247+
}
241248
if (result.state == RunState.skipped) {
242249
final String message =
243250
'${indentation}SKIPPING: ${result.details.first}';

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

Lines changed: 70 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ enum PlatformSupport {
1717
federated,
1818
}
1919

20+
/// Returns true if [package] is a Flutter plugin.
21+
bool isFlutterPlugin(RepositoryPackage package) {
22+
return _readPluginPubspecSection(package) != null;
23+
}
24+
2025
/// Returns true if [package] is a Flutter [platform] plugin.
2126
///
2227
/// It checks this by looking for the following pattern in the pubspec:
@@ -40,46 +45,43 @@ bool pluginSupportsPlatform(
4045
platform == kPlatformMacos ||
4146
platform == kPlatformWindows ||
4247
platform == kPlatformLinux);
43-
try {
44-
final YamlMap? platformEntry =
45-
_readPlatformPubspecSectionForPlugin(platform, plugin);
46-
if (platformEntry == null) {
48+
49+
final YamlMap? platformEntry =
50+
_readPlatformPubspecSectionForPlugin(platform, plugin);
51+
if (platformEntry == null) {
52+
return false;
53+
}
54+
55+
// If the platform entry is present, then it supports the platform. Check
56+
// for required mode if specified.
57+
if (requiredMode != null) {
58+
final bool federated = platformEntry.containsKey('default_package');
59+
if (federated != (requiredMode == PlatformSupport.federated)) {
4760
return false;
4861
}
62+
}
4963

50-
// If the platform entry is present, then it supports the platform. Check
51-
// for required mode if specified.
52-
if (requiredMode != null) {
53-
final bool federated = platformEntry.containsKey('default_package');
54-
if (federated != (requiredMode == PlatformSupport.federated)) {
64+
// If a variant is specified, check for that variant.
65+
if (variant != null) {
66+
const String variantsKey = 'supportedVariants';
67+
if (platformEntry.containsKey(variantsKey)) {
68+
if (!(platformEntry['supportedVariants']! as YamlList)
69+
.contains(variant)) {
5570
return false;
5671
}
57-
}
58-
59-
// If a variant is specified, check for that variant.
60-
if (variant != null) {
61-
const String variantsKey = 'supportedVariants';
62-
if (platformEntry.containsKey(variantsKey)) {
63-
if (!(platformEntry['supportedVariants']! as YamlList)
64-
.contains(variant)) {
65-
return false;
66-
}
67-
} else {
68-
// Platforms with variants have a default variant when unspecified for
69-
// backward compatibility. Must match the flutter tool logic.
70-
const Map<String, String> defaultVariants = <String, String>{
71-
kPlatformWindows: platformVariantWin32,
72-
};
73-
if (variant != defaultVariants[platform]) {
74-
return false;
75-
}
72+
} else {
73+
// Platforms with variants have a default variant when unspecified for
74+
// backward compatibility. Must match the flutter tool logic.
75+
const Map<String, String> defaultVariants = <String, String>{
76+
kPlatformWindows: platformVariantWin32,
77+
};
78+
if (variant != defaultVariants[platform]) {
79+
return false;
7680
}
7781
}
78-
79-
return true;
80-
} on YamlException {
81-
return false;
8282
}
83+
84+
return true;
8385
}
8486

8587
/// Returns true if [plugin] includes native code for [platform], as opposed to
@@ -89,24 +91,18 @@ bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
8991
// Web plugins are always Dart-only.
9092
return false;
9193
}
92-
try {
93-
final YamlMap? platformEntry =
94-
_readPlatformPubspecSectionForPlugin(platform, plugin);
95-
if (platformEntry == null) {
96-
return false;
97-
}
98-
// All other platforms currently use pluginClass for indicating the native
99-
// code in the plugin.
100-
final String? pluginClass = platformEntry['pluginClass'] as String?;
101-
// TODO(stuartmorgan): Remove the check for 'none' once none of the plugins
102-
// in the repository use that workaround. See
103-
// https://github.com/flutter/flutter/issues/57497 for context.
104-
return pluginClass != null && pluginClass != 'none';
105-
} on FileSystemException {
106-
return false;
107-
} on YamlException {
94+
final YamlMap? platformEntry =
95+
_readPlatformPubspecSectionForPlugin(platform, plugin);
96+
if (platformEntry == null) {
10897
return false;
10998
}
99+
// All other platforms currently use pluginClass for indicating the native
100+
// code in the plugin.
101+
final String? pluginClass = platformEntry['pluginClass'] as String?;
102+
// TODO(stuartmorgan): Remove the check for 'none' once none of the plugins
103+
// in the repository use that workaround. See
104+
// https://github.com/flutter/flutter/issues/57497 for context.
105+
return pluginClass != null && pluginClass != 'none';
110106
}
111107

112108
/// Returns the
@@ -118,26 +114,33 @@ bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
118114
/// or the pubspec couldn't be read.
119115
YamlMap? _readPlatformPubspecSectionForPlugin(
120116
String platform, RepositoryPackage plugin) {
121-
try {
122-
final File pubspecFile = plugin.pubspecFile;
123-
final YamlMap pubspecYaml =
124-
loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
125-
final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
126-
if (flutterSection == null) {
127-
return null;
128-
}
129-
final YamlMap? pluginSection = flutterSection['plugin'] as YamlMap?;
130-
if (pluginSection == null) {
131-
return null;
132-
}
133-
final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
134-
if (platforms == null) {
135-
return null;
136-
}
137-
return platforms[platform] as YamlMap?;
138-
} on FileSystemException {
117+
final YamlMap? pluginSection = _readPluginPubspecSection(plugin);
118+
if (pluginSection == null) {
119+
return null;
120+
}
121+
final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
122+
if (platforms == null) {
123+
return null;
124+
}
125+
return platforms[platform] as YamlMap?;
126+
}
127+
128+
/// Returns the
129+
/// flutter:
130+
/// plugin:
131+
/// platforms:
132+
/// section from [plugin]'s pubspec.yaml, or null if either it is not present,
133+
/// or the pubspec couldn't be read.
134+
YamlMap? _readPluginPubspecSection(RepositoryPackage package) {
135+
final File pubspecFile = package.pubspecFile;
136+
if (!pubspecFile.existsSync()) {
139137
return null;
140-
} on YamlException {
138+
}
139+
final YamlMap pubspecYaml =
140+
loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
141+
final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
142+
if (flutterSection == null) {
141143
return null;
142144
}
145+
return flutterSection['plugin'] as YamlMap?;
143146
}

script/tool/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: flutter_plugin_tools
22
description: Productivity utils for flutter/plugins and flutter/packages
33
repository: https://github.com/flutter/plugins/tree/master/script/tool
4-
version: 0.6.0
4+
version: 0.6.0+1
55

66
dependencies:
77
args: ^2.1.0

0 commit comments

Comments
 (0)