shangxinli commented on code in PR #989:
URL: https://github.com/apache/iceberg-cpp/pull/989#discussion_r4274099437


##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -258,6 +299,9 @@ function(resolve_arrow_dependency)
     # Arrow's exported static target interface may reference system libraries
     # (e.g. Threads, OpenSSL, CURL, ZLIB) that consumers need to find.
     list(APPEND ICEBERG_SYSTEM_DEPENDENCIES Threads ZLIB)
+    if(ICEBERG_ENCRYPTION)

Review Comment:
   Moved the append below the if/else so OpenSSL lands in 
ICEBERG_SYSTEM_DEPENDENCIES for both vendored and system Arrow, matching the 
bundle's exported OpenSSL::Crypto interface.



##########
src/iceberg/iceberg-config.cmake.in:
##########
@@ -78,6 +78,11 @@ macro(iceberg_find_components components)
   endforeach()
 endmacro()
 
+# OpenSSL used at build time (table encryption), e.g. Homebrew's keg-only 
OpenSSL.
+if(NOT DEFINED OPENSSL_ROOT_DIR AND NOT "@ICEBERG_OPENSSL_ROOT_DIR@" STREQUAL 
"")

Review Comment:
   Removed the export and the whole ICEBERG_OPENSSL_ROOT_DIR plumbing; 
consumers now resolve OpenSSL from their own toolchain. One consequence worth 
confirming: on macOS a consumer of the installed package will need 
OPENSSL_ROOT_DIR or CMAKE_PREFIX_PATH for keg-only openssl@3. That seems right 
to me versus baking in the producer's path, but say the word if you want 
discovery on the consumer side.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to