paleolimbot commented on code in PR #677:
URL: https://github.com/apache/arrow-nanoarrow/pull/677#discussion_r1835526851
##########
CMakeLists.txt:
##########
@@ -422,26 +422,31 @@ if(NANOARROW_BUILD_TESTS)
)
include(CTest)
- find_package(Arrow REQUIRED)
- message(STATUS "Arrow version: ${ARROW_VERSION}")
- message(STATUS "Arrow SO version: ${ARROW_FULL_SO_VERSION}")
+ find_package(Arrow)
+ if(Arrow_FOUND)
+ message(STATUS "Arrow version: ${ARROW_VERSION}")
+ message(STATUS "Arrow SO version: ${ARROW_FULL_SO_VERSION}")
- # Give caller the option to link a static version of Arrow C++
- if(NANOARROW_ARROW_STATIC)
- set(NANOARROW_ARROW_TARGET arrow_static)
- else()
- set(NANOARROW_ARROW_TARGET arrow_shared)
- endif()
+ # TODO: does this propoagate to projects using as a subproject?
+ add_compile_definitions("-DNANOARROW_ARROW_FOUND")
Review Comment:
I think it does. You probably want `target_compile_definitions(xxxx_test
PRIVATE -DNANOARROW_ARROW_FOUND)` for every test because the tests are what
actually need this define set. You could do something fancy like we do with the
`nanoarrow_coverage_config` (define an interface target that the tests "link
to"), or bite the bullet and use a `foreach()` loop to link tests (sort of like
you do in the meson config), which we probably should have done a while ago
anyway.
--
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]
