Skip to content

Add RE2 library to desktop packaging & Update MSVC dependencies #981

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jun 3, 2022
Merged

Add RE2 library to desktop packaging & Update MSVC dependencies #981

merged 6 commits into from
Jun 3, 2022

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Jun 2, 2022 •
edited by a-maurice
Loading

The PR firebase/firebase-ios-sdk#9488 in the firebase-ios-sdk added a dependency on a new third-party library, "re2". This was picked up by the firebase-cpp-sdk in #977.

All of the CI builds and tests worked fine; however, the "blastdoor" post-processing step renamed the re2 namespace that caused linking error when linking against the packaged SDK (e.g. https://github.com/firebase/firebase-cpp-sdk/runs/6701070693).

This PR fixes the linking error by configuring blastdoor to properly handle re2. This was 100% based on the same thing for the snappy dependency in #885.

Also, in the same PR the versions of grpc and absl were updated, which on Windows requires linking against dbghelp and bcrypt, so add those to the set of libraries in the integration test.

@dconeybe dconeybe added api: firestore skip-release-notes Skip release notes check labels Jun 2, 2022
@dconeybe dconeybe self-assigned this Jun 2, 2022
@dconeybe
Copy link
Contributor Author

dconeybe commented Jun 2, 2022 •
edited
Loading

CPP binary SDK packaging workflow started: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2425793137 https://github.com/firebase/firebase-cpp-sdk/actions/runs/2425814817

@dconeybe
Copy link
Contributor Author

dconeybe commented Jun 2, 2022

Integration test triggered against the packaging workflow: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2428059862

@dconeybe
Copy link
Contributor Author

dconeybe commented Jun 2, 2022

Trying the packaging workflow again: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2428095690

@dconeybe
Copy link
Contributor Author

dconeybe commented Jun 2, 2022

I've kicked off the integration tests to test the build artifacts of the packaging workflow 2428095690 mentioned in the previous comment: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2429785219

@dconeybe dconeybe requested a review from jonsimantov June 2, 2022 17:18
@jonsimantov jonsimantov changed the title blastdoor configuration for re2 Add RE2 library to desktop packaging Jun 2, 2022
@dconeybe
Copy link
Contributor Author

dconeybe commented Jun 2, 2022

Newer packaging workflow: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2430137781

@dconeybe dconeybe assigned jonsimantov and unassigned dconeybe Jun 2, 2022
jonsimantov
jonsimantov approved these changes Jun 2, 2022
View reviewed changes
Copy link
Contributor

@jonsimantov jonsimantov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in the packaging run log. Hopefully the integration tests work: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2430865138

@a-maurice a-maurice changed the title Add RE2 library to desktop packaging Add RE2 library to desktop packaging & Update MSVC dependencies Jun 2, 2022
@a-maurice a-maurice added the tests-requested: full Trigger a FULL set of integration tests (uses expanded test matrix). label Jun 2, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: full Trigger a FULL set of integration tests (uses expanded test matrix). labels Jun 2, 2022
@github-actions
Copy link

github-actions bot commented Jun 2, 2022 •
edited
Loading

❌  Integration test FAILED

Requested by @jonsimantov on commit 7e50de8
Last updated: Fri Jun 3 14:22 PDT 2022
View integration test log & download artifacts

Failures Configs
firestore [TEST] [FAILURE] [tvOS] [macos] [tvos_simulator]
(1 failed tests)  CleanupTest.DocumentChangeIsBlankAfterCleanup
[TEST] [FLAKINESS] [Android] [1/3 os: macos] [1/2 android_device: android_target]
(1 failed tests)  CRASH/TIMEOUT

Add flaky tests to go/fpl-cpp-flake-tracker

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Jun 3, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 3, 2022
jonsimantov
jonsimantov requested changes Jun 3, 2022
View reviewed changes
@dconeybe dconeybe mentioned this pull request Jun 3, 2022
Closed
@jonsimantov jonsimantov self-requested a review June 3, 2022 17:30
@jonsimantov jonsimantov removed the tests: failed This PR's integration tests failed. label Jun 3, 2022
@jonsimantov jonsimantov enabled auto-merge (squash) June 3, 2022 17:47
@jonsimantov jonsimantov removed the skip-release-notes Skip release notes check label Jun 3, 2022
@jonsimantov jonsimantov merged commit 7e50de8 into main Jun 3, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. labels Jun 3, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 3, 2022
@firebase firebase locked and limited conversation to collaborators Jul 4, 2022
@a-maurice a-maurice deleted the dconeybe/Re2Blastdoor branch October 25, 2022 21:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cynthiajoan cynthiajoan cynthiajoan approved these changes

@jonsimantov jonsimantov jonsimantov approved these changes

Assignees

@jonsimantov jonsimantov

Labels
api: firestore tests: failed This PR's integration tests failed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dconeybe @cynthiajoan @jonsimantov @a-maurice