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

chore(sdks, tests): update SDKs / Fix tests compile for xcode12 #4401

Merged
merged 10 commits into from
Oct 16, 2020

Conversation

mikehardy
Copy link
Collaborator

@mikehardy mikehardy commented Oct 16, 2020

Description

This is based on the good work from @MujtabaFR

They got the underlying SDK up to the point we noticed Android failures.

Turns out if you take MLKit past SDK BOM 25.7.0 you have incompatibilities you need to work around:
firebase/firebase-android-sdk#1904
https://firebase.google.com/support/release-notes/android#mlkit-self-serve-fixes

So I applied the necessary changes

In the process I tested ios and realized we don't work well with Xcode 12 (#4397) so I applied the necessary workarounds there

This now passes tests on local machine for android and ios and will hopefully go through e2e

@Salakar I'd especially like a look at this specific commit: 4f74c17 - I'm not 100% sure on it and it will fail in the future with Apple Silicon - you may have handled this elsewhere already?

Related issues

#4397 - Xcode 12 issue here
#4388 - PR that is the base for this

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Oct 16, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/cnnr7839o
✅ Preview: https://react-native-firebase-git-tests-xcode12.invertase.vercel.app

@mikehardy mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Oct 16, 2020
@mikehardy mikehardy changed the title Update SDKs / Fix tests compile for xcode12 chore(sdks, tests): update SDKs / Fix tests compile for xcode12 Oct 16, 2020
@mikehardy mikehardy requested a review from Salakar October 16, 2020 03:48
@codecov
Copy link

codecov bot commented Oct 16, 2020

Codecov Report

Merging #4401 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4401   +/-   ##
=======================================
  Coverage   67.05%   67.05%           
=======================================
  Files         114      114           
  Lines        3820     3820           
  Branches      278      278           
=======================================
  Hits         2561     2561           
  Misses       1150     1150           
  Partials      109      109           

@mikehardy
Copy link
Collaborator Author

mikehardy commented Oct 16, 2020

Yeah, so still some work to do https://github.com/invertase/react-native-firebase/runs/1262569666#step:15:11

Might be that the .github workflow taking a bump to Xcode12 would fix it, I'll try that next

[edit: trying Xcode12 now, pushed the change and it's building...]

@mikehardy mikehardy linked an issue Oct 16, 2020 that may be closed by this pull request
@mikehardy mikehardy added Workflow: Needs Second Review Waiting on a second review before merge and removed Workflow: Needs Review Pending feedback or review from a maintainer. labels Oct 16, 2020
@mikehardy
Copy link
Collaborator Author

All working now! My default action will be to merge this before end of day in order to unblock RNFB development but I'll wait that long in case anyone has comments / wisdom about the changes

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Oct 16, 2020
@mikehardy mikehardy merged commit dc4f57a into invertase:master Oct 16, 2020
@mikehardy mikehardy deleted the tests-xcode12 branch October 16, 2020 21:34
@mikehardy mikehardy removed the Workflow: Needs Second Review Waiting on a second review before merge label Oct 16, 2020
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Oct 16, 2020
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
…rtase#4401)

* Upgrade firebase sdk version

Upgrade firebase sdk version to 25.12.0 to fix [issue invertase#4189](invertase#4189)

* Update iid android library

Co-authored-by: Mike Hardy <[email protected]>

* update sdk for ios firebase, iid and playServicesAuth
* update firebase bom version
* Update FirebaseSDKVersion
* Install firebase-tools as dev dep in tests, reference locally
* De-integrate pre-compiled firestore, incompatible with Xcode12+detox

invertase/firestore-ios-sdk-frameworks#15

* Exclude arm64/i386 from simulator build archs for Xcode12 compatibility

Without these, there are linker failures as Xcode12 attempts to build for
all archs, even when it's not really possible

* Workaround upstream MLKit dependency-related compile failures

https://firebase.google.com/support/release-notes/android#mlkit-self-serve-fixes
firebase/firebase-android-sdk#1904

* Use Xcode "latest-stable" (12 now) in E2E CI

This should fix invertase#4397

Co-authored-by: Mujtaba F. Radhi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI - Update Xcode to 12+ for E2E tests
2 participants