-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[DO NOT MERGE] Verify post submit tests fail without #154645 #154750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
loic-sharma
wants to merge
1
commit into
flutter:master
from
loic-sharma:spm_only_active_archs_baseline_test
Closed
[DO NOT MERGE] Verify post submit tests fail without #154645 #154750
loic-sharma
wants to merge
1
commit into
flutter:master
from
loic-sharma:spm_only_active_archs_baseline_test
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 6, 2024
auto-submit bot
pushed a commit
that referenced
this pull request
Sep 11, 2024
#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) "] ``` #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: #154649 This unblocks: #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: #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: #154749 | [Link](https://ci.chromium.org/ui/p/flutter/builders/try.shadow/Mac_ios%20rrect_blur_perf_ios__timeline_summary/8/overview) | â�
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CI test: https://ci.chromium.org/ui/p/flutter/builders/try.shadow/Mac_ios%20rrect_blur_perf_ios__timeline_summary/7/overview