diff --git a/cpp/CMake/Findglog.cmake b/cpp/CMake/Findglog.cmake index 6d9dbdacf1b..05c15e2100c 100644 --- a/cpp/CMake/Findglog.cmake +++ b/cpp/CMake/Findglog.cmake @@ -22,9 +22,23 @@ if(NOT BUILD_GLOG) include(FindPackageHandleStandardArgs) include(SelectLibraryConfigurations) + # On macOS, prefer the static libglog.a over libglog.dylib. libvelox.dylib has + # static gflags baked in via folly, so we must avoid libglog.dylib which + # transitively loads libgflags.dylib at runtime and triggers the "linked both + # statically and dynamically" abort in gflags. + if(CMAKE_SYSTEM_NAME MATCHES "Darwin") + set(_glog_find_suffixes_saved ${CMAKE_FIND_LIBRARY_SUFFIXES}) + set(CMAKE_FIND_LIBRARY_SUFFIXES ".a" ${CMAKE_FIND_LIBRARY_SUFFIXES}) + endif() + find_library(GLOG_LIBRARY_RELEASE glog PATHS ${GLOG_LIBRARYDIR}) find_library(GLOG_LIBRARY_DEBUG glogd PATHS ${GLOG_LIBRARYDIR}) + if(CMAKE_SYSTEM_NAME MATCHES "Darwin") + set(CMAKE_FIND_LIBRARY_SUFFIXES ${_glog_find_suffixes_saved}) + unset(_glog_find_suffixes_saved) + endif() + find_path(GLOG_INCLUDE_DIR glog/logging.h PATHS ${GLOG_INCLUDEDIR}) select_library_configurations(GLOG) @@ -48,13 +62,43 @@ else() set(libgflags_component shared) endif() +# On macOS, force static gflags regardless of glog linkage. libvelox.dylib has +# static gflags baked in via folly (-DGFLAGS_SHARED=FALSE). If libgluten.dylib +# were to load libgflags.dylib dynamically, gflags would detect the same flag +# registered both statically (inside libvelox.dylib) and dynamically (via +# libgflags.dylib) and abort with "being linked both statically and +# dynamically". +if(CMAKE_SYSTEM_NAME MATCHES "Darwin") + set(libgflags_component static) +endif() + +# On macOS with a static libglog.a we link glog and gflags via -load_hidden so +# their symbols stay local to libgluten.dylib. This prevents dyld from +# coalescing the weak symbols (C++ function-local statics like +# FlagRegistry::GlobalRegistry) with the static gflags copy already inside +# libvelox.dylib, which otherwise triggers "flag 'flagfile' ... linked both +# statically and dynamically". +# +# For this to work, google::glog must be an INTERFACE IMPORTED target so that +# CMake does not try to add GLOG_LIBRARY through the normal link path. +set(_gluten_glog_hidden_load OFF) +if(CMAKE_SYSTEM_NAME MATCHES "Darwin" AND libglog_ext STREQUAL ".a") + set(_gluten_glog_hidden_load ON) +endif() + # glog::glog may already exist. Use google::glog to avoid conflicts. -add_library(google::glog ${libglog_type} IMPORTED) -set_target_properties(google::glog PROPERTIES INTERFACE_INCLUDE_DIRECTORIES - "${GLOG_INCLUDE_DIR}") -set_target_properties( - google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LANGUAGES "C" - IMPORTED_LOCATION "${GLOG_LIBRARY}") +if(_gluten_glog_hidden_load) + add_library(google::glog INTERFACE IMPORTED) + set_target_properties(google::glog PROPERTIES INTERFACE_INCLUDE_DIRECTORIES + "${GLOG_INCLUDE_DIR}") +else() + add_library(google::glog ${libglog_type} IMPORTED) + set_target_properties(google::glog PROPERTIES INTERFACE_INCLUDE_DIRECTORIES + "${GLOG_INCLUDE_DIR}") + set_target_properties( + google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LANGUAGES "C" + IMPORTED_LOCATION "${GLOG_LIBRARY}") +endif() set(GLUTEN_GFLAGS_VERSION 2.2.2) find_package(gflags ${GLUTEN_GFLAGS_VERSION} CONFIG @@ -73,17 +117,65 @@ if(gflags_FOUND) FATAL_ERROR "Found Gflags but missing component gflags_${libgflags_component}") endif() - if(TARGET gflags::gflags_${libgflags_component}) - set_target_properties( - google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES - gflags::gflags_${libgflags_component}) - else() - set_target_properties( - google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES - gflags_${libgflags_component}) + if(NOT _gluten_glog_hidden_load) + if(TARGET gflags::gflags_${libgflags_component}) + set_target_properties( + google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES + gflags::gflags_${libgflags_component}) + else() + set_target_properties( + google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES + gflags_${libgflags_component}) + endif() endif() else() include(BuildGflags) + if(NOT _gluten_glog_hidden_load) + set_target_properties( + google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES gflags_static) + endif() +endif() + +if(_gluten_glog_hidden_load) + set(_gflags_static_target "") + if(TARGET gflags::gflags_static) + set(_gflags_static_target gflags::gflags_static) + elseif(TARGET gflags_static) + set(_gflags_static_target gflags_static) + endif() + + set(_gflags_static_path "") + if(_gflags_static_target) + foreach(_cfg RELEASE NOCONFIG "" DEBUG RELWITHDEBINFO MINSIZEREL) + if(_cfg STREQUAL "") + set(_prop IMPORTED_LOCATION) + else() + set(_prop IMPORTED_LOCATION_${_cfg}) + endif() + get_target_property(_maybe_path ${_gflags_static_target} ${_prop}) + if(_maybe_path AND NOT _maybe_path STREQUAL "_maybe_path-NOTFOUND") + set(_gflags_static_path ${_maybe_path}) + break() + endif() + endforeach() + endif() + + if(NOT _gflags_static_path) + message( + FATAL_ERROR + "Could not resolve libgflags.a path for -load_hidden; expected a gflags_static target with an IMPORTED_LOCATION* property." + ) + endif() + + message( + STATUS "Linking gflags (static, -load_hidden): ${_gflags_static_path}") + set_target_properties( - google::glog PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES gflags_static) + google::glog PROPERTIES INTERFACE_LINK_OPTIONS + "LINKER:-load_hidden,${GLOG_LIBRARY}") + set_property( + TARGET google::glog + APPEND + PROPERTY INTERFACE_LINK_OPTIONS + "LINKER:-load_hidden,${_gflags_static_path}") endif() diff --git a/cpp/core/CMakeLists.txt b/cpp/core/CMakeLists.txt index e16464b8111..3d7fcec8a6c 100644 --- a/cpp/core/CMakeLists.txt +++ b/cpp/core/CMakeLists.txt @@ -154,6 +154,16 @@ file(MAKE_DIRECTORY ${root_directory}/releases) add_library(gluten SHARED ${SPARK_COLUMNAR_PLUGIN_SRCS}) add_dependencies(gluten jni_proto) +# On macOS, libgluten.dylib links libgflags.a with -load_hidden (see +# cpp/CMake/Findglog.cmake). That hides the gflags copy pulled in by +# libgluten.dylib, but libvelox.dylib has an undefined reference to +# google::HandleCommandLineHelpFlags that previously resolved against +# libgluten.dylib's exported copy. Provide an exported no-op stub so +# libvelox.dylib can still find the symbol at dlopen time. +if(CMAKE_SYSTEM_NAME MATCHES "Darwin") + target_sources(gluten PRIVATE utils/GflagsStubDarwin.cc) +endif() + # Hide symbols of some static dependencies. Otherwise, if such dependencies are # already statically linked to libvelox.so, a runtime error will be reported: # xxx is being linked both statically and dynamically. diff --git a/cpp/core/utils/GflagsStubDarwin.cc b/cpp/core/utils/GflagsStubDarwin.cc new file mode 100644 index 00000000000..a1288cea7e0 --- /dev/null +++ b/cpp/core/utils/GflagsStubDarwin.cc @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// macOS only: libgluten.dylib loads libgflags.a with -load_hidden +// (cpp/CMake/Findglog.cmake) so its gflags symbols do not coalesce with the +// static gflags already baked into libvelox.dylib. libvelox.dylib's own +// static gflags build pulled gflags.cc.o but not gflags_reporting.cc.o, so +// google::HandleCommandLineHelpFlags remains undefined inside libvelox.dylib +// and is expected to be supplied by libgluten.dylib at dlopen time. Provide a +// no-op stub with default visibility; Gluten never invokes help processing. +namespace google { +// NOLINTNEXTLINE(readability-identifier-naming) - name dictated by gflags ABI. +__attribute__((visibility("default"))) void HandleCommandLineHelpFlags() {} +} // namespace google diff --git a/cpp/velox/CMakeLists.txt b/cpp/velox/CMakeLists.txt index 532dd6e6a79..d932e0ec93f 100644 --- a/cpp/velox/CMakeLists.txt +++ b/cpp/velox/CMakeLists.txt @@ -293,6 +293,18 @@ endif() target_link_libraries(velox PUBLIC gluten) +# On Darwin, propagate google::glog's INTERFACE_LINK_OPTIONS +# (LINKER:-load_hidden for libglog.a and libgflags.a) through libvelox.dylib to +# all downstream consumers (test binaries, benchmarks). Without this, +# Folly::folly's INTERFACE_LINK_LIBRARIES pulls libgflags.a/libglog.a into +# libvelox.dylib and test binaries with default visibility, so their +# FlagRegistry::GlobalRegistry function-local statics coalesce with +# libgluten.dylib's copy and gflags aborts with "flag 'flagfile' linked both +# statically and dynamically" at startup. +if(CMAKE_SYSTEM_NAME MATCHES "Darwin") + target_link_libraries(velox PUBLIC google::glog) +endif() + # Requires VELOX_MONO_LIBRARY=ON when building Velox. import_library(facebook::velox ${VELOX_BUILD_PATH}/lib/libvelox.a) diff --git a/cpp/velox/compute/VeloxBackend.cc b/cpp/velox/compute/VeloxBackend.cc index 85a8622508a..e493ccf3251 100644 --- a/cpp/velox/compute/VeloxBackend.cc +++ b/cpp/velox/compute/VeloxBackend.cc @@ -113,7 +113,9 @@ void VeloxBackend::init( } } FLAGS_logtostderr = true; - google::InitGoogleLogging("gluten"); + if (!google::IsGoogleLoggingInitialized()) { + google::InitGoogleLogging("gluten"); + } globalMemoryManager_ = std::make_unique(kVeloxBackendKind, std::move(listener), *backendConf_); @@ -352,6 +354,9 @@ std::unique_ptr VeloxBackend::instance_ = nullptr; void VeloxBackend::create( std::unique_ptr listener, const std::unordered_map& conf) { + if (instance_) { + return; + } instance_ = std::unique_ptr(new VeloxBackend(std::move(listener), conf)); }