Proposed build system changes

[thompsonbry]

This diff accomplishes a few things.

• It supports the AWS internal build systems (if -DAWS_BUILD=TRUE is specified to cmake).
• It accounts for some differences in gtest and gmock handling
• It provides an install semantics

Please take a look when you get a chance and let's discuss.

diff --git a/third-party-src/CMakeLists.txt b/third-party-src/CMakeLists.txt
index 755029a..901b23f 100644
--- a/third-party-src/CMakeLists.txt
+++ b/third-party-src/CMakeLists.txt
@@ -295,6 +295,13 @@ set(THREADS_PREFER_PTHREAD_FLAG ON)

 find_package(Threads REQUIRED)

+if(AWS_BUILD)
+   find_package( "GSL" REQUIRED)
+   find_package( "GoogleBenchmark" REQUIRED)
+   find_package( "Googletest" REQUIRED )
+   find_package( "Googlemock" REQUIRED )
+endif()
+
 find_package(Boost REQUIRED)
 # TODO(laurynas): once the minimum CMake version is at least 3.13, convert to a
 # find_package(Boost) component check
@@ -365,9 +372,11 @@ endmacro()
 # For Windows: Prevent overriding the parent project's compiler/linker settings
 set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)

-ADD_CXX_FLAGS_FOR_SUBDIR()
-add_subdirectory(3rd_party/googletest)
-RESTORE_CXX_FLAGS_FOR_SUBDIR()
+if(!AWS_BUILD)
+  ADD_CXX_FLAGS_FOR_SUBDIR()
+  add_subdirectory(3rd_party/googletest)
+  RESTORE_CXX_FLAGS_FOR_SUBDIR()
+endif()

 set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "Suppressing Google Benchmark tests"
   FORCE)
@@ -389,9 +398,11 @@ if(IPO_SUPPORTED)
   endif()
 endif()

-ADD_CXX_FLAGS_FOR_SUBDIR()
-add_subdirectory(3rd_party/benchmark)
-RESTORE_CXX_FLAGS_FOR_SUBDIR()
+if(!AWS_BUILD)
+  ADD_CXX_FLAGS_FOR_SUBDIR()
+  add_subdirectory(3rd_party/benchmark)
+  RESTORE_CXX_FLAGS_FOR_SUBDIR()
+endif()

 # Do not build DeepState:
 # - under Windows as it's not supported
@@ -399,7 +410,7 @@ RESTORE_CXX_FLAGS_FOR_SUBDIR()
 # - if 32-bit build is not possible
 # - with GCC under macOS due to https://github.com/trailofbits/deepstate/issues/374
 # - under macOS with ASan or TSan enabled
-if(NOT (is_darwin AND (is_gxx OR SANITIZE_ADDRESS OR SANITIZE_THREAD))
+if(NOT(AWS_BUILD) AND NOT (is_darwin AND (is_gxx OR SANITIZE_ADDRESS OR SANITIZE_THREAD))
     AND NOT is_windows AND is_x86_64)
   CHECK_INCLUDE_FILE(stdio.h CAN_BUILD_32BIT -m32)

@@ -484,14 +495,18 @@ set(is_standalone "$<BOOL:${STANDALONE}>")
 set(is_gxx_not_release_standalone
   $<AND:${is_gxx_genex},${is_not_release_genex},${is_standalone}>)

-target_compile_definitions(benchmark PUBLIC
-  "$<${is_gxx_not_release_standalone}:_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC>")
+if(!AWS_BUILD)
+  target_compile_definitions(benchmark PUBLIC
+    "$<${is_gxx_not_release_standalone}:_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC>")
+endif()

 # Add benchmark_include_dirs by target_include_directories(... SYSTEM ...)
 # before target_link_libraries so that benchmark headers are included through
 # -isystem not -I, resulting in build-breaking diagnostics.
-get_target_property(benchmark_include_dirs benchmark::benchmark
-  INTERFACE_INCLUDE_DIRECTORIES)
+if(!AWS_BUILD)
+  get_target_property(benchmark_include_dirs benchmark::benchmark
+    INTERFACE_INCLUDE_DIRECTORIES)
+endif()

 if(NOT is_any_clang)
   message(STATUS "Not using clang-tidy due to non-clang compiler being used")
@@ -585,7 +600,9 @@ if(IWYU OR MAINTAINER_MODE)
 endif()

 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
-set(GSL_INCLUDES "3rd_party/GSL/include")
+if(!AWS_BUILD)
+  set(GSL_INCLUDES "3rd_party/GSL/include")
+endif()

 function(COMMON_TARGET_PROPERTIES TARGET)
   cmake_parse_arguments(PARSE_ARGV 1 CTP "SKIP_CHECKS" "" "")
@@ -662,16 +679,26 @@ function(SET_CLANG_TIDY_OPTIONS TARGET COMMAND)
 endfunction()

 function(ADD_UNODB_LIBRARY LIB)
-  add_library(${LIB} ${ARGN})
+  if(SHARED_LIBRARY_BUILD)
+    add_library(${LIB} SHARED ${ARGN})
+  else()
+    add_library(${LIB} ${ARGN})
+  endif()
   common_target_properties(${LIB})
-  target_include_directories(${LIB} SYSTEM PUBLIC "${GSL_INCLUDES}")
+  if(!AWS_BUILD)
+    target_include_directories(${LIB} SYSTEM PUBLIC "${GSL_INCLUDES}")
+  endif()
+  target_link_libraries(${LIB} PUBLIC GSL::Headers)
   set_clang_tidy_options(${LIB} "${DO_CLANG_TIDY}")

   if(LIBFUZZER_AVAILABLE)
     set(LIB_LF "${LIB}_lf")
     add_library(${LIB_LF} ${ARGN})
     common_target_properties(${LIB_LF} SKIP_CHECKS)
-    target_include_directories(${LIB_LF} SYSTEM PUBLIC "${GSL_INCLUDES}")
+    if(!AWS_BUILD)
+      target_include_directories(${LIB_LF} SYSTEM PUBLIC "${GSL_INCLUDES}")
+    endif()
+    target_link_libraries(${LIB} PUBLIC GSL::Headers)
     target_compile_options(${LIB_LF} PRIVATE "-fsanitize=fuzzer-no-link")
   endif()
 endfunction()
@@ -690,9 +717,26 @@ if(LIBFUZZER_AVAILABLE)
   target_link_libraries(unodb_qsbr_lf PUBLIC unodb_util Threads::Threads)
 endif()

-add_unodb_library(unodb art.cpp art.hpp art_common.cpp art_common.hpp
-  mutex_art.hpp optimistic_lock.hpp art_internal_impl.hpp olc_art.hpp
-  olc_art.cpp art_internal.cpp art_internal.hpp node_type.hpp)
+# Collect up the .cpp files and the .hpp files separately. The .hpp files will be
+# collected and exposed as a headers build artifact.
+add_unodb_library(unodb art.cpp art_common.cpp olc_art.cpp art_internal.cpp)
+target_sources(unodb
+    PUBLIC
+    FILE_SET unodb_headers
+    TYPE HEADERS
+    FILES
+    ${CMAKE_CURRENT_SOURCE_DIR}/art.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/art_common.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/mutex_art.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/optimistic_lock.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/art_internal_impl.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/olc_art.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/art_internal.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/node_type.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/qsbr.hpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/qsbr_ptr.hpp
+)
+
 target_link_libraries(unodb PUBLIC unodb_util unodb_qsbr)
 if(LIBFUZZER_AVAILABLE)
   target_link_libraries(unodb_lf PUBLIC unodb_util unodb_qsbr_lf)
@@ -735,6 +779,11 @@ if(NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows")
     ${BIN_DIR_CDB} ${SRC_DIR_CDB})
 endif()

+install(TARGETS unodb unodb_qsbr
+    LIBRARY DESTINATION lib
+    FILE_SET unodb_headers DESTINATION include
+)
+
 message(STATUS "User-set CMake options:")
 message(STATUS "STANDALONE: ${STANDALONE}")
 message(STATUS "MAINTAINER_MODE: ${MAINTAINER_MODE}")
diff --git a/third-party-src/benchmark/CMakeLists.txt b/third-party-src/benchmark/CMakeLists.txt
index a346004..5a19259 100644
--- a/third-party-src/benchmark/CMakeLists.txt
+++ b/third-party-src/benchmark/CMakeLists.txt
@@ -62,7 +62,11 @@ target_compile_definitions(micro_benchmark_utils PRIVATE
   BENCHMARK_STATIC_DEFINE)
 target_include_directories(micro_benchmark_utils PUBLIC ".")
 target_link_libraries(micro_benchmark_utils PUBLIC unodb)
-target_link_libraries(micro_benchmark_utils PUBLIC benchmark::benchmark)
+if(AWS_BUILD)
+  target_link_libraries(micro_benchmark_utils PUBLIC GoogleBenchmark::benchmark)
+else()
+  target_link_libraries(micro_benchmark_utils PUBLIC benchmark::benchmark)
+endif()
 target_include_directories(micro_benchmark_utils SYSTEM PUBLIC
   ${benchmark_include_dirs})
 set_clang_tidy_options(micro_benchmark_utils "${DO_CLANG_TIDY}")
diff --git a/third-party-src/test/CMakeLists.txt b/third-party-src/test/CMakeLists.txt
index 189bbf9..8ac9378 100644
--- a/third-party-src/test/CMakeLists.txt
+++ b/third-party-src/test/CMakeLists.txt
@@ -26,20 +26,32 @@ endfunction()

 add_library(db_test_utils STATIC db_test_utils.hpp db_test_utils.cpp)
 common_target_properties(db_test_utils)
-target_link_libraries(db_test_utils PUBLIC unodb gtest_main gmock_main)
+if(AWS_BUILD)
+  target_link_libraries(db_test_utils PUBLIC unodb Googletest::gtest Googletest::gtest_main Googlemock::gmock_main)
+else()
+  target_link_libraries(db_test_utils PUBLIC unodb gtest_main gmock_main)
+endif()
 set_clang_tidy_options(db_test_utils "${DO_CLANG_TIDY}")

 add_library(qsbr_test_utils STATIC qsbr_test_utils.hpp qsbr_test_utils.cpp
   qsbr_gtest_utils.hpp qsbr_gtest_utils.cpp)
 common_target_properties(qsbr_test_utils)
-target_link_libraries(qsbr_test_utils PUBLIC gtest_main)
 target_link_libraries(micro_benchmark_utils PUBLIC unodb)
-target_link_libraries(micro_benchmark_utils PUBLIC benchmark::benchmark)
+if(AWS_BUILD)
+  target_link_libraries(micro_benchmark_utils PUBLIC GoogleBenchmark::benchmark)
+else()
+  target_link_libraries(micro_benchmark_utils PUBLIC benchmark::benchmark)
+endif()
 target_include_directories(micro_benchmark_utils SYSTEM PUBLIC
   ${benchmark_include_dirs})
 set_clang_tidy_options(micro_benchmark_utils "${DO_CLANG_TIDY}")
diff --git a/third-party-src/test/CMakeLists.txt b/third-party-src/test/CMakeLists.txt
index 189bbf9..8ac9378 100644
--- a/third-party-src/test/CMakeLists.txt
+++ b/third-party-src/test/CMakeLists.txt
@@ -26,20 +26,32 @@ endfunction()

 add_library(db_test_utils STATIC db_test_utils.hpp db_test_utils.cpp)
 common_target_properties(db_test_utils)
-target_link_libraries(db_test_utils PUBLIC unodb gtest_main gmock_main)
+if(AWS_BUILD)
+  target_link_libraries(db_test_utils PUBLIC unodb Googletest::gtest Googletest::gtest_main Googlemock::gmock_main)
+else()
+  target_link_libraries(db_test_utils PUBLIC unodb gtest_main gmock_main)
+endif()
 set_clang_tidy_options(db_test_utils "${DO_CLANG_TIDY}")

 add_library(qsbr_test_utils STATIC qsbr_test_utils.hpp qsbr_test_utils.cpp
   qsbr_gtest_utils.hpp qsbr_gtest_utils.cpp)
 common_target_properties(qsbr_test_utils)
-target_link_libraries(qsbr_test_utils PUBLIC gtest_main)
+if(AWS_BUILD)
+  target_link_libraries(qsbr_test_utils PUBLIC Googletest::gtest Googletest::gtest_main)
+else()
+  target_link_libraries(qsbr_test_utils PUBLIC gtest_main)
+endif()
 target_link_libraries(qsbr_test_utils PRIVATE unodb_qsbr)
 set_clang_tidy_options(qsbr_test_utils "${DO_CLANG_TIDY}")

 function(ADD_TEST_TARGET TARGET)
   add_executable("${TARGET}" "${TARGET}.cpp")
   common_target_properties("${TARGET}")
-  target_link_libraries("${TARGET}" PRIVATE unodb_qsbr unodb_test gtest_main)
+  if(AWS_BUILD)
+    target_link_libraries("${TARGET}" PRIVATE unodb_qsbr unodb_test Googletest::gtest Googletest::gtest_main)
+  else()
+    target_link_libraries("${TARGET}" PRIVATE unodb_qsbr unodb_test gtest_main)
+  endif()
   set_clang_tidy_options("${TARGET}" "${DO_CLANG_TIDY}")
   add_sanitized_test(NAME "${TARGET}" COMMAND "${TARGET}")
 endfunction()

[thompsonbry]

Let me find a way to attach that as a file....

[thompsonbry]

unodb_cmake_delta.diff.txt

[laurynas-biveinis]

For patches, pull requests should be easier to work with than diff attachments.

Quick observations:

• The changes to use find_package if AWS and build subdirs otherwise should be guarded by the existing standalone option, IMHO
• Likewise the GLIBCXX_DEBUG bits should just disappear if AWS == !standalone
• What is the reason to explicitly disable DeepState under AWS?
• third-party-src/benchmark/CMakeLists.txt - this path does not exist, likewise third-party-src/test/CMakeLists.txt

[thompsonbry]

• I will try to figure out a way to use PRs for this. The OSS import is not the same as a git clone (everything comes into a subdirectory of an internal project), so I can't trivially use git PRs.
• Yes, find_package should be guarded. I thought I got them all. I will fix that.
• GLIBCXX_DEBUG -- I have to look at where that crept in.
• DeepState -- it was failing with the IF(). I can look at whether it can be enabled or whether I can infer that 32-bit support will not be available. I am not sure why your previous changes were not sufficient for the internal build environment.
• Please remove third-party-src/ from those paths. Apologies for not fixing that.

The internal project structure looks like this:

16      ./third-party-src/.circleci
4       ./third-party-src/3rd_party/GSL
4       ./third-party-src/3rd_party/deepstate
4       ./third-party-src/3rd_party/googletest
4       ./third-party-src/3rd_party/benchmark
24      ./third-party-src/3rd_party <======== unodb winds up here ============
148     ./third-party-src/test
60      ./third-party-src/fuzz_deepstate
8       ./third-party-src/.muse
148     ./third-party-src/benchmark
924     ./third-party-src
32      ./build-tools/bin
36      ./build-tools

[laurynas-biveinis]

I see, then my assumption was wrong, feel free to continue sending diffs instead of PRs