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

[NTV-614] Firebase Binaries To SPM #1729

Merged
merged 1 commit into from Sep 14, 2022
Merged

Conversation

msadoon
Copy link
Contributor

@msadoon msadoon commented Sep 13, 2022

πŸ“² What

As part of our migration to SPM from Carthage to support M1 builds.

πŸ€” Why

Relevant context here.

πŸ›  How

Before:

Cartfile (.resolved reflects same changes)

### Binaries

binary "https://raw.githubusercontent.com/Appboy/appboy-ios-sdk/master/appboy_ios_sdk_full.json" == 4.3.2
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseAnalyticsBinary.json" "7.4.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseCrashlyticsBinary.json" "7.4.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseInAppMessagingBinary.json" "7.4.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebasePerformanceBinary.json" "7.4.0"
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseProtobufBinary.json" "7.4.0"
binary "https://raw.githubusercontent.com/PerimeterX/px-iOS-Framework/master/PerimeterX.json" == 1.13.9

After:

Cartfile (.resolved reflects same changes)

### Binaries

binary "https://raw.githubusercontent.com/Appboy/appboy-ios-sdk/master/appboy_ios_sdk_full.json" == 4.3.2
binary "https://raw.githubusercontent.com/PerimeterX/px-iOS-Framework/master/PerimeterX.json" == 1.13.9

Using version 9.0 because it 8.0 is earliest supported version that is not beta and available on SPM, and 9.0 was defaulted to in SPM, tested it with 9.0, no code changes so it should be fine.

So investigating each library we found that direct package translations using this chart.

Compare the above with what we had in the previous section above (the Cartfile).

We don't import Firebase In-App Messaging (like before) into SPM because we don't use it. Here is the page for it. Judging by the description we need to have a console set up on firebase's dashboard, which currently doesn't exist for either native platform.

Also FirebaseProtobufBinary doesn't seem to have a direct translation package in the chart above but it included FirebasePerformance along with Protobuf when used within Carthage.

Browsing the web found this reference to Google Protocol buffers, but no direct Protobuf.framework reference (as FirebasePerformance is a package we did include into SPM). It seems to be unsupported at the moment, as it isn't listed on Carthage's firebase page. There's no immediate need to find a use for it, as it seems like a dependency that came with FirebasePerformance.

It seems like we can use the exact same upload-dsyms-firebase.sh script that we previously used with the Carthage firebase packages.

Use with Carthage instructions:

For Crashlytics, do the following steps to automatically upload your app's symbols so your app's crashes are symbolicated:

    Download [upload-symbols](https://github.com/firebase/firebase-ios-sdk/raw/master/Crashlytics/upload-symbols) and [run](https://github.com/firebase/firebase-ios-sdk/raw/master/Crashlytics/run). Note: please see the https://github.com/firebase/firebase-ios-sdk/issues/4720#issuecomment-577213858 for details why it has to be done manually.
    Put these in the directory where your .xcodeproj file lives, eg. scripts/run and scripts/upload-symbols
    Make sure that the files are executable - chmod +x scripts/run scripts/upload-symbols
    Open your project in Xcode, then select its project file in the left navigator.
    From the Select a project or target dropdown, select your main build target.
    Select the Build Phases tab, then click "+" add > New Run Script Phase.
    Paste the following into your new Run Script, replacing "scripts" with whatever you named your folder: "${PROJECT_DIR}/scripts/run"
    Add the following dependencies as Input Files to the Run Script:
        ${DWARF_DSYM_FOLDER_PATH}/${DWARF_DSYM_FILE_NAME}/Contents/Resources/DWARF/${TARGET_NAME}
        ${BUILT_PRODUCTS_DIR}/${INFOPLIST_PATH}

Use with SPM instructions:

Another option is to use the [upload-symbols](https://github.com/firebase/firebase-ios-sdk/raw/master/Crashlytics/upload-symbols) script. Place it in the directory where your .xcodeproj file lives, eg. scripts/upload-symbols, and make sure that the file is executable: chmod +x scripts/upload-symbols. This script can be used to manually upload dSYM files (for usage notes and additional instructions, run with the --help parameter).

Assuming its the same upload-symbols executable, which worse case scenario won't be and we fail automatically updating dSyms after a build is released. Probably be able to catch this pretty early (alpha/beta) stages and fix if not being uploaded.

After this branch is merged, beta is created we should see the dSyms get uploaded to firebase console to signal all is good. Will keep an eye out for crashlytics, analytics, performance as well on the dashboard to ensure they are properly reporting.

βœ… Acceptance criteria

  • Project builds/runs well on device/tests pass

⏰ TODO

  • Smoke test where-ever we import Firebase but also note we are using FirebasePerformance, FirebaseAnalytics, FirebaseCrashlytics and FirebaseInAppMessaging-Beta. Do we need them all? Try removing ones we don't need, get project building.
  • Ensure all these instructions are followed. Especially because we do use FirebaseCrashlytics (so if its' still used, then make the changes.)
  • Test in-app functionality once above changes are made - on device.

@msadoon msadoon added this to the release-5.5.0 milestone Sep 13, 2022
@msadoon msadoon self-assigned this Sep 13, 2022
@msadoon msadoon changed the title added firebase spm changes, after rebasing with facebook spm changes. [NTV-614] Firebase Binaries To SPM Sep 13, 2022
@msadoon
Copy link
Contributor Author

msadoon commented Sep 13, 2022

@scottkicks if its cool can we merge this guy in first before FBSnapShotTestCase, I just wanted to not deal with merge conflicts lol

@codecov
Copy link

codecov bot commented Sep 13, 2022

Codecov Report

Merging #1729 (ffee3a7) into main (e06eda7) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1729   +/-   ##
=======================================
  Coverage   85.38%   85.38%           
=======================================
  Files        1270     1270           
  Lines      114706   114706           
  Branches    30296    30296           
=======================================
  Hits        97941    97941           
  Misses      15713    15713           
  Partials     1052     1052           

πŸ“£ We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

awesome job! πŸŽ‰

@msadoon msadoon merged commit b70ed49 into main Sep 14, 2022
@msadoon msadoon deleted the ntv-614/firebase-binaries-to-spm branch September 14, 2022 01:46
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

2 participants