Skip to content

Commit

Permalink
[GLUTEN-8627][VL] Fix cpp build and build script on MacOS (apache#8628)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
marin-ma authored and baibaichen committed Feb 1, 2025
1 parent c1bd14d commit 67faece
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 18 deletions.
19 changes: 19 additions & 0 deletions cpp/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
31 changes: 20 additions & 11 deletions cpp/velox/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cpp/velox/tests/FunctionTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:";
Expand Down
2 changes: 1 addition & 1 deletion cpp/velox/tests/VeloxRowToColumnarTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
31 changes: 26 additions & 5 deletions dev/builddeps-veloxbe.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 67faece

Please sign in to comment.