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

Branch SDK v1.42-1.43.2 causes crash on launch for iOS 10-12 even with LinkPresentation optionally linked #1207

Open
AlexGingell opened this issue Oct 28, 2022 · 9 comments

Comments

@AlexGingell
Copy link

AlexGingell commented Oct 28, 2022

Branch SDK v1.42 - 1.43 causes crash on launch for iOS 10-12 with:

dyld: Library not loaded: /System/Library/Frameworks/LinkPresentation.framework/LinkPresentation
  Referenced from: /Users/Hiro/Library/Developer/Xcode/DerivedData/REDACTED-fhchuwfdawvhjigaguzjpsyrsory/Build/Products/Debug-iphonesimulator/Branch.framework/Branch
  Reason: image not found
dyld: launch, loading dependent libraries
DYLD_FRAMEWORK_PATH=/Users/Hiro/Library/Developer/Xcode/DerivedData/REDACTED-fhchuwfdawvhjigaguzjpsyrsory/Build/Products/Debug-iphonesimulator
DYLD_FALLBACK_LIBRARY_PATH=/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 12.4.simruntime/Contents/Resources/RuntimeRoot/usr/lib
DYLD_ROOT_PATH=/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 12.4.simruntime/Contents/Resources/RuntimeRoot
DYLD_FALLBACK_FRAMEWORK_PATH=/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 12.4.simruntime/Contents/Resources/RuntimeRoot/System/Library/Frameworks
DYLD_INSERT_LIBRARIES=/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 12.4.simruntime/Contents/Resources/RuntimeRoot/usr/lib/libBacktraceRecording.dylib:/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 12.4.simruntime/Contents/Resources/RuntimeRoot/usr/lib/libMainThreadChecker.dylib:/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 12.4.simruntime/Contents/Resources/RuntimeRoot/Developer/Library/PrivateFrame

Ours is a manual integration. Note that the LinkPresentation framework has been added to Link Binary With Libraries with the 'Optional' status. LinkPresentation is only available from iOS 13.

We note a similar issue here:
fluttercommunity/plus_plugins#222
which was fixed as follows:
fluttercommunity/plus_plugins#224

Is it possible the Branch SDK is strongly requiring a framework which should be weakly required because it is not available on iOS 10-12 ?

We've downgraded to v1.41.0 for now - this works fine. We note that 1.44 now requires iOS 11, and have not tested it as we support iOS 10+.

Steps to reproduce

  1. Integrate Branch v1.41 - 1.43
  2. Link the LinkPresentation framework optionally.
  3. Run on simulator or device iOS 10-12.

Expected behavior

The LinkPresentation framework should not be required on iOS 10 - 12 and Branch should launch and operate normally, omitting LinkPresentation functionality gracefully.

SDK Version

1.42.0 - 1.43.2

XCode Version

13.4.1

Device

All

OS

10-12

Additional Information/Context

No response

@echo-branch
Copy link
Contributor

Sorry for the delay. This was a confusing documentation issue. Basically, the framework should be marked optional in xcode.

@AlexGingell
Copy link
Author

AlexGingell commented Nov 21, 2022

Sorry for the delay. This was a confusing documentation issue. Basically, the framework should be marked optional in xcode.

As stated in my original post, I did have the LinkPresentation framework marked optional in Xcode's linking options. This does not prevent the crashing issue.

@echo-branch
Copy link
Contributor

@AlexGingell
Ah yea, I totally overlooked that detail. So it looks like it's an issue with the pre-built SDK library, as the error does not occur with other integration methods. Creating a ticket for this internally.

@LeadAssimilator
Copy link

LeadAssimilator commented Nov 27, 2023

Why hasn't this been fixed yet? It has been over a year for a 32 character insertion in 3 targets - aka -weak_framework LinkPresentation in other linker flags. This is affecting the xamarin version of the sdk because it uses the binary distributions! Do I need to file a support case for this? I probably will anyway to get a refund for some months given this is a known issue that has been ignored for such a long time.

@echo-branch
Copy link
Contributor

@LeadAssimilator
Unfortunately the fix you're suggesting does not work. I also thought it was a simple linker setting, but the original reporter pointed out that they did set the linker setting correctly and it still fails when using the pre-built binary.

On native iOS, the issue can be worked around by using Swift Package Manager or Cocoapods, so the ticket priority had fallen behind iOS 17 related work.

But for Xamarin, we'll likely need to provide a different binary without LinkPresentation.

@LeadAssimilator
Copy link

What do you mean it doesn't it work? And why? Because it should and does work!

Adding -weak_framework LinkPresentation to other linker flags for the 3 main targets in the project is no different than s.weak_framework = 'LinkPresentation' in the .podspec. If pods work then so does this fix. I think the original reporter was stating they set the linker flags on their project (as in they weren't strongly linking against LinkPresentation explicitly) which of course won't work to fix branch as it needs set on the Branch project itself given its a dynamic lib!

Just add the flags as described to the branch project and run the build script. You can confirm the framework is linked as weak by inspecting the output of otool -l for each dylib in the resulting xcframework.

otool -l BranchSDK.xcframework/ios-arm64/BranchSDK.framework/BranchSDK | grep -B 3 -A 3 LinkPresentation
otool -l BranchSDK.xcframework/ios-arm64_x86_64-simulator/BranchSDK.framework/BranchSDK | grep -B 3 -A 3 LinkPresentation
otool -l BranchSDK.xcframework/ios-arm64_x86_64-maccatalyst/BranchSDK.framework/BranchSDK | grep -B 3 -A 3 LinkPresentation
otool -l BranchSDK.xcframework/tvos-arm64/BranchSDK.framework/BranchSDK | grep -B 3 -A 3 LinkPresentation

@LeadAssimilator
Copy link

Since Xamarin is using 2.1.2, I suggest fixing this in main, backporting the fix to 2.1.2 to make 2.1.3 (even if it just remains internal) and use that build on Xamarin 9.0.0 to make 9.0.1. That way we can get an immediate fix for Xamarin without needing to do any sort of in depth testing/validation since the bits should be identical outside of the linker flags and timestamps.

Clearly this issue was never triaged or prioritized properly if it ended up behind new feature work for something that is so trivial to address. Hopefully you will recognize this error and address the problem appropriately in a timely manner this time.

@LeadAssimilator
Copy link

Also I recommend setting Link Frameworks Automatically to No.

Automatically linking frameworks via imports of their headers is not a good idea. I think Apple only made it Yes by default to reduce their support cases and/or to make it seem easier for new/inexperienced developers to make apps. But it ends up hiding a project's dependencies, makes inspection/audit of dependencies harder and encourages one to forget about linking entirely. All of which probably contributed to this particular bug in the first place.

For example, if one had imported LinkPresentation.h with LFA=No, then compile would have initially failed. To make it work an explicit reference to LinkPresentation would have to be added in Frameworks and Libraries. Being forced add the reference manually serves as a reminder to check the framework availability vs. the minimum deployment target and to adjust the linker flags accordingly.

While it isn't perfect, it is better than the alternative, because you can define standard practices/processes/procedures around the adding of new Frameworks and Libraries from ensuring legal/license compliance and entitlements to deployment target availability and linker flags. All of which would only have to apply when adding something new there, rather than having to be something you must remember to do every #import or @import. Nobody wants to have to remember to do that so frequently and most of the time it is useless because you are importing something in use already. Contrast that with leveraging the compiler to remind you extra thought is required shifts the burden to just in time, which is clearly the superior option.

@echo-branch
Copy link
Contributor

@LeadAssimilator
Thanks for pushing back! I am indeed mistaken in how deep the initial triage testing was, that's on me. Currently looking into how soon we can meet the overall goal of getting Xamarin updated.

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

No branches or pull requests

3 participants