Re: [PR] chore(ci): Fix verification builds for Windows and centos7 [arrow-nanoarrow]

["assignUser (via GitHub)" <gi...@apache.org>]

assignUser commented on code in PR #341:
URL: https://github.com/apache/arrow-nanoarrow/pull/341#discussion_r1433356766


##########
extensions/nanoarrow_ipc/CMakeLists.txt:
##########
@@ -215,6 +215,16 @@ if(NANOARROW_IPC_BUILD_TESTS)
     fetchcontent_makeavailable(nlohmann_json)
   endif()
 
+  # zlib to decode gzipped integration testing JSON files
+  # We don't use Arrow C++ for this because building Arrow C++ with zlib
+  # is not trivial on Windows.
+  find_package(ZLIB)
+  if(NOT ZLIB_FOUND)
+    # Wrapper around FetchContent that better isolates the zlib CMakeLists.txt

Review Comment:
   Yeah, separate dir scope can really help prevent global options from being modified ^^



##########
dev/release/verify-release-candidate.sh:
##########
@@ -217,6 +217,7 @@ test_cmake_project() {
   show_info "Configure CMake Project"
   ${CMAKE_BIN} "${NANOARROW_SOURCE_DIR}/${2}" \
     "${@:3}" \
+    -DCMAKE_POSITION_INDEPENDENT_CODE=ON \

Review Comment:
   This works of course but out of interest what was the issue with the target version? It reads like you set the flags directly but you can also apply this option on a target basis and let cmake add the flags: `set_property(TARGET fpic-test PROPERTY POSITION_INDEPENDENT_CODE ON)`



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org