NotificationService is an Application Extension and hence a separate XCode target so it needs its own Compile Sources definition. Moreover using UI - related API is not allow in application extension. Therefore in order to use some expo libraries (parts of which use UI API, but not the ones we use in NotificationService) I needed to programatically disable APPLICATION_EXTENSION_API_ONLY flag in Podfile.
Details
Details
Firstly succesfully compile iOS app for both release and debug version. Then place calls to methods from CommonCpp (SQLiteQueryExecutor in particular) inside NotificationService callback and send push notification to running app instance on physical device. Check whether CommonCpp is working inside NotificationService callbacks as expected.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-490
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Change NotificationService target so that it uses as little react as possible to compile and run EXSecureStore.
Comment Actions
Podfile changes look clean!
| native/ios/Comm.xcodeproj/project.pbxproj | ||
|---|---|---|
| 1113 ↗ | (On Diff #10140) | Why is that? Does it get set to NO for some other target? Does it matter that it is being set to YES for Pods-NotificationService.debug.xcconfig here? |
| native/ios/Podfile | ||
| 61 ↗ | (On Diff #10140) | I don't think this was addressed |
| native/ios/Comm.xcodeproj/project.pbxproj | ||
|---|---|---|
| 1113 ↗ | (On Diff #10140) | Answering 1): I did not set this flag to NO for any target in XCode. When I attempted to do so for NotificationService I could not compile this target. Only after setting it in Podfile inside post_install call I was able to compile Notification Service with expo dependencies. |
| native/ios/Podfile | ||
| 61 ↗ | (On Diff #10140) | Sorry, my mistake. I should remove this unnecessary space shortly. |
| native/ios/Comm.xcodeproj/project.pbxproj | ||
|---|---|---|
| 1113 ↗ | (On Diff #10140) | I don't think either of my questions has been answered in detail. My first question: does this flag get set to NO for some other target? To answer this, you will have to go through every single related target (the pods for the NotificationService across debug/release, and the actual NotificationService itself) and figure out what the flag is set to, and then share that information here. My second question: you need to determine IF IT MATTERS that we are setting this to NO. What does it mean to set this to YES vs. NO? How does it work if one target with YES includes one with NO, or the other way around? Your answer will probably need to be multiple paragraphs long, with links to documentation. |
Comment Actions
APPLICATION_EXTENSION_API_ONLY is set to:
- YES every time it appears in native/ios/Comm.xcodeproj/project.pbxproj, which is for Pods-NotificationService.debug.xcconfig and Pods-NotificationService.release.xcconfig
- NO every time it appears in native/ios/Pods/Pods.xcodeproj/project.pbxproj, which is for all pods both .debug.xcconfig and .release.xcconfig
I found the following article that explains why this flag has to be explicitly set to YES for NotificationService: https://swift-cast.com/2021/04/38/.
I have also found the following github issue: https://github.com/CocoaPods/CocoaPods/issues/9233.
After reading it I came to a conclusion that if I explicitly set APPLICATION_EXTENSION_API_ONLY flag to YES for Notification service then it is automatically applied to Pods included in this target. Then compilation fails since UI-related API is used in Pods we want to compile in NotificationService. Setting this flag to NO for all Pods in post_install hook in Podfile is a way to overwrite this compiler flag inheritance.
Moreover, I have found an iOS library which official installation guide directly advises users to set this flag to NO inside a Podfile to overcome the same exact issue we are trying to solve in this diff: https://cocoapods.org/pods/CrowdinSDK#installation.
Finally, if Apple reviewers for some reason disapprove of using this flag to compile expo and react native staff for Notification Service (App Extension) I have created an alternative solution that is more complex but does not utilise this flag and allows us to compile CommonCpp sources for Notification service without need to compile libraries that use UI-related API. It can be seen on the following branch: https://github.com/CommE2E/comm/tree/marcin/eng-490-expo-patching
@ashoat please let me know if this explanation is sufficient.
| native/ios/Podfile | ||
|---|---|---|
| 56 | Does this apply to all targets now? Are we sure we want to apply it to all targets? Is it possibly that we're inadvertedly applying it to some targets we haven't considered? (Targets that aren't the Comm app, or the notification service extension) | |
| 63 | It's a bit confusing to me that we exclude both of these here, but in the project.pbxproj only arm64 is mentioned. Do you know why that is? | |
| native/ios/Podfile | ||
|---|---|---|
| 56 | There are two changes here compared to master branch:
Let me explain why those changes should not raise concern. | |
| 63 | Build settings in the XCode project refer to targets: Comm and NotificationService. This line changes build settings for pods. I did not modify this line and according to the explanation above it has alway been that we exclude both architectures for all pods. I do not know why it is that, but it is not related to any change introduced in this diff. Target 'Comm' excluded only amr64 architecture before I started to work on this issue and the very recent changes I introduced were to unify excluded architectures for both Comm and NotificationService. Would you like me to try whether we can exclude i386 arch for XCode targets as well? | |