kevingurney commented on a change in pull request #10614:
URL: https://github.com/apache/arrow/pull/10614#discussion_r661920751
##########
File path: matlab/CMakeLists.txt
##########
@@ -72,3 +89,28 @@ else()
set_target_properties(featherwritemex PROPERTIES LIBRARY_OUTPUT_DIRECTORY
$<1:${CMAKE_SOURCE_DIR}/src>)
endif()
+
+# ######################
+# GoogleTest Integration
+# ######################
+# If the user has specified a GTEST_ROOT value, use their pre-built GoogleTest
binaries.
+# Otherwise, download and build GoogleTest automatically.
+if(GTEST_ROOT)
+ find_package(GTest REQUIRED)
+else()
+ download_and_build_google_test()
Review comment:
Thanks for sharing your thoughts, @kou!
-------------
> Can we clarify target people of the MATLAB library's test?
I agree with you that _Developers_ are the primary target audience of the
C++ tests for the MATLAB interface.
> For 2., custom Apache Arrow C++ build is useful. For example, developers
may want to build with -DCMAKE_BUILD_TYPE=debug for debugging and with
-DCMAKE_BUILD_TYPE=release for benchmarking
You make an excellent point about providing developers with the flexibility
to build their own custom `libarrow` for debugging and benchmarking purposes!
This is definitely something that we would like to support. If we use
`find_package`, as you previously suggested, this should support custom
`libarrow` builds via `ARROW_HOME`, as well as package managers.
> For 1., users may not want to enable tests by default. Because they just
want to use the MATLAB library.
Good point! I think it would make sense to introduce a flag like
`MATLAB_BUILD_TESTS`, with a default value of `OFF`, so that users can toggle
building of the C++ tests for the MATLAB interface.
-------------
Based on your input, we went back to the drawing board and learned more
about `ExternalProject_Add` and `IMPORTED` targets. By studying the source code
for `build_dependency` in `ThirdpartyToolchain` more closely, I now understand
how `ExternalProject_Add` can be used to trigger an automatic build of the
Apache Arrow C++ libraries (and bundled GoogleTest).
I put together a prototype which automatically builds Arrow C++ and bundled
GoogleTest on Linux (only if the user has specified `-DMATLAB_BUILD_TESTS=ON`).
**You can find the prototype here:**
https://github.com/mathworks/arrow/commit/42609d9671501c8fe1e8c613337eedb207d32e91
I tested this manually under the following conditions:
1. GoogleTest installed via the `apt` package manager to `/usr/lib/...` on
Debian 10
2. A custom `GTEST_ROOT` and a custom `ARROW_HOME` value are specified
3. `find_package` fails to find a valid Arrow or GTest installation via 1.
or 2., and an automatic C++ source build is triggered
The code could probably benefit from some additional refactoring, and I'm
not sure whether "Modern CMake Best Practices" would deem it acceptable to
depend on the bundled GoogleTest binaries in this way, but it does seem to be
working as expected.
Let me know what you think.
Thanks!
--
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]
