diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 4fba7009e80..cce42a9bbe5 100644 --- a/cpp/CMakeLists.txt +++ b/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/ diff --git a/cpp/core/CMakeLists.txt b/cpp/core/CMakeLists.txt index f3860166a49..c6da36dbd0a 100644 --- a/cpp/core/CMakeLists.txt +++ b/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 $ || true - COMMENT "Checking ld result of libgluten.so") -add_custom_command( - TARGET gluten - POST_BUILD - COMMAND ldd $ || true - COMMENT "Checking ldd result of libgluten.so") +if(CMAKE_SYSTEM_NAME MATCHES "Linux") + add_custom_command( + TARGET gluten + POST_BUILD + COMMAND ld $ || true + COMMENT "Checking ld result of libgluten.so") + add_custom_command( + TARGET gluten + POST_BUILD + COMMAND ldd $ || true + COMMENT "Checking ldd result of libgluten.so") +endif() diff --git a/cpp/velox/CMakeLists.txt b/cpp/velox/CMakeLists.txt index d935a12d048..56cd88b9960 100644 --- a/cpp/velox/CMakeLists.txt +++ b/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 $ || true - COMMENT "Checking ld result of libvelox.so") -add_custom_command( - TARGET velox - POST_BUILD - COMMAND ldd $ || true - COMMENT "Checking ldd result of libvelox.so") +if(CMAKE_SYSTEM_NAME MATCHES "Linux") + add_custom_command( + TARGET velox + POST_BUILD + COMMAND ld $ || true + COMMENT "Checking ld result of libvelox.so") + add_custom_command( + TARGET velox + POST_BUILD + COMMAND ldd $ || true + COMMENT "Checking ldd result of libvelox.so") +endif() diff --git a/cpp/velox/tests/CMakeLists.txt b/cpp/velox/tests/CMakeLists.txt index 59408cd2331..87331de818c 100644 --- a/cpp/velox/tests/CMakeLists.txt +++ b/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 diff --git a/cpp/velox/tests/FilePathGenerator.cc b/cpp/velox/tests/FilePathGenerator.cc index 0ee42740b38..a6281251f9c 100644 --- a/cpp/velox/tests/FilePathGenerator.cc +++ b/cpp/velox/tests/FilePathGenerator.cc @@ -16,10 +16,9 @@ */ #include "FilePathGenerator.h" + #include 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(); } diff --git a/dev/build-helper-functions.sh b/dev/build-helper-functions.sh index a8a257c286f..4692065a664 100644 --- a/dev/build-helper-functions.sh +++ b/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 diff --git a/dev/builddeps-veloxbe.sh b/dev/builddeps-veloxbe.sh index 0e7398d29e5..f36be37377d 100755 --- a/dev/builddeps-veloxbe.sh +++ b/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") diff --git a/ep/build-velox/src/build-velox.sh b/ep/build-velox/src/build-velox.sh index e84a8faab77..31109ed9192 100755 --- a/ep/build-velox/src/build-velox.sh +++ b/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 diff --git a/ep/build-velox/src/get-velox.sh b/ep/build-velox/src/get-velox.sh index 0ba44acfaa9..6443710a936 100755 --- a/ep/build-velox/src/get-velox.sh +++ b/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