feat: preliminary find_package support #452

cwaldren-ld · 2024-10-07T21:52:26Z

This PR adds preliminary support for:

find_package(launchdarkly REQUIRED)

target_link_libraries(app 
        PRIVATE 
        launchdarkly::{launchdarkly-cpp-server,launchdarkly-cpp-client}
)

It treats the C++ monorepo as the installable unit. In the very near future, this will likely be refactored so that we have two packages. If you want both on the same host, you'd need to install both (and there'd be some redundancy.)

Currently there is only one version, 0.1.0. This isn't updated by release-please.

For these two reasons, I'm indicating that this is preliminary and subject to change form.

cwaldren-ld · 2024-10-07T22:12:04Z

CMakeLists.txt

@@ -6,7 +6,7 @@ cmake_minimum_required(VERSION 3.19)
 include(CMakeDependentOption)

 project(
-        LaunchDarklyCPPSDKs


Attempting to unify the project name with the target alias namespace (e.g. launchdarkly::server).

This is what users will encounter when they call find_package, so it would be predictable to have that be the same as what they link against.

cwaldren-ld · 2024-10-07T22:13:23Z

CMakeLists.txt

@@ -6,7 +6,7 @@ cmake_minimum_required(VERSION 3.19)
 include(CMakeDependentOption)

 project(
-        LaunchDarklyCPPSDKs
+        launchdarkly
        VERSION 0.1


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.

cwaldren-ld · 2024-10-07T23:52:47Z

.github/actions/cmake-test/action.yml

@@ -36,6 +35,15 @@ runs:
        # along in the same manner to those test projects via the command line.
        BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }}
        OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }}
+        CMAKE_INSTALL_PREFIX: ../LAUNCHDARKLY_INSTALL


I'm arbitrarily installing into this folder here. Later, this folder is injected into the cmake configure step so find_package looks there for launchdarklyConfig.cmake.

That's one use-case, but it would also be good to check:

If you don't give it an install prefix, and just run cmake --install .

Then if you invoke find_package without specifying a custom CMAKE_PREFIX_PATH

It will work, due to find_package's lookup logic magic.

cwaldren-ld · 2024-10-07T23:53:37Z

CMakeLists.txt

 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)


I added a new FOXY_BUILD_TESTING to avoid the whole "get the previous value of BUILD_TESTING, set it to off, add_subdir, set the value back.." song and dance.

cwaldren-ld · 2024-10-07T23:54:20Z

README.md


 ```bash
-mkdir -p build && cd build
-cmake -G"Unix Makefiles" ..
+# Use 'make' as the build system.


Might as well advocate for "modern" cmake commands (-B build -S .)

cwaldren-ld · 2024-10-07T23:55:46Z

cmake/googletest.cmake

@@ -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


We could guard this with some kind of config option, that maybe is tied into LD_BUILD_UNIT_TESTS or something. For now, don't attempt to install googletest when the SDK is installed.

cwaldren-ld · 2024-10-07T23:58:21Z

cmake/launchdarklyConfig.cmake

@@ -0,0 +1,16 @@
+include(CMakeFindDependencyMacro)
+


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.

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.)

cwaldren-ld · 2024-10-07T23:58:49Z

libs/client-sdk/src/CMakeLists.txt

@@ -37,7 +37,6 @@ target_sources(${LIBNAME} PRIVATE
        flag_manager/context_index.cpp
        flag_manager/flag_manager.cpp
        flag_manager/flag_persistence.cpp
-        bindings/c/sdk.cpp


This was just a redundant entry.

cwaldren-ld · 2024-10-08T00:00:15Z

libs/client-sdk/src/CMakeLists.txt

@@ -46,22 +45,29 @@ target_link_libraries(${LIBNAME}
        PRIVATE Boost::headers Boost::json Boost::url launchdarkly::sse launchdarkly::internal foxy)



The changes in most of these sub-lib CMakeLists.txt are:

Switch from target_include_directories(${LIBNAME} PUBLIC ../include to one that uses generator expressions, so it works at build and installation time

Swap out raw lib or bin paths with GNUInstallDir variables

Install the targets to our LD_TARGETS_EXPORT_NAME

cwaldren-ld · 2024-10-08T00:04:41Z

cmake-tests/test_find_package/project/CMakeLists.txt

+
+find_package(launchdarkly REQUIRED)
+
+add_executable(use_find_package_server main_server.cpp)


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 server or client target."

Not sure how to make this match our launchdarkly::{client,server} aliases that are usable in the add_subdirectory case.

cwaldren-ld · 2024-10-08T00:04:56Z

cmake/certify.cmake

@@ -1,23 +0,0 @@
-cmake_minimum_required(VERSION 3.11)


Moved into foxy, since this is really a foxy dependency.

feat: preliminary find_package support

a9bfe15

cwaldren-ld force-pushed the cw/SDK-354-cmake-find-package branch from 6818481 to a9bfe15 Compare October 7, 2024 21:53

update vendor/foxy to use custom certify

201e989

cwaldren-ld commented Oct 7, 2024

View reviewed changes

cwaldren-ld added 2 commits October 7, 2024 15:15

don't install .pc file

bdcc9c4

maybe a quotting issue?:

61f1108

cwaldren-ld force-pushed the cw/SDK-354-cmake-find-package branch from da6370e to 410e69c Compare October 7, 2024 23:14

fix exports name

c51b5c2

cwaldren-ld force-pushed the cw/SDK-354-cmake-find-package branch from 410e69c to c51b5c2 Compare October 7, 2024 23:16

cwaldren-ld marked this pull request as ready for review October 7, 2024 23:50

cwaldren-ld requested a review from a team as a code owner October 7, 2024 23:50

cwaldren-ld commented Oct 7, 2024

View reviewed changes

cwaldren-ld commented Oct 8, 2024

View reviewed changes

switch to launchdarkly/certify

6eba487

cwaldren-ld requested a review from kinyoklion October 8, 2024 16:23

kinyoklion approved these changes Oct 8, 2024

View reviewed changes

cwaldren-ld merged commit 1ca48d5 into main Oct 8, 2024
23 checks passed

cwaldren-ld deleted the cw/SDK-354-cmake-find-package branch October 8, 2024 23:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: preliminary find_package support #452

feat: preliminary find_package support #452

cwaldren-ld commented Oct 7, 2024 •

edited

Loading

cwaldren-ld Oct 7, 2024 •

edited

Loading

cwaldren-ld Oct 7, 2024

cwaldren-ld Oct 7, 2024

cwaldren-ld Oct 7, 2024

cwaldren-ld Oct 7, 2024

cwaldren-ld Oct 7, 2024

cwaldren-ld Oct 7, 2024

kinyoklion Oct 8, 2024

cwaldren-ld Oct 7, 2024

cwaldren-ld Oct 8, 2024

cwaldren-ld Oct 8, 2024

cwaldren-ld Oct 8, 2024

		@@ -46,22 +45,29 @@ target_link_libraries(${LIBNAME}
		PRIVATE Boost::headers Boost::json Boost::url launchdarkly::sse launchdarkly::internal foxy)


		find_package(launchdarkly REQUIRED)

		add_executable(use_find_package_server main_server.cpp)

feat: preliminary find_package support #452

feat: preliminary find_package support #452

Conversation

cwaldren-ld commented Oct 7, 2024 • edited Loading

cwaldren-ld Oct 7, 2024 • edited Loading

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

cwaldren-ld commented Oct 7, 2024 •

edited

Loading

cwaldren-ld Oct 7, 2024 •

edited

Loading