-
Notifications
You must be signed in to change notification settings - Fork 3
feat: preliminary find_package support #452
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
Changes from 2 commits
a9bfe15
201e989
bdcc9c4
61f1108
c51b5c2
6eba487
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ cmake_minimum_required(VERSION 3.19) | |
| include(CMakeDependentOption) | ||
|
|
||
| project( | ||
| LaunchDarklyCPPSDKs | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Attempting to unify the project name with the target alias namespace (e.g. This is what users will encounter when they call |
||
| launchdarkly | ||
| VERSION 0.1 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: currently the version number isn't being updated, and I don't have a great plan for how that will work. Likely I will change to exporting two different package configs rather than the monorepo itself. |
||
| DESCRIPTION "LaunchDarkly C++ SDK Monorepo (Server/Client)" | ||
| LANGUAGES CXX C | ||
|
|
@@ -21,8 +21,6 @@ if (POLICY CMP0144) | |
| cmake_policy(SET CMP0144 NEW) | ||
| endif () | ||
|
|
||
| include(GNUInstallDirs) | ||
|
|
||
| option(BUILD_TESTING "Top-level switch for testing. Turn off to disable unit and contract tests." ON) | ||
|
|
||
| option(LD_BUILD_SHARED_LIBS "Build the SDKs as shared libraries" OFF) | ||
|
|
@@ -155,12 +153,20 @@ endif () | |
|
|
||
| set(Boost_USE_MULTITHREADED ON) | ||
| set(Boost_USE_STATIC_RUNTIME OFF) | ||
|
|
||
| if (POLICY CMP0167) | ||
| # TODO: Update to use the Boost project's cmake config directly, since FindBoost was deprecated in | ||
| # cmake >= 3.30. | ||
| cmake_policy(SET CMP0167 OLD) | ||
| endif () | ||
|
|
||
| find_package(Boost 1.81 REQUIRED COMPONENTS json url coroutine) | ||
| message(STATUS "LaunchDarkly: using Boost v${Boost_VERSION}") | ||
|
|
||
| include(${CMAKE_FILES}/certify.cmake) | ||
| set(FOXY_BUILD_TESTING OFF) | ||
| add_subdirectory(vendor/foxy) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a new |
||
|
|
||
|
|
||
| # Common, internal, and server-sent-events are built as "object" libraries. | ||
| add_subdirectory(libs/common) | ||
| add_subdirectory(libs/internal) | ||
|
|
@@ -185,3 +191,36 @@ if (LD_BUILD_EXAMPLES) | |
| message(STATUS "LaunchDarkly: building examples") | ||
| add_subdirectory(examples) | ||
| endif () | ||
|
|
||
|
|
||
| # Support installation of a cmake package. | ||
| include(CMakePackageConfigHelpers) | ||
| include(GNUInstallDirs) | ||
|
|
||
| write_basic_package_version_file( | ||
| "${CMAKE_CURRENT_BINARY_DIR}/launchdarklyConfigVersion.cmake" | ||
| COMPATIBILITY SameMajorVersion | ||
| ) | ||
|
|
||
| install(FILES | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/cmake/launchdarklyConfig.cmake" | ||
| "${CMAKE_CURRENT_BINARY_DIR}/launchdarklyConfigVersion.cmake" | ||
| DESTINATION "${CMAKE_INSTALL_DATADIR}/cmake/launchdarkly" | ||
| ) | ||
|
|
||
| configure_file( | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/cmake/launchdarkly.pc.in | ||
| ${CMAKE_CURRENT_BINARY_DIR}/launchdarkly.pc | ||
| ) | ||
|
|
||
| install( | ||
| FILES ${CMAKE_CURRENT_BINARY_DIR}/launchdarkly.pc | ||
| DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig | ||
| ) | ||
|
|
||
|
|
||
| install( | ||
| EXPORT launchdarklyTargets | ||
| NAMESPACE launchdarkly:: | ||
| DESTINATION "${CMAKE_INSTALL_DATADIR}/cmake/launchdarkly" | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ GoogleTest is used for testing. | |
|
|
||
| For information on integrating an SDK package please refer to the SDK specific README. | ||
|
|
||
| ## CMake Usage | ||
| ## CMake Options | ||
|
|
||
| Various CMake options are available to customize the client/server SDK builds. | ||
|
|
||
|
|
@@ -66,30 +66,59 @@ Various CMake options are available to customize the client/server SDK builds. | |
| | `LD_DYNAMIC_LINK_OPENSSL` | Whether OpenSSL is dynamically linked or not. | Off (static link) | N/A | | ||
| | `LD_BUILD_REDIS_SUPPORT` | Whether the server-side Redis Source is built or not. | Off | N/A | | ||
|
|
||
| **Note:** _if building the SDKs as shared libraries, then unit tests won't be able to link correctly since the SDK's C++ | ||
| symbols aren't exposed. To run unit tests, build a static library._ | ||
|
|
||
| > [!WARNING] | ||
| > When building shared libraries C++ symbols are not exported, only the C API will be exported. This is because C++ does | ||
| > not have a stable ABI. | ||
| > not have a stable ABI. For this reason, the SDK's unit tests are not built in shared library mode. | ||
|
|
||
| ## Building the SDK from Source | ||
|
|
||
| Basic usage example: | ||
| To configure the SDK's CMake project: | ||
|
|
||
| ```bash | ||
| mkdir -p build && cd build | ||
| cmake -G"Unix Makefiles" .. | ||
| # Use 'make' as the build system. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might as well advocate for "modern" cmake commands ( |
||
| cmake -B build -S . -G"Unix Makefiles" | ||
| ``` | ||
|
|
||
| Slightly more advanced example - build shared libraries, and don't build any of the testing components: | ||
| To pass in config options defined in the table above, add them using `-D`: | ||
|
|
||
| ```bash | ||
| mkdir -p build && cd build | ||
| cmake -G"Unix Makefiles" -DLD_BUILD_SHARED_LIBS=On -DBUILD_TESTING=Off .. | ||
| # Use 'make' as the build system, build shared libs, and disable testing. | ||
| cmake -B build -S . -G"Unix Makefiles" \ | ||
| -DLD_BUILD_SHARED_LIBS=On \ | ||
| -DBUILD_TESTING=Off .. | ||
| ``` | ||
|
|
||
| The example uses `make`, but you might instead use [Ninja](https://ninja-build.org/), | ||
| MSVC, [etc.](https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html) | ||
|
|
||
| ## Incorporating the SDK via `add_subdirectory` | ||
|
|
||
| The SDK can be incorporated into an existing application using CMake via `add_subdirectory.`. | ||
|
|
||
| ```cmake | ||
| # Set SDK build options, for example: | ||
| set(LD_BUILD_SHARED_LIBS On) | ||
|
|
||
| add_subdirectory(path-to-cpp-sdks-repo) | ||
| target_link_libraries(your-app PRIVATE launchdarkly::client) | ||
| # ... or launchdarkly::server | ||
| ```` | ||
|
|
||
| ## Incorporating the SDK via `find_package` | ||
|
|
||
| > [!WARNING] | ||
| > Preliminary support for `find_package` is available. The package configuration is subject to change, do not expect it | ||
| > to be stable as long as this notice is present. | ||
|
|
||
| If you've installed the SDK on the build system via `cmake --install`, you can consume it from | ||
| the target application like so: | ||
|
|
||
| ```cmake | ||
| find_package(launchdarkly REQUIRED) | ||
| target_link_libraries(your-app PRIVATE launchdarkly::launchdarkly-cpp-client) | ||
| # ... or launchdarkly::launchdarkly-cpp-server | ||
| ``` | ||
|
|
||
| ## LaunchDarkly overview | ||
|
|
||
| [LaunchDarkly](https://www.launchdarkly.com) is a feature management platform that serves trillions of feature flags | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| include(declareProjectTest.cmake) | ||
| add_subdirectory(test_find_package) | ||
| add_subdirectory(test_add_subdirectory) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # This test assumes that the SDK has been installed at CMAKE_INSTALL_PREFIX. | ||
| declare_find_package_test(test_find_package) | ||
| add_build_step(test_find_package) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| cmake_minimum_required(VERSION 3.19) | ||
|
|
||
| project(UseFindPackageTest) | ||
|
|
||
| find_package(launchdarkly REQUIRED) | ||
|
|
||
| add_executable(use_find_package_server main_server.cpp) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may wonder "why launchdarkly::launchdarkly-cpp-server"? Because when I originally named the targets, I was thinking "global namespace, it might clash with someone's Not sure how to make this match our |
||
| target_link_libraries(use_find_package_server launchdarkly::launchdarkly-cpp-server) | ||
|
|
||
|
|
||
| add_executable(use_find_package_client main_client.cpp) | ||
| target_link_libraries(use_find_package_client launchdarkly::launchdarkly-cpp-client) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #include <launchdarkly/client_side/client.hpp> | ||
| #include <launchdarkly/context_builder.hpp> | ||
|
|
||
| #include <cstring> | ||
| #include <iostream> | ||
|
|
||
| using namespace launchdarkly; | ||
| using namespace launchdarkly::client_side; | ||
|
|
||
| int main() { | ||
| auto config = ConfigBuilder("sdk-key").Build(); | ||
| if (!config) { | ||
| std::cout << "error: config is invalid: " << config.error() << '\n'; | ||
| return 1; | ||
| } | ||
|
|
||
| auto context = | ||
| ContextBuilder().Kind("user", "example-user-key").Name("Sandy").Build(); | ||
|
|
||
| auto client = Client(std::move(*config), std::move(context)); | ||
|
|
||
| client.StartAsync(); | ||
|
|
||
| std::cout << client.Initialized() << '\n'; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #include <launchdarkly/server_side/client.hpp> | ||
| #include <launchdarkly/server_side/config/config_builder.hpp> | ||
|
|
||
| #include <cstring> | ||
| #include <iostream> | ||
|
|
||
| using namespace launchdarkly; | ||
| using namespace launchdarkly::server_side; | ||
|
|
||
| int main() { | ||
| auto config = ConfigBuilder("sdk-key").Build(); | ||
| if (!config) { | ||
| std::cout << "error: config is invalid: " << config.error() << '\n'; | ||
| return 1; | ||
| } | ||
|
|
||
| auto client = Client(std::move(*config)); | ||
|
|
||
| client.StartAsync(); | ||
|
|
||
| std::cout << client.Initialized() << '\n'; | ||
|
|
||
| } |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,4 +13,8 @@ FetchContent_Declare(googletest | |
| ) | ||
| # For Windows: Prevent overriding the parent project's compiler/linker settings | ||
| set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) | ||
|
|
||
| # Disable installation of googletest | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could guard this with some kind of config option, that maybe is tied into |
||
| set(INSTALL_GTEST OFF CACHE BOOL "Disable googletest installation" FORCE) | ||
|
|
||
| FetchContent_MakeAvailable(googletest) | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| include(CMakeFindDependencyMacro) | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not entirely sure how to support this properly. With find_package, the idea is that the SDK was already built. Now, you may want to decide whether to statically or dynamically link boost/openSSL. But that depends on how you built the SDK in the first place.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, they basically just need to match. Unless we start building a bunch of permutations. I don't think that sounds amazing though. (Boost basically does this and it never seems to work when I actually use it.) |
||
| if (NOT DEFINED Boost_USE_STATIC_LIBS) | ||
| if (LD_DYNAMIC_LINK_BOOST) | ||
| set(Boost_USE_STATIC_LIBS OFF) | ||
| else () | ||
| set(Boost_USE_STATIC_LIBS ON) | ||
| endif () | ||
| endif () | ||
|
|
||
| find_dependency(Boost 1.81 COMPONENTS json url coroutine) | ||
| find_dependency(OpenSSL) | ||
| find_dependency(tl-expected) | ||
| find_dependency(certify) | ||
|
|
||
| include(${CMAKE_CURRENT_LIST_DIR}/launchdarklyTargets.cmake) | ||
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.
I'm arbitrarily installing into this folder here. Later, this folder is injected into the cmake configure step so
find_packagelooks there forlaunchdarklyConfig.cmake.That's one use-case, but it would also be good to check:
cmake --install .find_packagewithout specifying a customCMAKE_PREFIX_PATHfind_package's lookup logic magic.