This is an automated email from the ASF dual-hosted git repository.
assignuser 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 a4a5562e6b GH-43400: [C++] Ensure using bundled GoogleTest when we use
bundled GoogleTest (#43465)
a4a5562e6b is described below
commit a4a5562e6b5a82ff297faeac45eb2ce1f391bfec
Author: Sutou Kouhei <[email protected]>
AuthorDate: Tue Jul 30 11:55:21 2024 +0900
GH-43400: [C++] Ensure using bundled GoogleTest when we use bundled
GoogleTest (#43465)
### Rationale for this change
If we use bundled GoogleTest and system other dependencies such as Boost,
our include path options may be:
* `-isystem /opt/homebrew/include` (for Boost)
* `-isystem build_dir/_deps/googletest-src/googletest` (for bundled
GoogleTest)
* `-isystem build_dir/_deps/googletest-src/googlemock` (for bundled
GoogleTest)
With this order, GoogleTest headers in `/opt/homebrew/include/` are used
with bundled GoogleTest. It may cause link errors.
### What changes are included in this PR?
This change introduces a new CMake target
`arrow::GTest::gtest_headers` that has include paths for bundled
GoogleTest. And it's always used as the first link library of all test program.
With this change, our include path options are:
* `-isystem build_dir/_deps/googletest-src/googletest` (for bundled
GoogleTest)
* `-isystem build_dir/_deps/googletest-src/googlemock` (for bundled
GoogleTest)
* `-isystem /opt/homebrew/include` (for Boost)
With this order, we can always use our bundled GoogleTest.
`arrow::GTest::gtest_headers` is defined only when we use bundled
GoogleTest. So this doesn't change the system GoogleTest case.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* GitHub Issue: #43400
Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
---
cpp/cmake_modules/BuildUtils.cmake | 5 +++++
cpp/cmake_modules/ThirdpartyToolchain.cmake | 6 ++++++
dev/tasks/java-jars/github.yml | 5 -----
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/cpp/cmake_modules/BuildUtils.cmake
b/cpp/cmake_modules/BuildUtils.cmake
index e7523add27..692efa7837 100644
--- a/cpp/cmake_modules/BuildUtils.cmake
+++ b/cpp/cmake_modules/BuildUtils.cmake
@@ -721,6 +721,11 @@ function(ADD_TEST_CASE REL_TEST_NAME)
"${EXECUTABLE_OUTPUT_PATH};$ENV{CONDA_PREFIX}/lib")
endif()
+ # Ensure using bundled GoogleTest when we use bundled GoogleTest.
+ # ARROW_GTEST_GTEST_HEADERS is defined only when we use bundled
+ # GoogleTest.
+ target_link_libraries(${TEST_NAME} PRIVATE ${ARROW_GTEST_GTEST_HEADERS})
+
if(ARG_STATIC_LINK_LIBS)
# Customize link libraries
target_link_libraries(${TEST_NAME} PRIVATE ${ARG_STATIC_LINK_LIBS})
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake
b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 1c8c40d6f9..92bd80014e 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -2306,6 +2306,10 @@ function(build_gtest)
install(DIRECTORY "${googletest_SOURCE_DIR}/googlemock/include/"
"${googletest_SOURCE_DIR}/googletest/include/"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
+ add_library(arrow::GTest::gtest_headers INTERFACE IMPORTED)
+ target_include_directories(arrow::GTest::gtest_headers
+ INTERFACE
"${googletest_SOURCE_DIR}/googlemock/include/"
+
"${googletest_SOURCE_DIR}/googletest/include/")
install(TARGETS gmock gmock_main gtest gtest_main
EXPORT arrow_testing_targets
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"
@@ -2350,12 +2354,14 @@ if(ARROW_TESTING)
string(APPEND ARROW_TESTING_PC_LIBS " $<TARGET_FILE:GTest::gtest>")
endif()
+ set(ARROW_GTEST_GTEST_HEADERS)
set(ARROW_GTEST_GMOCK GTest::gmock)
set(ARROW_GTEST_GTEST GTest::gtest)
set(ARROW_GTEST_GTEST_MAIN GTest::gtest_main)
else()
string(APPEND ARROW_TESTING_PC_CFLAGS " -I\${includedir}/arrow-gtest")
string(APPEND ARROW_TESTING_PC_LIBS " -larrow_gtest")
+ set(ARROW_GTEST_GTEST_HEADERS arrow::GTest::gtest_headers)
set(ARROW_GTEST_GMOCK arrow::GTest::gmock)
set(ARROW_GTEST_GTEST arrow::GTest::gtest)
set(ARROW_GTEST_GTEST_MAIN arrow::GTest::gtest_main)
diff --git a/dev/tasks/java-jars/github.yml b/dev/tasks/java-jars/github.yml
index 58c1cedb11..77e8867652 100644
--- a/dev/tasks/java-jars/github.yml
+++ b/dev/tasks/java-jars/github.yml
@@ -138,11 +138,6 @@ jobs:
# used on test We uninstall Homebrew's Protobuf to ensure using
# bundled Protobuf.
brew uninstall protobuf
- # We want to use the bundled googletest for static linking. Since
- # both BUNDLED and brew options are enabled, it could cause a
conflict
- # when there is a version mismatch.
- # We uninstall googletest to ensure using the bundled googletest.
- brew uninstall googletest
brew bundle --file=arrow/java/Brewfile
- name: Build C++ libraries
