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 ff762a5878 GH-38339: [C++][CMake] Use transitive dependency for system
GoogleTest (#38340)
ff762a5878 is described below
commit ff762a5878603a0cca3d1498d5f8895078ffbb0a
Author: Sutou Kouhei <[email protected]>
AuthorDate: Thu Nov 2 15:34:26 2023 +0900
GH-38339: [C++][CMake] Use transitive dependency for system GoogleTest
(#38340)
### Rationale for this change
Our bundled GoogleTest has transitive dependency. So we can omit explicit
`GTest::gtest` linking because `GTest::gtest_main`/`GTest::gmock` has
transitive dependency.
But GoogleTest related targets found by CMake's `FindGTest.cmake` don't
have transitive dependency. So our code base needs to link `GTest::gtest`
explicitly.
We can remove needless links by setting transitive dependency to GoogleTest
related targets found by CMake's `FindGTest.cmake`.
### What changes are included in this PR?
* Set transitive dependencies to `GTest::gtest_main` and `GTest::gmock`
* Remove needless links
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
* Closes: #38339
Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
ci/scripts/cpp_test.sh | 2 +-
cpp/CMakeLists.txt | 6 ++--
cpp/cmake_modules/FindGTestAlt.cmake | 3 ++
cpp/src/arrow/acero/CMakeLists.txt | 3 +-
cpp/src/arrow/adapters/orc/CMakeLists.txt | 10 ++----
cpp/src/arrow/compute/CMakeLists.txt | 4 +--
cpp/src/arrow/compute/kernels/CMakeLists.txt | 3 +-
cpp/src/arrow/dataset/CMakeLists.txt | 3 +-
cpp/src/arrow/filesystem/CMakeLists.txt | 2 +-
cpp/src/arrow/flight/CMakeLists.txt | 1 -
.../arrow/flight/integration_tests/CMakeLists.txt | 7 ++--
cpp/src/arrow/gpu/CMakeLists.txt | 9 +++---
cpp/src/parquet/CMakeLists.txt | 37 ++++++++++------------
13 files changed, 38 insertions(+), 52 deletions(-)
diff --git a/ci/scripts/cpp_test.sh b/ci/scripts/cpp_test.sh
index 3acf56bae0..0c6e1c6ef7 100755
--- a/ci/scripts/cpp_test.sh
+++ b/ci/scripts/cpp_test.sh
@@ -86,7 +86,7 @@ ctest \
--parallel ${n_jobs} \
--timeout ${ARROW_CTEST_TIMEOUT:-300} \
"${ctest_options[@]}" \
- $@
+ "$@"
if [ "${ARROW_BUILD_EXAMPLES}" == "ON" ]; then
examples=$(find ${binary_output_dir} -executable -name "*example")
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index 524a9bebf7..bcb298407b 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -889,8 +889,8 @@ if(NOT MSVC_TOOLCHAIN)
list(APPEND ARROW_SHARED_LINK_LIBS ${CMAKE_DL_LIBS})
endif()
-set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers ${ARROW_GTEST_GTEST_MAIN}
- ${ARROW_GTEST_GTEST} ${ARROW_GTEST_GMOCK})
+set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers ${ARROW_GTEST_GMOCK}
+ ${ARROW_GTEST_GTEST_MAIN})
if(ARROW_BUILD_TESTS)
add_dependencies(arrow_test_dependencies ${ARROW_TEST_LINK_TOOLCHAIN})
@@ -909,7 +909,7 @@ set(ARROW_TEST_SHARED_LINK_LIBS arrow_testing_shared
arrow_shared
${ARROW_SHARED_LINK_LIBS}
${ARROW_TEST_LINK_TOOLCHAIN})
if(NOT MSVC)
- set(ARROW_TEST_SHARED_LINK_LIBS ${ARROW_TEST_SHARED_LINK_LIBS}
${CMAKE_DL_LIBS})
+ list(APPEND ARROW_TEST_SHARED_LINK_LIBS ${CMAKE_DL_LIBS})
endif()
if("${ARROW_TEST_LINKAGE}" STREQUAL "shared")
diff --git a/cpp/cmake_modules/FindGTestAlt.cmake
b/cpp/cmake_modules/FindGTestAlt.cmake
index 77d4f39d9e..d1873d138e 100644
--- a/cpp/cmake_modules/FindGTestAlt.cmake
+++ b/cpp/cmake_modules/FindGTestAlt.cmake
@@ -63,4 +63,7 @@ TEST(CXX_STANDARD, MatcherStringView) {
find_package_handle_standard_args(GTestAlt
REQUIRED_VARS
GTestAlt_CXX_STANDARD_AVAILABLE)
endif()
+
+ target_link_libraries(GTest::gmock INTERFACE GTest::gtest)
+ target_link_libraries(GTest::gtest_main INTERFACE GTest::gtest)
endif()
diff --git a/cpp/src/arrow/acero/CMakeLists.txt
b/cpp/src/arrow/acero/CMakeLists.txt
index 44fbb26f08..153413b33c 100644
--- a/cpp/src/arrow/acero/CMakeLists.txt
+++ b/cpp/src/arrow/acero/CMakeLists.txt
@@ -123,8 +123,7 @@ if(ARROW_TESTING)
add_library(arrow_acero_testing OBJECT test_util_internal.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_acero_testing ${ARROW_ACERO_TEST_LINK_LIBS})
- target_link_libraries(arrow_acero_testing ${ARROW_GTEST_GTEST})
+ target_link_libraries(arrow_acero_testing PRIVATE
${ARROW_ACERO_TEST_LINK_LIBS})
list(APPEND ARROW_ACERO_TEST_LINK_LIBS arrow_acero_testing)
endif()
diff --git a/cpp/src/arrow/adapters/orc/CMakeLists.txt
b/cpp/src/arrow/adapters/orc/CMakeLists.txt
index 4471937999..4d66151cd3 100644
--- a/cpp/src/arrow/adapters/orc/CMakeLists.txt
+++ b/cpp/src/arrow/adapters/orc/CMakeLists.txt
@@ -27,19 +27,15 @@ install(FILES adapter.h options.h
arrow_add_pkg_config("arrow-orc")
if(ARROW_BUILD_STATIC)
- set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_static arrow_static)
+ set(ARROW_ORC_STATIC_LINK_LIBS ${ARROW_TEST_STATIC_LINK_LIBS})
else()
- set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_shared arrow_shared)
+ set(ARROW_ORC_STATIC_LINK_LIBS ${ARROW_TEST_SHARED_LINK_LIBS})
endif()
-
-set(ORC_STATIC_TEST_LINK_LIBS orc::orc ${ARROW_LIBRARIES_FOR_STATIC_TESTS}
- ${ARROW_GTEST_GTEST_MAIN} ${ARROW_GTEST_GTEST})
-
add_arrow_test(adapter_test
PREFIX
"arrow-orc"
STATIC_LINK_LIBS
- ${ORC_STATIC_TEST_LINK_LIBS})
+ ${ARROW_ORC_STATIC_LINK_LIBS})
set_source_files_properties(adapter_test.cc PROPERTIES SKIP_PRECOMPILE_HEADERS
ON
SKIP_UNITY_BUILD_INCLUSION ON)
diff --git a/cpp/src/arrow/compute/CMakeLists.txt
b/cpp/src/arrow/compute/CMakeLists.txt
index 001424dd42..1134e0a98a 100644
--- a/cpp/src/arrow/compute/CMakeLists.txt
+++ b/cpp/src/arrow/compute/CMakeLists.txt
@@ -89,9 +89,7 @@ add_arrow_test(internals_test
kernel_test.cc
light_array_test.cc
registry_test.cc
- key_hash_test.cc
- EXTRA_LINK_LIBS
- ${ARROW_GTEST_GMOCK})
+ key_hash_test.cc)
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 7874305062..4350cd57ff 100644
--- a/cpp/src/arrow/compute/kernels/CMakeLists.txt
+++ b/cpp/src/arrow/compute/kernels/CMakeLists.txt
@@ -23,8 +23,7 @@ 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}
- ${ARROW_GTEST_GMOCK})
+ target_link_libraries(arrow_compute_kernels_testing PRIVATE
${ARROW_GTEST_GMOCK})
endif()
add_arrow_test(scalar_cast_test
diff --git a/cpp/src/arrow/dataset/CMakeLists.txt
b/cpp/src/arrow/dataset/CMakeLists.txt
index eb8fb54803..1afef3e3b0 100644
--- a/cpp/src/arrow/dataset/CMakeLists.txt
+++ b/cpp/src/arrow/dataset/CMakeLists.txt
@@ -113,8 +113,7 @@ if(ARROW_TESTING)
add_library(arrow_dataset_testing OBJECT test_util_internal.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_dataset_testing ${ARROW_DATASET_TEST_LINK_LIBS})
- target_link_libraries(arrow_dataset_testing ${ARROW_GTEST_GTEST})
+ target_link_libraries(arrow_dataset_testing PRIVATE
${ARROW_DATASET_TEST_LINK_LIBS})
list(APPEND ARROW_DATASET_TEST_LINK_LIBS arrow_dataset_testing)
endif()
diff --git a/cpp/src/arrow/filesystem/CMakeLists.txt
b/cpp/src/arrow/filesystem/CMakeLists.txt
index b997ca0a38..a42a8d0f8c 100644
--- a/cpp/src/arrow/filesystem/CMakeLists.txt
+++ b/cpp/src/arrow/filesystem/CMakeLists.txt
@@ -90,7 +90,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} ${ARROW_GTEST_GTEST})
+ ${GFLAGS_LIBRARIES})
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 c37d2c5670..91e0fbf913 100644
--- a/cpp/src/arrow/flight/CMakeLists.txt
+++ b/cpp/src/arrow/flight/CMakeLists.txt
@@ -67,7 +67,6 @@ list(APPEND
Boost::headers
Boost::filesystem
Boost::system
- ${ARROW_GTEST_GTEST}
${ARROW_GTEST_GMOCK})
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS gRPC::grpc++)
diff --git a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt
b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt
index 98a7a2a7af..7ac314531f 100644
--- a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt
+++ b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt
@@ -22,11 +22,8 @@ if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND
ARROW_BUILD_STATIC)
else()
set(ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_shared)
endif()
-list(APPEND
- ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS
- ${ARROW_FLIGHT_TEST_LINK_LIBS}
- ${GFLAGS_LIBRARIES}
- ${ARROW_GTEST_GTEST})
+list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS
${ARROW_FLIGHT_TEST_LINK_LIBS}
+ ${GFLAGS_LIBRARIES})
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 a5b176793e..7238a0e0b7 100644
--- a/cpp/src/arrow/gpu/CMakeLists.txt
+++ b/cpp/src/arrow/gpu/CMakeLists.txt
@@ -100,9 +100,10 @@ if(ARROW_BUILD_TESTS)
endif()
if(ARROW_BUILD_BENCHMARKS)
- add_arrow_benchmark(cuda_benchmark PREFIX "arrow-gpu")
- target_link_libraries(arrow-gpu-cuda-benchmark
- PUBLIC ${ARROW_CUDA_LIBRARY} ${ARROW_GTEST_GTEST}
- ${ARROW_BENCHMARK_LINK_LIBS})
+ add_arrow_benchmark(cuda_benchmark
+ PREFIX
+ "arrow-gpu"
+ EXTRA_LINK_LIBS
+ ${ARROW_CUDA_LIBRARY})
add_dependencies(arrow_cuda-benchmarks arrow-gpu-cuda-benchmark)
endif()
diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt
index 0d04ec3e30..04028431ba 100644
--- a/cpp/src/parquet/CMakeLists.txt
+++ b/cpp/src/parquet/CMakeLists.txt
@@ -46,13 +46,15 @@ function(ADD_PARQUET_TEST REL_TEST_NAME)
if(ARROW_TEST_LINKAGE STREQUAL "static")
add_test_case(${REL_TEST_NAME}
STATIC_LINK_LIBS
- ${PARQUET_STATIC_TEST_LINK_LIBS}
+ parquet_static
+ ${PARQUET_TEST_LINK_LIBS}
${TEST_ARGUMENTS}
${ARG_UNPARSED_ARGUMENTS})
else()
add_test_case(${REL_TEST_NAME}
STATIC_LINK_LIBS
- ${PARQUET_SHARED_TEST_LINK_LIBS}
+ parquet_shared
+ ${PARQUET_TEST_LINK_LIBS}
${TEST_ARGUMENTS}
${ARG_UNPARSED_ARGUMENTS})
endif()
@@ -118,28 +120,20 @@ endfunction()
if(ARROW_BUILD_STATIC)
set(PARQUET_STATIC_LINK_LIBS arrow_static ${ARROW_STATIC_LINK_LIBS})
set(PARQUET_STATIC_INSTALL_INTERFACE_LIBS Arrow::arrow_static)
- set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_static arrow_static
- ${ARROW_STATIC_LINK_LIBS})
else()
set(PARQUET_STATIC_INSTALL_INTERFACE_LIBS)
- set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_shared arrow_shared)
endif()
-set(PARQUET_MIN_TEST_LIBS ${ARROW_GTEST_GMOCK} ${ARROW_GTEST_GTEST}
- ${ARROW_GTEST_GTEST_MAIN} Boost::headers)
-
+set(PARQUET_TEST_LINK_LIBS ${ARROW_TEST_LINK_LIBS} thrift::thrift
Boost::headers)
if(APPLE)
- list(APPEND PARQUET_MIN_TEST_LIBS ${CMAKE_DL_LIBS})
+ list(APPEND PARQUET_TEST_LINK_LIBS ${CMAKE_DL_LIBS})
elseif(NOT MSVC)
- list(APPEND PARQUET_MIN_TEST_LIBS pthread ${CMAKE_DL_LIBS})
+ if(ARROW_ENABLE_THREADING)
+ list(APPEND PARQUET_TEST_LINK_LIBS Threads::Threads)
+ endif()
+ list(APPEND PARQUET_TEST_LINK_LIBS ${CMAKE_DL_LIBS})
endif()
-set(PARQUET_SHARED_TEST_LINK_LIBS arrow_testing_shared ${PARQUET_MIN_TEST_LIBS}
- parquet_shared thrift::thrift)
-
-set(PARQUET_STATIC_TEST_LINK_LIBS ${PARQUET_MIN_TEST_LIBS} parquet_static
thrift::thrift
- ${ARROW_LIBRARIES_FOR_STATIC_TESTS})
-
#
# Generated Thrift sources
set(PARQUET_THRIFT_SOURCE_DIR "${ARROW_SOURCE_DIR}/src/generated/")
@@ -302,15 +296,16 @@ if(WIN32 AND NOT (ARROW_TEST_LINKAGE STREQUAL "static"))
"${PARQUET_THRIFT_SOURCE_DIR}/parquet_constants.cpp"
"${PARQUET_THRIFT_SOURCE_DIR}/parquet_types.cpp")
target_link_libraries(parquet_test_support thrift::thrift)
- set(PARQUET_SHARED_TEST_LINK_LIBS ${PARQUET_SHARED_TEST_LINK_LIBS}
parquet_test_support)
- set(PARQUET_LIBRARIES ${PARQUET_LIBRARIES} parquet_test_support)
+ list(PREPEND PARQUET_TEST_LINK_LIBS parquet_test_support)
+ list(APPEND PARQUET_LIBRARIES parquet_test_support)
endif()
if(NOT ARROW_BUILD_SHARED)
- set(PARQUET_BENCHMARK_LINK_OPTION STATIC_LINK_LIBS benchmark::benchmark_main
- ${PARQUET_STATIC_TEST_LINK_LIBS})
+ set(PARQUET_BENCHMARK_LINK_OPTION STATIC_LINK_LIBS parquet_static
+ ${PARQUET_TEST_LINK_LIBS}
benchmark::benchmark_main)
else()
- set(PARQUET_BENCHMARK_LINK_OPTION EXTRA_LINK_LIBS
${PARQUET_SHARED_TEST_LINK_LIBS})
+ set(PARQUET_BENCHMARK_LINK_OPTION EXTRA_LINK_LIBS parquet_shared
+ ${PARQUET_TEST_LINK_LIBS})
endif()
if(ARROW_BUILD_STATIC AND WIN32)
