Skip to content
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

feat(ios): use/support/build xcframeworks #12026

Merged
merged 6 commits into from
Sep 10, 2020

Conversation

sgtcoolguy
Copy link
Contributor

@sgtcoolguy sgtcoolguy commented Sep 9, 2020

JIRA: https://jira.appcelerator.org/browse/TIMOB-28116

Description:
This is required for #11955

These are the changes to move to using xcframeworks for APS, tiverify, TitaniumKit and for native modules.

This includes changes to the libraries:

Xcode minimum is bumped to 11 as that is required for xcframework support.

This PR does build/include the built maccatalyst binaries in the xcframeworks. It does not enable macOS as a target platform. #11955 will be used for that.

Note that to avoid breaking existing native modules we will selectively exclude arm64 from simulator builds of apps if we detect a native module that is not an xcframework - or is built with Xcode 11 and is missing arm64 simulator binaries.

If the developer is running an arm64 Mac build of Node (there are none yet officially!), and a native module is assumed to not have arm64 sim support, we'll error out with a message about it. (the "fix" there is for us/them to rebuild the native modules using Xcode 12 w/ xcframework support)

TODO

  • Address @vijaysingh-axway review feedback around use of Info.plist from xcframework to determine arch/folder name support.
  • make the arm64 sim exclusion smarter by handling if an xcframework has no arm64 sim support.

@build
Copy link
Contributor

build commented Sep 9, 2020

Fails
🚫 Tests have failed, see below for more information.
Warnings
⚠️

iphone/cli/hooks/frameworks.js#L60 - iphone/cli/hooks/frameworks.js line 60 – Avoid calling back inside of a promise. (promise/no-callback-in-promise)

⚠️

iphone/cli/hooks/frameworks.js#L60 - iphone/cli/hooks/frameworks.js line 60 – Expected catch() or return (promise/catch-or-return)

⚠️

iphone/cli/hooks/frameworks.js#L79 - iphone/cli/hooks/frameworks.js line 79 – Avoid wrapping return values in Promise.resolve (promise/no-return-wrap)

⚠️

iphone/cli/hooks/frameworks.js#L429 - iphone/cli/hooks/frameworks.js line 429 – Each then() should return a value or throw (promise/always-return)

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 1 tests have failed There are 1 tests failing and 711 skipped out of 8156 total tests.

Tests:

ClassnameNameTimeError
ios.ipad.Titanium.UI.iOS.CollisionBehavior.exampleworks (14.0)15
Error: timeout of 15000ms exceeded
file:///Users/build/Library/Developer/CoreSimulator/Devices/AEB0AAF4-FED7-4BF5-8DDC-233DE4C6A1FF/data/Containers/Bundle/Application/D13CAFE1-684E-411F-8DCE-C42F4582C3BA/mocha.app/ti-mocha.js:4326:27

Generated by 🚫 dangerJS against 692b1c0

@sgtcoolguy
Copy link
Contributor Author

@vijaysingh-axway This is the xcframeworks only changes. I addressed your initial feedback re: Xcode 11 built xcframeworks in a lazy/dumb way - checking for the Xcode 12+ dir name and falling back to the Xcode 11 folder name (rather than try and get fancy with parsing Info.plist).

If we see an xcframework built on Xcode 11 missing arm64, we'll exclude arm64 on app simulator builds just as we would for a non-xcframework based native module.

@@ -6,7 +6,7 @@
*/
#import <TitaniumKit/TiModule.h>

#ifdef USE_TI_CONTACTS
#if defined(USE_TI_CONTACTS) && (!TARGET_OS_MACCATALYST || IS_SDK_IOS_14)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that TARGET_OS_MACCATALYST is getting used. I guess it is not removed intentionally to save time. I think it is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Contacts API is available in Mac Catalyst 14+. So I intentionally guarded here like this.

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

@sgtcoolguy Changes looks good. I have left few comments. Basically questions. Can you look in that? Thanks!

iphone/cli/commands/_buildModule.js Show resolved Hide resolved
iphone/iphone/Titanium.entitlements Outdated Show resolved Hide resolved
iphone/package.json Show resolved Hide resolved
@ewanharris
Copy link
Collaborator

@sgtcoolguy couple notes from me using this.

[DEBUG] Running: /Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -create-xcframework -library /Users/awam/git/modules/ti.coremotion/ios/build/iphoneos.xcarchive/Products/usr/local/lib/libti.coremotion.a -headers /Users/awam/git/modules/ti.coremotion/ios/build/dummyheader/ -library /Users/awam/git/modules/ti.coremotion/ios/build/iphonesimulator.xcarchive/Products/usr/local/lib/libti.coremotion.a -headers /Users/awam/git/modules/ti.coremotion/ios/build/dummyheader/ -library /Users/awam/git/modules/ti.coremotion/ios/build/macosx.xcarchive/Products/usr/local/lib/libti.coremotion.a -headers /Users/awam/git/modules/ti.coremotion/ios/build/dummyheader/ -output /Users/awam/git/modules/ti.coremotion/ios/build/ti.coremotion.xcframework
[ERROR] Failed to generate universal binary (code 70):
[ERROR] “libti.coremotion.a” couldn’t be copied to “ios-arm64_x86_64-maccatalyst” because an item with the same name already exists.

…xcframeworks

Modules built before xcframework support do not have an arm64 sim arch/target
To avoid having to break all modules, we'll try and exclude arm64 from sim builds
This will eventually break if app is built on arm64 apple silicon and a new
xcframework based module hasn't been shipped.
- preserve symlinks in module xcframeworks/zips
@sgtcoolguy
Copy link
Contributor Author

@ewanharris I've incorporated your feedback. Updated the line for the framework search path sniffing, and right before we create-xcframework we'll wipe the destination if it exists to avoid the "couldn't be copied" error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants