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

Add FirebaseAppCheckInterop umbrella header #11860

Merged
merged 23 commits into from
Oct 11, 2023
Merged

Conversation

andrewheard
Copy link
Contributor

@andrewheard andrewheard commented Sep 27, 2023

  • Added a FirebaseAppCheckInterop.h umbrella header for the framework.
  • Replaced semantic imports of App Check Interop (@import FirebaseAppCheckInterop) with umbrella header imports (#import <FirebaseAppCheckInterop/FirebaseAppCheckInterop.h>).
  • Updated Auth, RTDB and Firestore to depend on FirebaseAppCheckInterop via pod / SPM target dependencies instead of direct file paths.
    • This is in preparation for moving FirebaseAppCheckInterop into a separate repo.

Fix #11916

@google-oss-bot
Copy link

google-oss-bot commented Sep 27, 2023

Coverage Report 1

Affected Products

  • FirebaseAuth-iOS-FirebaseAuth.framework

    Overall coverage changed from 68.82% (8b46c6a) to 68.81% (8e878a6) by -0.01%.

    FilenameBase (8b46c6a)Merge (8e878a6)Diff
    FIRAuthKeychainServices.m59.66%58.80%-0.86%
  • FirebaseDatabase-iOS-FirebaseDatabase.framework

    Overall coverage changed from 56.94% (8b46c6a) to 57.09% (8e878a6) by +0.15%.

    FilenameBase (8b46c6a)Merge (8e878a6)Diff
    FSRWebSocket.m38.14%40.13%+2.00%
  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from 88.16% (8b46c6a) to 88.07% (8e878a6) by -0.09%.

    FilenameBase (8b46c6a)Merge (8e878a6)Diff
    exception.cc84.21%23.68%-60.53%
    leveldb_key.cc98.63%98.82%+0.20%
    leveldb_remote_document_cache.cc96.55%94.83%-1.72%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/RzEhJ9OVvX.html

@google-oss-bot
Copy link

1 Warning
⚠️ New public headers were added, did you remember to add them to the umbrella header?

Generated by 🚫 Danger

@ncooke3
Copy link
Member

ncooke3 commented Oct 11, 2023

I think the CMake failures may be because the CMakeLists.txt within the FirebaseAppCheck/Interop only globs files from the directory it is in:

https://github.com/firebase/firebase-ios-sdk/blob/ah/appcheck-interop-umbrella/FirebaseAppCheck/Interop/CMakeLists.txt#L19

Assuming that's the issue, I think either **/*.h or prefixing the glob path with the directory that the headers are in might do the trick (example).

@andrewheard andrewheard added this to the 10.17.0 - M139 milestone Oct 11, 2023
@andrewheard andrewheard marked this pull request as ready for review October 11, 2023 19:52
FirebaseAppCheck/CHANGELOG.md Outdated Show resolved Hide resolved
@ncooke3
Copy link
Member

ncooke3 commented Oct 11, 2023

CI is all green. I'm going to merge this to unblock failures in #11914.

@ncooke3 ncooke3 merged commit f3a14c2 into master Oct 11, 2023
97 checks passed
@ncooke3 ncooke3 deleted the ah/appcheck-interop-umbrella branch October 11, 2023 22:39
@firebase firebase locked and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App Check no longer compiles as an Objective-C++ module as of 10.16.0
4 participants