add option to build shared and enforce bundled build

CMake/resolve_dependency_modules/folly/CMakeLists.txt

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 project(Folly)
-cmake_minimum_required(VERSION 3.14)
+cmake_minimum_required(VERSION 3.28)
 
 set(VELOX_FOLLY_BUILD_VERSION v2024.04.01.00)
 set(VELOX_FOLLY_BUILD_SHA256_CHECKSUM
@@ -34,41 +34,23 @@ FetchContent_Declare(
   URL ${VELOX_FOLLY_SOURCE_URL}
   URL_HASH ${VELOX_FOLLY_BUILD_SHA256_CHECKSUM}
   PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/folly-no-export.patch
-                ${glog_patch})
+                ${glog_patch} OVERRIDE_FIND_PACKAGE SYSTEM EXCLUDE_FROM_ALL)
 
 if(ON_APPLE_M1)
   # folly will wrongly assume x86_64 if this is not set
   set(CMAKE_LIBRARY_ARCHITECTURE aarch64)
 endif()
 
-# TODO: handle this better
-set(BUILD_SHARED_LIBS ON)
-# Suppress all warnings
-set(FOLLY_CXX_FLAGS -w)
+set(BUILD_SHARED_LIBS ${VELOX_BUILD_SHARED})
+
 # Enable INT128 support
 set(FOLLY_HAVE_INT128_T ON)
 FetchContent_MakeAvailable(folly)
-unset(BUILD_SHARED_LIBS)
 
 # Folly::folly is not valid for FC but we want to match FindFolly
 add_library(Folly::folly ALIAS folly)
-
-# The folly target does not contain any include directories, they are propagated
-# from folly_base. This marks them as system headers which should suppress
-# warnigs generated by them when they are included else where.
-set_target_properties(
-  folly_deps
-  PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES
-             $<TARGET_PROPERTY:folly_deps,INTERFACE_INCLUDE_DIRECTORIES>)
-set_target_properties(
-  folly_base
-  PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES
-             $<TARGET_PROPERTY:folly_base,INTERFACE_INCLUDE_DIRECTORIES>)
+add_library(Folly::follybenchmark ALIAS follybenchmark)
 
 if(${gflags_SOURCE} STREQUAL "BUNDLED")
   add_dependencies(folly glog gflags::gflags fmt::fmt)
 endif()
-
-set(FOLLY_BENCHMARK_STATIC_LIB
-    ${folly_BINARY_DIR}/folly/libfollybenchmark${CMAKE_STATIC_LIBRARY_SUFFIX}
-    PARENT_SCOPE)

CMake/resolve_dependency_modules/gflags.cmake

@@ -27,30 +27,22 @@ FetchContent_Declare(
   gflags
   URL ${VELOX_GFLAGS_SOURCE_URL}
   URL_HASH ${VELOX_GFLAGS_BUILD_SHA256_CHECKSUM}
-  PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/gflags/gflags-config.patch)
+  PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/gflags/gflags-config.patch
+                OVERRIDE_FIND_PACKAGE EXCLUDE_FROM_ALL SYSTEM)
+
+# glog relies on the old `google` namespace
+set(GFLAGS_NAMESPACE "google;gflags")
+
+set(GFLAGS_BUILD_SHARED_LIBS ${VELOX_BUILD_SHARED})
+set(GFLAGS_BUILD_STATIC_LIBS ${VELOX_BUILD_STATIC})
 
-set(GFLAGS_BUILD_SHARED_LIBS ON)
-set(GFLAGS_BUILD_STATIC_LIBS OFF)
 set(GFLAGS_BUILD_gflags_LIB ON)
 set(GFLAGS_BUILD_gflags_nothreads_LIB ON)
 set(GFLAGS_IS_SUBPROJECT ON)
-# glog relies on the old `google` namespace
-set(GFLAGS_NAMESPACE "google;gflags")
-# unset(BUILD_SHARED_LIBS)
-FetchContent_MakeAvailable(gflags)
-# set(BUILD_SHARED_LIBS ON)
-add_library(gflags::gflags ALIAS gflags_shared)
-if(TARGET gflags::gflags)
-  get_target_property(AliasedTarget gflags::gflags ALIASED_TARGET)
-  message(FATAL_ERROR "gflags: ${AliasedTarget}")
-endif()
-# the flag has to be added to each target we build so adjust to settings choosen
-# above target_compile_options(gflags_shared PRIVATE -Wno-cast-function-type)
-# target_compile_options(gflags_nothreads_shared PRIVATE
-# -Wno-cast-function-type)
 
+# Workaround for https://github.com/gflags/gflags/issues/277
+unset(BUILD_SHARED_LIBS)
+FetchContent_MakeAvailable(gflags)
 # this causes find_package(gflags) to search in the build directory and prevents
-# the system gflags from being found
-set(gflags_DIR ${gflags_BINARY_DIR})
-set(gflags_LIBRARY gflags::gflags)
-set(gflags_INCLUDE_DIR ${gflags_BINARY_DIR}/include)
+# the system gflags from being found set(gflags_LIBRARY gflags::gflags)
+# set(gflags_INCLUDE_DIR ${gflags_BINARY_DIR}/include)

CMake/resolve_dependency_modules/glog.cmake

@@ -27,8 +27,10 @@ FetchContent_Declare(
   glog
   URL ${VELOX_GLOG_SOURCE_URL}
   URL_HASH ${VELOX_GLOG_BUILD_SHA256_CHECKSUM}
-  PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/glog/glog-no-export.patch
-                && git apply ${CMAKE_CURRENT_LIST_DIR}/glog/glog-config.patch)
+  PATCH_COMMAND
+    git apply ${CMAKE_CURRENT_LIST_DIR}/glog/glog-no-export.patch && git apply
+    ${CMAKE_CURRENT_LIST_DIR}/glog/glog-config.patch OVERRIDE_FIND_PACKAGE
+    SYSTEM EXCLUDE_FROM_ALL)
 
 set(BUILD_SHARED_LIBS OFF)
 set(WITH_UNWIND OFF)

CMake/resolve_dependency_modules/gtest.cmake

@@ -26,7 +26,8 @@ message(STATUS "Building gtest from source")
 FetchContent_Declare(
   gtest
   URL ${VELOX_GTEST_SOURCE_URL}
-  URL_HASH ${VELOX_GTEST_BUILD_SHA256_CHECKSUM})
+  URL_HASH ${VELOX_GTEST_BUILD_SHA256_CHECKSUM}
+  OVERRIDE_FIND_PACKAGE SYSTEM EXCLUDE_FROM_ALL)
 
 FetchContent_MakeAvailable(gtest)
 

CMakeLists.txt

@@ -50,6 +50,7 @@ list(PREPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/CMake"
 
 # Include our ThirdPartyToolchain dependencies macros
 include(ResolveDependency)
+include(CMakeDependentOption)
 
 set_with_default(VELOX_DEPENDENCY_SOURCE_DEFAULT VELOX_DEPENDENCY_SOURCE AUTO)
 message(STATUS "Build type: ${CMAKE_BUILD_TYPE}")
@@ -68,6 +69,11 @@ option(
   "Build a minimal set of components, including DWIO (file format readers/writers).
    This will override other build options."
   OFF)
+option(VELOX_BUILD_SHARED "Build Velox as shared libraries." OFF)
+# While it's possible to build both in one go we currently want to build either
+# static or shared.
+cmake_dependent_option(VELOX_BUILD_STATIC "Build Velox as static libraries." ON
+                       "NOT VELOX_BUILD_SHARED" OFF)
 
 # option() always creates a BOOL variable so we have to use a normal cache
 # variable with STRING type for this option.
@@ -378,10 +384,14 @@ set(BOOST_INCLUDE_LIBRARIES
 set_source(Boost)
 resolve_dependency(Boost 1.77.0 COMPONENTS ${BOOST_INCLUDE_LIBRARIES})
 
-# Range-v3 will be enable when the codegen code actually lands keeping it here
-# for reference. find_package(range-v3)
+if(VELOX_BUILD_SHARED)
+  # We have to enforce the bundled build for now to make sure folly uses the
+  # shared gflags to avoid linking issues
+  set(gflags_SOURCE BUNDLED)
+else()
+  set_source(gflags)
+endif()
 
-set_source(gflags)
 resolve_dependency(gflags COMPONENTS ${VELOX_GFLAGS_TYPE})
 if(NOT TARGET gflags::gflags)
   # This is a bit convoluted, but we want to be able to use gflags::gflags as a
@@ -442,8 +452,13 @@ set_source(simdjson)
 resolve_dependency(simdjson 3.8.0)
 
 # Locate or build folly.
-add_compile_definitions(FOLLY_HAVE_INT128_T=1)
-set_source(folly)
+if(VELOX_BUILD_SHARED)
+  # We have to enforce the bundled build for now to avoid linking issues with
+  # gflags
+  set(folly_SOURCE BUNDLED)
+else()
+  set_source(folly)
+endif()
 resolve_dependency(folly)
 
 if(VELOX_ENABLE_REMOTE_FUNCTIONS)
@@ -454,12 +469,6 @@ if(VELOX_ENABLE_REMOTE_FUNCTIONS)
   find_package(FBThrift CONFIG REQUIRED)
 endif()
 
-if(DEFINED FOLLY_BENCHMARK_STATIC_LIB)
-  set(FOLLY_BENCHMARK ${FOLLY_BENCHMARK_STATIC_LIB})
-else()
-  set(FOLLY_BENCHMARK Folly::follybenchmark)
-endif()
-
 # DWIO (ORC/DWRF) and Substrait depend on protobuf.
 if(${VELOX_BUILD_MINIMAL_WITH_DWIO}
    OR ${VELOX_ENABLE_HIVE_CONNECTOR}
@@ -564,5 +573,8 @@ if("${TREAT_WARNINGS_AS_ERRORS}")
 endif()
 
 add_subdirectory(third_party)
-set(BUILD_SHARED_LIBS ON)
+
+if(VELOX_BUILD_SHARED)
+  set(BUILD_SHARED_LIBS ON)
+endif()
 add_subdirectory(velox)

velox/vector/benchmarks/CMakeLists.txt

@@ -30,5 +30,6 @@ target_link_libraries(copy_benchmark velox_vector_test_lib Folly::folly
                       Folly::follybenchmark)
 
 add_executable(velox_vector_map_update_benchmark MapUpdateBenchmark.cpp)
-target_link_libraries(velox_vector_map_update_benchmark velox_vector_test_lib
-                      Folly::folly Folly::follybenchmark gflags::gflags glog::glog)
+target_link_libraries(
+  velox_vector_map_update_benchmark velox_vector_test_lib Folly::folly
+  Folly::follybenchmark gflags::gflags glog::glog)