szaszm commented on code in PR #2293:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2293#discussion_r4436791638


##########
cmake/BundledOpenSSL.cmake:
##########


Review Comment:
   can we keep the default symbol visibility hidden? even if due to the symbol 
tags, we work around the issue for exported symbols, there is no need to export 
more symbols than before 



##########
libminifi/CMakeLists.txt:
##########
@@ -92,6 +92,13 @@ endif()
 if (UNIX AND (CMAKE_SYSTEM_PROCESSOR MATCHES "armv7"))
     target_link_libraries(core-minifi "-latomic")
 endif()
+if (CMAKE_SYSTEM_NAME STREQUAL "Linux")
+    # core-minifi exports the symbols of its statically linked dependencies 
(e.g. OpenSSL), which extensions rely on.
+    # Tagging them with a minifi specific version prevents libraries loaded 
later (e.g. python's _hashlib referencing
+    # EVP_*@OPENSSL_3.0.0) from binding to these copies instead of their own 
dependencies (e.g. the system libcrypto).

Review Comment:
   I'd move this comment to core-minifi.map



##########
libminifi/CMakeLists.txt:
##########
@@ -92,6 +92,13 @@ endif()
 if (UNIX AND (CMAKE_SYSTEM_PROCESSOR MATCHES "armv7"))
     target_link_libraries(core-minifi "-latomic")
 endif()
+if (CMAKE_SYSTEM_NAME STREQUAL "Linux")

Review Comment:
   Is this still an issue on windows and macos?



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

Reply via email to