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

### 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) "] ``` https://github.com/flutter/flutter/pull/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: 6abef22251/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: https://github.com/flutter/flutter/pull/154649 This unblocks: https://github.com/flutter/flutter/issues/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: https://github.com/flutter/flutter/pull/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: https://github.com/flutter/flutter/pull/154749 | [Link](https://ci.chromium.org/ui/p/flutter/builders/try.shadow/Mac_ios%20rrect_blur_perf_ios__timeline_summary/8/overview) | â
2024-09-11 13:03:22 -07:00 · 2024-09-11 13:03:22 -07:00 · ea208f824b
commit ea208f824b
parent 3c2e25d7e4
6 changed files with 200 additions and 20 deletions
--- 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'] ?? ''}',
--- 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:
--- 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.
--- 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}');
    }
  }

--- 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,
--- 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);