assignUser commented on code in PR #614:
URL: https://github.com/apache/arrow-nanoarrow/pull/614#discussion_r1764027237
##########
CMakeLists.txt:
##########
@@ -337,9 +338,10 @@ if(NANOARROW_TESTING
PUBLIC
$<BUILD_INTERFACE:${NANOARROW_BUILD_INCLUDE_DIR}>
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/src>
$<INSTALL_INTERFACE:include>)
- target_link_libraries(nanoarrow_testing PRIVATE nlohmann_json::nlohmann_json
nanoarrow)
- target_link_libraries(nanoarrow_testing PUBLIC nanoarrow)
+ target_link_libraries(nanoarrow_testing PRIVATE nlohmann_json::nlohmann_json)
+ target_link_libraries(nanoarrow_testing PUBLIC nanoarrow
nanoarrow_coverage_config)
Review Comment:
nit: you can combine these into one invocation but if you prefer it this way
that's also fine.
##########
examples/cmake-scenarios/CMakeLists.txt:
##########
@@ -29,21 +29,31 @@ option(FIND_NANOARROW "Find an existing nanoarrow" ON)
# link to their own copy of nanoarrow. You may wish to include the namespace
only
# on release builds, since the namespace implementation obscures inline help
# available in many text editors.
-set(NANOARROW_NAMESPACE "ExampleCmakeMinimal")
+set(NANOARROW_NAMESPACE "ExampleCmakeScenarios")
+
+include(FetchContent)
if(FIND_NANOARROW)
# Users should pin to a specific version of nanoarrow if they choose
# to find nanoarrow rather than pinning to a specific version via
# FetchContent.
find_package(nanoarrow REQUIRED)
else()
- include(FetchContent)
- fetchcontent_declare(nanoarrow_example_cmake_minimal
- GIT_REPOSITORY
https://github.com/apache/arrow-nanoarrow.git
- GIT_TAG main
- GIT_SHALLOW TRUE)
- fetchcontent_makeavailable(nanoarrow_example_cmake_minimal)
+ # We need all these components for our test
+ set(NANOARROW_DEVICE ON)
+ set(NANOARROW_IPC ON)
+ set(NANOARROW_TESTING ON)
+
+ # Using SOURCE_DIR here such that CI accurately reflects the state of the
+ # source; however, using GIT_TAG or URL of a pinned version is a more
realistic
+ # user pattern.
Review Comment:
:+1:
##########
examples/cmake-scenarios/build.sh:
##########
@@ -20,12 +20,18 @@
set -exuo pipefail
# Build nanoarrow statically.
-cmake -S ../.. -B scratch/nanoarrow_build_static/
-DCMAKE_INSTALL_PREFIX=scratch/nanoarrow_install_static/
+cmake -S ../.. -B scratch/nanoarrow_build_static/ \
+ -DCMAKE_INSTALL_PREFIX=scratch/nanoarrow_install_static/ \
+ -DNANOARROW_IPC=ON -DNANOARROW_DEVICE=ON -DNANOARROW_TESTING=ON
Review Comment:
Do the cmake presets need updating as well?
--
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]
