You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2023/04/06 23:52:22 UTC
[arrow] branch main updated: GH-34813: [C++] Improve GoogleTest detection (#34920)
This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new f7644ae88c GH-34813: [C++] Improve GoogleTest detection (#34920)
f7644ae88c is described below
commit f7644ae88cafc4b86d7eb294c238f34b35791388
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Fri Apr 7 08:52:14 2023 +0900
GH-34813: [C++] Improve GoogleTest detection (#34920)
### Rationale for this change
#34765 approach has some corner cases. They causes CI failures.
### What changes are included in this PR?
If incompatible GoogleTest is detected, we can fallback to bundled GoogleTest automatically.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #34813
Authored-by: Sutou Kouhei <ko...@clear-code.com>
Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
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 +--
.../arrow/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(-)
diff --git a/ci/docker/alpine-linux-3.16-cpp.dockerfile b/ci/docker/alpine-linux-3.16-cpp.dockerfile
index 6edc405dc6..f269fa548c 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 9a9f54478b..ac7067c0d6 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 0000000000..8c8bd14af2
--- /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 <string_view>
+#include <gtest/gtest.h>
+
+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 f05e1ddb5f..14939b5268 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 " $<TARGET_FILE:GTest::gtest>")
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 22b2bf6e44..4c91dc57b4 100644
--- a/cpp/src/arrow/CMakeLists.txt
+++ b/cpp/src/arrow/CMakeLists.txt
@@ -718,18 +718,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 3c695abb5a..e8ff69c191 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 6888231a35..97aa01ea9f 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 a97047e7d4..42dafdf496 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 1bbd923160..98a7a2a7af 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 00265a8fc1..2556f30f0d 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 073224d519..19c2a83efe 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 07336412a0..b655d788e1 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})