Remove the Firestore Snappy patch by dconeybe · Pull Request #932 · firebase/firebase-cpp-sdk · GitHub

Remove the logic to patch the firebase-ios-sdk dependency to add Snappy support.

This PR will need to be merged when this SDK updates its dependency on the firebase-ios-sdk repository since firebase/firebase-ios-sdk#9596 has been merged to add Snappy support directly in the iOS SDK.

The Snappy patching was originally added by #885. This PR effectively reverts that PR now that Snappy support is natively added to the firebase-ios-sdk repository.

Googlers see b/227782613 for more details.

…version

✅ Integration test succeeded!

Requested by @dconeybe on commit a31c796
Last updated: Tue Jun 21 15:49 PDT 2022
View integration test log & download artifacts

Looks like there are some failures, at least on Windows

Looks like there are some failures, at least on Windows

I'm not sure what was going on there. I've merged in main to see if that fixes anything. The desktop.yml that was used was not consistent with HEAD of the main branch.

@jonsimantov It looks like ctest is trying to run the grpc tests, and those tests don't get compiled. This is odd because when I build everything via cmake locally, ctest does not try to run the grpc tests. Any ideas why it's trying to run the grpc tests and failing to build them?

Hmm, I'm not sure why that would be any different here than in previous occasions. @a-maurice any ideas?

@jonsimantov @a-maurice Many of the same build errors occur when pointing the C++ SDK at the HEAD of the master branch of the firebase-ios-sdk repository: #950. I'm going to move my investigation to that PR until it goes green.

I'm guessing that the upgrade of grpc in the iOS SDK is the culprit: firebase/firebase-ios-sdk#9488. I think we just need to set the cmake cache variable to disable the "re2" tests (RE2_BUILD_TESTING).

… PR that adds Snappy support has been merged there

…f the firebase-ios-sdk

…tic python_setup.cmake bug

…, which fixes a problematic usage of designated initialization

…ledFromBackend (#972)

@jonsimantov Can you take a look at this PR? I won't merge it into main but rather into the PR that updates the iOS dependencies.

Shouldn't GIT_TAG be ${version}, which will be updated to 9.2.0 or whatever when the time comes?

Ahh yes, this particular part of the PR will be reverted. It's just here so I can test again the HEAD of the firebase-ios-sdk.

LGTM, but as noted in the PR, we'll keep this a draft and merge it into the upcoming iOS dependency update.

### iOS - Firebase/Analytics → 9.2.0 - Firebase/Auth → 9.2.0 - Firebase/Core → 9.2.0 - Firebase/Crashlytics → 9.2.0 - Firebase/Database → 9.2.0 - Firebase/DynamicLinks → 9.2.0 - Firebase/Firestore → 9.2.0 - Firebase/Functions → 9.2.0 - Firebase/Installations → 9.2.0 - Firebase/Messaging → 9.2.0 - Firebase/RemoteConfig → 9.2.0 - Firebase/Storage → 9.2.0 > Created by [Update Android and iOS dependencies workflow](https://github.com/firebase/firebase-cpp-sdk/actions/runs/2544841237).

…220622-200757' into SnappyPatchRevert

The changes in this PR were merged into the main branch in #1003

firestore.cmake: remove the snappy patch and just update the leveldb …

83306fd

…version

dconeybe added api: firestore skip-release-notes Skip release notes check labels May 6, 2022

dconeybe self-assigned this May 6, 2022

dconeybe and others added 2 commits May 9, 2022 18:34

REVERT ME: Pin firebase-ios-sdk to firebase/firebase-ios-sdk@ece6774

346a4a4

Merge branch 'main' into dconeybe/SnappyPatchRevert

e8f77a4

github-actions Bot added the tests: in-progress This PR's integration tests are in progress. label May 9, 2022

Merge branch 'main' into dconeybe/SnappyPatchRevert

6557527

github-actions Bot added the tests: failed This PR's integration tests failed. label May 10, 2022

firebase-workflow-trigger Bot removed the tests: in-progress This PR's integration tests are in progress. label May 10, 2022

dconeybe mentioned this pull request May 10, 2022

Enable Snappy compression support in LevelDb in cmake builds firebase/firebase-ios-sdk#9596

Merged

dconeybe added 3 commits May 20, 2022 13:58

Merge remote-tracking branch 'origin/main' into SnappyPatchRevert

12bc437

firestore.cmake: updated pinned firebase-ios-sdk to HEAD now that the…

57d03a9

… PR that adds Snappy support has been merged there

CMakeLists.txt: set(RE2_BUILD_TESTING OFF CACHE BOOL "")

bdcbbbc

dconeybe mentioned this pull request May 20, 2022

DO NOT MERGE: firestore.cmake: point at HEAD commit in master branch: https://githu… #950

Closed

dconeybe assigned wu-hui May 20, 2022

dconeybe added a commit that referenced this pull request May 25, 2022

Patch in #932 to get the C++ SDK to build successfully against HEAD o…

a1d4c3e

…f the firebase-ios-sdk

dconeybe and others added 6 commits May 26, 2022 09:29

Merge remote-tracking branch 'origin/main' into SnappyPatchRevert

c9dbc89

firestore.cmake: update pinned commit to #967 , which fixes a problema…

ce9ca4f

…tic python_setup.cmake bug

firestore.cmake: update pinned commit to firebase/firebase-ios-sdk#9851 …

c834bc6

…, which fixes a problematic usage of designated initialization

cherry-pick bundle_test.cc: Disable LoadBundleWithDocumentsAlreadyPul…

b39452a

…ledFromBackend (#972)

bundle_test.cc: disable ALL of the tests

f8e38b1

Support git longpaths (#967 )

61d6ff9

firebase-workflow-trigger Bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 1, 2022

github-actions Bot added the tests: succeeded This PR's integration tests succeeded. label Jun 2, 2022

dconeybe removed the tests: failed This PR's integration tests failed. label Jun 2, 2022

dconeybe added 2 commits June 3, 2022 12:34

Merge remote-tracking branch 'origin/main' into SnappyPatchRevert

655b7b6

Merge remote-tracking branch 'origin/main' into SnappyPatchRevert

f033074

dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jun 7, 2022

github-actions Bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. tests: succeeded This PR's integration tests succeeded. labels Jun 7, 2022

dconeybe added 2 commits June 7, 2022 15:08

replace the fancy python script with a simple patch

6a7e47d

leveldb.cmake: fix comment: firestore.patch -> firestore.patch.txt

5e75846

dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jun 7, 2022

github-actions Bot added tests: succeeded This PR's integration tests succeeded. and removed tests-requested: quick Trigger a quick set of integration tests. labels Jun 7, 2022

firebase-workflow-trigger Bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 7, 2022

dconeybe requested a review from jonsimantov June 7, 2022 22:53

dconeybe assigned jonsimantov Jun 7, 2022

jonsimantov reviewed Jun 8, 2022

View reviewed changes

jonsimantov self-requested a review June 10, 2022 17:34

jonsimantov previously approved these changes Jun 10, 2022

View reviewed changes

Merge remote-tracking branch 'origin/main' into SnappyPatchRevert

e145df3

dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jun 21, 2022

github-actions Bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. tests: succeeded This PR's integration tests succeeded. labels Jun 21, 2022

firebase-workflow-trigger-bot and others added 3 commits June 22, 2022 20:30

Merge remote-tracking branch 'origin/workflow/auto-update-deps-116-20…

1f15efb

…220622-200757' into SnappyPatchRevert

remove pinned firebase-ios-sdk commit

1fcbbeb

-                  URL https://github.com/firebase/firebase-ios-sdk/archive/${version}.tar.gz
+                  GIT_REPOSITORY "https://github.com/firebase/firebase-ios-sdk.git"
+                  # Pinned HEAD commit as of June 01, 2022 @ 10:51 EDT.
+                  GIT_TAG 89c38d1ed908dbc10d5f9f7aded4cf271113773f

Remove the Firestore Snappy patch by dconeybe · Pull Request #932 · firebase/firebase-cpp-sdk · GitHub

Uh oh!

Remove the Firestore Snappy patch - #932

Remove the Firestore Snappy patch#932
dconeybe wants to merge 40 commits into
mainfrom
dconeybe/SnappyPatchRevert

dconeybe commented May 6, 2022 •
edited
Loading

Uh oh!

Uh oh!

github-actions Bot commented May 9, 2022 •
edited
Loading

Uh oh!

Uh oh!

jonsimantov commented May 9, 2022

Uh oh!

dconeybe commented May 10, 2022

Uh oh!

dconeybe commented May 10, 2022

Uh oh!

jonsimantov commented May 10, 2022

Uh oh!

dconeybe commented May 11, 2022

Uh oh!

dconeybe commented May 11, 2022

Uh oh!

dconeybe commented Jun 7, 2022

Uh oh!

jonsimantov Jun 8, 2022

Uh oh!

dconeybe Jun 9, 2022

Uh oh!

jonsimantov left a comment

Uh oh!

dconeybe commented Jun 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Remove the Firestore Snappy patch by dconeybe · Pull Request #932 · firebase/firebase-cpp-sdk · GitHub

Uh oh!

Remove the Firestore Snappy patch - #932

Remove the Firestore Snappy patch#932dconeybe wants to merge 40 commits intomainfirebase/firebase-cpp-sdk:mainfrom dconeybe/SnappyPatchRevertfirebase/firebase-cpp-sdk:dconeybe/SnappyPatchRevertCopy head branch name to clipboard

Conversation

dconeybe commented May 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Integration test succeeded!

Uh oh!

jonsimantov commented May 9, 2022

Uh oh!

dconeybe commented May 10, 2022

Uh oh!

dconeybe commented May 10, 2022

Uh oh!

jonsimantov commented May 10, 2022

Uh oh!

dconeybe commented May 11, 2022

Uh oh!

dconeybe commented May 11, 2022

Uh oh!

dconeybe commented Jun 7, 2022

Uh oh!

jonsimantov Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

dconeybe Jun 9, 2022

Choose a reason for hiding this comment

Uh oh!

jonsimantov left a comment

Choose a reason for hiding this comment

Uh oh!

dconeybe commented Jun 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Remove the Firestore Snappy patch#932
dconeybe wants to merge 40 commits into
mainfrom
dconeybe/SnappyPatchRevert

dconeybe commented May 6, 2022 •
edited
Loading

github-actions Bot commented May 9, 2022 •
edited
Loading