From 994ee4da7cce8f542dc1745f949d04aebd1e5e84 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 17 Jan 2025 12:46:22 -0500 Subject: [PATCH 1/6] fix: update macos shorebird.yaml before signing --- .../lib/src/build_system/targets/macos.dart | 19 ++++++++++++- packages/flutter_tools/lib/src/ios/mac.dart | 2 +- .../lib/src/macos/build_macos.dart | 27 ------------------- .../lib/src/shorebird/shorebird_yaml.dart | 5 ++-- .../lib/src/windows/build_windows.dart | 2 +- 5 files changed, 22 insertions(+), 33 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/macos.dart b/packages/flutter_tools/lib/src/build_system/targets/macos.dart index 1dfd62849c382..73e56ebb2a82d 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/macos.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/macos.dart @@ -10,8 +10,9 @@ import '../../base/file_system.dart'; import '../../base/io.dart'; import '../../base/process.dart'; import '../../build_info.dart'; -import '../../globals.dart' as globals show xcode; +import '../../globals.dart' as globals show platform, xcode; import '../../reporting/reporting.dart'; +import '../../shorebird/shorebird_yaml.dart'; import '../build_system.dart'; import '../depfile.dart'; import '../exceptions.dart'; @@ -683,6 +684,22 @@ class ReleaseMacOSBundleFlutterAssets extends MacOSBundleFlutterAssets { bool buildSuccess = true; try { await super.build(environment); + final ResolvedFiles resolvedOutputs = resolveOutputs(environment); + for (final File outputSource in resolvedOutputs.sources) { + if (outputSource.basename == 'shorebird.yaml') { + try { + updateShorebirdYaml( + environment.defines[kFlavor], + outputSource.path, + environment: globals.platform.environment, + ); + } on Exception catch (error) { + throw Exception( + 'Failed to generate shorebird configuration. Error: $error', + ); + } + } + } } catch (_) { // ignore: avoid_catches_without_on_clauses buildSuccess = false; rethrow; diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart index 2490364405026..3c82996dfbdfb 100644 --- a/packages/flutter_tools/lib/src/ios/mac.dart +++ b/packages/flutter_tools/lib/src/ios/mac.dart @@ -550,7 +550,7 @@ Future buildXcodeProject({ } try { - updateShorebirdYaml(buildInfo, app.shorebirdYamlPath, environment: globals.platform.environment); + updateShorebirdYaml(buildInfo.flavor, app.shorebirdYamlPath, environment: globals.platform.environment); } on Exception catch (error) { globals.printError('[shorebird] failed to generate shorebird configuration.\n$error'); return XcodeBuildResult(success: false); diff --git a/packages/flutter_tools/lib/src/macos/build_macos.dart b/packages/flutter_tools/lib/src/macos/build_macos.dart index 69dcc06e2dafe..5b8b186a238a7 100644 --- a/packages/flutter_tools/lib/src/macos/build_macos.dart +++ b/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 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); diff --git a/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart b/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart index ba103d0109bf4..805fd5c9c0f83 100644 --- a/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart +++ b/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 environment}) { +void updateShorebirdYaml(String? flavor, String shorebirdYamlPath, {required Map 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 compiled = compileShorebirdYaml(yamlMap, flavor: buildInfo.flavor, environment: environment); + final Map 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. diff --git a/packages/flutter_tools/lib/src/windows/build_windows.dart b/packages/flutter_tools/lib/src/windows/build_windows.dart index d82c68345c94b..c512582b960cd 100644 --- a/packages/flutter_tools/lib/src/windows/build_windows.dart +++ b/packages/flutter_tools/lib/src/windows/build_windows.dart @@ -137,7 +137,7 @@ Future 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'); From c9071ad40b03a52ee07be32df5fd6366e89687fe Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 17 Jan 2025 12:54:13 -0500 Subject: [PATCH 2/6] fix test --- .../test/general.shard/shorebird/shorebird_yaml_test.dart | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/flutter_tools/test/general.shard/shorebird/shorebird_yaml_test.dart b/packages/flutter_tools/test/general.shard/shorebird/shorebird_yaml_test.dart index 7b1ba4a5622a6..602de37681555 100644 --- a/packages/flutter_tools/test/general.shard/shorebird/shorebird_yaml_test.dart +++ b/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: {'SHOREBIRD_PUBLIC_KEY': '4-a'}, ); From d60f7e63d16e958aedb1c3d960347588005650b0 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 17 Jan 2025 16:16:38 -0500 Subject: [PATCH 3/6] tests --- .../build_system/targets/macos_test.dart | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart index a9178c78036aa..da1a9affa43e6 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart +++ b/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: { + FileSystem: () => fileSystem, + ProcessManager: () => processManager, + }); + testUsingContext('DebugMacOSFramework creates expected binary with arm64 only arch', () async { environment.defines[kDarwinArchs] = 'arm64'; processManager.addCommand( From aace41afa22b8d0e15be493e1dfe338c91ada87e Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Tue, 21 Jan 2025 15:51:25 -0500 Subject: [PATCH 4/6] Move shorebird.yaml processing to assets.dart to update the file on copy --- .../lib/src/build_system/targets/assets.dart | 15 +++++++++++++++ .../lib/src/build_system/targets/macos.dart | 19 +------------------ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/assets.dart b/packages/flutter_tools/lib/src/build_system/targets/assets.dart index ce10d1222dfef..b3c8f671d5398 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/assets.dart +++ b/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 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()); diff --git a/packages/flutter_tools/lib/src/build_system/targets/macos.dart b/packages/flutter_tools/lib/src/build_system/targets/macos.dart index 73e56ebb2a82d..1dfd62849c382 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/macos.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/macos.dart @@ -10,9 +10,8 @@ import '../../base/file_system.dart'; import '../../base/io.dart'; import '../../base/process.dart'; import '../../build_info.dart'; -import '../../globals.dart' as globals show platform, xcode; +import '../../globals.dart' as globals show xcode; import '../../reporting/reporting.dart'; -import '../../shorebird/shorebird_yaml.dart'; import '../build_system.dart'; import '../depfile.dart'; import '../exceptions.dart'; @@ -684,22 +683,6 @@ class ReleaseMacOSBundleFlutterAssets extends MacOSBundleFlutterAssets { bool buildSuccess = true; try { await super.build(environment); - final ResolvedFiles resolvedOutputs = resolveOutputs(environment); - for (final File outputSource in resolvedOutputs.sources) { - if (outputSource.basename == 'shorebird.yaml') { - try { - updateShorebirdYaml( - environment.defines[kFlavor], - outputSource.path, - environment: globals.platform.environment, - ); - } on Exception catch (error) { - throw Exception( - 'Failed to generate shorebird configuration. Error: $error', - ); - } - } - } } catch (_) { // ignore: avoid_catches_without_on_clauses buildSuccess = false; rethrow; From 42a54d8967e2b80b6dac09d3f981363a8158aacb Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Tue, 21 Jan 2025 17:18:33 -0500 Subject: [PATCH 5/6] Remove unnecessary update of shorebird.yaml in ios/mac.dart --- packages/flutter_tools/lib/src/ios/mac.dart | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart index 3c82996dfbdfb..38fa6ebb20ad6 100644 --- a/packages/flutter_tools/lib/src/ios/mac.dart +++ b/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 buildXcodeProject({ } } - try { - updateShorebirdYaml(buildInfo.flavor, 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, From 201d16d2e43ea51a9771153d35da4260a9d8a112 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Tue, 21 Jan 2025 17:33:24 -0500 Subject: [PATCH 6/6] Handle empty string flavor --- .../gradle/src/main/groovy/flutter.groovy | 37 ------------------- .../lib/src/shorebird/shorebird_yaml.dart | 2 +- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy index 935f2cf9f5111..6555e19485a8c 100644 --- a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy +++ b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy @@ -1387,43 +1387,6 @@ class FlutterPlugin implements Plugin { 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 diff --git a/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart b/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart index 805fd5c9c0f83..9e40f392b7eb8 100644 --- a/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart +++ b/packages/flutter_tools/lib/src/shorebird/shorebird_yaml.dart @@ -25,7 +25,7 @@ void updateShorebirdYaml(String? flavor, String shorebirdYamlPath, {required Map } 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');