Update GoogleUtilities' clients with repo-relative imports #5824

paulb777 · 2020-06-15T17:51:10Z

Update client imports to GoogleUtilities to use repo-relative imports - following up GoogleUtilities repo-relative imports #5810
- The clients were updated with the next incarnation of scripts/change_headers.swift
Initial Swift Package Manager CI for GoogleUtilities and FirebaseCore - (productizing prototype code in the spm-master2020 branch.
Bump GoogleUtilities minor version and its clients podspec dependency
Updated GoogleUtilities header structure to minimize deltas between CocoaPods and SPM
- Where possible, treated the CocoaPods Private folder as public.
- When not possible because of intra-pod imports in the Private headers, created a Public/dummy.h to treat the Privates as Private.
- The GoogleUtilities header organization should be revisited and made more consistent when breaking changes are allowed with Firebase 7. There is not a consistent definition between public and private header usage currently.

google-oss-bot · 2020-06-15T17:52:42Z

	1 Warning
⚠️	Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

maksymmalyhin · 2020-06-16T01:55:18Z

scripts/change_headers.swift

@@ -107,16 +113,22 @@ let headerMap = getHeaderMap(url)
 for root in changeImports {
  let rootURL = url.appendingPathComponent(root)
  let enumerator = FileManager.default.enumerator(atPath: rootURL.path)
-  while let file = enumerator?.nextObject() as? String {
+  whileLoop: while let file = enumerator?.nextObject() as? String {


I wonder what whileLoop: does?

The label enables the continue from a doubly nested loop. See line 128.

maksymmalyhin · 2020-06-16T02:14:06Z

GoogleUtilities/Environment/SecureStorage/GULKeychainStorage.m

@@ -14,7 +14,7 @@
 * limitations under the License.
 */

-#import "GoogleUtilities/Environment/Public/GULKeychainStorage.h"
+#import "GoogleUtilities/Environment/Private/GULKeychainStorage.h"


I wonder if there is a way to implement test apps like #5828 with this structure.

I don't think that test apps should depend on the repo structure. My intent is to only change the library sources and unit test imports.

The issue (existing one already) that there are no actual public header in GoogleUtilities/Environment (they are marked as private now). I was going to make them public. Do you think it may be an issue?

Yes. We should avoid public headers since they require ifdef'ing between Swift Package Manager and CocoaPods. See the Logger and NSData changes in this PR for details.

It feels a bit weird to have a library without public API, but it's probably out of scope of the PR, we may discuss it later

paulb777 · 2020-06-16T15:16:13Z

cc: @eldhosembabu @chliangGoogle @karenyz @christibbs

mikehardy · 2020-07-15T01:32:28Z

FirebaseMessaging/Sources/FIRMessaging.m

@@ -22,10 +22,10 @@
 #import <FirebaseInstanceID/FirebaseInstanceID.h>
 #import <FirebaseMessaging/FIRMessaging.h>
 #import <FirebaseMessaging/FIRMessagingExtensionHelper.h>
-#import <GoogleUtilities/GULAppDelegateSwizzler.h>


Hey @paulb777 :-), turns out react-native-firebase was using the GULAppDelegateSwizzler:

https://github.com/invertase/react-native-firebase/blob/67c52a8aaeac32d2253e73769313e49700bc497c/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BAppDelegate.m#L20

And with 6.7.0 of GoogleUtilities / 6.28.0 of firebase-ios-sdk we get a build error invertase/react-native-firebase#3938 as GULAppDelegateSwizzler.h tries to pull in GULApplication.h but it's not found:

ios/Pods/Headers/Private/GoogleUtilities/GULAppDelegateSwizzler.h:19:9: 'GoogleUtilities/AppDelegateSwizzler/Private/GULApplication.h' file not found

I'm ace at the Java side of the react-native house but my Objective-C skills sadly leave something to be desired.

I attempted to alter the import in our repo to match the change here ( #import "GoogleUtilities/AppDelegateSwizzler/Private/GULAppDelegateSwizzler.h") without success

Is there a simple trick I'm missing that will resolve the headers and allow our auto-swizzling for react-native library consumers to work, or is this a more serious structural issue?

Sorry to bother, but thanks in advance for any help

Turns out that the Firebase SDK for Unity was also using GULAppDelegateSwizzler. :/. We temporarily fixed the issue by adding this entry to the Podfile on our project:

pod 'GoogleUtilities', '6.6.0'

Hope this is fixed soon.

At least our repo isn't alone @alejobrainz :-), we have a user report confirming that pinning the GoogleUtilities dep at 6.6.0 works for us as well, at least for firebase-ios-sdk pods 6.27.1, ~~still building against 6.28.0 but with GoogleUtilities pinned on 6.6.0 to see~~ [update: 6.28.0 may not be used by react-native-firebase until we figure out how to resolve it, 6.27.1 is the latest that appears to work]

Sorry for the issue - I see it's been reported at #6047 so we'll make all progress updates there and continue the discussion for greater visibility.

This allows the messaging module to compile against firebase-ios-sdk <= 6.27.1 successfully It is not compatible with firebase-ios-sdk >= 6.28.0 A real fix is pending resolution of #3938 / Adaptation to firebase/firebase-ios-sdk#5824

googlebot added the cla: yes label Jun 15, 2020

paulb777 marked this pull request as draft June 15, 2020 17:51

google-oss-bot added api: auth api: core api: dynamiclinks api: inappmessaging Firebase In App Messaging api: installations api: instanceid api: messaging api: remoteconfig GoogleUtilities labels Jun 15, 2020

paulb777 mentioned this pull request Jun 15, 2020

GoogleUtilities repo-relative imports #5810

Merged

paulb777 marked this pull request as ready for review June 15, 2020 23:32

paulb777 requested review from maksymmalyhin and ryanwilson June 15, 2020 23:40

maksymmalyhin approved these changes Jun 16, 2020

View reviewed changes

Base automatically changed from pb-gu-rr to master June 16, 2020 13:19

paulb777 added 12 commits June 16, 2020 06:22

Update GoogleUtilities clients for repo-relative

be37872

checkpoint

0d1aae3

Fixes

20dd314

logger import fixes

d503de1

fix bad move

eb6f1b4

NSData+zlib.h is public

3e2e24e

fix

970a9e2

Fix paths

27e3b61

Missing add file

2919cd1

fix path

55684bc

podspec updates

7e4ec3c

style

aa63c87

paulb777 added 8 commits June 16, 2020 06:23

Remove test code

5e1ea59

Turn on travis

ebed32b

Fix module check

52ec54d

Swift Package CI

710829e

Swift PM fixes

bf5207e

missing comma

1491fb0

copyrights

6932f30

Restore Gemfile and travis

714e391

paulb777 force-pushed the pb-gu-rr-clients branch from 258de96 to 714e391 Compare June 16, 2020 13:24

paulb777 added 2 commits June 16, 2020 07:41

Add comment to Package.swift

929c068

Verify GoogleUtilities regression is fixed

3937529

paulb777 merged commit 3f556dd into master Jun 16, 2020

paulb777 deleted the pb-gu-rr-clients branch June 16, 2020 16:05

granluo pushed a commit that referenced this pull request Jun 18, 2020

Update GoogleUtilities' clients with repo-relative imports (#5824)

64aaf87

granluo pushed a commit that referenced this pull request Jul 7, 2020

Update GoogleUtilities' clients with repo-relative imports (#5824)

6a9d6dc

mikehardy mentioned this pull request Jul 15, 2020

GoogleUtilites pod 6.7.0 / firebase-ios-sdk 6.28.0 incompatibility (was 'GoogleUtilities/AppDelegateSwizzler/Private/GULApplication.h' file not found) invertase/react-native-firebase#3938

Closed

7 tasks

mikehardy reviewed Jul 15, 2020

View reviewed changes

mikehardy mentioned this pull request Jul 15, 2020

[upstream fix coming, obsoleted] fix(ios,messaging): pin GoogleUtilities at 6.6.0 in Messaging invertase/react-native-firebase#3939

Closed

8 tasks

ryanwilson mentioned this pull request Jul 15, 2020

Updated GoogleUtilities failing in azure pipeline for ios platform #6047

Closed

firebase locked and limited conversation to collaborators Jul 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update GoogleUtilities' clients with repo-relative imports #5824

Update GoogleUtilities' clients with repo-relative imports #5824

paulb777 commented Jun 15, 2020 •

edited

google-oss-bot commented Jun 15, 2020

maksymmalyhin Jun 16, 2020

paulb777 Jun 16, 2020

maksymmalyhin Jun 16, 2020

paulb777 Jun 16, 2020

maksymmalyhin Jun 16, 2020

paulb777 Jun 16, 2020

maksymmalyhin Jun 16, 2020

paulb777 commented Jun 16, 2020

mikehardy Jul 15, 2020

alejobrainz Jul 15, 2020 •

edited

mikehardy Jul 15, 2020 •

edited

ryanwilson Jul 15, 2020

Update GoogleUtilities' clients with repo-relative imports #5824

Update GoogleUtilities' clients with repo-relative imports #5824

Conversation

paulb777 commented Jun 15, 2020 • edited

google-oss-bot commented Jun 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulb777 commented Jun 16, 2020

Choose a reason for hiding this comment

alejobrainz Jul 15, 2020 • edited

Choose a reason for hiding this comment

mikehardy Jul 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulb777 commented Jun 15, 2020 •

edited

alejobrainz Jul 15, 2020 •

edited

mikehardy Jul 15, 2020 •

edited