fix: update macos shorebird.yaml before signing (#73)

* fix: update macos shorebird.yaml before signing

* fix test

* tests

* Move shorebird.yaml processing to assets.dart to update the file on copy

* Remove unnecessary update of shorebird.yaml in ios/mac.dart

* Handle empty string flavor

Author: bryanoltman

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

@@ -1387,43 +1387,6 @@ class FlutterPlugin implements Plugin<Project> {
                     return
                 }
                 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
-
-                  // Read the shorebird.yaml file into a map.
-                  def shorebirdYamlFile = new File("${outputDir}/flutter_assets/shorebird.yaml")
-                  def shorebirdYamlData = yaml.load(shorebirdYamlFile.text)
-
-                  // Update the app_id to the correct flavor if one was provided.
-                  if (variant.flavorName != null && !variant.flavorName.isEmpty()) {
-                    def shorebirdFlavor = shorebirdYamlData.flavors[variant.flavorName]
-                    if (shorebirdFlavor == null) {
-                      throw new GradleException("Flavor '${variant.flavorName}' not found in shorebird.yaml")
-                    }
-                    shorebirdYamlData['app_id'] = shorebirdFlavor
-                  }
-
-                  // Remove any flavors. This is a no-op if the flavors key doesn't exist.
-                  shorebirdYamlData.remove('flavors')
-
-                  // Add a public key if one was provided via an env var.
-                  // Ideally we'd have a way to pass the key as a parameter, but
-                  // an env var was the easiest way to get this working.
-                  def shorebirdPublicKeyEnvVar = System.getenv('SHOREBIRD_PUBLIC_KEY')
-                  if (shorebirdPublicKeyEnvVar != null && !shorebirdPublicKeyEnvVar.isEmpty()) {
-                    shorebirdYamlData['patch_public_key'] = shorebirdPublicKeyEnvVar
-                  }
-
-                  // Write the updated map back to the shorebird.yaml file.
-                  def updatedYamlString = yaml.dumpAsMap(shorebirdYamlData)
-                  shorebirdYamlFile.write(updatedYamlString)
-                }
                 def variantOutput = variant.outputs.first()
                 def processResources = variantOutput.hasProperty(propProcessResourcesProvider) ?
                     variantOutput.processResourcesProvider.get() : variantOutput.processResources

packages/flutter_tools/lib/src/build_system/targets/assets.dart

@@ -14,6 +14,8 @@ import '../../convert.dart';
 import '../../dart/package_map.dart';
 import '../../devfs.dart';
 import '../../flutter_manifest.dart';
+import '../../globals.dart' as globals show platform;
+import '../../shorebird/shorebird_yaml.dart';
 import '../build_system.dart';
 import '../depfile.dart';
 import '../exceptions.dart';
@@ -179,6 +181,19 @@ Future<Depfile> copyAssets(
           }
           if (doCopy) {
             await (content.file as File).copy(file.path);
+            if (file.basename == 'shorebird.yaml') {
+              try {
+                updateShorebirdYaml(
+                  environment.defines[kFlavor],
+                  file.path,
+                  environment: globals.platform.environment,
+                );
+              } on Exception catch (error) {
+                throw Exception(
+                  'Failed to generate shorebird configuration. Error: $error',
+                );
+              }
+            }
           }
         } else {
           await file.writeAsBytes(await entry.value.content.contentsAsBytes());

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

@@ -32,7 +32,6 @@ import '../migrations/xcode_thin_binary_build_phase_input_paths_migration.dart';
 import '../plugins.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';
@@ -549,13 +548,6 @@ Future<XcodeBuildResult> buildXcodeProject({
       }
     }
 
-    try {
-      updateShorebirdYaml(buildInfo, app.shorebirdYamlPath, environment: globals.platform.environment);
-    } on Exception catch (error) {
-      globals.printError('[shorebird] failed to generate shorebird configuration.\n$error');
-      return XcodeBuildResult(success: false);
-    }
-
     return XcodeBuildResult(
         success: true,
         output: outputDir,

packages/flutter_tools/lib/src/macos/build_macos.dart

@@ -22,7 +22,6 @@ import '../migrations/xcode_project_object_version_migration.dart';
 import '../migrations/xcode_script_build_phase_migration.dart';
 import '../migrations/xcode_thin_binary_build_phase_input_paths_migration.dart';
 import '../project.dart';
-import '../shorebird/shorebird_yaml.dart';
 import 'application_package.dart';
 import 'cocoapod_utils.dart';
 import 'migrations/flutter_application_migration.dart';
@@ -222,32 +221,6 @@ Future<void> buildMacOS({
   }
   final String? applicationBundle = MacOSApp.fromMacOSProject(flutterProject.macos).applicationBundle(buildInfo);
   if (applicationBundle != null) {
-    final String shorebirdYamlPath = globals.fs.path.join(
-      applicationBundle,
-      'Contents',
-      'Frameworks',
-      'App.framework',
-      'Resources',
-      'flutter_assets',
-      'shorebird.yaml',
-    );
-    if (globals.fs.isFileSync(shorebirdYamlPath)) {
-      try {
-        updateShorebirdYaml(
-          buildInfo,
-          shorebirdYamlPath,
-          environment: globals.platform.environment,
-        );
-      } on Exception catch (error) {
-        globals.printError(
-          '[shorebird] failed to generate shorebird configuration.\n$error',
-        );
-        throw Exception(
-          'Failed to generate shorebird configuration. Error: $error',
-        );
-      }
-    }
-
     final Directory outputDirectory = globals.fs.directory(applicationBundle);
     // This output directory is the .app folder itself.
     final int? directorySize = globals.os.getDirectorySize(outputDirectory);

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

@@ -6,17 +6,16 @@ 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, {required Map<String, String> environment}) {
+void updateShorebirdYaml(String? flavor, String shorebirdYamlPath, {required Map<String, String> environment}) {
   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: environment);
+  final Map<String, dynamic> compiled = compileShorebirdYaml(yamlMap, flavor: flavor, environment: 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.
@@ -26,7 +25,7 @@ void updateShorebirdYaml(BuildInfo buildInfo, String shorebirdYamlPath, {require
 }
 
 String appIdForFlavor(YamlMap yamlMap, {required String? flavor}) {
-  if (flavor == null) {
+  if (flavor == null || flavor.isEmpty) {
     final String? defaultAppId = yamlMap['app_id'] as String?;
     if (defaultAppId == null || defaultAppId.isEmpty) {
       throw Exception('Cannot find "app_id" in shorebird.yaml');

packages/flutter_tools/lib/src/windows/build_windows.dart

@@ -137,7 +137,7 @@ Future<void> buildWindows(
 
   if (shorebirdYamlFile.existsSync()) {
     try {
-      updateShorebirdYaml(buildInfo, shorebirdYamlFile.path, environment: globals.platform.environment);
+      updateShorebirdYaml(buildInfo.flavor, shorebirdYamlFile.path, environment: globals.platform.environment);
     } on Exception catch (error) {
       globals.printError('[shorebird] failed to generate shorebird configuration.\n$error');
       throw Exception('Failed to generate shorebird configuration');

packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart

@@ -564,6 +564,43 @@ void main() {
     ProcessManager: () => processManager,
   });
 
+  testUsingContext('ReleaseMacOSBundleFlutterAssets updates shorebird.yaml if present', () async {
+    environment.defines[kBuildMode] = 'release';
+    environment.defines[kXcodeAction] = 'install';
+    environment.defines[kFlavor] = 'internal';
+
+    fileSystem.file('bin/cache/artifacts/engine/darwin-x64/vm_isolate_snapshot.bin')
+        .createSync(recursive: true);
+    fileSystem.file('bin/cache/artifacts/engine/darwin-x64/isolate_snapshot.bin')
+        .createSync(recursive: true);
+    fileSystem.file(fileSystem.path.join(environment.buildDir.path, 'App.framework', 'App'))
+        .createSync(recursive: true);
+    final String shorebirdYamlPath = fileSystem.path.join(
+      environment.buildDir.path,
+      'App.framework','Versions',
+      'A',
+      'Resources',
+      'flutter_assets',
+      'shorebird.yaml',
+    );
+    fileSystem.file(fileSystem.path.join(environment.buildDir.path, 'App.framework', 'App'))
+        ..createSync(recursive: true)
+        ..writeAsStringSync('''
+# Some other text that should be removed
+app_id: base-app-id
+flavors:
+  internal: internal-app-id
+  stable: stable-app-id
+''');
+
+    await const ReleaseMacOSBundleFlutterAssets().build(environment);
+
+    expect(fileSystem.file(shorebirdYamlPath).readAsStringSync(), 'app_id: internal-app-id');
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => processManager,
+  });
+
   testUsingContext('DebugMacOSFramework creates expected binary with arm64 only arch', () async {
     environment.defines[kDarwinArchs] = 'arm64';
     processManager.addCommand(

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

@@ -4,7 +4,6 @@
 
 import 'dart:io';
 
-import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/shorebird/shorebird_yaml.dart';
 import 'package:test/test.dart';
 import 'package:yaml/yaml.dart';
@@ -86,12 +85,7 @@ base_url: https://example.com
       final File tempFile = File('${tempDir.path}/shorebird.yaml');
       tempFile.writeAsStringSync(yamlContents);
       updateShorebirdYaml(
-        const BuildInfo(
-          BuildMode.release,
-          'foo',
-          treeShakeIcons: false,
-          packageConfigPath: '',
-        ),
+        'foo',
         tempFile.path,
         environment: <String, String>{'SHOREBIRD_PUBLIC_KEY': '4-a'},
       );