[GitHub] [arrow] kou commented on a change in pull request #7388: ARROW-9084: [C++] CMake is unable to find zstd target when ZSTD_SOURCE=SYSTEM

[GitBox <gi...@apache.org>]

kou commented on a change in pull request #7388:
URL: https://github.com/apache/arrow/pull/7388#discussion_r437780818



##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -1879,13 +1879,19 @@ macro(build_zstd)
 
   add_dependencies(toolchain zstd_ep)
   add_dependencies(ZSTD::zstd zstd_ep)
+  set(ZSTD_LIBRARIES ZSTD::zstd)
 endmacro()
 
 if(ARROW_WITH_ZSTD)
   resolve_dependency(ZSTD)
 
+  # "SYSTEM" source will prioritize cmake config, which exports
+  # zstd::libzstd_{static,shared}
+  if(TARGET zstd::libzstd_static)
+    set(ZSTD_LIBRARIES zstd::libzstd_static)

Review comment:
       Why do you choose `zstd_static` not `zstd_shared`.
   I think that we should `zstd_shared` for security reason.
   If zstd has a security problem, we just need to update zstd with `zstd_shared`. But we need to update zstd and rebuild Apache Arrow with `zstd_static`.




----------------------------------------------------------------
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.

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