refactor: Move logic for compiling shorebird.yaml into a new file

.github/workflows/shorebird_ci.yml

@@ -1,31 +1,14 @@
-name: shorebird_tests
+name: shorebird_ci
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 
 on:
   pull_request:
-    paths:
-      - ".github/workflows/shorebird_tests.yaml"
-      - "packages/shorebird_tests/lib/**"
-      - "packages/shorebird_tests/test/**"
-      - "packages/shorebird_tests/pubspec.yaml"
-      - "packages/flutter_tools/lib/**"
-      - "packages/flutter_tools/test/**"
-      - "packages/flutter_tools/pubspec.yaml"
   push:
     branches:
-      - main
-    paths:
-      - ".github/workflows/shorebird_tests.yaml"
-      - "packages/shorebird_tests/lib/**"
-      - "packages/shorebird_tests/test/**"
-      - "packages/shorebird_tests/pubspec.yaml"
-      - "packages/flutter_tools/lib/**"
-      - "packages/flutter_tools/test/**"
-      - "packages/flutter_tools/pubspec.yaml"
-  workflow_dispatch:
+      - shorebird/dev
 
 jobs:
   test:
@@ -38,6 +21,8 @@ jobs:
 
     name: 🐦 Shorebird Test
 
+    # TODO(eseidel): This is also set inside shorebird_tests, unclear if
+    # if it's needed here as well.
     env:
       FLUTTER_STORAGE_BASE_URL: https://download.shorebird.dev
 
@@ -48,6 +33,8 @@ jobs:
           # Fetch all branches and tags to ensure that Flutter can determine its version
           fetch-depth: 0
 
+      # TODO(eseidel): shorebird_tests seems to assume flutter is available
+      # yet it doesn't seem to set it up here?
       - name: 🎯 Setup Dart
         uses: dart-lang/setup-dart@v1
 
@@ -56,6 +43,14 @@ jobs:
           distribution: "zulu"
           java-version: "11"
 
+      - name: 🐦 Run Flutter Tools Tests
+        # TODO(eseidel): Find a nice way to run this on windows.
+        if: matrix.os == 'ubuntu-latest' || matrix.os == 'macos-latest'
+        # TODO(eseidel): We can't run all flutter_tools tests until we make
+        # our changes not throw exceptions on missing shorebird.yaml.
+        run: ../../bin/flutter test test/general.shard/shorebird
+        working-directory: packages/flutter_tools
+
       - name: 🐦 Run Shorebird Tests
         run: dart test
         working-directory: packages/shorebird_tests

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

@@ -1332,6 +1332,11 @@ class FlutterPlugin implements Plugin<Project> {
                 }
                 Task copyFlutterAssetsTask = addFlutterDeps(variant)
                 copyFlutterAssetsTask.doLast {
+                  // TODO(eseidel): This is currently duplicating logic
+                  // inside shorebird_yaml.dart in the flutter tool.  We should
+                  // just call `flutter build shorebird-yaml` or something
+                  // instead, but I don't know how to call `flutter build`
+                  // from here yet.
                   def yaml = new Yaml()
                   def outputDir = copyFlutterAssetsTask.destinationDir
 

packages/flutter_tools/lib/src/ios/application_package.dart

@@ -128,6 +128,18 @@ class BuildableIOSApp extends IOSApp {
   @override
   String? get name => _hostAppBundleName;
 
+  String get shorebirdYamlPath =>
+    globals.fs.path.join(
+      archiveBundleOutputPath,
+      'Products',
+      'Applications',
+      _hostAppBundleName ?? 'Runner.app',
+      'Frameworks',
+      'App.framework',
+      'flutter_assets',
+      'shorebird.yaml',
+    );
+
   @override
   String get simulatorBundlePath => _buildAppPath('iphonesimulator');
 

packages/flutter_tools/lib/src/ios/mac.dart

@@ -8,11 +8,9 @@ import 'dart:io';
 import 'package:meta/meta.dart';
 import 'package:process/process.dart';
 import 'package:unified_analytics/unified_analytics.dart';
-import 'package:yaml/yaml.dart';
 
 import '../artifacts.dart';
 import '../base/file_system.dart';
-import '../base/io.dart';
 import '../base/logger.dart';
 import '../base/process.dart';
 import '../base/project_migrator.dart';
@@ -29,6 +27,7 @@ import '../migrations/xcode_script_build_phase_migration.dart';
 import '../migrations/xcode_thin_binary_build_phase_input_paths_migration.dart';
 import '../project.dart';
 import '../reporting/reporting.dart';
+import '../shorebird/shorebird_yaml.dart';
 import 'application_package.dart';
 import 'code_signing.dart';
 import 'migrations/host_app_info_plist_migration.dart';
@@ -520,7 +519,7 @@ Future<XcodeBuildResult> buildXcodeProject({
     }
 
     try {
-      updateShorebirdYaml(buildInfo, app);
+      updateShorebirdYaml(buildInfo, app.shorebirdYamlPath);
     } on Exception catch (error) {
       globals.printError('[shorebird] failed to generate shorebird configuration.\n$error');
       return XcodeBuildResult(success: false);
@@ -540,93 +539,6 @@ Future<XcodeBuildResult> buildXcodeProject({
   }
 }
 
-void updateShorebirdYaml(BuildInfo buildInfo, BuildableIOSApp app) {
-  final String resolvedAppName = app.name ?? 'Runner.app';
-  final File shorebirdYaml = globals.fs.file(
-    globals.fs.path.join(
-      app.archiveBundleOutputPath,
-      'Products',
-      'Applications',
-      resolvedAppName,
-      'Frameworks',
-      'App.framework',
-      'flutter_assets',
-      'shorebird.yaml',
-    ),
-  );
-  if (!shorebirdYaml.existsSync()) {
-    // Find the closest existing parent of the file.
-    Directory parent = shorebirdYaml.parent;
-
-    int i = 0;
-    // The max depth is just a hard limit to prevent the cli from going too far back in the
-    // folder tree and unintentionally "invading" a user folder that isn't the project. 
-    //
-    // This limit should never be reached though, since at least the `Applications` or
-    // `Products` folder should exist, no matter what changed in the app.
-    // This is really just an overcautious from our side to make sure we never
-    // access files that we don't need.
-    const int maxDepth = 7;
-    while (!parent.existsSync() && i < maxDepth) {
-      parent = parent.parent;
-      i++;
-    }
-
-    String parentChildren = '';
-    if (parent.existsSync()) {
-      parentChildren = parent.listSync().map((FileSystemEntity entity) => entity.basename).join(', ');
-    }
-
-    throw Exception('''
-Cannot find shorebird.yaml in ${shorebirdYaml.absolute.path}.
-Resolved app name: $resolvedAppName
-
-Closest existing parent:
-  PATH: ${parent.absolute.path}
-  CHILDREN: $parentChildren
-
-Please file an issue at: https://github.com/shorebirdtech/shorebird/issues/new
-''');
-  }
-  final YamlDocument yaml = loadYamlDocument(shorebirdYaml.readAsStringSync());
-  final YamlMap yamlMap = yaml.contents as YamlMap;
-  final String? flavor = buildInfo.flavor;
-  String appId = '';
-  if (flavor == null) {
-    final String? defaultAppId = yamlMap['app_id'] as String?;
-    if (defaultAppId == null || defaultAppId.isEmpty) {
-      throw Exception('Cannot find "app_id" in shorebird.yaml');
-    }
-    appId = defaultAppId;
-  } else {
-    final YamlMap? yamlFlavors = yamlMap['flavors'] as YamlMap?;
-    if (yamlFlavors == null) {
-      throw Exception('Cannot find "flavors" in shorebird.yaml.');
-    }
-    final String? flavorAppId = yamlFlavors[flavor] as String?;
-    if (flavorAppId == null || flavorAppId.isEmpty) {
-      throw Exception('Cannot find "app_id" for $flavor in shorebird.yaml');
-    }
-    appId = flavorAppId;
-  }
-  final StringBuffer yamlContent = StringBuffer();
-  final String? baseUrl = yamlMap['base_url'] as String?;
-  yamlContent.writeln('app_id: $appId');
-  if (baseUrl != null) {
-    yamlContent.writeln('base_url: $baseUrl');
-  }
-  final bool? autoUpdate = yamlMap['auto_update'] as bool?;
-  if (autoUpdate != null) {
-    yamlContent.writeln('auto_update: $autoUpdate');
-  }
-
-  final String? shorebirdPublicKeyEnvVar = Platform.environment['SHOREBIRD_PUBLIC_KEY'];
-  if (shorebirdPublicKeyEnvVar != null) {
-    yamlContent.writeln('patch_public_key: $shorebirdPublicKeyEnvVar');
-  }
-  shorebirdYaml.writeAsStringSync(yamlContent.toString(), flush: true);
-}
-
 /// Extended attributes applied by Finder can cause code signing errors. Remove them.
 /// https://developer.apple.com/library/archive/qa/qa1940/_index.html
 Future<void> removeFinderExtendedAttributes(FileSystemEntity projectDirectory, ProcessUtils processUtils, Logger logger) async {

packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart

@@ -0,0 +1,65 @@
+// Copyright 2024 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:yaml/yaml.dart';
+import 'package:yaml_edit/yaml_edit.dart';
+
+import '../base/file_system.dart';
+import '../build_info.dart';
+import '../globals.dart' as globals;
+
+void updateShorebirdYaml(BuildInfo buildInfo, String shorebirdYamlPath) {
+  final File shorebirdYaml = globals.fs.file(shorebirdYamlPath);
+  if (!shorebirdYaml.existsSync()) {
+    throw Exception('shorebird.yaml not found at $shorebirdYamlPath');
+  }
+  final YamlDocument input = loadYamlDocument(shorebirdYaml.readAsStringSync());
+  final YamlMap yamlMap = input.contents as YamlMap;
+  final Map<String, dynamic> compiled = compileShorebirdYaml(yamlMap, flavor: buildInfo.flavor, environment: globals.platform.environment);
+  // Currently we write out over the same yaml file, we should fix this to
+  // write to a new .json file instead and avoid naming confusion between the
+  // input and compiled files.
+  final YamlEditor yamlEditor = YamlEditor('');
+  yamlEditor.update(<Object?>[], compiled);
+  shorebirdYaml.writeAsStringSync(yamlEditor.toString(), flush: true);
+}
+
+String appIdForFlavor(YamlMap yamlMap, {required String? flavor}) {
+  if (flavor == null) {
+    final String? defaultAppId = yamlMap['app_id'] as String?;
+    if (defaultAppId == null || defaultAppId.isEmpty) {
+      throw Exception('Cannot find "app_id" in shorebird.yaml');
+    }
+    return defaultAppId;
+  }
+
+  final YamlMap? yamlFlavors = yamlMap['flavors'] as YamlMap?;
+  if (yamlFlavors == null) {
+    throw Exception('Cannot find "flavors" in shorebird.yaml.');
+  }
+  final String? flavorAppId = yamlFlavors[flavor] as String?;
+  if (flavorAppId == null || flavorAppId.isEmpty) {
+    throw Exception('Cannot find "app_id" for $flavor in shorebird.yaml');
+  }
+  return flavorAppId;
+}
+
+Map<String, dynamic> compileShorebirdYaml(YamlMap yamlMap, {required String? flavor, required Map<String, String> environment}) {
+  final String appId = appIdForFlavor(yamlMap, flavor: flavor);
+  final Map<String, dynamic> compiled = <String, dynamic>{
+    'app_id': appId,
+  };
+  void copyIfSet(String key) {
+    if (yamlMap[key] != null) {
+      compiled[key] = yamlMap[key];
+    }
+  }
+  copyIfSet('base_url');
+  copyIfSet('auto_update');
+  final String? shorebirdPublicKeyEnvVar = environment['SHOREBIRD_PUBLIC_KEY'];
+  if (shorebirdPublicKeyEnvVar != null) {
+    compiled['patch_public_key'] = shorebirdPublicKeyEnvVar;
+  }
+  return compiled;
+}

packages/flutter_tools/test/general.shard/shorebird/shorebird_yaml_test.dart

@@ -0,0 +1,73 @@
+// Copyright 2024 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_tools/src/shorebird/shorebird_yaml.dart';
+import 'package:test/test.dart';
+import 'package:yaml/yaml.dart';
+
+void main() {
+  group('ShorebirdYaml', () {
+    test('yaml ignores comments', () {
+      const String yamlContents = '''
+# This file is used to configure the Shorebird updater used by your app.
+app_id: 6160a7d8-cc18-4928-1233-05b51c0bb02c
+
+# auto_update controls if Shorebird should automatically update in the background on launch.
+auto_update: false
+''';
+      final YamlDocument input = loadYamlDocument(yamlContents);
+      final YamlMap yamlMap = input.contents as YamlMap;
+      final Map<String, dynamic> compiled =
+          compileShorebirdYaml(yamlMap, flavor: null, environment: <String, String>{});
+      expect(compiled, <String, dynamic>{
+        'app_id': '6160a7d8-cc18-4928-1233-05b51c0bb02c',
+        'auto_update': false,
+      });
+    });
+    test('flavors', () {
+      // These are invalid app_ids but make for easy testing.
+      const String yamlContents = '''
+app_id: 1-a
+auto_update: false
+flavors:
+  foo: 2-a
+  bar: 3-a
+''';
+      final YamlDocument input = loadYamlDocument(yamlContents);
+      final YamlMap yamlMap = input.contents as YamlMap;
+      expect(appIdForFlavor(yamlMap, flavor: null), '1-a');
+      expect(appIdForFlavor(yamlMap, flavor: 'foo'), '2-a');
+      expect(appIdForFlavor(yamlMap, flavor: 'bar'), '3-a');
+      expect(() => appIdForFlavor(yamlMap, flavor: 'unknown'), throwsException);
+    });
+    test('all values', () {
+      // These are invalid app_ids but make for easy testing.
+      const String yamlContents = '''
+app_id: 1-a
+auto_update: false
+flavors:
+  foo: 2-a
+  bar: 3-a
+base_url: https://example.com
+''';
+      final YamlDocument input = loadYamlDocument(yamlContents);
+      final YamlMap yamlMap = input.contents as YamlMap;
+      final Map<String, dynamic> compiled1 =
+          compileShorebirdYaml(yamlMap, flavor: null, environment: <String, String>{});
+      expect(compiled1, <String, dynamic>{
+        'app_id': '1-a',
+        'auto_update': false,
+        'base_url': 'https://example.com',
+      });
+      final Map<String, dynamic> compiled2 =
+          compileShorebirdYaml(yamlMap, flavor: 'foo', environment: <String, String>{'SHOREBIRD_PUBLIC_KEY': '4-a'});
+      expect(compiled2, <String, dynamic>{
+        'app_id': '2-a',
+        'auto_update': false,
+        'base_url': 'https://example.com',
+        'patch_public_key': '4-a',
+      });
+    });
+  });
+}