build(cmake): Clean up various issues (#11751)

Summary: I have pulled these changes from #10732 as they are not specifically part of the shared build. - Add `[<dependency name>]` as an indicator that a dependency is running CMake. This makes it much easier to parse the log and track issues: ``` -- [simdjson] Using BUNDLED simdjson -- Setting folly source to BUNDLED -- [folly] Building Folly from source -- [folly] Using Boost - Bundled ``` - use targets instead of variables for Folly::follybenchmark and glog::glog - remove explicit linking of std::fs as this is no longer necessary for gcc >= 9 (our minimum is 11) - unify generation and linking of dwrf protobuf files - don't set C++ std for dependencies Pull Request resolved: #11751 Reviewed By: DanielHunte Differential Revision: D66843322 Pulled By: kgpai fbshipit-source-id: a3444a3ff919028523f403e78ade05afb40295c5
facebookincubator · Dec 6, 2024 · 24b41e5 · 24b41e5
1 parent 7205403
commit 24b41e5
Show file tree

Hide file tree

Showing 38 changed files with 90 additions and 129 deletions.
diff --git a/CMake/ResolveDependency.cmake b/CMake/ResolveDependency.cmake
@@ -64,6 +64,9 @@ endmacro()
 # is not found the build will fail and will not fall back to download and build
 # from source.
 macro(velox_resolve_dependency dependency_name)
+  # INDENT is always visible, CONTEXT has to be turned on.
+  list(APPEND CMAKE_MESSAGE_INDENT "[${dependency_name}] ")
+
   set(find_package_args ${dependency_name} ${ARGN})
   list(REMOVE_ITEM find_package_args REQUIRED QUIET)
   if(${dependency_name}_SOURCE STREQUAL "AUTO")
@@ -84,6 +87,8 @@ macro(velox_resolve_dependency dependency_name)
       FATAL_ERROR
         "Invalid source for ${dependency_name}: ${${dependency_name}_SOURCE}")
   endif()
+
+  list(POP_BACK CMAKE_MESSAGE_INDENT)
 endmacro()
 
 # By using a macro we don't need to propagate the value into the parent scope.

diff --git a/CMake/resolve_dependency_modules/folly/CMakeLists.txt b/CMake/resolve_dependency_modules/folly/CMakeLists.txt
@@ -44,6 +44,7 @@ FetchContent_MakeAvailable(folly)
 
 # Folly::folly is not valid for FC but we want to match FindFolly
 add_library(Folly::folly ALIAS folly)
+add_library(Folly::follybenchmark ALIAS follybenchmark)
 
 # The folly target does not contain any include directories, they are propagated
 # from folly_base. This marks them as system headers which should suppress
@@ -60,7 +61,3 @@ set_target_properties(
 if(${gflags_SOURCE} STREQUAL "BUNDLED")
   add_dependencies(folly glog gflags_static fmt::fmt)
 endif()
-
-set(FOLLY_BENCHMARK_STATIC_LIB
-    ${folly_BINARY_DIR}/folly/libfollybenchmark${CMAKE_STATIC_LIBRARY_SUFFIX}
-    PARENT_SCOPE)
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -233,10 +233,6 @@ if(${VELOX_FORCE_COLORED_OUTPUT})
   endif()
 endif()
 
-# At the moment we prefer static linking but by default cmake looks for shared
-# libs first. This will still fallback to shared libs when static ones are not
-# found
-list(INSERT CMAKE_FIND_LIBRARY_SUFFIXES 0 a)
 if(VELOX_ENABLE_S3)
   # Set AWS_ROOT_DIR if you have a custom install location of AWS SDK CPP.
   if(AWSSDK_ROOT_DIR)
@@ -293,7 +289,8 @@ endif()
 set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)
 
 set(CMAKE_CXX_STANDARD 17)
-set(CMAKE_CXX_STANDARD_REQUIRED True)
+set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
+set(CMAKE_CXX_EXTENSIONS ON) # Big Int is an extension
 
 execute_process(
   COMMAND
@@ -373,7 +370,6 @@ if(${VELOX_ENABLE_GPU})
   if(CMAKE_BUILD_TYPE MATCHES Debug)
     add_compile_options("$<$<COMPILE_LANGUAGE:CUDA>:-G>")
   endif()
-  include_directories("${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}")
   find_package(CUDAToolkit REQUIRED)
 endif()
 
@@ -424,11 +420,10 @@ set(BOOST_INCLUDE_LIBRARIES
 velox_set_source(Boost)
 velox_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)
-
 velox_set_source(gflags)
+
 velox_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
   # target even when velox is built as a subproject which uses
@@ -494,8 +489,6 @@ endif()
 velox_set_source(simdjson)
 velox_resolve_dependency(simdjson 3.9.3)
 
-# Locate or build folly.
-add_compile_definitions(FOLLY_HAVE_INT128_T=1)
 velox_set_source(folly)
 velox_resolve_dependency(folly)
 
@@ -521,12 +514,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()
-
 if(VELOX_ENABLE_GCS)
   velox_set_source(google_cloud_cpp_storage)
   velox_resolve_dependency(google_cloud_cpp_storage CONFIG 2.22.0 REQUIRED)
@@ -535,7 +522,6 @@ endif()
 
 # GCC needs to link a library to enable std::filesystem.
 if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
-  set(FILESYSTEM "stdc++fs")
 
   # Ensure we have gcc at least 9+.
   if(CMAKE_CXX_COMPILER_VERSION LESS 9.0)
@@ -545,8 +531,6 @@ if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
 
   # Find Threads library
   find_package(Threads REQUIRED)
-else()
-  set(FILESYSTEM "")
 endif()
 
 if(VELOX_BUILD_TESTING AND NOT VELOX_ENABLE_DUCKDB)
@@ -595,7 +579,6 @@ find_package(double-conversion 3.1.5 REQUIRED)
 include_directories(SYSTEM velox)
 include_directories(SYSTEM velox/external)
 
-# these were previously vendored in third-party/
 if(NOT VELOX_DISABLE_GOOGLETEST)
   velox_set_source(GTest)
   velox_resolve_dependency(GTest)

diff --git a/scripts/setup-helper-functions.sh b/scripts/setup-helper-functions.sh
@@ -111,15 +111,15 @@ function get_cxx_flags {
   case $CPU_ARCH in
 
     "arm64")
-      echo -n "-mcpu=apple-m1+crc -std=c++17 -fvisibility=hidden"
+      echo -n "-mcpu=apple-m1+crc"
     ;;
 
     "avx")
-      echo -n "-mavx2 -mfma -mavx -mf16c -mlzcnt -std=c++17 -mbmi2"
+      echo -n "-mavx2 -mfma -mavx -mf16c -mlzcnt  -mbmi2"
     ;;
 
     "sse")
-      echo -n "-msse4.2 -std=c++17"
+      echo -n "-msse4.2 "
     ;;
 
     "aarch64")
@@ -138,16 +138,16 @@ function get_cxx_flags {
         ARM_CPU_PRODUCT=${hex_ARM_CPU_DETECT: -4:3}
 
         if [ "$ARM_CPU_PRODUCT" = "$Neoverse_N1" ]; then
-          echo -n "-mcpu=neoverse-n1 -std=c++17"
+          echo -n "-mcpu=neoverse-n1 "
         elif [ "$ARM_CPU_PRODUCT" = "$Neoverse_N2" ]; then
-          echo -n "-mcpu=neoverse-n2 -std=c++17"
+          echo -n "-mcpu=neoverse-n2 "
         elif [ "$ARM_CPU_PRODUCT" = "$Neoverse_V1" ]; then
-          echo -n "-mcpu=neoverse-v1 -std=c++17"
+          echo -n "-mcpu=neoverse-v1 "
         else
-          echo -n "-march=armv8-a+crc+crypto -std=c++17"
+          echo -n "-march=armv8-a+crc+crypto "
         fi
       else
-        echo -n "-std=c++17"
+        echo -n ""
       fi
     ;;
   *)

diff --git a/velox/benchmarks/CMakeLists.txt b/velox/benchmarks/CMakeLists.txt
@@ -23,7 +23,7 @@ set(velox_benchmark_deps
     velox_parse_expression
     velox_serialization
     Folly::folly
-    ${FOLLY_BENCHMARK}
+    Folly::follybenchmark
     ${DOUBLE_CONVERSION}
     gflags::gflags
     glog::glog)
@@ -60,6 +60,6 @@ target_link_libraries(
   velox_type_fbhive
   velox_caching
   velox_vector_test_lib
-  ${FOLLY_BENCHMARK}
   Folly::folly
+  Folly::follybenchmark
   fmt::fmt)
diff --git a/velox/benchmarks/basic/CMakeLists.txt b/velox/benchmarks/basic/CMakeLists.txt
@@ -23,7 +23,7 @@ set(velox_benchmark_deps
     velox_benchmark_builder
     velox_vector_test_lib
     Folly::folly
-    ${FOLLY_BENCHMARK}
+    Folly::follybenchmark
     ${DOUBLE_CONVERSION}
     gflags::gflags
     glog::glog)

diff --git a/velox/benchmarks/tpch/CMakeLists.txt b/velox/benchmarks/tpch/CMakeLists.txt
@@ -34,7 +34,7 @@ target_link_libraries(
   velox_type_fbhive
   velox_caching
   velox_vector_test_lib
-  ${FOLLY_BENCHMARK}
+  Folly::follybenchmark
   Folly::folly
   fmt::fmt)
 

diff --git a/velox/benchmarks/unstable/CMakeLists.txt b/velox/benchmarks/unstable/CMakeLists.txt
@@ -21,7 +21,7 @@ set(velox_benchmark_deps
     velox_parse_expression
     velox_serialization
     Folly::folly
-    ${FOLLY_BENCHMARK}
+    Folly::follybenchmark
     ${DOUBLE_CONVERSION}
     gflags::gflags
     glog::glog)

diff --git a/velox/common/base/benchmarks/CMakeLists.txt b/velox/common/base/benchmarks/CMakeLists.txt
@@ -15,11 +15,12 @@ add_executable(velox_common_base_benchmarks BitUtilBenchmark.cpp)
 
 target_link_libraries(
   velox_common_base_benchmarks
-  PUBLIC ${FOLLY_BENCHMARK}
+  PUBLIC Folly::follybenchmark
   PRIVATE velox_common_base Folly::folly)
 
 add_executable(velox_common_stringsearch_benchmarks StringSearchBenchmark.cpp)
+
 target_link_libraries(
   velox_common_stringsearch_benchmarks
-  PUBLIC ${FOLLY_BENCHMARK}
+  PUBLIC Folly::follybenchmark
   PRIVATE velox_common_base Folly::folly)
diff --git a/velox/common/hyperloglog/benchmarks/CMakeLists.txt b/velox/common/hyperloglog/benchmarks/CMakeLists.txt
@@ -16,4 +16,4 @@ add_executable(velox_common_hyperloglog_dense_hll_bm DenseHll.cpp)
 
 target_link_libraries(
   velox_common_hyperloglog_dense_hll_bm velox_common_hyperloglog
-  ${FOLLY_BENCHMARK})
+  Folly::follybenchmark)
diff --git a/velox/connectors/hive/benchmarks/CMakeLists.txt b/velox/connectors/hive/benchmarks/CMakeLists.txt
@@ -30,5 +30,5 @@ target_link_libraries(
   velox_hive_partition_function
   velox_memory
   Folly::folly
-  ${FOLLY_BENCHMARK}
+  Folly::follybenchmark
   fmt::fmt)
diff --git a/velox/connectors/hive/iceberg/tests/CMakeLists.txt b/velox/connectors/hive/iceberg/tests/CMakeLists.txt
@@ -19,7 +19,7 @@ target_link_libraries(
   velox_exec
   velox_hive_connector
   Folly::folly
-  ${FOLLY_BENCHMARK}
+  Folly::follybenchmark
   ${TEST_LINK_LIBS})
 
 if(VELOX_ENABLE_BENCHMARKS)
@@ -28,12 +28,11 @@ if(VELOX_ENABLE_BENCHMARKS)
   target_link_libraries(
     velox_dwio_iceberg_reader_benchmark
     velox_dwio_iceberg_reader_benchmark_lib
-    velox_dwio_dwrf_proto
     velox_exec_test_lib
     velox_exec
     velox_hive_connector
     Folly::folly
-    ${FOLLY_BENCHMARK}
+    Folly::follybenchmark
     ${TEST_LINK_LIBS})
 endif()
 
@@ -51,12 +50,11 @@ if(NOT VELOX_DISABLE_GOOGLETEST)
     velox_hive_partition_function
     velox_dwio_common_exception
     velox_dwio_common_test_utils
-    velox_dwio_dwrf_proto
     velox_vector_test_lib
     velox_exec
     velox_exec_test_lib
     Folly::folly
-    ${FOLLY_BENCHMARK}
+    Folly::follybenchmark
     GTest::gtest
     GTest::gtest_main)
 

diff --git a/velox/dwio/common/tests/CMakeLists.txt b/velox/dwio/common/tests/CMakeLists.txt
@@ -85,7 +85,7 @@ if(VELOX_ENABLE_BENCHMARKS)
     velox_memory
     velox_dwio_common_exception
     Folly::folly
-    ${FOLLY_BENCHMARK})
+    Folly::follybenchmark)
 
   add_executable(velox_dwio_common_int_decoder_benchmark
                  IntDecoderBenchmark.cpp)
@@ -95,7 +95,7 @@ if(VELOX_ENABLE_BENCHMARKS)
     velox_exception
     velox_dwio_dwrf_common
     Folly::folly
-    ${FOLLY_BENCHMARK})
+    Folly::follybenchmark)
 
   if(VELOX_ENABLE_ARROW)
     add_subdirectory(Lemire/FastPFor)
@@ -112,6 +112,6 @@ if(VELOX_ENABLE_BENCHMARKS)
       velox_fastpforlib
       duckdb_static
       Folly::folly
-      ${FOLLY_BENCHMARK})
+      Folly::follybenchmark)
   endif()
 endif()
diff --git a/velox/dwio/dwrf/CMakeLists.txt b/velox/dwio/dwrf/CMakeLists.txt
@@ -12,8 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-add_subdirectory(common)
 add_subdirectory(proto)
+add_subdirectory(common)
 add_subdirectory(reader)
 if(${VELOX_BUILD_TESTING})
   add_subdirectory(test)

diff --git a/velox/dwio/dwrf/common/CMakeLists.txt b/velox/dwio/dwrf/common/CMakeLists.txt
@@ -25,12 +25,8 @@ velox_add_library(
   RLEv1.cpp
   RLEv2.cpp
   Statistics.cpp
-  wrap/dwrf-proto-wrapper.cpp
-  wrap/orc-proto-wrapper.cpp)
-
-if(NOT VELOX_MONO_LIBRARY)
-  add_dependencies(velox_dwio_dwrf_common velox_dwio_dwrf_proto)
-endif()
+  wrap/orc-proto-wrapper.cpp
+  wrap/dwrf-proto-wrapper.cpp)
 
 velox_link_libraries(
   velox_dwio_dwrf_common
@@ -39,7 +35,15 @@ velox_link_libraries(
   velox_common_config
   velox_dwio_common
   velox_dwio_common_compression
-  velox_dwio_dwrf_proto
   velox_caching
   Snappy::snappy
-  zstd::zstd)
+  zstd::zstd
+  protobuf::libprotobuf)
+
+# required for the wrapped protobuf headers/sources
+velox_include_directories(velox_dwio_dwrf_common PUBLIC ${PROJECT_BINARY_DIR})
+
+if(NOT VELOX_MONO_LIBRARY)
+  # trigger generation of pb files
+  add_dependencies(velox_dwio_dwrf_common dwio_proto)
+endif()
diff --git a/velox/dwio/dwrf/proto/CMakeLists.txt b/velox/dwio/dwrf/proto/CMakeLists.txt
@@ -47,10 +47,3 @@ add_custom_target(dwio_proto ALL DEPENDS ${PROTO_OUTPUT_FILES})
 if(VELOX_MONO_LIBRARY)
   add_dependencies(velox dwio_proto)
 endif()
-velox_add_library(velox_dwio_dwrf_proto ${PROTO_HDRS} ${PROTO_SRCS})
-
-# Access generated proto file with.
-#
-# #include "velox/dwio/dwrf/proto/dwrf_proto.pb.h"
-velox_link_libraries(velox_dwio_dwrf_proto protobuf::libprotobuf)
-velox_include_directories(velox_dwio_dwrf_proto PUBLIC ${PROJECT_BINARY_DIR})
diff --git a/velox/dwio/dwrf/test/CMakeLists.txt b/velox/dwio/dwrf/test/CMakeLists.txt
@@ -16,16 +16,14 @@ add_subdirectory(utils)
 
 set(TEST_LINK_LIBS
     velox_dwio_common_test_utils
-    velox_dwio_dwrf_proto
     velox_dwio_dwrf_reader
     velox_dwio_dwrf_writer
     velox_row_fast
     gflags::gflags
     GTest::gtest
     GTest::gtest_main
     GTest::gmock
-    glog::glog
-    ${FILESYSTEM})
+    glog::glog)
 
 add_executable(velox_dwio_dwrf_buffered_output_stream_test
                TestBufferedOutputStream.cpp)
@@ -549,7 +547,7 @@ if(VELOX_ENABLE_BENCHMARKS)
     velox_memory
     velox_dwio_common_exception
     Folly::folly
-    ${FOLLY_BENCHMARK})
+    Folly::follybenchmark)
 
   add_executable(velox_dwrf_float_column_writer_benchmark
                  FloatColumnWriterBenchmark.cpp)
@@ -559,7 +557,7 @@ if(VELOX_ENABLE_BENCHMARKS)
     velox_dwio_common_exception
     velox_dwio_dwrf_writer
     Folly::folly
-    ${FOLLY_BENCHMARK}
+    Folly::follybenchmark
     fmt::fmt)
 endif()
 

diff --git a/velox/dwio/dwrf/test/utils/CMakeLists.txt b/velox/dwio/dwrf/test/utils/CMakeLists.txt
@@ -27,8 +27,3 @@ target_link_libraries(
   velox_vector
   GTest::gtest
   GTest::gtest_main)
-
-# older versions of GCC need it to allow std::filesystem
-if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9)
-  target_link_libraries(velox_dwrf_test_utils stdc++fs)
-endif()