- Notifications
You must be signed in to change notification settings - Fork 125
Allow all dependencies (curl, websockets, grpc) to use boringssl instead of openssl when making packaged C++ SDK. #207
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
Conversation
Add missing Firestore core headers to desktop package.
…irebase/firebase-cpp-sdk into feature/js-switch-to-boringssl
…ust the ones they are embedded in. Also pass along build options to the cmake build for boringssl. Also set curl to only build http/https support and not other things like SMTP, FTP, etc.
…irebase/firebase-cpp-sdk into feature/js-switch-to-boringssl
…tional sub-builds.
This is now fixed, you can link the SDK from source either with openssl (default) or specifying FIREBASE_USING_BORINGSSL. PTAL |
| Now that there is option to also use openssl, do we want to re-add openssl to vcpkg's list of packages? vcpkg will only make sense if we don't want to use a system specific package (vcpkg packages are localized in a directory and only used for that project) and if we care about the versions of openssl library. |
| I think we should keep it out of vcpkg - again, I do think it might make sense to move off of vcpkg and build all dependencies in the binary directory in the future. Also putting it into vcpkg causes cmake to sometimes think it wants to use that library. |
| Cool, sounds good. I agree, it would be awesome to have a self contained build in the future and avoid dealing with external packages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR gives you the option to switch all desktop platforms over to using boringssl rather than openssl, which does not like being embedded in our SDKs. If you are building a binary SDK, you may want to enable this option. If you are linking directly to our SDK from source via cmake, you probably don't need it (but you should ensure that OpenSSL is installed on your system.
It will be enabled if you turn on FIREBASE_USE_BORINGSSL at cmake configure time, e.g.:
cmake [source directory] -DFIREBASE_USE_BORINGSSL=ON
With this option enabled, it will build boringssl during the configure step by shelling out to another instance of cmake. Much work is done in build_external_dependencies to ensure that this (and future) sub-builds correctly match the main project's selection of:
A few notes on switching to boringSSL:
Additional changes in this PR:
You'll also see a few workaround at the end of the download_external_sources function, this is to work around a few issues in subprojects that expect openssl but are now getting boringssl. There are PRs to fix these in the upstream sources, but until we update to their latest version these will need to be there.