From 943f0b9f1220c5c8d0f243b1dd5cc87e52be949f Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 10:35:08 +0900 Subject: [PATCH 01/13] GH-37067: [C++] Install bundled GoogleTest Because `libarrow*_testing.so` depend it. --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 183 +++++--------------- cpp/src/arrow/CMakeLists.txt | 3 + 2 files changed, 49 insertions(+), 137 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index b2360dab2ec..b9b119aff23 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -998,6 +998,18 @@ endif() include(FetchContent) +macro(prepare_fetchcontent) + set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY TRUE) + set(CMAKE_COMPILE_WARNING_AS_ERROR FALSE) + if(MSVC) + string(APPEND CMAKE_C_FLAGS_DEBUG " /WX:NO") + string(APPEND CMAKE_CXX_FLAGS_DEBUG " /WX:NO") + else() + string(APPEND CMAKE_C_FLAGS_DEBUG " -Wno-error") + string(APPEND CMAKE_CXX_FLAGS_DEBUG " -Wno-error") + endif() +endmacro() + # ---------------------------------------------------------------------- # Find pthreads @@ -2086,136 +2098,43 @@ endif() macro(build_gtest) message(STATUS "Building gtest from source") set(GTEST_VENDORED TRUE) - set(GTEST_CMAKE_CXX_FLAGS ${EP_CXX_FLAGS}) - - if(CMAKE_BUILD_TYPE MATCHES DEBUG) - set(CMAKE_GTEST_DEBUG_EXTENSION "d") - else() - set(CMAKE_GTEST_DEBUG_EXTENSION "") - endif() - + fetchcontent_declare(googletest + URL ${GTEST_SOURCE_URL} + URL_HASH "SHA256=${ARROW_GTEST_BUILD_SHA256_CHECKSUM}") + prepare_fetchcontent() if(APPLE) string(APPEND - GTEST_CMAKE_CXX_FLAGS - " -DGTEST_USE_OWN_TR1_TUPLE=1" + CMAKE_CXX_FLAGS " -Wno-unused-value" " -Wno-ignored-attributes") endif() - - if(WIN32) - string(APPEND GTEST_CMAKE_CXX_FLAGS " -DGTEST_CREATE_SHARED_LIBRARY=1") - endif() - - set(GTEST_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/googletest_ep-prefix") - set(GTEST_INCLUDE_DIR "${GTEST_PREFIX}/include") - - set(_GTEST_LIBRARY_DIR "${GTEST_PREFIX}/lib") - - if(WIN32) - set(_GTEST_IMPORTED_TYPE IMPORTED_IMPLIB) - set(_GTEST_LIBRARY_SUFFIX - "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_IMPORT_LIBRARY_SUFFIX}") - else() - set(_GTEST_IMPORTED_TYPE IMPORTED_LOCATION) - set(_GTEST_LIBRARY_SUFFIX - "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_SHARED_LIBRARY_SUFFIX}") - - endif() - - set(GTEST_SHARED_LIB - "${_GTEST_LIBRARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${_GTEST_LIBRARY_SUFFIX}") - set(GMOCK_SHARED_LIB - "${_GTEST_LIBRARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gmock${_GTEST_LIBRARY_SUFFIX}") - set(GTEST_MAIN_SHARED_LIB - "${_GTEST_LIBRARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${_GTEST_LIBRARY_SUFFIX}" - ) - set(GTEST_INSTALL_NAME_DIR "$/lib") - # Fix syntax highlighting mess introduced by unclosed bracket above - set(dummy ">") - - set(GTEST_CMAKE_ARGS - ${EP_COMMON_CMAKE_ARGS} - -DBUILD_SHARED_LIBS=ON - -DBUILD_STATIC_LIBS=OFF - -DCMAKE_CXX_FLAGS=${GTEST_CMAKE_CXX_FLAGS} - -DCMAKE_INSTALL_NAME_DIR=${GTEST_INSTALL_NAME_DIR} - -DCMAKE_INSTALL_PREFIX=${GTEST_PREFIX} - -DCMAKE_MACOSX_RPATH=OFF) - set(GMOCK_INCLUDE_DIR "${GTEST_PREFIX}/include") - - if(WIN32 AND NOT ARROW_USE_STATIC_CRT) - list(APPEND GTEST_CMAKE_ARGS -Dgtest_force_shared_crt=ON) - endif() - - externalproject_add(googletest_ep - ${EP_COMMON_OPTIONS} - URL ${GTEST_SOURCE_URL} - URL_HASH "SHA256=${ARROW_GTEST_BUILD_SHA256_CHECKSUM}" - BUILD_BYPRODUCTS ${GTEST_SHARED_LIB} ${GTEST_MAIN_SHARED_LIB} - ${GMOCK_SHARED_LIB} - CMAKE_ARGS ${GTEST_CMAKE_ARGS}) - if(WIN32) - # Copy the built shared libraries to the same directory as our - # test programs because Windows doesn't provided rpath (run-time - # search path) feature. We need to put these shared libraries to - # the same directory as our test programs or add - # _GTEST_LIBRARY_DIR to PATH when we run our test programs. We - # choose the former because the latter may be forgotten. - set(_GTEST_RUNTIME_DIR "${GTEST_PREFIX}/bin") - set(_GTEST_RUNTIME_SUFFIX - "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_SHARED_LIBRARY_SUFFIX}") - set(_GTEST_RUNTIME_LIB - "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${_GTEST_RUNTIME_SUFFIX}" - ) - set(_GMOCK_RUNTIME_LIB - "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gmock${_GTEST_RUNTIME_SUFFIX}" - ) - set(_GTEST_MAIN_RUNTIME_LIB - "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${_GTEST_RUNTIME_SUFFIX}" - ) - get_property(_GENERATOR_IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) - if(_GENERATOR_IS_MULTI_CONFIG) - set(_GTEST_RUNTIME_OUTPUT_DIR "${BUILD_OUTPUT_ROOT_DIRECTORY}/${CMAKE_BUILD_TYPE}") - else() - set(_GTEST_RUNTIME_OUTPUT_DIR ${BUILD_OUTPUT_ROOT_DIRECTORY}) - endif() - externalproject_add_step(googletest_ep copy - COMMAND ${CMAKE_COMMAND} -E make_directory - ${_GTEST_RUNTIME_OUTPUT_DIR} - COMMAND ${CMAKE_COMMAND} -E copy ${_GTEST_RUNTIME_LIB} - ${_GTEST_RUNTIME_OUTPUT_DIR} - COMMAND ${CMAKE_COMMAND} -E copy ${_GMOCK_RUNTIME_LIB} - ${_GTEST_RUNTIME_OUTPUT_DIR} - COMMAND ${CMAKE_COMMAND} -E copy ${_GTEST_MAIN_RUNTIME_LIB} - ${_GTEST_RUNTIME_OUTPUT_DIR} - DEPENDEES install) + set(BUILD_SHARED_LIBS ON) + set(BUILD_STATIC_LIBS OFF) + set(INSTALL_GTEST OFF) + string(APPEND CMAKE_INSTALL_INCLUDEDIR "/arrow-gtest") + fetchcontent_makeavailable(googletest) + set_target_properties(gmock PROPERTIES OUTPUT_NAME "arrow_gmock") + set_target_properties(gmock_main PROPERTIES OUTPUT_NAME "arrow_gmock_main") + set_target_properties(gtest PROPERTIES OUTPUT_NAME "arrow_gtest") + set_target_properties(gtest_main PROPERTIES OUTPUT_NAME "arrow_gtest_main") + install(DIRECTORY "${googletest_SOURCE_DIR}/googlemock/include/" + "${googletest_SOURCE_DIR}/googletest/include/" + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") + install(TARGETS gmock gmock_main gtest gtest_main + EXPORT arrow_testing_targets + RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" + ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" + LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}") + if(MSVC) + install(FILES $ $ + $ $ + DESTINATION "${CMAKE_INSTALL_BINDIR}" + OPTIONAL) endif() - - # The include directory must exist before it is referenced by a target. - file(MAKE_DIRECTORY "${GTEST_INCLUDE_DIR}") - - add_library(arrow::GTest::gtest SHARED IMPORTED) - set_target_properties(arrow::GTest::gtest - PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_SHARED_LIB}" - INTERFACE_COMPILE_DEFINITIONS - "GTEST_LINKED_AS_SHARED_LIBRARY=1" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") - - add_library(arrow::GTest::gtest_main SHARED IMPORTED) - set_target_properties(arrow::GTest::gtest_main - PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_MAIN_SHARED_LIB}" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") - - add_library(arrow::GTest::gmock SHARED IMPORTED) - set_target_properties(arrow::GTest::gmock - PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GMOCK_SHARED_LIB}" - INTERFACE_COMPILE_DEFINITIONS - "GMOCK_LINKED_AS_SHARED_LIBRARY=1" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") - add_dependencies(toolchain-tests googletest_ep) - add_dependencies(arrow::GTest::gtest googletest_ep) - add_dependencies(arrow::GTest::gtest_main googletest_ep) - add_dependencies(arrow::GTest::gmock googletest_ep) + add_library(arrow::GTest::gmock ALIAS gmock) + add_library(arrow::GTest::gmock_main ALIAS gmock_main) + add_library(arrow::GTest::gtest ALIAS gtest) + add_library(arrow::GTest::gtest_main ALIAS gtest_main) endmacro() if(ARROW_TESTING) @@ -2249,9 +2168,8 @@ if(ARROW_TESTING) set(ARROW_GTEST_GTEST GTest::gtest) set(ARROW_GTEST_GTEST_MAIN GTest::gtest_main) else() - # TODO: How to solve BUNDLED case? Do we install bundled GoogleTest? - # string(APPEND ARROW_TESTING_PC_CFLAGS " -I${GTEST_INCLUDE_DIR}") - # string(APPEND ARROW_TESTING_PC_LIBS " -lgtest") + string(APPEND ARROW_TESTING_PC_CFLAGS " -I\${includedir}/arrow-gtest") + string(APPEND ARROW_TESTING_PC_LIBS " -larrow_gtest") set(ARROW_GTEST_GMOCK arrow::GTest::gmock) set(ARROW_GTEST_GTEST arrow::GTest::gtest) set(ARROW_GTEST_GTEST_MAIN arrow::GTest::gtest_main) @@ -5121,24 +5039,15 @@ function(build_azure_sdk) fetchcontent_declare(azure_sdk URL ${ARROW_AZURE_SDK_URL} URL_HASH "SHA256=${ARROW_AZURE_SDK_BUILD_SHA256_CHECKSUM}") + prepare_fetchcontent() set(BUILD_PERFORMANCE_TESTS FALSE) set(BUILD_SAMPLES FALSE) set(BUILD_TESTING FALSE) set(BUILD_WINDOWS_UWP TRUE) - set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY TRUE) set(CMAKE_UNITY_BUILD FALSE) set(DISABLE_AZURE_CORE_OPENTELEMETRY TRUE) set(ENV{AZURE_SDK_DISABLE_AUTO_VCPKG} TRUE) set(WARNINGS_AS_ERRORS FALSE) - # TODO: Configure flags in a better way. FetchContent builds inherit - # global flags but we want to disable -Werror for Azure SDK for C++ builds. - if(MSVC) - string(REPLACE "/WX" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") - string(REPLACE "/WX" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") - else() - string(REPLACE "-Werror" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") - string(REPLACE "-Werror" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") - endif() fetchcontent_makeavailable(azure_sdk) set(AZURE_SDK_VENDORED TRUE diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index ba3f0ef30dd..b24b477ddc6 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -710,6 +710,9 @@ if(ARROW_TESTING) if(GTest_SOURCE STREQUAL "SYSTEM") list(APPEND ARROW_TESTING_SHARED_INSTALL_INTERFACE_LIBS ${ARROW_GTEST_GTEST}) list(APPEND ARROW_TESTING_STATIC_INSTALL_INTERFACE_LIBS ${ARROW_GTEST_GTEST}) + else() + list(APPEND ARROW_TESTING_SHARED_INSTALL_INTERFACE_LIBS ArrowTesting::gtest) + list(APPEND ARROW_TESTING_STATIC_INSTALL_INTERFACE_LIBS ArrowTesting::gtest) endif() add_arrow_lib(arrow_testing CMAKE_PACKAGE_NAME From 583a54ccd1122be6c526e77d0fc4eac6875e240b Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 11:23:58 +0900 Subject: [PATCH 02/13] Add missing gmock dependencies --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 5 +---- cpp/src/arrow/CMakeLists.txt | 17 +++++++++-------- cpp/src/arrow/compute/CMakeLists.txt | 4 +++- cpp/src/arrow/compute/kernels/CMakeLists.txt | 3 ++- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index b9b119aff23..d0e8e4d09c9 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2103,10 +2103,7 @@ macro(build_gtest) URL_HASH "SHA256=${ARROW_GTEST_BUILD_SHA256_CHECKSUM}") prepare_fetchcontent() if(APPLE) - string(APPEND - CMAKE_CXX_FLAGS - " -Wno-unused-value" - " -Wno-ignored-attributes") + string(APPEND CMAKE_CXX_FLAGS " -Wno-unused-value" " -Wno-ignored-attributes") endif() set(BUILD_SHARED_LIBS ON) set(BUILD_STATIC_LIBS OFF) diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index b24b477ddc6..9ecf2b51c91 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -324,6 +324,11 @@ if(ARROW_WITH_ZSTD) list(APPEND ARROW_SRCS util/compression_zstd.cc) endif() +set(ARROW_TESTING_SHARED_LINK_LIBS arrow::flatbuffers rapidjson::rapidjson arrow_shared + ${ARROW_GTEST_GTEST}) +set(ARROW_TESTING_STATIC_LINK_LIBS arrow::flatbuffers rapidjson::rapidjson arrow_static + ${ARROW_GTEST_GTEST}) + set(ARROW_TESTING_SRCS io/test_common.cc ipc/test_common.cc @@ -503,6 +508,8 @@ if(ARROW_FILESYSTEM) SKIP_UNITY_BUILD_INCLUSION ON) endif() + list(APPEND ARROW_TESTING_SHARED_LINK_LIBS ${ARROW_GTEST_GMOCK}) + list(APPEND ARROW_TESTING_STATIC_LINK_LIBS ${ARROW_GTEST_GMOCK}) list(APPEND ARROW_TESTING_SRCS filesystem/test_util.cc) endif() @@ -728,17 +735,11 @@ if(ARROW_TESTING) DEPENDENCIES arrow_test_dependencies SHARED_LINK_LIBS - arrow::flatbuffers - rapidjson::rapidjson - arrow_shared - ${ARROW_GTEST_GTEST} + ${ARROW_TESTING_SHARED_LINK_LIBS} SHARED_INSTALL_INTERFACE_LIBS ${ARROW_TESTING_SHARED_INSTALL_INTERFACE_LIBS} STATIC_LINK_LIBS - arrow::flatbuffers - rapidjson::rapidjson - arrow_static - ${ARROW_GTEST_GTEST} + ${ARROW_TESTING_STATIC_LINK_LIBS} STATIC_INSTALL_INTERFACE_LIBS ${ARROW_TESTING_STATIC_INSTALL_INTERFACE_LIBS}) diff --git a/cpp/src/arrow/compute/CMakeLists.txt b/cpp/src/arrow/compute/CMakeLists.txt index 1134e0a98ae..001424dd420 100644 --- a/cpp/src/arrow/compute/CMakeLists.txt +++ b/cpp/src/arrow/compute/CMakeLists.txt @@ -89,7 +89,9 @@ add_arrow_test(internals_test kernel_test.cc light_array_test.cc registry_test.cc - key_hash_test.cc) + key_hash_test.cc + EXTRA_LINK_LIBS + ${ARROW_GTEST_GMOCK}) add_arrow_compute_test(expression_test SOURCES expression_test.cc) diff --git a/cpp/src/arrow/compute/kernels/CMakeLists.txt b/cpp/src/arrow/compute/kernels/CMakeLists.txt index 0bd6fe86134..78743050625 100644 --- a/cpp/src/arrow/compute/kernels/CMakeLists.txt +++ b/cpp/src/arrow/compute/kernels/CMakeLists.txt @@ -23,7 +23,8 @@ if(ARROW_TESTING) add_library(arrow_compute_kernels_testing OBJECT test_util.cc) # Even though this is still just an object library we still need to "link" our # dependencies so that include paths are configured correctly - target_link_libraries(arrow_compute_kernels_testing ${ARROW_GTEST_GTEST}) + target_link_libraries(arrow_compute_kernels_testing ${ARROW_GTEST_GTEST} + ${ARROW_GTEST_GMOCK}) endif() add_arrow_test(scalar_cast_test From 5313e99b538ed3ac157c36e1a305567ce87cfe23 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 11:44:29 +0900 Subject: [PATCH 03/13] Can't use /WX:NO and use function for scope --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index d0e8e4d09c9..0fc2b0e8a13 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1002,8 +1002,8 @@ macro(prepare_fetchcontent) set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY TRUE) set(CMAKE_COMPILE_WARNING_AS_ERROR FALSE) if(MSVC) - string(APPEND CMAKE_C_FLAGS_DEBUG " /WX:NO") - string(APPEND CMAKE_CXX_FLAGS_DEBUG " /WX:NO") + string(REPLACE "/WX" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") + string(REPLACE "/WX" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") else() string(APPEND CMAKE_C_FLAGS_DEBUG " -Wno-error") string(APPEND CMAKE_CXX_FLAGS_DEBUG " -Wno-error") @@ -2095,7 +2095,7 @@ endif() # ---------------------------------------------------------------------- # Google gtest -macro(build_gtest) +function(build_gtest) message(STATUS "Building gtest from source") set(GTEST_VENDORED TRUE) fetchcontent_declare(googletest @@ -2132,7 +2132,7 @@ macro(build_gtest) add_library(arrow::GTest::gmock_main ALIAS gmock_main) add_library(arrow::GTest::gtest ALIAS gtest) add_library(arrow::GTest::gtest_main ALIAS gtest_main) -endmacro() +endfunction() if(ARROW_TESTING) set(GTestAlt_NEED_CXX_STANDARD_CHECK TRUE) From a110cc9755f42c8bcb98471ea2f6b9d64dd47053 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 11:47:21 +0900 Subject: [PATCH 04/13] Use static link by default --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 0fc2b0e8a13..eb76105d5f1 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -999,6 +999,8 @@ endif() include(FetchContent) macro(prepare_fetchcontent) + set(BUILD_SHARED_LIBS OFF) + set(BUILD_STATIC_LIBS ON) set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY TRUE) set(CMAKE_COMPILE_WARNING_AS_ERROR FALSE) if(MSVC) From 64f27667d150c7d2def953f988b949e5e1dba3c6 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 13:52:59 +0900 Subject: [PATCH 05/13] Adjust MATLAB --- matlab/CMakeLists.txt | 57 +++++++++++++------------------------------ 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index feda2cc6adb..18e08fe4ea5 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -115,35 +115,21 @@ function(build_arrow) endfunction() macro(enable_gtest) - set(ARROW_GTEST_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") - set(ARROW_GTEST_MAIN_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") + set(ARROW_GTEST_SOURCE_PREFIX "${ARROW_BINARY_DIR}/_deps/googletest-src/googletest") + set(ARROW_GTEST_INCLUDE_DIR "${ARROW_GTEST_SOURCE_PREFIX}/include") - if(WIN32) - set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_GTEST_PREFIX}/bin") - set(ARROW_GTEST_MAIN_SHARED_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/bin") - - set(ARROW_GTEST_LINK_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") - set(ARROW_GTEST_LINK_LIB - "${ARROW_GTEST_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest${CMAKE_IMPORT_LIBRARY_SUFFIX}" - ) - - set(ARROW_GTEST_MAIN_LINK_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") - set(ARROW_GTEST_MAIN_LINK_LIB - "${ARROW_GTEST_MAIN_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest_main${CMAKE_IMPORT_LIBRARY_SUFFIX}" - ) - else() - set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") - set(ARROW_GTEST_MAIN_SHARED_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") - endif() - - set(ARROW_GTEST_INCLUDE_DIR "${ARROW_GTEST_PREFIX}/include") + set(ARROW_GTEST_LIB_DIR "${ARROW_BINARY_DIR}/$") + set(ARROW_GTEST_IMPORT_LIB + "${ARROW_GTEST_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}arrow_gtest${CMAKE_IMPORT_LIBRARY_SUFFIX}" + ) + set(ARROW_GTEST_MAIN_IMPORT_LIB + "${ARROW_GTEST_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}arrow_gtest_main${CMAKE_IMPORT_LIBRARY_SUFFIX}" + ) set(ARROW_GTEST_SHARED_LIB - "${ARROW_GTEST_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}" + "${ARROW_GTEST_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow_gtest${CMAKE_SHARED_LIBRARY_SUFFIX}" ) - - set(ARROW_GTEST_MAIN_INCLUDE_DIR "${ARROW_GTEST_MAIN_PREFIX}/include") set(ARROW_GTEST_MAIN_SHARED_LIB - "${ARROW_GTEST_MAIN_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" + "${ARROW_GTEST_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow_gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" ) list(APPEND ARROW_CMAKE_ARGS "-DARROW_BUILD_TESTS=ON") @@ -152,8 +138,8 @@ macro(enable_gtest) # executables. if(WIN32) # On Windows, add the gtest link libraries as BUILD_BYPRODUCTS for arrow_ep. - list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_LINK_LIB}" - "${ARROW_GTEST_MAIN_LINK_LIB}") + list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_IMPORT_LIB}" + "${ARROW_GTEST_MAIN_IMPORT_LIB}") else() # On Linux and macOS, add the gtest shared libraries as BUILD_BYPRODUCTS for arrow_ep. list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_SHARED_LIB}" @@ -168,26 +154,17 @@ macro(build_gtest) # Create target GTest::gtest add_library(GTest::gtest SHARED IMPORTED) set_target_properties(GTest::gtest - PROPERTIES IMPORTED_LOCATION ${ARROW_GTEST_SHARED_LIB} + PROPERTIES IMPORTED_IMPLIB ${ARROW_GTEST_IMPORT_LIB} + IMPORTED_LOCATION ${ARROW_GTEST_SHARED_LIB} INTERFACE_INCLUDE_DIRECTORIES ${ARROW_GTEST_INCLUDE_DIR}) - if(WIN32) - set_target_properties(GTest::gtest PROPERTIES IMPORTED_IMPLIB ${ARROW_GTEST_LINK_LIB}) - endif() - add_dependencies(GTest::gtest arrow_ep) # Create target GTest::gtest_main add_library(GTest::gtest_main SHARED IMPORTED) set_target_properties(GTest::gtest_main - PROPERTIES IMPORTED_LOCATION ${ARROW_GTEST_MAIN_SHARED_LIB} - INTERFACE_INCLUDE_DIRECTORIES - ${ARROW_GTEST_MAIN_INCLUDE_DIR}) - if(WIN32) - set_target_properties(GTest::gtest_main PROPERTIES IMPORTED_IMPLIB - ${ARROW_GTEST_MAIN_LINK_LIB}) - endif() - + PROPERTIES IMPORTED_IMPLIB ${ARROW_GTEST_MAIN_IMPORT_LIB} + IMPORTED_LOCATION ${ARROW_GTEST_MAIN_SHARED_LIB}) add_dependencies(GTest::gtest_main arrow_ep) endmacro() From b512457bc873f50b6b6ac4a5165c17a4860e893c Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 14:03:30 +0900 Subject: [PATCH 06/13] Fix path --- matlab/CMakeLists.txt | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 18e08fe4ea5..b89f265633f 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -116,20 +116,25 @@ endfunction() macro(enable_gtest) set(ARROW_GTEST_SOURCE_PREFIX "${ARROW_BINARY_DIR}/_deps/googletest-src/googletest") - set(ARROW_GTEST_INCLUDE_DIR "${ARROW_GTEST_SOURCE_PREFIX}/include") + set(ARROW_GTEST_INCLUDE_DIR "${ARROW_PREFIX}/include") - set(ARROW_GTEST_LIB_DIR "${ARROW_BINARY_DIR}/$") + set(ARROW_GTEST_IMPORT_LIB_DIR "${ARROW_PREFIX}/lib") + if(WIN32) + set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_PREFIX}/bin") + else() + set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_PREFIX}/lib") + endif() set(ARROW_GTEST_IMPORT_LIB - "${ARROW_GTEST_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}arrow_gtest${CMAKE_IMPORT_LIBRARY_SUFFIX}" + "${ARROW_GTEST_IMPORT_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}arrow_gtest${CMAKE_IMPORT_LIBRARY_SUFFIX}" ) set(ARROW_GTEST_MAIN_IMPORT_LIB - "${ARROW_GTEST_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}arrow_gtest_main${CMAKE_IMPORT_LIBRARY_SUFFIX}" + "${ARROW_GTEST_IMPORT_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}arrow_gtest_main${CMAKE_IMPORT_LIBRARY_SUFFIX}" ) set(ARROW_GTEST_SHARED_LIB - "${ARROW_GTEST_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow_gtest${CMAKE_SHARED_LIBRARY_SUFFIX}" + "${ARROW_GTEST_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow_gtest${CMAKE_SHARED_LIBRARY_SUFFIX}" ) set(ARROW_GTEST_MAIN_SHARED_LIB - "${ARROW_GTEST_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow_gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" + "${ARROW_GTEST_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow_gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" ) list(APPEND ARROW_CMAKE_ARGS "-DARROW_BUILD_TESTS=ON") From 4decaf904867686fb501125812c3fd3e6d1b8171 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 31 Aug 2023 14:23:08 +0900 Subject: [PATCH 07/13] Fix path --- matlab/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index b89f265633f..36350a53ccf 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -115,8 +115,7 @@ function(build_arrow) endfunction() macro(enable_gtest) - set(ARROW_GTEST_SOURCE_PREFIX "${ARROW_BINARY_DIR}/_deps/googletest-src/googletest") - set(ARROW_GTEST_INCLUDE_DIR "${ARROW_PREFIX}/include") + set(ARROW_GTEST_INCLUDE_DIR "${ARROW_PREFIX}/include/arrow-gtest") set(ARROW_GTEST_IMPORT_LIB_DIR "${ARROW_PREFIX}/lib") if(WIN32) From 4fff568ef38cd34b7aca0aef11776997dbc165c6 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 1 Sep 2023 12:19:39 +0900 Subject: [PATCH 08/13] Enable MACOSX_RPATH by default --- cpp/CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index f8e7b1eb271..d343de836e5 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -23,6 +23,13 @@ message(STATUS "Building using CMake version: ${CMAKE_VERSION}") # Compiler id for Apple Clang is now AppleClang. cmake_policy(SET CMP0025 NEW) +# https://cmake.org/cmake/help/latest/policy/CMP0042.html +# +# Enable MACOSX_RPATH by default. @rpath in a target's install name is +# a more flexible and powerful mechanism than @executable_path or +# @loader_path for locating shared libraries. +cmake_policy(SET CMP0042 NEW) + # https://www.cmake.org/cmake/help/latest/policy/CMP0054.html # # Only interpret if() arguments as variables or keywords when unquoted. From 6be71275c69e30320fbaa12cf4c781644fff86a1 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 1 Sep 2023 12:43:58 +0900 Subject: [PATCH 09/13] Set MACOSX_RPATH explicitly --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index eb76105d5f1..c7928f6bdb7 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2112,10 +2112,12 @@ function(build_gtest) set(INSTALL_GTEST OFF) string(APPEND CMAKE_INSTALL_INCLUDEDIR "/arrow-gtest") fetchcontent_makeavailable(googletest) - set_target_properties(gmock PROPERTIES OUTPUT_NAME "arrow_gmock") - set_target_properties(gmock_main PROPERTIES OUTPUT_NAME "arrow_gmock_main") - set_target_properties(gtest PROPERTIES OUTPUT_NAME "arrow_gtest") - set_target_properties(gtest_main PROPERTIES OUTPUT_NAME "arrow_gtest_main") + set_target_properties(gmock PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME "arrow_gmock") + set_target_properties(gmock_main PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME + "arrow_gmock_main") + set_target_properties(gtest PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME "arrow_gtest") + set_target_properties(gtest_main PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME + "arrow_gtest_main") install(DIRECTORY "${googletest_SOURCE_DIR}/googlemock/include/" "${googletest_SOURCE_DIR}/googletest/include/" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") From d0aa12342f83e58c6e5269793a4da865be17c927 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 1 Sep 2023 14:47:52 +0900 Subject: [PATCH 10/13] Use CMAKE_MACOSX_RPATH --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index c7928f6bdb7..402f02dbec9 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1001,8 +1001,9 @@ include(FetchContent) macro(prepare_fetchcontent) set(BUILD_SHARED_LIBS OFF) set(BUILD_STATIC_LIBS ON) - set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY TRUE) set(CMAKE_COMPILE_WARNING_AS_ERROR FALSE) + set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY TRUE) + set(CMAKE_MACOSX_RPATH ${ARROW_INSTALL_NAME_RPATH}) if(MSVC) string(REPLACE "/WX" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") string(REPLACE "/WX" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") @@ -2112,11 +2113,11 @@ function(build_gtest) set(INSTALL_GTEST OFF) string(APPEND CMAKE_INSTALL_INCLUDEDIR "/arrow-gtest") fetchcontent_makeavailable(googletest) - set_target_properties(gmock PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME "arrow_gmock") - set_target_properties(gmock_main PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME + set_target_properties(gmock PROPERTIES OUTPUT_NAME "arrow_gmock") + set_target_properties(gmock_main PROPERTIES OUTPUT_NAME "arrow_gmock_main") - set_target_properties(gtest PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME "arrow_gtest") - set_target_properties(gtest_main PROPERTIES MACOSX_RPATH TRUE OUTPUT_NAME + set_target_properties(gtest PROPERTIES OUTPUT_NAME "arrow_gtest") + set_target_properties(gtest_main PROPERTIES OUTPUT_NAME "arrow_gtest_main") install(DIRECTORY "${googletest_SOURCE_DIR}/googlemock/include/" "${googletest_SOURCE_DIR}/googletest/include/" From 3be85b2426cafb3c3824cb852687f6164ca7b934 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 3 Sep 2023 16:09:06 +0900 Subject: [PATCH 11/13] Fix style --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 402f02dbec9..92931676162 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2114,11 +2114,9 @@ function(build_gtest) string(APPEND CMAKE_INSTALL_INCLUDEDIR "/arrow-gtest") fetchcontent_makeavailable(googletest) set_target_properties(gmock PROPERTIES OUTPUT_NAME "arrow_gmock") - set_target_properties(gmock_main PROPERTIES OUTPUT_NAME - "arrow_gmock_main") + set_target_properties(gmock_main PROPERTIES OUTPUT_NAME "arrow_gmock_main") set_target_properties(gtest PROPERTIES OUTPUT_NAME "arrow_gtest") - set_target_properties(gtest_main PROPERTIES OUTPUT_NAME - "arrow_gtest_main") + set_target_properties(gtest_main PROPERTIES OUTPUT_NAME "arrow_gtest_main") install(DIRECTORY "${googletest_SOURCE_DIR}/googlemock/include/" "${googletest_SOURCE_DIR}/googletest/include/" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") From 893da3bcbc8b6cee86468d486562094a29707724 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 3 Sep 2023 16:16:06 +0900 Subject: [PATCH 12/13] Set BUILD_RPATH --- matlab/CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 36350a53ccf..d73173b58e7 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -326,6 +326,13 @@ if(MATLAB_BUILD_TESTS) # targets. target_link_libraries(placeholder_test GTest::gtest GTest::gtest_main) + # Ensure using GTest:gtest and GTest::gtest_main on macOS without + # specifying DYLD_LIBRARY_DIR. + set_target_properties(placeholder_test + PROPERTIES BUILD_RPATH + "$;$" + ) + # Add test targets for C++ tests. add_test(PlaceholderTestTarget placeholder_test) From 52138c14c89c962d2bd32614daf56690c8265736 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Mon, 4 Sep 2023 06:28:29 +0900 Subject: [PATCH 13/13] Use cache variable --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 92931676162..a9ac0b94514 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2110,7 +2110,14 @@ function(build_gtest) endif() set(BUILD_SHARED_LIBS ON) set(BUILD_STATIC_LIBS OFF) - set(INSTALL_GTEST OFF) + # We need to use "cache" variable to override the default + # INSTALL_GTEST option by this value. See also: + # https://cmake.org/cmake/help/latest/policy/CMP0077.html + set(INSTALL_GTEST + OFF + CACHE "BOOL" + "Enable installation of googletest. (Projects embedding googletest may want to turn this OFF.)" + FORCE) string(APPEND CMAKE_INSTALL_INCLUDEDIR "/arrow-gtest") fetchcontent_makeavailable(googletest) set_target_properties(gmock PROPERTIES OUTPUT_NAME "arrow_gmock")