EmergeTools / ETTrace

Easily and accurately profile iOS apps
https://www.emergetools.com/ettrace
MIT License
570 stars 23 forks source link

FirebaseCrashlytics symbol is bundled with Binary #26

Closed bdfox325 closed 1 year ago

bdfox325 commented 1 year ago

When compiling application, getting issue:

objc[3368]: Class PodsDummy_FirebaseCrashlytics is implemented in both 
/private/var/containers/Bundle/Application/8700DB0D-AE97-4424-803C-98DA8B67E3B0/Robinhood.app/Frameworks/ETTrace.framework/ETTrace (0x11f526490) and 
/private/var/containers/Bundle/Application/8700DB0D-AE97-4424-803C-98DA8B67E3B0/Robinhood.app/Robinhood 
(0x10a88db68). One of the two will be used. Which one is undefined.

Is it necessary to compile FirebaseCrashlytics in the binary?

noahsmartin commented 1 year ago

Is this happening when you compile? From the log it looks like this is at runtime. This happens because the app links to firebase, but so does ETTrace. Firebase is needed because it's used to record the backtraces. It shouldn't be a problem in actual use, and since ETTrace isn't design to ship to production it wouldn't affect users.

We might be able to remove that class "PodsDummy_FirebaseCrashlytics" if we stopped integrating it with cocoapods

bdfox325 commented 1 year ago

Hey. Yes, this is happening at runtime. We only package the app in our DEBUG builds so its not an issue for the end user.

KhaosT commented 1 year ago

The main concern on our side around ETTrace itself also bundling a copy of Firebase is if the bundled version mismatches, it may cause unexpected crashes for the developers during development because Firebase internal methods calls the wrong implementation.

If ETTrace needs to include Firebase with itself, can it try to rename the symbols internally so we don't run into the potential issues down the road? Thanks!

noahsmartin commented 1 year ago

Thanks for explaining! We do plan to rename it. We are also evaluating switching to SwiftPM so will probably wait for that to complete, which can make renaming unnecessary. The only duplicated symbol appears to be the PodsDummy_FirebaseCrashlytics class which is just an empty class added by cocoapods, more details here: https://github.com/CocoaPods/CocoaPods/issues/1767. So I don't think there's anything to worry about in terms of wrong methods being called.

Since we use a local copy of the pod already (in the Unwinding folder) I suspect just renaming the pod in our Podfile would prevent the warning. So if you're interested you could try that out as a solution until we try switching to SwiftPM

noahsmartin commented 1 year ago

Hi @bdfox325 @KhaosT We have just released an update that switches to using Swift Package Manager for building the xcframework, you can now link to the new framework, or install it directly as a SPM dependency. There won't be this Firebase class included either way. Let us know if this works for you!