From 6bc3f413fc86e6f993d036888dd101aff7ba5e82 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 19:19:58 +0200 Subject: [PATCH 1/7] Raise minimum required CMake version to 3.25 This allows us to use FIND_PACKAGE_ARGS for FetchContent with the SYSTEM argument. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ed4040c3..1f047f95 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ # object-introspection -cmake_minimum_required(VERSION 3.20) +cmake_minimum_required(VERSION 3.25) project(object-introspection) # Lets find_program() locate SETUID binaries From db8c33daed665763d1a58cfa7ae9bfbb4646e20e Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 19:23:31 +0200 Subject: [PATCH 2/7] Use SYSTEM for FetchContent_Declare statements Warnings in thirdparty dependencies are not very interesting for the development of an application. By setting the SYSTEM arg we will include the dependency header via `-isystem` instead. --- CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1f047f95..997a7967 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,6 +35,7 @@ find_package(gflags REQUIRED) ### tomlplusplus (for configuration files) FetchContent_Declare( tomlplusplus + SYSTEM GIT_REPOSITORY https://github.com/marzer/tomlplusplus.git GIT_TAG 4b166b69f28e70a416a1a04a98f365d2aeb90de8 # v3.2.0 GIT_PROGRESS TRUE @@ -44,6 +45,7 @@ FetchContent_MakeAvailable(tomlplusplus) ### glog FetchContent_Declare( glog + SYSTEM GIT_REPOSITORY https://github.com/google/glog.git GIT_TAG 96a2f23dca4cc7180821ca5f32e526314395d26a GIT_PROGRESS TRUE @@ -63,6 +65,7 @@ target_compile_options(utilities_unittest PRIVATE "-w") # After this is fixed with FetchContent, move to test/CMakeLists.txt. FetchContent_Declare( googletest + SYSTEM GIT_REPOSITORY https://github.com/google/googletest.git GIT_TAG 1ed6a8c67a0bd675149ece27bbec0ef1759854cf GIT_PROGRESS TRUE @@ -72,6 +75,7 @@ FetchContent_MakeAvailable(googletest) ### rocksdb FetchContent_Declare( rocksdb + SYSTEM GIT_REPOSITORY https://github.com/facebook/rocksdb.git GIT_TAG f32521662acf3352397d438b732144c7813bbbec # v8.5.3 GIT_PROGRESS TRUE @@ -91,6 +95,7 @@ include_directories(SYSTEM "${rocksdb_SOURCE_DIR}/include") ### use folly as a header only library. some features won't be supported. FetchContent_Declare( folly + SYSTEM GIT_REPOSITORY https://github.com/JakeHillion/folly.git GIT_TAG 8db54418e3ccdd97619ac8b69bb3702f82bb0f66 GIT_PROGRESS TRUE From 58763e133625b47c3d1f6c3d7754adcca73f2272 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 19:58:10 +0200 Subject: [PATCH 3/7] Use FIND_PACKAGE_ARGS for tomlplusplus Straightforward adaption to leverage `find_package` on those systems that have a tomlplusplus that's new enough for our purposes. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 997a7967..166acd20 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,6 +39,7 @@ FetchContent_Declare( GIT_REPOSITORY https://github.com/marzer/tomlplusplus.git GIT_TAG 4b166b69f28e70a416a1a04a98f365d2aeb90de8 # v3.2.0 GIT_PROGRESS TRUE + FIND_PACKAGE_ARGS 3.2.0 CONFIG ) FetchContent_MakeAvailable(tomlplusplus) From 7297265d9a9539b1d3e853a0a0efde7cf8fdb716 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 19:58:52 +0200 Subject: [PATCH 4/7] Leverage FIND_PACKAGE_ARGS for glog This is also relatively straight forward, except for the fact that we don't get the glog unittest targets when the dependency is found. Thus the changeset is slightly larger than one might expect otherwise. --- CMakeLists.txt | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 166acd20..8583137a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -48,18 +48,22 @@ FetchContent_Declare( glog SYSTEM GIT_REPOSITORY https://github.com/google/glog.git - GIT_TAG 96a2f23dca4cc7180821ca5f32e526314395d26a + GIT_TAG 96a2f23dca4cc7180821ca5f32e526314395d26a # v0.4.0 GIT_PROGRESS TRUE + FIND_PACKAGE_ARGS 0.4.0 CONFIG ) FetchContent_MakeAvailable(glog) -# These glog executable targets still generate warnings - disable warnings for -# them explicitly -target_compile_options(demangle_unittest PRIVATE "-w") -target_compile_options(logging_unittest PRIVATE "-w") -target_compile_options(stl_logging_unittest PRIVATE "-w") -target_compile_options(symbolize_unittest PRIVATE "-w") -target_compile_options(utilities_unittest PRIVATE "-w") +if (TARGET demangle_unittest) + # These glog executable targets still generate warnings - disable warnings for + # them explicitly + # NOTE: we only inherit these targets when FIND_PACKAGE_ARGS isn't used + target_compile_options(demangle_unittest PRIVATE "-w") + target_compile_options(logging_unittest PRIVATE "-w") + target_compile_options(stl_logging_unittest PRIVATE "-w") + target_compile_options(symbolize_unittest PRIVATE "-w") + target_compile_options(utilities_unittest PRIVATE "-w") +endif() ### googletest # Do this in the main file so it can be fetched before setting project warnings. From acdb28a44396d5a18ea3392327dae9e451957126 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 20:00:23 +0200 Subject: [PATCH 5/7] Use FetchContent + FIND_PACKAGE_ARGS for msgpack v5.0.0 or higher The version bump was done so that I can test it with the package I have available on my system. Previously, the embedded msgpack code didn't compile there. Now I simply can use the system provided msgpack here which is much better. --- CMakeLists.txt | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8583137a..5d067445 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -107,6 +107,17 @@ FetchContent_Declare( ) FetchContent_Populate(folly) +### msgpack +FetchContent_Declare( + msgpackc-cxx + SYSTEM + GIT_REPOSITORY https://github.com/msgpack/msgpack-c.git + GIT_TAG 8824c626b4c05698ec94ed77ababd64457f5574e # v5.0.0 + GIT_PROGRESS TRUE + FIND_PACKAGE_ARGS 5.0.0 CONFIG +) +FetchContent_MakeAvailable(msgpackc-cxx) + set_project_warnings() if (ASAN) @@ -188,14 +199,6 @@ find_package(Clang REQUIRED CONFIG) message(STATUS "Found Clang ${LLVM_PACKAGE_VERSION}") message(STATUS "Using ClangConfig.cmake in: ${Clang_DIR}") -### msgpack -# msgpack v3.0.0 doesn't define the msgpackc-cxx target, but since the library is header only, -# we can locate the header dir and add it to our include directories. -# Ideally, we would use a more modern version, like v3.3.0, and directly use the msgpackc-cxx target. -find_package(msgpack REQUIRED CONFIG) -get_target_property(MSGPACK_INCLUDE_DIRS msgpackc INTERFACE_INCLUDE_DIRECTORIES) -include_directories(SYSTEM ${MSGPACK_INCLUDE_DIRS}) - ### zstd (for rocksdb) find_package(zstd REQUIRED) From c671e906e9d00ee9d79ce5b5821c0ab0f7171489 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 20:08:52 +0200 Subject: [PATCH 6/7] Import FindRocksDB.cmake from caffe2 This code is copied from [1] which is Apache-2.0 licensed and owned by Meta, so I hope this is fine licensing wise. [1]: https://github.com/facebookarchive/caffe2/blob/157ff81a12cf48eb4b0a2ad854c4ae205ad8aa07/cmake/Modules/FindRocksDB.cmake --- cmake/FindRocksDB.cmake | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 cmake/FindRocksDB.cmake diff --git a/cmake/FindRocksDB.cmake b/cmake/FindRocksDB.cmake new file mode 100644 index 00000000..ef7f3b5f --- /dev/null +++ b/cmake/FindRocksDB.cmake @@ -0,0 +1,23 @@ +# Find the RocksDB libraries +# +# The following variables are optionally searched for defaults +# ROCKSDB_ROOT_DIR: Base directory where all RocksDB components are found +# +# The following are set after configuration is done: +# ROCKSDB_FOUND +# RocksDB_INCLUDE_DIR +# RocksDB_LIBRARIES + +find_path(RocksDB_INCLUDE_DIR NAMES rocksdb/db.h + PATHS ${ROCKSDB_ROOT_DIR} ${ROCKSDB_ROOT_DIR}/include) + +find_library(RocksDB_LIBRARIES NAMES rocksdb + PATHS ${ROCKSDB_ROOT_DIR} ${ROCKSDB_ROOT_DIR}/lib) + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(RocksDB DEFAULT_MSG RocksDB_INCLUDE_DIR RocksDB_LIBRARIES) + +if(ROCKSDB_FOUND) + message(STATUS "Found RocksDB (include: ${RocksDB_INCLUDE_DIR}, library: ${RocksDB_LIBRARIES})") + mark_as_advanced(RocksDB_INCLUDE_DIR RocksDB_LIBRARIES) +endif() From 5a6ef38c376a7ec56c748f76fef647258408c92e Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 29 Sep 2023 20:22:35 +0200 Subject: [PATCH 7/7] Make it possible to link against a system provided RocksDB Due to the custom build system integration for RocksDB we cannot simply use FIND_PACKAGE_ARGS. Instead, we first do a `find_package` and only if that fails do we fallback to the old rocksdb integration. To better facilitate this, I took the liberty to also modernize the CMake code a bit. Consumers of the lib just need to link against the new `rocksdb::librocksdb` alias target to get the required library and include directory. --- CMakeLists.txt | 56 ++++++++++++++++++++++------------------- cmake/FindRocksDB.cmake | 4 +++ 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5d067445..41fc6e56 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -78,23 +78,34 @@ FetchContent_Declare( FetchContent_MakeAvailable(googletest) ### rocksdb -FetchContent_Declare( - rocksdb - SYSTEM - GIT_REPOSITORY https://github.com/facebook/rocksdb.git - GIT_TAG f32521662acf3352397d438b732144c7813bbbec # v8.5.3 - GIT_PROGRESS TRUE -) -FetchContent_Populate(rocksdb) -add_custom_target(librocksdb ALL - WORKING_DIRECTORY ${rocksdb_SOURCE_DIR} - COMMAND cmake -G Ninja -B ${rocksdb_BINARY_DIR} -DCMAKE_BUILD_TYPE=Release -DWITH_GFLAGS=Off -DWITH_LIBURING=Off -DWITH_ZSTD=On -DFAIL_ON_WARNINGS=Off - COMMAND cmake --build ${rocksdb_BINARY_DIR} --target rocksdb - BYPRODUCTS ${rocksdb_BINARY_DIR}/librocksdb.a - COMMENT "Building RocksDB" - USES_TERMINAL -) -include_directories(SYSTEM "${rocksdb_SOURCE_DIR}/include") +find_package(RocksDB 8.5.3) +if (NOT RocksDB_FOUND) + FetchContent_Declare( + rocksdb + SYSTEM + GIT_REPOSITORY https://github.com/facebook/rocksdb.git + GIT_TAG f32521662acf3352397d438b732144c7813bbbec # v8.5.3 + GIT_PROGRESS TRUE + ) + FetchContent_Populate(rocksdb) + add_custom_target(librocksdb_build ALL + WORKING_DIRECTORY ${rocksdb_SOURCE_DIR} + COMMAND cmake -G Ninja -B ${rocksdb_BINARY_DIR} -DCMAKE_BUILD_TYPE=Release -DWITH_GFLAGS=Off -DWITH_LIBURING=Off -DWITH_ZSTD=On -DFAIL_ON_WARNINGS=Off + COMMAND cmake --build ${rocksdb_BINARY_DIR} --target rocksdb + BYPRODUCTS ${rocksdb_BINARY_DIR}/librocksdb.a + COMMENT "Building RocksDB" + USES_TERMINAL + ) + add_library(librocksdb INTERFACE) + add_dependencies(librocksdb librocksdb_build) + target_include_directories(librocksdb INTERFACE SYSTEM "${rocksdb_SOURCE_DIR}/include") + add_library(rocksdb::librocksdb ALIAS librocksdb) + + ### zstd (for rocksdb) + find_package(zstd REQUIRED) + + target_link_libraries(librocksdb INTERFACE zstd::zstd ${rocksdb_BINARY_DIR}/librocksdb.a) +endif() ### folly ### use folly as a header only library. some features won't be supported. @@ -130,7 +141,6 @@ if (CODE_COVERAGE) add_link_options(--coverage) endif() - ## System checks ## These checks are potentially fatal so perform them first. @@ -199,9 +209,6 @@ find_package(Clang REQUIRED CONFIG) message(STATUS "Found Clang ${LLVM_PACKAGE_VERSION}") message(STATUS "Using ClangConfig.cmake in: ${Clang_DIR}") -### zstd (for rocksdb) -find_package(zstd REQUIRED) - ### drgn # The setup.py script in drgn is really meant to build drgn (python # debugger). It shoves the C headers/lib in a temporary directory (which @@ -345,9 +352,8 @@ add_library(treebuilder ) add_dependencies(treebuilder librocksdb) target_link_libraries(treebuilder - ${rocksdb_BINARY_DIR}/librocksdb.a + rocksdb::librocksdb oicore # overkill but it does need a lot of stuff - zstd::zstd ) @@ -383,10 +389,8 @@ target_link_libraries(oip oicore) ### Object Introspection RocksDB Printer (OIRP) add_executable(oirp tools/OIRP.cpp) -add_dependencies(oirp librocksdb) target_link_libraries(oirp - ${rocksdb_BINARY_DIR}/librocksdb.a - zstd::zstd + rocksdb::librocksdb msgpackc ) diff --git a/cmake/FindRocksDB.cmake b/cmake/FindRocksDB.cmake index ef7f3b5f..a173fe54 100644 --- a/cmake/FindRocksDB.cmake +++ b/cmake/FindRocksDB.cmake @@ -20,4 +20,8 @@ find_package_handle_standard_args(RocksDB DEFAULT_MSG RocksDB_INCLUDE_DIR RocksD if(ROCKSDB_FOUND) message(STATUS "Found RocksDB (include: ${RocksDB_INCLUDE_DIR}, library: ${RocksDB_LIBRARIES})") mark_as_advanced(RocksDB_INCLUDE_DIR RocksDB_LIBRARIES) + add_library(librocksdb UNKNOWN IMPORTED) + set_target_properties(librocksdb PROPERTIES IMPORTED_LOCATION ${RocksDB_LIBRARIES}) + target_include_directories(librocksdb INTERFACE ${RocksDB_INCLUDE_DIR}) + add_library(rocksdb::librocksdb ALIAS librocksdb) endif()