Fix flutter run on Mac x64 hosts if Swift Package Manager is enabled (flutter#154645)

loic-sharma · web-flow · commit ea208f824b53 · 2024-09-11T20:03:22.000Z
### Problem Enabling the Swift Package Manager feature caused post-submit tests to fail on Mac x64 hosts: <details> <summary>Example error...</summary> https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20rrect_blur_perf_ios__timeline_summary/575/overview ``` â�¦ ... flutter --verbose assemble ... -dIosArchs=x86_64 ... profile_unpack_ios Target profile_unpack_ios failed: Exception: Binary ... build/ios/Profile-iphoneos/Flutter.framework/Flutter does not contain x86_64. Running lipo -info: Non-fat file: ... build/ios/Profile-iphoneos/Flutter.framework/Flutter is architecture: arm64 #0 UnpackIOS._thinFramework (package:flutter_tools/src/build_system/targets/ios.dart:351:7) <asynchronous suspension> #1 UnpackIOS.build (package:flutter_tools/src/build_system/targets/ios.dart:298:5) <asynchronous suspension> ... ``` </details> ### Reproduction On a mac x64 host: 1. Switch to the latest master channel: `flutter channel master ; flutter upgrade` 1. Disable the Swift Package Manager feature: `flutter config --no-enable-swift-package-manager` 2. Create a Flutter project 2. [Edit the Xcode project manually to add the prepare pre-action](https://docs.flutter.dev/packages-and-plugins/swift-package-manager/for-app-developers#step-2-add-run-prepare-flutter-framework-script-pre-action) 3. Run `flutter run` (`flutter build ios` does not reproduce this issue). ### Background Previously, the Flutter framework was unpacked in the Xcode target's build. Unfortunately, this happens after Swift packages are built; this prevented Swift packages from using the Flutter framework. To fix this, we added an Xcode pre-action that unpacks the Flutter framework _before_ Swift Package Manager builds packages. The Xcode target still runs the Flutter framework unpack step, but this step no-ops if the unpack step has the exact same inputs. ```mermaid flowchart LR A[flutter run -d iphone] --> B(Build Xcode project) B --> C(Xcode 'prepare framework' pre-action) B --> G[Build Swift packages] B --> D(Build 'Runner' target) C --> E[Unpack Flutter framework #1] D --> F[" Unpack Flutter framework #2 (No-ops if inputs are same as #1) "] ``` flutter#150052 added an optimization that made it more likely the second unpack step no-ops by fixing a case where the target architecture input could be different: > When using SwiftPM, we use `flutter assemble` in an Xcode Pre-action to run the `debug_unpack_macos` (or profile/release) target. This target is also later used in a Run Script build phase. Depending on `ARCHS` build setting, the Flutter/FlutterMacOS binary is thinned. In the Run Script build phase, `ARCHS` is filtered to the active arch. However, in the Pre-action it doesn't always filter to the active arch. As a workaround, assume arm64 if the [`NATIVE_ARCH`](https://developer.apple.com/documentation/xcode/build-settings-reference/#NATIVEARCH) is arm, otherwise assume x86_64. This optimization is only applied if [`ONLY_ACTIVE_ARCH`](https://developer.apple.com/library/archive/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW157) is `YES`. > [!IMPORTANT] > [`ONLY_ACTIVE_ARCH`](https://developer.apple.com/library/archive/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW157)'s name is misleading. It specifies whether the product includes only object code for the native architecture. > > A value of `YES` means the product includes only code for the native architecture ([NATIVE_ARCH](https://developer.apple.com/library/archive/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW59)). > > A value of `NO` means the product includes code for the architectures specified in [ARCHS (Architectures)](https://developer.apple.com/library/archive/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW62). ### Problem `buildXcodeProject` incorrectly always sets `ONLY_ACTIVE_ARCH` to `YES` if the Xcode built is for a single architecture: https://github.com/flutter/flutter/blob/6abef222514e8039bde5c47ab7864abbc4caf7e8/packages/flutter_tools/lib/src/ios/mac.dart#L353-L361 This is incorrect! If the host architecture is `x64` but the target architecture is `arm64`, [`ONLY_ACTIVE_ARCH`](https://developer.apple.com/library/archive/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW157) should be `NO`. This caused the prepare pre-action to incorrectly use x64 as the target architecture for arm64 devices on an x64 host, which in turn caused builds to fail if Swift Package Manager was enabled. ### Solution This change updates `buildXcodeProject` to set `ONLY_ACTIVE_ARCH` correctly. This change also updates the prepare pre-action's to be more conservative in applying the optimization that filters the target architecture. This ensures that the build still works (but without the optimization) if `ONLY_ACTIVE_ARCH` is incorrectly set. Follow-up PR: flutter#154649 This unblocks: flutter#151567 ### DeviceLab test This problem reproduces if you `flutter run` to an iPhone Arm64 device from an x64 mac host with the Swift Package Manager feature enabled. I ran an affected DeviceLab test to verify the fix works as expected: Description | CI test | Result -- | -- | -- SwiftPM enabled without this fix: flutter#154750 | [Link](https://ci.chromium.org/ui/p/flutter/builders/try.shadow/Mac_ios%20rrect_blur_perf_ios__timeline_summary/7/overview) | â�� SwiftPM enabled with this fix: flutter#154749 | [Link](https://ci.chromium.org/ui/p/flutter/builders/try.shadow/Mac_ios%20rrect_blur_perf_ios__timeline_summary/8/overview) | â�
diff --git a/packages/flutter_tools/bin/xcode_backend.dart b/packages/flutter_tools/bin/xcode_backend.dart
@@ -461,18 +461,20 @@ class Context {
       flutterArgs.add('--local-engine-host=${environment['LOCAL_ENGINE_HOST']}');
     }
 
-    String architectures = environment['ARCHS'] ?? '';
-    if (command == 'prepare') {
-      // The "prepare" command runs in a pre-action script, which doesn't always
-      // filter the "ARCHS" build setting to only the active arch. To workaround,
-      // if "ONLY_ACTIVE_ARCH" is true and the "NATIVE_ARCH" is arm, assume the
-      // active arch is also arm to improve caching. If this assumption is
-      // incorrect, it will later be corrected by the "build" command.
-      if (environment['ONLY_ACTIVE_ARCH'] == 'YES' && environment['NATIVE_ARCH'] != null) {
-        if (environment['NATIVE_ARCH']!.contains('arm')) {
-          architectures = 'arm64';
-        } else {
-          architectures = 'x86_64';
+    // The "prepare" command runs in a pre-action script, which doesn't always
+    // filter the "ARCHS" build setting. Attempt to filter the architecture
+    // to improve caching. If this filter is incorrect, it will later be
+    // corrected by the "build" command.
+    String archs = environment['ARCHS'] ?? '';
+    if (command == 'prepare' && archs.contains(' ')) {
+      // If "ONLY_ACTIVE_ARCH" is "YES", the product includes only code for the
+      // native architecture ("NATIVE_ARCH").
+      final String? nativeArch = environment['NATIVE_ARCH'];
+      if (environment['ONLY_ACTIVE_ARCH'] == 'YES' && nativeArch != null) {
+        if (nativeArch.contains('arm64') && archs.contains('arm64')) {
+          archs = 'arm64';
+        } else if (nativeArch.contains('x86_64') && archs.contains('x86_64')) {
+          archs = 'x86_64';
         }
       }
     }
@@ -485,7 +487,7 @@ class Context {
       '-dTargetFile=$targetPath',
       '-dBuildMode=$buildMode',
       if (environment['FLAVOR'] != null) '-dFlavor=${environment['FLAVOR']}',
-      '-dIosArchs=$architectures',
+      '-dIosArchs=$archs',
       '-dSdkRoot=${environment['SDKROOT'] ?? ''}',
       '-dSplitDebugInfo=${environment['SPLIT_DEBUG_INFO'] ?? ''}',
       '-dTreeShakeIcons=${environment['TREE_SHAKE_ICONS'] ?? ''}',
diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart
@@ -648,8 +648,17 @@ class CachedArtifacts implements Artifacts {
     switch (artifact) {
       case Artifact.genSnapshot:
         assert(mode != BuildMode.debug, 'Artifact $artifact only available in non-debug mode.');
-        final String hostPlatform = getNameForHostPlatform(getCurrentHostPlatform());
-        return _fileSystem.path.join(engineDir, hostPlatform, _artifactToFileName(artifact, _platform));
+
+        // TODO(cbracken): Build Android gen_snapshot as Arm64 binary to run
+        // natively on Apple Silicon. See:
+        // https://github.com/flutter/flutter/issues/152281
+        HostPlatform hostPlatform = getCurrentHostPlatform();
+        if (hostPlatform == HostPlatform.darwin_arm64) {
+          hostPlatform = HostPlatform.darwin_x64;
+        }
+
+        final String hostPlatformName = getNameForHostPlatform(hostPlatform);
+        return _fileSystem.path.join(engineDir, hostPlatformName, _artifactToFileName(artifact, _platform));
       case Artifact.engineDartSdkPath:
       case Artifact.engineDartBinary:
       case Artifact.engineDartAotRuntime:
diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart
@@ -788,9 +788,23 @@ AndroidArch getAndroidArchForName(String platform) {
   };
 }
 
+DarwinArch getCurrentDarwinArch() {
+  return switch (globals.os.hostPlatform) {
+    HostPlatform.darwin_arm64 => DarwinArch.arm64,
+    HostPlatform.darwin_x64 => DarwinArch.x86_64,
+    final HostPlatform unsupported => throw Exception(
+      'Unsupported Darwin host platform "$unsupported"',
+    ),
+  };
+}
+
 HostPlatform getCurrentHostPlatform() {
   if (globals.platform.isMacOS) {
-    return HostPlatform.darwin_x64;
+    return switch (getCurrentDarwinArch()) {
+      DarwinArch.arm64 => HostPlatform.darwin_arm64,
+      DarwinArch.x86_64 => HostPlatform.darwin_x64,
+      DarwinArch.armv7 => throw Exception('Unsupported macOS arch "amv7"'),
+    };
   }
   if (globals.platform.isLinux) {
     // support x64 and arm64 architecture.
diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart
@@ -351,12 +351,15 @@ Future<XcodeBuildResult> buildXcodeProject({
   }
 
   if (activeArch != null) {
-    final String activeArchName = activeArch.name;
-    buildCommands.add('ONLY_ACTIVE_ARCH=YES');
     // Setting ARCHS to $activeArchName will break the build if a watchOS companion app exists,
     // as it cannot be build for the architecture of the Flutter app.
     if (!hasWatchCompanion) {
-      buildCommands.add('ARCHS=$activeArchName');
+      // ONLY_ACTIVE_ARCH specifies whether the product includes only code for
+      // the native architecture.
+      final bool onlyActiveArch = activeArch == getCurrentDarwinArch();
+
+      buildCommands.add('ONLY_ACTIVE_ARCH=${onlyActiveArch? 'YES' : 'NO'}');
+      buildCommands.add('ARCHS=${activeArch.name}');
     }
   }
 
diff --git a/packages/flutter_tools/test/general.shard/ios/ios_device_start_nonprebuilt_test.dart b/packages/flutter_tools/test/general.shard/ios/ios_device_start_nonprebuilt_test.dart
@@ -10,6 +10,7 @@ import 'package:file_testing/file_testing.dart';
 import 'package:flutter_tools/src/artifacts.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/base/version.dart';
 import 'package:flutter_tools/src/build_info.dart';
@@ -79,6 +80,10 @@ final FakePlatform macPlatform = FakePlatform(
   environment: <String, String>{},
 );
 
+final FakeOperatingSystemUtils os = FakeOperatingSystemUtils(
+  hostPlatform: HostPlatform.darwin_arm64,
+);
+
 void main() {
   late Artifacts artifacts;
   late String iosDeployPath;
@@ -153,6 +158,7 @@ void main() {
       ProcessManager: () => processManager,
       FileSystem: () => fileSystem,
       Logger: () => logger,
+      OperatingSystemUtils: () => os,
       Platform: () => macPlatform,
       XcodeProjectInterpreter: () => FakeXcodeProjectInterpreter(buildSettings: const <String, String>{
         'WRAPPER_NAME': 'My Super Awesome App.app',
@@ -243,6 +249,92 @@ void main() {
       ProcessManager: () => processManager,
       FileSystem: () => fileSystem,
       Logger: () => logger,
+      OperatingSystemUtils: () => os,
+      Platform: () => macPlatform,
+      XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
+      Xcode: () => xcode,
+    });
+
+    testUsingContext('ONLY_ACTIVE_ARCH is NO if different host and target architectures', () async {
+      // Host architecture is x64, target architecture is arm64.
+      final IOSDevice iosDevice = setUpIOSDevice(
+        fileSystem: fileSystem,
+        processManager: processManager,
+        logger: logger,
+        artifacts: artifacts,
+      );
+      setUpIOSProject(fileSystem);
+      final FlutterProject flutterProject = FlutterProject.fromDirectory(fileSystem.currentDirectory);
+      final BuildableIOSApp buildableIOSApp = BuildableIOSApp(flutterProject.ios, 'flutter', 'My Super Awesome App');
+      fileSystem.directory('build/ios/Release-iphoneos/My Super Awesome App.app').createSync(recursive: true);
+
+      processManager.addCommand(FakeCommand(command: _xattrArgs(flutterProject)));
+      processManager.addCommand(const FakeCommand(command: <String>[
+        'xcrun',
+        'xcodebuild',
+        '-configuration',
+        'Release',
+        '-quiet',
+        '-workspace',
+        'Runner.xcworkspace',
+        '-scheme',
+        'Runner',
+        'BUILD_DIR=/build/ios',
+        '-sdk',
+        'iphoneos',
+        '-destination',
+        'id=123',
+        'ONLY_ACTIVE_ARCH=NO',
+        'ARCHS=arm64',
+        '-resultBundlePath',
+        '/.tmp_rand0/flutter_ios_build_temp_dirrand0/temporary_xcresult_bundle',
+        '-resultBundleVersion',
+        '3',
+        'FLUTTER_SUPPRESS_ANALYTICS=true',
+        'COMPILER_INDEX_STORE_ENABLE=NO',
+      ]));
+      processManager.addCommand(const FakeCommand(command: <String>[
+        'rsync',
+        '-8',
+        '-av',
+        '--delete',
+        'build/ios/Release-iphoneos/My Super Awesome App.app',
+        'build/ios/iphoneos',
+      ]));
+      processManager.addCommand(FakeCommand(
+        command: <String>[
+          iosDeployPath,
+          '--id',
+          '123',
+          '--bundle',
+          'build/ios/iphoneos/My Super Awesome App.app',
+          '--app_deltas',
+          'build/ios/app-delta',
+          '--no-wifi',
+          '--justlaunch',
+          '--args',
+          const <String>[
+            '--enable-dart-profiling',
+          ].join(' '),
+        ])
+      );
+
+      final LaunchResult launchResult = await iosDevice.startApp(
+        buildableIOSApp,
+        debuggingOptions: DebuggingOptions.disabled(BuildInfo.release),
+        platformArgs: <String, Object>{},
+      );
+
+      expect(fileSystem.directory('build/ios/iphoneos'), exists);
+      expect(launchResult.started, true);
+      expect(processManager, hasNoRemainingExpectations);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => processManager,
+      FileSystem: () => fileSystem,
+      Logger: () => logger,
+      OperatingSystemUtils: () => FakeOperatingSystemUtils(
+        hostPlatform: HostPlatform.darwin_x64,
+      ),
       Platform: () => macPlatform,
       XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
       Xcode: () => xcode,
@@ -362,6 +454,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -396,6 +489,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -430,6 +524,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -465,6 +560,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -531,6 +627,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -606,6 +703,7 @@ void main() {
           ProcessManager: () => FakeProcessManager.any(),
           FileSystem: () => fileSystem,
           Logger: () => logger,
+          OperatingSystemUtils: () => os,
           Platform: () => macPlatform,
           XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
           Xcode: () => xcode,
@@ -686,6 +784,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -764,6 +863,7 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
         FileSystem: () => fileSystem,
         Logger: () => logger,
+        OperatingSystemUtils: () => os,
         Platform: () => macPlatform,
         XcodeProjectInterpreter: () => fakeXcodeProjectInterpreter,
         Xcode: () => xcode,
@@ -839,6 +939,7 @@ IOSDevice setUpIOSDevice({
   bool isCoreDevice = false,
   IOSCoreDeviceControl? coreDeviceControl,
   FakeXcodeDebug? xcodeDebug,
+  DarwinArch cpuArchitecture = DarwinArch.arm64,
 }) {
   artifacts ??= Artifacts.test();
   final Cache cache = Cache.test(
@@ -872,7 +973,7 @@ IOSDevice setUpIOSDevice({
     ),
     coreDeviceControl: coreDeviceControl ?? FakeIOSCoreDeviceControl(),
     xcodeDebug: xcodeDebug ?? FakeXcodeDebug(),
-    cpuArchitecture: DarwinArch.arm64,
+    cpuArchitecture: cpuArchitecture,
     connectionInterface: DeviceConnectionInterface.attached,
     isConnected: true,
     isPaired: true,
diff --git a/packages/flutter_tools/test/general.shard/xcode_backend_test.dart b/packages/flutter_tools/test/general.shard/xcode_backend_test.dart
@@ -450,6 +450,57 @@ void main() {
       expect(context.stderr, isEmpty);
     });
 
+    test('does not assumes ARCHS if ARCHS and NATIVE_ARCH are different', () {
+      final Directory buildDir = fileSystem.directory('/path/to/builds')
+        ..createSync(recursive: true);
+      final Directory flutterRoot = fileSystem.directory('/path/to/flutter')
+        ..createSync(recursive: true);
+      final File pipe = fileSystem.file('/tmp/pipe')
+        ..createSync(recursive: true);
+      const String buildMode = 'Debug';
+      final TestContext context = TestContext(
+        <String>['prepare'],
+        <String, String>{
+          'BUILT_PRODUCTS_DIR': buildDir.path,
+          'CONFIGURATION': buildMode,
+          'FLUTTER_ROOT': flutterRoot.path,
+          'INFOPLIST_PATH': 'Info.plist',
+          'ARCHS': 'arm64',
+          'ONLY_ACTIVE_ARCH': 'YES',
+          'NATIVE_ARCH': 'x86_64',
+        },
+        commands: <FakeCommand>[
+          FakeCommand(
+            command: <String>[
+              '${flutterRoot.path}/bin/flutter',
+              'assemble',
+              '--no-version-check',
+              '--output=${buildDir.path}/',
+              '-dTargetPlatform=ios',
+              '-dTargetFile=lib/main.dart',
+              '-dBuildMode=${buildMode.toLowerCase()}',
+              '-dIosArchs=arm64',
+              '-dSdkRoot=',
+              '-dSplitDebugInfo=',
+              '-dTreeShakeIcons=',
+              '-dTrackWidgetCreation=',
+              '-dDartObfuscation=',
+              '-dAction=',
+              '-dFrontendServerStarterPath=',
+              '--ExtraGenSnapshotOptions=',
+              '--DartDefines=',
+              '--ExtraFrontEndOptions=',
+              '-dPreBuildAction=PrepareFramework',
+              'debug_unpack_ios',
+            ],
+          ),
+        ],
+        fileSystem: fileSystem,
+        scriptOutputStreamFile: pipe,
+      )..run();
+      expect(context.stderr, isEmpty);
+    });
+
     test('does not assumes ARCHS if ONLY_ACTIVE_ARCH is not YES', () {
       final Directory buildDir = fileSystem.directory('/path/to/builds')
         ..createSync(recursive: true);

Original file line number	Diff line number	Diff line change
`@@ -351,12 +351,15 @@ Future<XcodeBuildResult> buildXcodeProject({`
`351`	`351`	`}`
`352`	`352`
`353`	`353`	`if (activeArch != null) {`
`354`		`- final String activeArchName = activeArch.name;`
`355`		`- buildCommands.add('ONLY_ACTIVE_ARCH=YES');`
`356`	`354`	`// Setting ARCHS to $activeArchName will break the build if a watchOS companion app exists,`
`357`	`355`	`// as it cannot be build for the architecture of the Flutter app.`
`358`	`356`	`if (!hasWatchCompanion) {`
`359`		`- buildCommands.add('ARCHS=$activeArchName');`
	`357`	`+ // ONLY_ACTIVE_ARCH specifies whether the product includes only code for`
	`358`	`+ // the native architecture.`
	`359`	`+ final bool onlyActiveArch = activeArch == getCurrentDarwinArch();`
	`360`	`+`
	`361`	`+ buildCommands.add('ONLY_ACTIVE_ARCH=${onlyActiveArch? 'YES' : 'NO'}');`
	`362`	`+ buildCommands.add('ARCHS=${activeArch.name}');`
`360`	`363`	`}`
`361`	`364`	`}`
`362`	`365`