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 <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
apache · Nov 2, 2023 · ff762a5 · ff762a5
1 parent fe90302
commit ff762a5
Show file tree

Hide file tree

Showing 13 changed files with 38 additions and 52 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)