Buildable and interoperable source pods #444

paulb777 · 2017-11-11T17:54:07Z

With this PR, it is now possible to intermix source pods with released Firebase pods.

FirebaseCommunity is deprecated
There is now an exact one to one mapping between the open source and released pods
Turn off CocoaPods pch files for pod builds
CocoaPods 1.4.0 or newer is required
Externally depends on Firebase 4.6.0 being public, so travis and merge after that

Proposed plan is to run this in parallel with old release process for a few releases and then switch to releasing the pods as source pods from the git repo.

…> Firebase

wilhuff

First pass

wilhuff · 2017-11-13T17:16:33Z

.travis.yml


 branches:
  only:
    - master
-
+    - pb-future


Is this intended to bleed out?

wilhuff · 2017-11-13T17:17:44Z

Example/Auth/Sample/ApplicationDelegate.m

 #import "FirebaseAuth.h"
-#import "FirebaseCommunity/FIRLogger.h"
+#import "FirebaseCore/FIRLogger.h"


Running scripts/style.sh should reorder these imports (unless Auth exempts itself).

Auth is exempt, but will run style.sh for rest

wilhuff · 2017-11-13T17:18:27Z

Example/Core/App/iOS/FIRAppDelegate.m

@@ -12,7 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.

-@import FirebaseCommunity;
+@import Firebase;
+//#import "FirebaseCore/FIRApp.h"


Please remove commented-out code.

wilhuff · 2017-11-13T17:19:03Z

Example/Database/Tests/Unit/FSyncPointTests.m

-#import "FirebaseCommunity/FIRLogger.h"
+
+#import "FirebaseCore/FIRLogger.h"
+


Excess newline

wilhuff · 2017-11-13T17:19:53Z

Example/Podfile

@@ -1,138 +1,136 @@
+#remove next two lines after Firebase 4.6.0 release and before merge


These likely shouldn't be here

wilhuff · 2017-11-13T17:26:06Z

FirebaseCore.podspec

+  s.license          = { :type => 'Apache', :file => 'LICENSE' }
+  s.authors          = 'Google, Inc.'
+
+  s.source           = { :git => 'https://github.com/firebase/firebase-ios-sdk.git', :tag => s.version.to_s }


This line is too long. Try:

s.source = { :git => 'https://github.com/firebase/firebase-ios-sdk.git', :tag => s.version.to_s }

wilhuff · 2017-11-13T17:26:27Z

FirebaseDatabase.podspec

+  s.dependency 'leveldb-library', '~> 1.18'
+  s.dependency 'FirebaseCore', '~> 4.0'
+  s.ios.dependency 'FirebaseAnalytics', '~> 4.0'
+  s.pod_target_xcconfig = { 'GCC_PREPROCESSOR_DEFINITIONS' =>


Newline after the brace.

wilhuff · 2017-11-13T17:26:40Z

FirebaseMessaging.podspec

+  s.requires_arc = base_dir + '*.m'
+  s.public_header_files = base_dir + 'Public/*.h'
+  s.library = 'sqlite3'
+  s.pod_target_xcconfig = { 'GCC_PREPROCESSOR_DEFINITIONS' =>


newline after the brace.

wilhuff · 2017-11-13T17:29:17Z

Firestore/Firestore.podspec

  s.library = 'c++'

-  s.xcconfig = { 'GCC_PREPROCESSOR_DEFINITIONS' =>
+  s.pod_target_xcconfig = {
+    'GCC_PREPROCESSOR_DEFINITIONS' =>


Next line can be pulled up one.

wilhuff · 2017-11-13T17:29:47Z

Firestore/Firestore.podspec

+  s.ios.deployment_target = '7.0'
+
+  s.cocoapods_version = '>= 1.4.0.beta.2'
+  s.static_framework = true


Are you sure this works for swift users?

Swift Firestore test builds and I tested Analytics and Auth quickstarts.

wilhuff · 2017-11-15T17:04:13Z

Example/Database/Tests/Unit/FSyncPointTests.m

@@ -15,9 +15,7 @@
 */

 #import <Foundation/Foundation.h>


System imports/includes should be in a separate group

wilhuff · 2017-11-15T17:24:18Z

Example/Podfile

 target 'Core_Example_iOS' do
  platform :ios, '8.0'
-
-  pod 'FirebaseCommunity/Core', :path => '../'
+  pod 'Firebase/Core', '4.6.0'


I don't understand "the forcing function for the Firebase pod". What are we forcing?

It seems like Example has a dependency on Firebase/Core, but that we're overriding the path of FirebaseCore, no?

wilhuff · 2017-11-15T17:27:36Z

FirebaseAuth.podspec

-  s.source           = { :git => 'https://github.com/firebase/firebase-ios-sdk.git', :tag => s.version.to_s }
+  s.source           = {
+    :git => 'https://github.com/firebase/firebase-ios-sdk.git',
+    :tag => s.version.to_s


This :tag doesn't make any sense. The tags we're laying down are the Firebase release numbers (though we haven't been terribly good about that either). There is no 4.3.2 tag, and even if there were that would have been for an other Firebase-wide release number.

Shouldn't this tag be something like auth-4.3.2 if were to exist at all?

Otherwise it seems like we should be using the Firebase version for the tag.

Now that the open source repo is interoperable with the released Firebase version, it needs to be specified which version of Firebase, since Firebase has exact version numbers for each of the subspec dependencies.

The specified version (4.6.0 here) needs to be updated the same time all of the respective versions in each podspec is updated to the component version that goes with 4.6.0.

In the interim period, while we're still doing the official release internally and only overriding from open source, the tags are no-ops. We can't push these podspec because we don't want to replace the official internally replaced version.

There is an ongoing discussion with product management about whether we want to keep versioning the components individually. If we do so, the tags will switch to something like "Auth-" + s.version.to_s and we'll add to the appropriate repo. This will happen once we're ready do the official release from open source.

wilhuff · 2017-11-15T17:28:45Z

FirebaseFirestore.podspec

@@ -16,7 +16,7 @@ Google Cloud Firestore is a NoSQL document database built for automatic scaling,
                       DESC

  s.homepage         = 'https://developers.google.com/'
-  s.license          = { :type => 'Apache', :file => '../LICENSE' }
+  s.license          = { :type => 'Apache', :file => 'LICENSE' }
  s.authors          = 'Google, Inc.'

  s.source           = { :git => 'https://github.com/TBD/Firestore.git', :tag => s.version.to_s }


There is no 0.9.2 tag; this doesn't seem like it could pass pod lib lint.

pod lib lint is only a local check. The missing tag is an issue for pod spec lint though.

Separately, we won't be able to run pod spec lint until we can push the dependencies. The public FirebaseCore is not satisfactory, since the open source components depend on the open source Core for private header resolution.

pod lib lint only works for FirebaseCore because pod lib lint also requires dependencies to be publicly pushed. There are some possible approaches to address this, but depending on priorities, it might make sense to punt on linting until we're ready to do official releases from open source and in the meantime monitor warnings in the Xcode builds.

ryanwilson

Awesome work @paulb777 👏 I'm at LGTM after @wilhuff's comments and the minor markdown fix.

ryanwilson · 2017-11-16T14:55:34Z

Gemfile.lock

  specs:
-    cocoapods (1.3.1)
+    cocoapods (1.4.0.beta.2)


This is required, right?

Yeah, this makes sure all our collaborators are using the same version of CocoaPods.

This sets the CocoaPods for the Travis runs.

It also can be used by repo developers if they prefix pod commands with bundle exec and have bundler installed via gem install bundler - (thanks to @morganchen12 for factoid!)

SGTM, thanks to you both for the explanation.

ryanwilson · 2017-11-16T14:56:22Z

README.md

@@ -15,21 +15,38 @@ the Firebase iOS SDK. If you're interested in using the Firebase iOS SDK, start
 ## Context

 This repo contains a fully functional development environment for FirebaseCore,
-FirebaseAuth, FirebaseDatabase, FirebaseMessaging, and FirebaseStorage. By
+FirebaseAuth, FirebaseDatabase, FirebaseFirestore, FirebaseMessaging, and
+FirebaseStorage. By


Nit: spacing here? The next line should be brought up to this line.

Looks right when rendered at https://github.com/firebase/firebase-ios-sdk/blob/pb-future/README.md. I thought it might be better to keep more of the blame intact in the source.

Thanks for the review. I'm planning to present in Tuesday's leads meeting and hope to merge after that.

This file is also viewed in text editors; let's keep things looking crisp.

morganchen12

LGTM, but please wait for Gil approval.

ryanwilson

LGTM!

wilhuff

LGTM

wilhuff · 2017-11-17T16:07:11Z

README.md

@@ -15,21 +15,38 @@ the Firebase iOS SDK. If you're interested in using the Firebase iOS SDK, start
 ## Context

 This repo contains a fully functional development environment for FirebaseCore,
-FirebaseAuth, FirebaseDatabase, FirebaseMessaging, and FirebaseStorage. By
+FirebaseAuth, FirebaseDatabase, FirebaseFirestore, FirebaseMessaging, and
+FirebaseStorage. By


This file is also viewed in text editors; let's keep things looking crisp.

paulb777 added 30 commits August 19, 2017 11:16

Future podspecs (WIP)

a0f01db

Run Core and Auth tests with Future

98514c1

Running Auth quick start (with import renames)

61297dd

Don't conflict with Auth generate version variables. FirebaseFuture -…

fc37e45

…> Firebase

vendored_frameworks don't link from Development Pods

386561f

podspec experiment

3a9a80a

Disable mac for now

e9fcec2

Turn macOS back on

4febf43

Merge remote-tracking branch 'origin/master' into pb-future

b7b6847

Checkpoint with all ios tests building and running

95480e7

inherited is added to flags by CocoaPods

781f674

incremental

0ec3722

All iOS and macOS unit tests run

b1b6a5d

checkpoint

37b93d8

s.prefix_header_file = false

d1ffe2e

incremental post master merge

9b6388c

Merge remote-tracking branch 'origin/master' into pb-future

1667bd7

fix auth build after merge

65e470c

Fix Auth macOS unit test build

00236b2

checkpoint

5100d95

Merge remote-tracking branch 'origin/master' into pb-future

0ed38e8

Checkpoint - unit tests running after merge

4882c91

Merge remote-tracking branch 'origin/master' into pb-future

af0bc74

Update podspecs with real content

972cadf

Merge remote-tracking branch 'origin/master' into pb-future

514a7d9

Xcode project update after merge

f398019

Merge remote-tracking branch 'origin/master' into pb-future

4eb8937

Use pod_target_xcconfig instead of xcconfig

57c4a3e

Unit tests run interoperably with Firebase pod

7aab4a0

Build integration tests

0e84d7e

paulb777 requested review from wilhuff, davidair, ryanwilson, protocol86, OleksiyA, schmidt-sebastian, morganchen12 and totoriott November 13, 2017 14:57

wilhuff reviewed Nov 13, 2017

View reviewed changes

paulb777 added 2 commits November 14, 2017 14:53

Review feedback and making FirebaseFirestore publishable

24f78bc

style

1a00824

This was referenced Nov 15, 2017

BuildFrameworks generates frameworks that cannot be signed #455

Closed

Add script to clean up generated frameworks #456

Closed

wilhuff reviewed Nov 15, 2017

View reviewed changes

System imports/includes should be in a separate group

e50f081

wilhuff assigned paulb777 Nov 15, 2017

ryanwilson reviewed Nov 16, 2017

View reviewed changes

morganchen12 approved these changes Nov 17, 2017

View reviewed changes

ryanwilson approved these changes Nov 17, 2017

View reviewed changes

wilhuff approved these changes Nov 17, 2017

View reviewed changes

paulb777 added 3 commits November 17, 2017 08:17

README.md formatting

b2b5e32

Delete deprecated FirebaseCommunity instructions

43fb2b6

Merge remote-tracking branch 'origin/master' into pb-future

daae87c

paulb777 mentioned this pull request Nov 17, 2017

Crashed: com.google.iid-token-operations (QOS: UTILITY) #402

Closed

Merge remote-tracking branch 'origin/master' into pb-future

ee33aee

paulb777 merged commit 3e59252 into master Nov 22, 2017

paulb777 mentioned this pull request Nov 22, 2017

Use brackets for module imports #487

Merged

paulb777 deleted the pb-future branch March 5, 2018 23:02

minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request Jun 6, 2018

Buildable and interoperable source pods (firebase#444)

6af05fb

firebase locked and limited conversation to collaborators Nov 11, 2019

		#import "FirebaseCommunity/FIRLogger.h"

		#import "FirebaseCore/FIRLogger.h"

		@@ -1,138 +1,136 @@
		#remove next two lines after Firebase 4.6.0 release and before merge

Buildable and interoperable source pods #444

Buildable and interoperable source pods #444

Conversation

paulb777 commented Nov 11, 2017

wilhuff left a 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

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

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

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

ryanwilson left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morganchen12 left a comment

Choose a reason for hiding this comment

ryanwilson left a comment

Choose a reason for hiding this comment

wilhuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment