[GLUTEN-8627][VL] Fix cpp build and build script on MacOS (#8628)

Fix the cpp compilation error and make builddeps-veloxbe.sh work on MacOS. Manually tested with ./dev/builddeps-veloxbe.sh --run_setup_script=ON --build_arrow=ON --build_type=Debug --build_tests=ON --build_examples=ON and also tested with switching OFF the build options and Release build type. Note --build_benchmarks=ON still fails because the function to set CPU affinity on macOS is not supported. I will fix that in a separate patch.
apache · Jan 28, 2025 · 80ffbf1 · 80ffbf1
1 parent cd7c790
commit 80ffbf1
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 18 deletions.
diff --git a/cpp/core/CMakeLists.txt b/cpp/core/CMakeLists.txt
@@ -52,6 +52,25 @@ endif(CCACHE_FOUND)
 macro(find_protobuf)
   set(CMAKE_FIND_LIBRARY_SUFFIXES_BCK ${CMAKE_FIND_LIBRARY_SUFFIXES})
   set(CMAKE_FIND_LIBRARY_SUFFIXES ".a")
+
+  if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
+    # brew --prefix protobuf will not return the correct path. Update `brew
+    # --prefix protobuf@21` if protobuf version is changed.
+    execute_process(
+      COMMAND brew --prefix protobuf@21
+      RESULT_VARIABLE BREW_PROTOBUF
+      OUTPUT_VARIABLE BREW_PROTOBUF_PREFIX
+      OUTPUT_STRIP_TRAILING_WHITESPACE)
+    if(BREW_PROTOBUF EQUAL 0 AND EXISTS "${BREW_PROTOBUF_PREFIX}")
+      message(
+        STATUS "Found protobuf installed by Homebrew at ${BREW_PROTOBUF_PREFIX}"
+      )
+      list(APPEND CMAKE_PREFIX_PATH "${BREW_PROTOBUF_PREFIX}")
+    else()
+      message(WARNING "Homebrew protobuf not found.")
+    endif()
+  endif()
+
   find_package(Protobuf)
   set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES_BCK})
   if("${Protobuf_LIBRARY}" STREQUAL "Protobuf_LIBRARY-NOTFOUND")

diff --git a/cpp/velox/CMakeLists.txt b/cpp/velox/CMakeLists.txt
@@ -282,18 +282,27 @@ endif()
 
 set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES_BCK})
 
-set(icu_components i18n uc data)
-foreach(component ${icu_components})
-  find_library(ICU_${component}_LIB NAMES icu${component})
-  if(NOT ICU_${component}_LIB)
-    message(FATAL_ERROR "icu${component} library not found")
+# Adopted from Velox's CMakeLists.txt.
+# https://github.com/facebookincubator/velox/pull/11410
+if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
+  execute_process(
+    COMMAND brew --prefix icu4c
+    RESULT_VARIABLE BREW_ICU4C
+    OUTPUT_VARIABLE BREW_ICU4C_PREFIX
+    OUTPUT_STRIP_TRAILING_WHITESPACE)
+  if(BREW_ICU4C EQUAL 0 AND EXISTS "${BREW_ICU4C_PREFIX}")
+    message(STATUS "Found icu4c installed by Homebrew at ${BREW_ICU4C_PREFIX}")
+    list(APPEND CMAKE_PREFIX_PATH "${BREW_ICU4C_PREFIX}")
+  else()
+    list(APPEND CMAKE_PREFIX_PATH "/usr/local/opt/icu4c")
   endif()
-  message(STATUS "Found ICU::${component}: ${ICU_${component}_LIB}")
-  add_library(ICU::${component} UNKNOWN IMPORTED)
-  set_target_properties(ICU::${component} PROPERTIES IMPORTED_LOCATION
-                                                     ${ICU_${component}_LIB})
-  target_link_libraries(velox PUBLIC ICU::${component})
-endforeach()
+endif()
+
+find_package(
+  ICU
+  COMPONENTS i18n uc data
+  REQUIRED)
+target_link_libraries(velox PUBLIC ICU::i18n ICU::uc ICU::data)
 
 if(BUILD_TESTS)
   add_subdirectory(tests)

diff --git a/cpp/velox/tests/FunctionTest.cc b/cpp/velox/tests/FunctionTest.cc
@@ -73,7 +73,7 @@ TEST_F(FunctionTest, getIdxFromNodeName) {
 
 TEST_F(FunctionTest, getNameBeforeDelimiter) {
   std::string functionSpec = "lte:fp64_fp64";
-  std::string_view funcName = SubstraitParser::getNameBeforeDelimiter(functionSpec);
+  auto funcName = SubstraitParser::getNameBeforeDelimiter(functionSpec);
   ASSERT_EQ(funcName, "lte");
 
   functionSpec = "lte:";

diff --git a/cpp/velox/tests/VeloxRowToColumnarTest.cc b/cpp/velox/tests/VeloxRowToColumnarTest.cc
@@ -43,7 +43,7 @@ class VeloxRowToColumnarTest : public ::testing::Test, public test::VectorTestBa
     uint8_t* address = columnarToRowConverter->getBufferAddress();
     auto lengthVec = columnarToRowConverter->getLengths();
 
-    long lengthArr[lengthVec.size()];
+    int64_t lengthArr[lengthVec.size()];
     for (int i = 0; i < lengthVec.size(); i++) {
       lengthArr[i] = lengthVec[i];
     }

diff --git a/dev/builddeps-veloxbe.sh b/dev/builddeps-veloxbe.sh
@@ -198,11 +198,32 @@ function build_gluten_cpp {
   rm -rf build
   mkdir build
   cd build
-  cmake -DBUILD_VELOX_BACKEND=ON -DCMAKE_BUILD_TYPE=$BUILD_TYPE \
-        -DVELOX_HOME=${VELOX_HOME} \
-        -DBUILD_TESTS=$BUILD_TESTS -DBUILD_EXAMPLES=$BUILD_EXAMPLES -DBUILD_BENCHMARKS=$BUILD_BENCHMARKS -DENABLE_JEMALLOC_STATS=$ENABLE_JEMALLOC_STATS \
-        -DENABLE_HBM=$ENABLE_HBM -DENABLE_QAT=$ENABLE_QAT -DENABLE_IAA=$ENABLE_IAA -DENABLE_GCS=$ENABLE_GCS \
-        -DENABLE_S3=$ENABLE_S3 -DENABLE_HDFS=$ENABLE_HDFS -DENABLE_ABFS=$ENABLE_ABFS ..
+
+  GLUTEN_CMAKE_OPTIONS="-DBUILD_VELOX_BACKEND=ON \
+    -DCMAKE_BUILD_TYPE=$BUILD_TYPE \
+    -DVELOX_HOME=$VELOX_HOME \
+    -DBUILD_TESTS=$BUILD_TESTS \
+    -DBUILD_EXAMPLES=$BUILD_EXAMPLES \
+    -DBUILD_BENCHMARKS=$BUILD_BENCHMARKS \
+    -DENABLE_JEMALLOC_STATS=$ENABLE_JEMALLOC_STATS \
+    -DENABLE_HBM=$ENABLE_HBM \
+    -DENABLE_QAT=$ENABLE_QAT \
+    -DENABLE_IAA=$ENABLE_IAA \
+    -DENABLE_GCS=$ENABLE_GCS \
+    -DENABLE_S3=$ENABLE_S3 \
+    -DENABLE_HDFS=$ENABLE_HDFS \
+    -DENABLE_ABFS=$ENABLE_ABFS"
+
+  if [ $OS == 'Darwin' ]; then
+    if [ -n "$INSTALL_PREFIX" ]; then
+      DEPS_INSTALL_DIR=$INSTALL_PREFIX
+    else
+      DEPS_INSTALL_DIR=$VELOX_HOME/deps-install
+    fi
+    GLUTEN_CMAKE_OPTIONS+=" -DCMAKE_PREFIX_PATH=$DEPS_INSTALL_DIR"
+  fi
+
+  cmake $GLUTEN_CMAKE_OPTIONS ..
   make -j $NUM_THREADS
 }