[VL] Support test/debug/benchmark native builds on macOS (follow-up to #12331) (#12470)

Author: jackylee-ch

cpp/CMakeLists.txt

@@ -66,6 +66,19 @@ if(NOT DEFINED VELOX_HOME)
   message(STATUS "Set VELOX_HOME to ${VELOX_HOME}")
 endif()
 
+set(GLUTEN_PREFIX_INCLUDE_DIRS)
+if(CMAKE_INSTALL_PREFIX
+   AND NOT CMAKE_INSTALL_PREFIX STREQUAL "/usr/local"
+   AND IS_DIRECTORY "${CMAKE_INSTALL_PREFIX}/include")
+  list(APPEND GLUTEN_PREFIX_INCLUDE_DIRS "${CMAKE_INSTALL_PREFIX}/include")
+endif()
+foreach(PREFIX_PATH IN LISTS CMAKE_PREFIX_PATH)
+  if(IS_DIRECTORY "${PREFIX_PATH}/include")
+    list(APPEND GLUTEN_PREFIX_INCLUDE_DIRS "${PREFIX_PATH}/include")
+  endif()
+endforeach()
+list(REMOVE_DUPLICATES GLUTEN_PREFIX_INCLUDE_DIRS)
+
 if(${CMAKE_BUILD_TYPE} STREQUAL "Debug")
   set(ARROW_HOME
       ${VELOX_HOME}/_build/debug/CMake/resolve_dependency_modules/arrow/arrow_ep/

cpp/core/CMakeLists.txt

@@ -220,6 +220,9 @@ target_include_directories(
   gluten
   PUBLIC ${CMAKE_SYSTEM_INCLUDE_PATH} ${JNI_INCLUDE_DIRS}
          ${CMAKE_CURRENT_SOURCE_DIR} ${PROTO_OUTPUT_DIR} ${PROTOBUF_INCLUDE})
+if(GLUTEN_PREFIX_INCLUDE_DIRS)
+  target_include_directories(gluten BEFORE PUBLIC ${GLUTEN_PREFIX_INCLUDE_DIRS})
+endif()
 set_target_properties(gluten PROPERTIES LIBRARY_OUTPUT_DIRECTORY
                                         ${root_directory}/releases)
 
@@ -241,13 +244,15 @@ install(TARGETS gluten DESTINATION ${CMAKE_INSTALL_LIBDIR})
 install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/resources/libhdfs.so
         DESTINATION ${LIBHDFS3_DESTINATION})
 
-add_custom_command(
-  TARGET gluten
-  POST_BUILD
-  COMMAND ld $<TARGET_FILE:gluten> || true
-  COMMENT "Checking ld result of libgluten.so")
-add_custom_command(
-  TARGET gluten
-  POST_BUILD
-  COMMAND ldd $<TARGET_FILE:gluten> || true
-  COMMENT "Checking ldd result of libgluten.so")
+if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+  add_custom_command(
+    TARGET gluten
+    POST_BUILD
+    COMMAND ld $<TARGET_FILE:gluten> || true
+    COMMENT "Checking ld result of libgluten.so")
+  add_custom_command(
+    TARGET gluten
+    POST_BUILD
+    COMMAND ldd $<TARGET_FILE:gluten> || true
+    COMMENT "Checking ldd result of libgluten.so")
+endif()

cpp/velox/CMakeLists.txt

@@ -262,6 +262,12 @@ if(ENABLE_GLUTEN_VCPKG AND NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
   target_link_options(
     velox PRIVATE -Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/symbols.map)
 endif()
+if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
+  # Make libvelox.dylib own the Velox mono archive. Test utilities can then
+  # resolve Velox symbols from the dylib instead of linking a second copy.
+  target_link_options(velox PRIVATE
+                      "LINKER:-force_load,${VELOX_BUILD_PATH}/lib/libvelox.a")
+endif()
 
 find_protobuf()
 message(STATUS "Found Protobuf: ${PROTOBUF_LIBRARY}")
@@ -289,6 +295,9 @@ target_include_directories(
          ${VELOX_HOME}
          ${PROTO_OUTPUT_DIR}
          ${PROTOBUF_INCLUDE})
+if(GLUTEN_PREFIX_INCLUDE_DIRS)
+  target_include_directories(velox BEFORE PUBLIC ${GLUTEN_PREFIX_INCLUDE_DIRS})
+endif()
 
 set_target_properties(velox PROPERTIES LIBRARY_OUTPUT_DIRECTORY
                                        ${root_directory}/releases)
@@ -308,6 +317,10 @@ target_link_libraries(velox PUBLIC gluten)
 
 # Requires VELOX_MONO_LIBRARY=ON when building Velox.
 import_library(facebook::velox ${VELOX_BUILD_PATH}/lib/libvelox.a)
+set(GLUTEN_VELOX_LINK_SCOPE PUBLIC)
+if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
+  set(GLUTEN_VELOX_LINK_SCOPE PRIVATE)
+endif()
 
 if(BUILD_TESTS)
   add_duckdb()
@@ -337,7 +350,7 @@ if(BUILD_TESTS)
   target_link_libraries(velox PUBLIC facebook::velox::exec_test_lib)
 endif()
 
-target_link_libraries(velox PUBLIC facebook::velox)
+target_link_libraries(velox ${GLUTEN_VELOX_LINK_SCOPE} facebook::velox)
 
 target_link_libraries(velox PUBLIC ${GLUTEN_ROARING_LINK_LIBRARY})
 
@@ -385,6 +398,24 @@ target_link_libraries(velox PUBLIC external::veloxthrift)
 # `-DCMAKE_PREFIX_PATH="${folly lib path}" to cmake arguments. It is also
 # applicable to other dependencies.
 find_package(Folly REQUIRED CONFIG)
+if(CMAKE_SYSTEM_NAME MATCHES "Darwin" AND TARGET gflags_shared)
+  # Folly's exported interface references the static gflags target while glog
+  # links the shared gflags library. Loading both copies into one process makes
+  # gflags register its built-in flags twice and abort test binaries. Point
+  # folly's interface at the shared gflags target instead.
+  foreach(FOLLY_TARGET Folly::folly Folly::folly_deps)
+    if(TARGET ${FOLLY_TARGET})
+      get_target_property(FOLLY_LINK_LIBS ${FOLLY_TARGET}
+                          INTERFACE_LINK_LIBRARIES)
+      if(FOLLY_LINK_LIBS MATCHES "gflags_static")
+        list(TRANSFORM FOLLY_LINK_LIBS REPLACE "gflags_static" "gflags_shared")
+        set_target_properties(
+          ${FOLLY_TARGET} PROPERTIES INTERFACE_LINK_LIBRARIES
+                                     "${FOLLY_LINK_LIBS}")
+      endif()
+    endif()
+  endforeach()
+endif()
 target_link_libraries(velox PUBLIC Folly::folly)
 
 # Link thrift libraries - check vcpkg first, then fall back to system libraries
@@ -555,13 +586,15 @@ if(ENABLE_GPU)
            velox_curl)
 endif()
 
-add_custom_command(
-  TARGET velox
-  POST_BUILD
-  COMMAND ld $<TARGET_FILE:velox> || true
-  COMMENT "Checking ld result of libvelox.so")
-add_custom_command(
-  TARGET velox
-  POST_BUILD
-  COMMAND ldd $<TARGET_FILE:velox> || true
-  COMMENT "Checking ldd result of libvelox.so")
+if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+  add_custom_command(
+    TARGET velox
+    POST_BUILD
+    COMMAND ld $<TARGET_FILE:velox> || true
+    COMMENT "Checking ld result of libvelox.so")
+  add_custom_command(
+    TARGET velox
+    POST_BUILD
+    COMMAND ldd $<TARGET_FILE:velox> || true
+    COMMENT "Checking ldd result of libvelox.so")
+endif()

cpp/velox/tests/CMakeLists.txt

@@ -28,12 +28,16 @@ function(add_velox_test TEST_EXEC)
   add_executable(${TEST_EXEC} ${SOURCES} ${VELOX_TEST_COMMON_SRCS})
   target_include_directories(${TEST_EXEC} PRIVATE ${CMAKE_SOURCE_DIR}/velox
                                                   ${CMAKE_SOURCE_DIR}/src)
+  target_compile_definitions(
+    ${TEST_EXEC}
+    PRIVATE GLUTEN_VELOX_TEST_DATA_DIR="${CMAKE_CURRENT_SOURCE_DIR}/data")
   target_link_libraries(${TEST_EXEC} velox ${GLUTEN_ROARING_LINK_LIBRARY}
                         GTest::gtest GTest::gtest_main)
   if(ENABLE_GPU)
     target_link_libraries(${TEST_EXEC} velox_curl)
   endif()
-  gtest_discover_tests(${TEST_EXEC} DISCOVERY_MODE PRE_TEST)
+  gtest_discover_tests(${TEST_EXEC} DISCOVERY_MODE PRE_TEST DISCOVERY_TIMEOUT
+                       60)
 endfunction()
 
 if(ENABLE_GPU)
@@ -68,7 +72,9 @@ if(ENABLE_GPU)
     target_include_directories(${TEST_EXEC} PRIVATE ${CMAKE_SOURCE_DIR}/velox
                                                     ${CMAKE_SOURCE_DIR}/src)
     target_compile_definitions(
-      ${TEST_EXEC} PUBLIC LIBCUDACXX_ENABLE_EXPERIMENTAL_MEMORY_RESOURCE)
+      ${TEST_EXEC}
+      PUBLIC LIBCUDACXX_ENABLE_EXPERIMENTAL_MEMORY_RESOURCE
+      PRIVATE GLUTEN_VELOX_TEST_DATA_DIR="${CMAKE_CURRENT_SOURCE_DIR}/data")
     target_link_libraries(
       ${TEST_EXEC}
       PUBLIC facebook::velox::velox_cudf_expression

cpp/velox/tests/FilePathGenerator.cc

@@ -16,10 +16,9 @@
  */
 
 #include "FilePathGenerator.h"
+
 #include <filesystem>
 
 const std::string FilePathGenerator::getDataFilePath(const std::string& fileName) {
-  const std::string currentPath = std::filesystem::current_path().c_str();
-  const std::string filePath = currentPath + "/../../../velox/tests/data/" + fileName;
-  return filePath;
+  return (std::filesystem::path(GLUTEN_VELOX_TEST_DATA_DIR) / fileName).string();
 }

dev/build-helper-functions.sh

@@ -180,12 +180,17 @@ function cmake_install {
 
   local MACOS_ISOLATION_FLAGS=""
   if [[ "$(uname)" == "Darwin" ]]; then
-    if [[ "${INSTALL_PREFIX:-}" == "/usr/local" || "${INSTALL_PREFIX:-}" == /usr/local/* ]]; then
+    if [[ -z "${INSTALL_PREFIX:-}" ]]; then
+      :
+    elif [[ "${INSTALL_PREFIX:-}" == "/usr/local" || "${INSTALL_PREFIX:-}" == /usr/local/* ]]; then
       echo "INFO: INSTALL_PREFIX=${INSTALL_PREFIX} is under /usr/local; keeping /usr/local visible to CMake." >&2
     else
-      COMPILER_FLAGS="$COMPILER_FLAGS -isystem /usr/local/include"
-      MACOS_ISOLATION_FLAGS="-DCMAKE_NO_SYSTEM_FROM_IMPORTED=ON \
-        -DCMAKE_IGNORE_PREFIX_PATH=/usr/local \
+      # CMake ignore paths only affect package discovery. AppleClang still adds
+      # /usr/local/include to the default header search path unless an SDK
+      # sysroot is selected, so pin SDKROOT here too for callers that invoke
+      # cmake_install directly (e.g. build-arrow.sh) without exporting it.
+      export SDKROOT="${SDKROOT:-$(xcrun --show-sdk-path)}"
+      MACOS_ISOLATION_FLAGS="-DCMAKE_IGNORE_PREFIX_PATH=/usr/local \
         -DCMAKE_IGNORE_PATH=/usr/local;/usr/local/include;/usr/local/lib;/usr/local/lib/cmake \
         -DCMAKE_SYSTEM_IGNORE_PATH=/usr/local;/usr/local/include;/usr/local/lib;/usr/local/lib/cmake"
     fi

dev/builddeps-veloxbe.sh

@@ -162,6 +162,12 @@ if [[ "$(uname)" == "Darwin" ]]; then
     export INSTALL_PREFIX=${INSTALL_PREFIX:-${VELOX_HOME}/deps-install}
     if [[ "$INSTALL_PREFIX" == "/usr/local" || "$INSTALL_PREFIX" == /usr/local/* ]]; then
         echo "INFO: INSTALL_PREFIX=$INSTALL_PREFIX is under /usr/local; keeping /usr/local visible to CMake." >&2
+    else
+        # AppleClang injects /usr/local/include into the default header search
+        # path unless an SDK sysroot is selected. Route this build and all child
+        # builds through the SDK so /usr/local headers cannot shadow the ones
+        # from INSTALL_PREFIX.
+        export SDKROOT="${SDKROOT:-$(xcrun --show-sdk-path)}"
     fi
 elif [ -n "${INSTALL_PREFIX:-}" ]; then
     export INSTALL_PREFIX
@@ -217,6 +223,7 @@ concat_velox_param
 export VELOX_HOME
 
 function build_arrow {
+  local GLUTEN_BUILD_TYPE="$BUILD_TYPE"
   if [ ! -d "$VELOX_HOME" ]; then
     get_velox
     if [ -z "${GLUTEN_VCPKG_ENABLED:-}" ] && [ $RUN_SETUP_SCRIPT == "ON" ]; then
@@ -227,6 +234,7 @@ function build_arrow {
   fi
   cd $GLUTEN_DIR/dev
   source ./build-arrow.sh
+  BUILD_TYPE="$GLUTEN_BUILD_TYPE"
 }
 
 function build_velox {
@@ -269,8 +277,7 @@ function build_gluten_cpp {
     GLUTEN_CMAKE_OPTIONS+=("-DCMAKE_INSTALL_PREFIX=$INSTALL_PREFIX")
   fi
   if [ $OS == 'Darwin' ]; then
-    if [[ "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]]; then
-      GLUTEN_CMAKE_OPTIONS+=("-DCMAKE_NO_SYSTEM_FROM_IMPORTED=ON")
+    if [[ -n "${INSTALL_PREFIX:-}" && "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]]; then
       GLUTEN_CMAKE_OPTIONS+=("-DCMAKE_IGNORE_PREFIX_PATH=/usr/local")
       GLUTEN_CMAKE_OPTIONS+=("-DCMAKE_IGNORE_PATH=/usr/local;/usr/local/include;/usr/local/lib;/usr/local/lib/cmake")
       GLUTEN_CMAKE_OPTIONS+=("-DCMAKE_SYSTEM_IGNORE_PATH=/usr/local;/usr/local/include;/usr/local/lib;/usr/local/lib/cmake")

ep/build-velox/src/build-velox.sh

@@ -114,8 +114,10 @@ function compile {
     -Wno-error=uninitialized -Wno-unknown-warning-option -Wno-deprecated-declarations'
   if [[ "$(uname)" == "Darwin" ]]; then
     CXX_FLAGS="$CXX_FLAGS -Wno-inconsistent-missing-override -Wno-macro-redefined"
-    if [[ "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]]; then
-      CXX_FLAGS="$CXX_FLAGS -isystem /usr/local/include"
+    if [[ -n "${INSTALL_PREFIX:-}" && "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]]; then
+      # Some prefix-installed deps only publish loose headers (e.g. xsimd), so
+      # keep the prefix include dir on the compiler command line.
+      CXX_FLAGS="$CXX_FLAGS -I${INSTALL_PREFIX}/include"
     fi
   fi
 
@@ -125,8 +127,7 @@ function compile {
   if [ -n "${INSTALL_PREFIX:-}" ]; then
     COMPILE_OPTION="$COMPILE_OPTION -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_INSTALL_PREFIX=${INSTALL_PREFIX}"
   fi
-  if [[ "$(uname)" == "Darwin" && "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]]; then
-    COMPILE_OPTION="$COMPILE_OPTION -DCMAKE_NO_SYSTEM_FROM_IMPORTED=ON"
+  if [[ "$(uname)" == "Darwin" && -n "${INSTALL_PREFIX:-}" && "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]]; then
     COMPILE_OPTION="$COMPILE_OPTION -DCMAKE_IGNORE_PREFIX_PATH=/usr/local"
     COMPILE_OPTION="$COMPILE_OPTION -DCMAKE_IGNORE_PATH=/usr/local\;/usr/local/include\;/usr/local/lib\;/usr/local/lib/cmake"
     COMPILE_OPTION="$COMPILE_OPTION -DCMAKE_SYSTEM_IGNORE_PATH=/usr/local\;/usr/local/include\;/usr/local/lib\;/usr/local/lib/cmake"
@@ -191,7 +192,9 @@ function compile {
     cd _build/$COMPILE_TYPE/_deps
     if [ -d xsimd-build ]; then
       echo "INSTALL xsimd."
-      install_cmake_dependency xsimd-build/
+      if [ -f xsimd-build/cmake_install.cmake ]; then
+        install_cmake_dependency xsimd-build/
+      fi
     fi
     if [ -d googletest-build ]; then
       echo "INSTALL gtest."
@@ -218,6 +221,11 @@ if [ "$OS" == 'Darwin' ]; then
   export INSTALL_PREFIX="${INSTALL_PREFIX:-${VELOX_HOME}/deps-install}"
   if [[ "$INSTALL_PREFIX" == "/usr/local" || "$INSTALL_PREFIX" == /usr/local/* ]]; then
     echo "INFO: INSTALL_PREFIX=$INSTALL_PREFIX is under /usr/local; keeping /usr/local visible to CMake." >&2
+  else
+    # AppleClang adds /usr/local/include to the default header search path
+    # unless an SDK sysroot is selected. Keep prefix-based builds on the SDK so
+    # /usr/local headers cannot shadow the ones from INSTALL_PREFIX.
+    export SDKROOT="${SDKROOT:-$(xcrun --show-sdk-path)}"
   fi
 elif [ -n "${INSTALL_PREFIX:-}" ]; then
   export INSTALL_PREFIX

ep/build-velox/src/get-velox.sh

@@ -89,17 +89,12 @@ function process_setup_tencentos32 {
   sed -i "/^[[:space:]]*#/!s/.*dnf config-manager --set-enabled powertools/#&/" ${CURRENT_DIR}/setup-centos8.sh
 }
 
-# Keep macOS dependency builds on INSTALL_PREFIX even when /usr/local is present.
-# Apple clang injects /usr/local/include as a normal include path before CMake's
-# imported system includes, which can mix /usr/local headers with INSTALL_PREFIX
-# libraries. Demote it to system include order. Folly also enables jemalloc from
-# Homebrew headers but does not link libjemalloc, so keep Folly's Linux-equivalent
-# no-jemalloc behavior here; Gluten's own jemalloc build is independent of this.
+# Folly enables jemalloc when Homebrew headers are visible but does not link
+# libjemalloc, leaving _mallocx/_nallocx undefined for downstream links. Keep
+# Folly's Linux-equivalent no-jemalloc behavior; Gluten's own jemalloc build is
+# independent of this. Header-search isolation from /usr/local is handled by
+# SDKROOT exported in builddeps-veloxbe.sh / build-velox.sh.
 function process_setup_macos {
-  if [[ "${INSTALL_PREFIX:-}" != "/usr/local" && "${INSTALL_PREFIX:-}" != /usr/local/* ]] &&
-      ! grep -Fq '/usr/local/include' scripts/setup-macos.sh; then
-    sed -i '' 's|OS_CXXFLAGS=" -isystem $(brew --prefix)/include "|OS_CXXFLAGS=" -isystem $(brew --prefix)/include -isystem /usr/local/include "|' scripts/setup-macos.sh
-  fi
   if ! grep -Fq 'FOLLY_USE_JEMALLOC=OFF' scripts/setup-common.sh; then
     sed -i '' 's/local FOLLY_FLAGS=(/local FOLLY_FLAGS=(-DFOLLY_USE_JEMALLOC=OFF /' scripts/setup-common.sh
   fi