Eliminate build_all_plugins_app.sh (flutter#4232)

Removes the `build_all_plugins_app.sh` bash script, in support of the goal of eliminating all use of bash from the repository (for maintainability, and for better Windows compatibility). - The exclusion list moves to a config file, match other recent repo changes - The exclusion logging moves into the tool itself, consistent with the tool doing more logging of skipped and excluded plugins - The bulk of the logic moves to a Cirrus task template. This was done instead of rewriting the script in Dart, even though it will mean more work for alternate CI support (e.g., bringing this up on a Windows LUCI bot), because breaking it into components makes it easier to pinpoint failures from the CI UI rather than having all the steps smashed together.
amantoux · Sep 27, 2021 · 24f4219 · 24f4219
1 parent 46fe9ab
commit 24f4219
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 114 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -28,6 +28,20 @@ flutter_upgrade_template: &FLUTTER_UPGRADE_TEMPLATE
     - flutter doctor -v
   << : *TOOL_SETUP_TEMPLATE
 
+build_all_plugins_app_template: &BUILD_ALL_PLUGINS_APP_TEMPLATE
+  create_all_plugins_app_script:
+    - dart $PLUGIN_TOOL all-plugins-app --output-dir=. --exclude script/configs/exclude_all_plugins_app.yaml
+  build_all_plugins_debug_script:
+    - cd all_plugins
+    - if [[ "$BUILD_ALL_ARGS" == "web" ]]; then
+    -   echo "Skipping; web does not support debug builds"
+    - else
+    -   flutter build $BUILD_ALL_ARGS --debug
+    - fi
+  build_all_plugins_release_script:
+    - cd all_plugins
+    - flutter build $BUILD_ALL_ARGS --release
+
 macos_template: &MACOS_TEMPLATE
   # Only one macOS task can run in parallel without credits, so use them for
   # PRs on macOS.
@@ -82,28 +96,29 @@ task:
     ### Android tasks ###
     - name: build_all_plugins_apk
       env:
+        BUILD_ALL_ARGS: "apk"
         matrix:
           CHANNEL: "master"
           CHANNEL: "stable"
-      script:
-        - ./script/build_all_plugins_app.sh apk
+      << : *BUILD_ALL_PLUGINS_APP_TEMPLATE
     ### Web tasks ###
     - name: build_all_plugins_web
       env:
+        BUILD_ALL_ARGS: "web"
         matrix:
           CHANNEL: "master"
           CHANNEL: "stable"
-      script:
-        - ./script/build_all_plugins_app.sh web
+      << : *BUILD_ALL_PLUGINS_APP_TEMPLATE
     ### Linux desktop tasks ###
     - name: build_all_plugins_linux
       env:
+        BUILD_ALL_ARGS: "linux"
         matrix:
           CHANNEL: "master"
           CHANNEL: "stable"
-      script:
+      setup_script:
         - flutter config --enable-linux-desktop
-        - ./script/build_all_plugins_app.sh linux
+      << : *BUILD_ALL_PLUGINS_APP_TEMPLATE
     - name: build-linux+drive-examples
       env:
         matrix:
@@ -200,11 +215,11 @@ task:
     ### iOS tasks ###
     - name: build_all_plugins_ipa
       env:
+        BUILD_ALL_ARGS: "ios --no-codesign"
         matrix:
           CHANNEL: "master"
           CHANNEL: "stable"
-      script:
-        - ./script/build_all_plugins_app.sh ios --no-codesign
+      << : *BUILD_ALL_PLUGINS_APP_TEMPLATE
     - name: build-ipas+drive-examples
       env:
         PATH: $PATH:/usr/local/bin
@@ -234,12 +249,13 @@ task:
     ### macOS desktop tasks ###
     - name: build_all_plugins_macos
       env:
+        BUILD_ALL_ARGS: "macos"
         matrix:
           CHANNEL: "master"
           CHANNEL: "stable"
-      script:
+      setup_script:
         - flutter config --enable-macos-desktop
-        - ./script/build_all_plugins_app.sh macos
+      << : *BUILD_ALL_PLUGINS_APP_TEMPLATE
     - name: build-macos+drive-examples
       env:
         matrix:

diff --git a/script/build_all_plugins_app.sh b/script/build_all_plugins_app.sh
diff --git a/script/common.sh b/script/common.sh
diff --git a/script/configs/exclude_all_plugins_app.yaml b/script/configs/exclude_all_plugins_app.yaml
@@ -0,0 +1,10 @@
+# This list should be kept as short as possible, and things should remain here
+# only as long as necessary, since in general the goal is for all of the latest
+# versions of plugins to be mutually compatible.
+#
+# An example use case for this list would be to temporarily add plugins while
+# updating multiple plugins for a breaking change in a common dependency in
+# cases where using a relaxed version constraint isn't possible.
+
+# This is a permament entry, as it should never be a direct app dependency.
+- plugin_platform_interface
diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart
@@ -191,7 +191,7 @@ abstract class PluginCommand extends Command<void> {
   }
 
   /// Returns the set of plugins to exclude based on the `--exclude` argument.
-  Set<String> _getExcludedPackageName() {
+  Set<String> getExcludedPackageNames() {
     final Set<String> excludedPackages = _excludedPackages ??
         getStringListArg(_excludeArg).expand<String>((String item) {
           if (item.endsWith('.yaml')) {
@@ -265,7 +265,7 @@ abstract class PluginCommand extends Command<void> {
   Stream<PackageEnumerationEntry> _getAllPackages() async* {
     Set<String> plugins = Set<String>.from(getStringListArg(_packagesArg));
 
-    final Set<String> excludedPluginNames = _getExcludedPackageName();
+    final Set<String> excludedPluginNames = getExcludedPackageNames();
 
     final bool runOnChangedPackages = getBoolArg(_runOnChangedPackagesArg);
     if (plugins.isEmpty &&

diff --git a/script/tool/lib/src/create_all_plugins_app_command.dart b/script/tool/lib/src/create_all_plugins_app_command.dart
@@ -12,22 +12,27 @@ import 'package:pubspec_parse/pubspec_parse.dart';
 import 'common/core.dart';
 import 'common/plugin_command.dart';
 
+const String _outputDirectoryFlag = 'output-dir';
+
 /// A command to create an application that builds all in a single application.
 class CreateAllPluginsAppCommand extends PluginCommand {
   /// Creates an instance of the builder command.
   CreateAllPluginsAppCommand(
     Directory packagesDir, {
     Directory? pluginsRoot,
-  })  : pluginsRoot = pluginsRoot ?? packagesDir.fileSystem.currentDirectory,
-        super(packagesDir) {
-    appDirectory = this.pluginsRoot.childDirectory('all_plugins');
+  }) : super(packagesDir) {
+    final Directory defaultDir =
+        pluginsRoot ?? packagesDir.fileSystem.currentDirectory;
+    argParser.addOption(_outputDirectoryFlag,
+        defaultsTo: defaultDir.path,
+        help: 'The path the directory to create the "all_plugins" project in.\n'
+            'Defaults to the repository root.');
   }
 
-  /// The root directory of the plugin repository.
-  Directory pluginsRoot;
-
   /// The location of the synthesized app project.
-  late Directory appDirectory;
+  Directory get appDirectory => packagesDir.fileSystem
+      .directory(getStringArg(_outputDirectoryFlag))
+      .childDirectory('all_plugins');
 
   @override
   String get description =>
@@ -43,6 +48,15 @@ class CreateAllPluginsAppCommand extends PluginCommand {
       throw ToolExit(exitCode);
     }
 
+    final Set<String> excluded = getExcludedPackageNames();
+    if (excluded.isNotEmpty) {
+      print('Exluding the following plugins from the combined build:');
+      for (final String plugin in excluded) {
+        print('  $plugin');
+      }
+      print('');
+    }
+
     await Future.wait(<Future<void>>[
       _genPubspecWithAllPlugins(),
       _updateAppGradle(),

diff --git a/script/tool/test/create_all_plugins_app_command_test.dart b/script/tool/test/create_all_plugins_app_command_test.dart
@@ -13,10 +13,10 @@ import 'util.dart';
 void main() {
   group('$CreateAllPluginsAppCommand', () {
     late CommandRunner<void> runner;
-    FileSystem fileSystem;
+    late CreateAllPluginsAppCommand command;
+    late FileSystem fileSystem;
     late Directory testRoot;
     late Directory packagesDir;
-    late Directory appDir;
 
     setUp(() {
       // Since the core of this command is a call to 'flutter create', the test
@@ -26,11 +26,10 @@ void main() {
       testRoot = fileSystem.systemTempDirectory.createTempSync();
       packagesDir = testRoot.childDirectory('packages');
 
-      final CreateAllPluginsAppCommand command = CreateAllPluginsAppCommand(
+      command = CreateAllPluginsAppCommand(
         packagesDir,
         pluginsRoot: testRoot,
       );
-      appDir = command.appDirectory;
       runner = CommandRunner<void>(
           'create_all_test', 'Test for $CreateAllPluginsAppCommand');
       runner.addCommand(command);
@@ -47,7 +46,7 @@ void main() {
 
       await runCapturingPrint(runner, <String>['all-plugins-app']);
       final List<String> pubspec =
-          appDir.childFile('pubspec.yaml').readAsLinesSync();
+          command.appDirectory.childFile('pubspec.yaml').readAsLinesSync();
 
       expect(
           pubspec,
@@ -65,7 +64,7 @@ void main() {
 
       await runCapturingPrint(runner, <String>['all-plugins-app']);
       final List<String> pubspec =
-          appDir.childFile('pubspec.yaml').readAsLinesSync();
+          command.appDirectory.childFile('pubspec.yaml').readAsLinesSync();
 
       expect(
           pubspec,
@@ -82,9 +81,38 @@ void main() {
 
       await runCapturingPrint(runner, <String>['all-plugins-app']);
       final String pubspec =
-          appDir.childFile('pubspec.yaml').readAsStringSync();
+          command.appDirectory.childFile('pubspec.yaml').readAsStringSync();
 
       expect(pubspec, contains(RegExp('sdk:\\s*(?:["\']>=|[^])2\\.12\\.')));
     });
+
+    test('handles --output-dir', () async {
+      createFakePlugin('plugina', packagesDir);
+
+      final Directory customOutputDir =
+          fileSystem.systemTempDirectory.createTempSync();
+      await runCapturingPrint(runner,
+          <String>['all-plugins-app', '--output-dir=${customOutputDir.path}']);
+
+      expect(command.appDirectory.path,
+          customOutputDir.childDirectory('all_plugins').path);
+    });
+
+    test('logs exclusions', () async {
+      createFakePlugin('plugina', packagesDir);
+      createFakePlugin('pluginb', packagesDir);
+      createFakePlugin('pluginc', packagesDir);
+
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['all-plugins-app', '--exclude=pluginb,pluginc']);
+
+      expect(
+          output,
+          containsAllInOrder(<String>[
+            'Exluding the following plugins from the combined build:',
+            '  pluginb',
+            '  pluginc',
+          ]));
+    });
   });
 }
diff --git a/script/tool_runner.sh b/script/tool_runner.sh
@@ -8,7 +8,11 @@ set -e
 readonly SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)"
 readonly REPO_DIR="$(dirname "$SCRIPT_DIR")"
 
-source "$SCRIPT_DIR/common.sh"
+# Runs the plugin tools from the in-tree source.
+function plugin_tools() {
+  (pushd "$REPO_DIR/script/tool" && dart pub get && popd) >/dev/null
+  dart run "$REPO_DIR/script/tool/bin/flutter_plugin_tools.dart" "$@"
+}
 
 ACTIONS=("$@")