Skip to content

Compile snapshot_assembly with sdk root set in Xcode #69731

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.

Sign up for GitHub

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

Jump to bottom
Merged
jmagman merged 1 commit into from
Nov 4, 2020
Merged

Compile snapshot_assembly with sdk root set in Xcode #69731

jmagman merged 1 commit into from
Nov 4, 2020

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Nov 4, 2020 •
edited
Loading

Description

With the upcoming support for Apple Silicon ARM simulators, App.framework can no longer contain architectures for both iphoneos and iphonesimulator since they need to be build against different SDKs, but they can no longer be lipod together (lipo complains both binaries contain arm64 and fails).

Primarily this is a problem when building the debug stub framework:

flutter/packages/flutter_tools/lib/src/build_system/targets/ios.dart

Lines 106 to 111 in cb67513

'lipo',
...darwinArchs.map((DarwinArch iosArch) =>
environment.fileSystem.path.join(buildOutputPath, getNameForDarwinArch(iosArch), 'App.framework', 'App')),
'-create',
'-output',
resultPath,

And in flutter build ios-framework that smush together plugin frameworks.

  1. Output App.framework to different iphoneos and iphonesimulator directories - done with Build App.framework directly to build directory #69699.
  2. Refactor the build system to take either the iphoneos and iphonesimulator SDK from Xcode instead of trying to guess from the architectures (to stop assuming x86 == simulator). Use it for aot (this PR).
  3. Remove lipoing from debug stub framework step and only use the requested iphoneos or iphonesimulator SDK, not for both. Refactor flutter build ios-framework as needed (next PR).
  4. Deprecate flutter build ios-framework --universal - Deprecate build ios-framework --universal #69720
  5. Deprecate flutter build aot - Deprecate and replace build aot #49562

This PR actually doesn't change any behavior, but I did steps 2 and 3 at the same time and the patch is too large to easily review. Some of this will make more sense on the next PR.

Related Issues

Support #69334

@jmagman jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. t: xcode "xcodebuild" on iOS and general Xcode project management labels Nov 4, 2020
@jmagman jmagman self-assigned this Nov 4, 2020
@google-cla google-cla bot added the cla: yes label Nov 4, 2020
@jmagman jmagman mentioned this pull request Nov 4, 2020
Closed
9 tasks
jmagman
jmagman commented Nov 4, 2020
View reviewed changes
packages/flutter_tools/lib/src/aot.dart Outdated
Copy link
Member Author

@jmagman jmagman Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is almost dead: b/172093077

packages/flutter_tools/lib/src/base/build.dart Outdated
Copy link
Member Author

@jmagman jmagman Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We take the architecture from the build settings instead.

packages/flutter_tools/lib/src/build_info.dart Outdated
Comment on lines +483 to +487
Copy link
Member Author

@jmagman jmagman Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be used in the next PR when the debug App.framework is migrated.

packages/flutter_tools/lib/src/commands/build_ios_framework.dart Outdated
Comment on lines 378 to +380
Copy link
Member Author

@jmagman jmagman Nov 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually wrong and isn't building anything for the simulator, but that's what it was doing before this PR. Will fix in the next PR.

@jmagman jmagman marked this pull request as ready for review November 4, 2020 02:19
@jmagman jmagman mentioned this pull request Nov 4, 2020
Merged
jonahwilliams
jonahwilliams approved these changes Nov 4, 2020
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xster
Copy link
Member

xster commented Nov 4, 2020

😮 TIL you can quote multiple code lines' permalinks

@jmagman jmagman merged commit 53410c4 into flutter:master Nov 4, 2020
@jmagman jmagman deleted the aot-sdkroot branch November 4, 2020 18:32
@jmagman jmagman mentioned this pull request Nov 5, 2020
Merged
@jmagman jmagman added the platform-host-arm Building on an ARM-based platform label Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xster xster Awaiting requested review from xster

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jmagman jmagman

Labels

platform-host-arm Building on an ARM-based platform platform-ios iOS applications specifically t: xcode "xcodebuild" on iOS and general Xcode project management tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmagman @xster @jonahwilliams