From 381532803c29fcdd003d10f0821289c21bea7d72 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 6 Apr 2023 10:12:54 +0900 Subject: [PATCH] GH-34813: [C++] Improve GoogleTest detection If incompatible GoogleTest is detected, we can fallback to bundled GoogleTest automatically. --- ci/docker/alpine-linux-3.16-cpp.dockerfile | 2 +- cpp/CMakeLists.txt | 4 +- cpp/cmake_modules/FindGTestAlt.cmake | 61 +++++++++++++++++++ cpp/cmake_modules/ThirdpartyToolchain.cmake | 46 ++++++-------- cpp/src/arrow/CMakeLists.txt | 8 +-- cpp/src/arrow/adapters/orc/CMakeLists.txt | 2 +- cpp/src/arrow/filesystem/CMakeLists.txt | 2 +- cpp/src/arrow/flight/CMakeLists.txt | 6 +- .../flight/integration_tests/CMakeLists.txt | 2 +- cpp/src/arrow/gpu/CMakeLists.txt | 2 +- cpp/src/arrow/testing/CMakeLists.txt | 2 +- cpp/src/parquet/CMakeLists.txt | 2 +- 12 files changed, 94 insertions(+), 45 deletions(-) create mode 100644 cpp/cmake_modules/FindGTestAlt.cmake diff --git a/ci/docker/alpine-linux-3.16-cpp.dockerfile b/ci/docker/alpine-linux-3.16-cpp.dockerfile index 6edc405dc61..f269fa548c1 100644 --- a/ci/docker/alpine-linux-3.16-cpp.dockerfile +++ b/ci/docker/alpine-linux-3.16-cpp.dockerfile @@ -37,6 +37,7 @@ RUN apk add \ glog-dev \ gmock \ grpc-dev \ + gtest-dev \ libxml2-dev \ llvm13-dev \ llvm13-static \ @@ -95,7 +96,6 @@ ENV ARROW_ACERO=ON \ ARROW_WITH_ZSTD=ON \ AWSSDK_SOURCE=BUNDLED \ google_cloud_cpp_storage_SOURCE=BUNDLED \ - GTest_SOURCE=BUNDLED \ ORC_SOURCE=BUNDLED \ PATH=/usr/lib/ccache/:$PATH \ xsimd_SOURCE=BUNDLED diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 9a9f54478bb..ac7067c0d6c 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -839,8 +839,8 @@ if(NOT MSVC_TOOLCHAIN) list(APPEND ARROW_SHARED_LINK_LIBS ${CMAKE_DL_LIBS}) endif() -set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers GTest::gtest_main GTest::gtest - GTest::gmock) +set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers ${ARROW_GTEST_GTEST_MAIN} + ${ARROW_GTEST_GTEST} ${ARROW_GTEST_GMOCK}) if(ARROW_BUILD_TESTS) add_dependencies(arrow_test_dependencies ${ARROW_TEST_LINK_TOOLCHAIN}) diff --git a/cpp/cmake_modules/FindGTestAlt.cmake b/cpp/cmake_modules/FindGTestAlt.cmake new file mode 100644 index 00000000000..8c8bd14af2d --- /dev/null +++ b/cpp/cmake_modules/FindGTestAlt.cmake @@ -0,0 +1,61 @@ +# 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. + +if(GTestAlt_FOUND) + return() +endif() + +set(find_package_args) +if(GTestAlt_FIND_VERSION) + list(APPEND find_package_args ${GTestAlt_FIND_VERSION}) +endif() +if(GTestAlt_FIND_QUIETLY) + list(APPEND find_package_args QUIET) +endif() +if(CMAKE_VERSION VERSION_LESS 3.23) + list(APPEND find_package_args CONFIG) +endif() +# We can't find shred library version of GoogleTest on Windows with +# Conda's gtest package because it doesn't provide GTestConfig.cmake +# provided by GoogleTest and CMake's built-in FindGTtest.cmake +# doesn't support gtest_dll.dll. +find_package(GTest ${find_package_args}) + +set(GTestAlt_FOUND ${GTest_FOUND}) +if(GTestAlt_FOUND) + set(KEEP_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) + set(CMAKE_TRY_COMPILE_TARGET_TYPE EXECUTABLE) + set(GTestAlt_CXX_STANDARD_TEST_SOURCE + "${CMAKE_CURRENT_BINARY_DIR}/gtest_cxx_standard_test.cc") + file(WRITE ${GTestAlt_CXX_STANDARD_TEST_SOURCE} + " +#include +#include + +TEST(CXX_STANDARD, MatcherStringView) { + testing::Matcher matcher(std::string_view(\"hello\")); +} + ") + try_compile(GTestAlt_CXX_STANDARD_AVAILABLE ${CMAKE_CURRENT_BINARY_DIR} + SOURCES ${GTestAlt_CXX_STANDARD_TEST_SOURCE} + LINK_LIBRARIES GTest::gtest_main) + set(CMAKE_TRY_COMPILE_TARGET_TYPE ${KEEP_CMAKE_TRY_COMPILE_TARGET_TYPE}) + if(NOT GTestAlt_CXX_STANDARD_AVAILABLE) + message(STATUS "GTest can't be used with ${CMAKE_CXX_STANDARD}") + set(GTestAlt_FOUND FALSE) + endif() +endif() diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index f05e1ddb5f0..14939b52688 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2190,54 +2190,38 @@ macro(build_gtest) # The include directory must exist before it is referenced by a target. file(MAKE_DIRECTORY "${GTEST_INCLUDE_DIR}") - add_library(GTest::gtest SHARED IMPORTED) - set_target_properties(GTest::gtest + 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(GTest::gtest_main SHARED IMPORTED) - set_target_properties(GTest::gtest_main + 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(GTest::gmock SHARED IMPORTED) - set_target_properties(GTest::gmock + 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(GTest::gtest googletest_ep) - add_dependencies(GTest::gtest_main googletest_ep) - add_dependencies(GTest::gmock googletest_ep) + add_dependencies(arrow::GTest::gtest googletest_ep) + add_dependencies(arrow::GTest::gtest_main googletest_ep) + add_dependencies(arrow::GTest::gmock googletest_ep) endmacro() if(ARROW_TESTING) - if(CMAKE_VERSION VERSION_LESS 3.23) - set(GTEST_USE_CONFIG TRUE) - else() - set(GTEST_USE_CONFIG FALSE) - endif() - # We can't find shred library version of GoogleTest on Windows with - # Conda's gtest package because it doesn't provide GTestConfig.cmake - # provided by GoogleTest and CMake's built-in FindGTtest.cmake - # doesn't support gtest_dll.dll. resolve_dependency(GTest + HAVE_ALT + TRUE REQUIRED_VERSION - 1.10.0 - USE_CONFIG - ${GTEST_USE_CONFIG}) + 1.10.0) if(GTest_SOURCE STREQUAL "SYSTEM") - get_target_property(gtest_cxx_standard GTest::gtest INTERFACE_COMPILE_FEATURES) - if((${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL - "cxx_std_14")) - message(FATAL_ERROR "System GTest is built with a C++ standard lower than 17. Use bundled GTest via passing in CMake flag --DGTest_SOURCE=\"BUNDLED\"") - endif() - find_package(PkgConfig QUIET) pkg_check_modules(gtest_PC gtest @@ -2254,10 +2238,16 @@ if(ARROW_TESTING) string(APPEND ARROW_TESTING_PC_LIBS " $") endif() + set(ARROW_GTEST_GMOCK GTest::gmock) + 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") + set(ARROW_GTEST_GMOCK arrow::GTest::gmock) + set(ARROW_GTEST_GTEST arrow::GTest::gtest) + set(ARROW_GTEST_GTEST_MAIN arrow::GTest::gtest_main) endif() endif() diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 143fb13ddcc..a0c1e477b60 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -717,18 +717,16 @@ if(ARROW_TESTING) arrow::flatbuffers rapidjson::rapidjson arrow_shared - GTest::gtest + ${ARROW_GTEST_GTEST} SHARED_INSTALL_INTERFACE_LIBS Arrow::arrow_shared - GTest::gtest STATIC_LINK_LIBS arrow::flatbuffers rapidjson::rapidjson arrow_static - GTest::gtest + ${ARROW_GTEST_GTEST} STATIC_INSTALL_INTERFACE_LIBS - Arrow::arrow_static - GTest::gtest) + Arrow::arrow_static) add_custom_target(arrow_testing) add_dependencies(arrow_testing ${ARROW_TESTING_LIBRARIES}) diff --git a/cpp/src/arrow/adapters/orc/CMakeLists.txt b/cpp/src/arrow/adapters/orc/CMakeLists.txt index 3c695abb5a0..e8ff69c191f 100644 --- a/cpp/src/arrow/adapters/orc/CMakeLists.txt +++ b/cpp/src/arrow/adapters/orc/CMakeLists.txt @@ -33,7 +33,7 @@ else() endif() set(ORC_STATIC_TEST_LINK_LIBS orc::liborc ${ARROW_LIBRARIES_FOR_STATIC_TESTS} - GTest::gtest_main GTest::gtest) + ${ARROW_GTEST_GTEST_MAIN} ${ARROW_GTEST_GTEST}) add_arrow_test(adapter_test PREFIX diff --git a/cpp/src/arrow/filesystem/CMakeLists.txt b/cpp/src/arrow/filesystem/CMakeLists.txt index 6888231a35a..97aa01ea9f9 100644 --- a/cpp/src/arrow/filesystem/CMakeLists.txt +++ b/cpp/src/arrow/filesystem/CMakeLists.txt @@ -81,7 +81,7 @@ if(ARROW_S3) if(ARROW_BUILD_TESTS) add_executable(arrow-s3fs-narrative-test s3fs_narrative_test.cc) target_link_libraries(arrow-s3fs-narrative-test ${ARROW_TEST_LINK_LIBS} - ${GFLAGS_LIBRARIES} GTest::gtest) + ${GFLAGS_LIBRARIES} ${ARROW_GTEST_GTEST}) add_dependencies(arrow-tests arrow-s3fs-narrative-test) endif() diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index a97047e7d4e..42dafdf4969 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -67,8 +67,8 @@ list(APPEND Boost::headers Boost::filesystem Boost::system - GTest::gtest - GTest::gmock) + ${ARROW_GTEST_GTEST} + ${ARROW_GTEST_GMOCK}) list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS gRPC::grpc++) # TODO(wesm): Protobuf shared vs static linking @@ -308,7 +308,7 @@ if(ARROW_TESTING) test_definitions.cc test_util.cc DEPENDENCIES - GTest::gtest + ${ARROW_GTEST_GTEST} flight_grpc_gen arrow_dependencies SHARED_LINK_LIBS diff --git a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt index 1bbd9231606..98a7a2a7af3 100644 --- a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt @@ -26,7 +26,7 @@ list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS ${ARROW_FLIGHT_TEST_LINK_LIBS} ${GFLAGS_LIBRARIES} - GTest::gtest) + ${ARROW_GTEST_GTEST}) add_executable(flight-test-integration-server test_integration_server.cc test_integration.cc) diff --git a/cpp/src/arrow/gpu/CMakeLists.txt b/cpp/src/arrow/gpu/CMakeLists.txt index 00265a8fc1f..2556f30f0d5 100644 --- a/cpp/src/arrow/gpu/CMakeLists.txt +++ b/cpp/src/arrow/gpu/CMakeLists.txt @@ -98,7 +98,7 @@ endif() if(ARROW_BUILD_BENCHMARKS) add_arrow_benchmark(cuda_benchmark PREFIX "arrow-gpu") target_link_libraries(arrow-gpu-cuda-benchmark - PUBLIC ${ARROW_CUDA_LIBRARY} GTest::gtest + PUBLIC ${ARROW_CUDA_LIBRARY} ${ARROW_GTEST_GTEST} ${ARROW_BENCHMARK_LINK_LIBS}) add_dependencies(arrow_cuda-benchmarks arrow-gpu-cuda-benchmark) endif() diff --git a/cpp/src/arrow/testing/CMakeLists.txt b/cpp/src/arrow/testing/CMakeLists.txt index 073224d519b..19c2a83efe2 100644 --- a/cpp/src/arrow/testing/CMakeLists.txt +++ b/cpp/src/arrow/testing/CMakeLists.txt @@ -30,7 +30,7 @@ if(ARROW_BUILD_TESTS) elseif(ARROW_BUILD_INTEGRATION) add_executable(arrow-json-integration-test json_integration_test.cc) target_link_libraries(arrow-json-integration-test ${ARROW_TEST_LINK_LIBS} - ${GFLAGS_LIBRARIES} GTest::gtest) + ${GFLAGS_LIBRARIES} ${ARROW_GTEST_GTEST}) add_dependencies(arrow-json-integration-test arrow arrow_testing) add_dependencies(arrow-integration arrow-json-integration-test) diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt index 07336412a0a..b655d788e14 100644 --- a/cpp/src/parquet/CMakeLists.txt +++ b/cpp/src/parquet/CMakeLists.txt @@ -114,7 +114,7 @@ else() set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_shared arrow_shared) endif() -set(PARQUET_MIN_TEST_LIBS GTest::gtest_main GTest::gtest Boost::headers) +set(PARQUET_MIN_TEST_LIBS ${ARROW_GTEST_GTEST_MAIN} ${ARROW_GTEST_GTEST} Boost::headers) if(APPLE) set(PARQUET_MIN_TEST_LIBS ${PARQUET_MIN_TEST_LIBS} ${CMAKE_DL_LIBS})