HaochengLIU commented on code in PR #34765:
URL: https://github.com/apache/arrow/pull/34765#discussion_r1152686155
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -115,6 +115,9 @@ elseif(ARROW_CPU_FLAG STREQUAL "aarch64")
if(ARROW_SIMD_LEVEL STREQUAL "DEFAULT")
set(ARROW_SIMD_LEVEL "NEON")
endif()
+ if(APPLE)
+ set(GTest_SOURCE "BUNDLED")
+ endif()
Review Comment:
After checking the upstream
[FindGTest.cmake](https://gitlab.kitware.com/cmake/cmake/blob/master/Modules/FindGTest.cmake)
as well as the pkgconfig from GoogleTest on my host, here is my finding:
In order to detect what C++ standard is used by GoogleTest, there is no
simple way to do so except by importing it first to my best knowledge.
Moreover, once a target is imported, you can not rename or delete it(See Ben's
- one of the main CMake developers - comment
[here](https://discourse.cmake.org/t/how-to-delete-or-rename-an-imported-target-using-cmake/2372/2?u=haocheng_liu1)).
Thus there is no way to fall back to bundled mode.
My new proposed fix is at CMake configuration time, the CMake code will
automatically detect and
compare the C++ standard used by GTest and arrow. If a mismatch is detected,
CMake will error out and provide meaningful guidance. With respect to The
principle of least astonishment, I prefer to let the users add these flags by
themselves.
```
// sample output from my host
CMake Error at cmake_modules/ThirdpartyToolchain.cmake:2238 (message):
SYSTEM GTest is built with c++14 but project is built
with a different standard(17). Use bundled GTest via passing in CMake flag
-DGTest_SOURCE="BUNDLED"
Call Stack (most recent call first):
CMakeLists.txt:505 (include)
-- Configuring incomplete, errors occurred!
```
Looking forward to your thoughts. @kou .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
