updates in response to code review

Author: craiglabenz

Makefile

@@ -11,6 +11,10 @@ test: generate_text test_text generate_core test_core
 # Runs `ffigen` for all packages and all tests for all packages
 test_only: test_core test_text
 
+# Runs `sdks_finder` to update manifest files
+sdks:
+	dart tool/builder/bin/main.dart sdks
+
 # Core ---
 
 # Runs `ffigen` for `mediapipe_core`

README.md

@@ -1,3 +1,21 @@
 # Flutter-MediaPipe
 
 This repository will be home to the source code for the mediapipe_task_vision, mediapipe_task_audio, and mediapipe_task_text plugins for Flutter.
+
+## Releasing
+
+### Updating MediaPipe SDKs
+
+Anytime MediaPipe releases new versions of their SDKs, this package will need to be updated to incorporate those latest builds. SDK versions are pinned in the `sdk_downloads.json` files in each package, which are updated by running the following command from the root of the repository:
+
+```
+$ make sdks
+```
+
+The Google Cloud Storage bucket in question only gives read-list access to a specific list of Googlers' accounts, so this command must be run from such a Googler's corp machines.
+
+After this, create and merge a PR with the changes and then proceed to `Releasing to pub.dev`.
+
+### Releasing to pub.dev
+
+TODO

tool/builder/lib/sdks_finder.dart

@@ -1,5 +1,6 @@
 import 'dart:convert';
 import 'dart:io';
+import 'dart:io' as io;
 
 import 'package:args/command_runner.dart';
 import 'package:builder/extensions.dart';
@@ -47,9 +48,12 @@ enum MediaPipeSdk {
 /// command helps by automating a portion of the task.
 ///
 /// The cache-busting mechanism of Flutter's native assets feature is a hash
-/// of the contents of any build dependencies, so if this command leads to any
-/// new build state, this must be reflected by *some change* to the associated
-/// library's `build.dart` script.
+/// of the contents of any build dependencies. The output files of this command
+/// are included in the build dependencies (as specified by the contents of each
+/// package's `build.dart` file), so if this command generates new SDK locations
+/// in those files, Flutter's CLI will have a cache miss, will re-run
+/// `build.dart` during the build phase, and in turn will download the newest
+/// versions of the MediaPipe SDKs onto the developer's machine.
 ///
 /// Operationally, [SdksFinderCommand]'s implementation involves orchestrating
 /// one [_OsFinder] instance for each supported [OS] value, which in turn
@@ -67,7 +71,7 @@ class SdksFinderCommand extends Command with RepoFinderMixin {
   }
   @override
   String description =
-      'Downloads the appropriate MediaPipe SDKs for the current build target';
+      'Updates MediaPipe SDK manifest files for the current build target';
 
   @override
   String name = 'sdks';
@@ -120,8 +124,24 @@ final Map<String, Map<String, String>> sdkDownloadUrls = ${encoder.convert(resul
   }
 
   void _checkGsUtil() async {
+    if (!io.Platform.isMacOS && !io.Platform.isLinux) {
+      // `which` is not available on Windows, so allow the command to attempt
+      // to run on Windows
+      // TODO: possibly add Windows-specific support
+      return;
+    }
     final process = await Process.start('which', ['gsutil']);
-    await process.exitCode;
+    final exitCode = await process.exitCode;
+    if (exitCode != 0) {
+      stderr.writeln(
+        wrapWith(
+          'Warning: Unexpected exit code $exitCode checking for gsutil. Output:'
+          '${(await process.processedStdOut).join('\n')}',
+          [yellow],
+        ),
+      );
+      // Not exiting here, since this could be a false-negative.
+    }
     if ((await process.processedStdOut).isEmpty) {
       stderr.writeln(
         wrapWith(
@@ -297,7 +317,21 @@ Future<List<String>> _gsUtil(String path, {bool recursive = false}) async {
   ];
   _log.finest('Running: `gsutil ${cmd.join(' ')}`');
   final process = await Process.start('gsutil', cmd);
-  await process.exitCode;
+  final exitCode = await process.exitCode;
+  if (exitCode > 1) {
+    // Exit codes of 1 appear when `gsutil` checks for a file that does not
+    // exist, which for our purposes does not constitute an actual error, and is
+    // handled later when `process.processedStdOut` is empty.
+    stderr.writeln(
+      wrapWith(
+        'Warning: Unexpected exit code $exitCode running '
+        '`gsutil ${cmd.join(' ')}`. Output: '
+        '${(await process.processedStdOut).join('\n')}',
+        [red],
+      ),
+    );
+    exit(exitCode);
+  }
   final processStdout = await process.processedStdOut;
   final filtered = (processStdout).where((String line) => line != '').toList();
   return filtered;