[GitHub] [incubator-mxnet] szha commented on a change in pull request #19057: Update symbol visibility control

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

szha commented on a change in pull request #19057:
URL: https://github.com/apache/incubator-mxnet/pull/19057#discussion_r480843407



##########
File path: CMakeLists.txt
##########
@@ -659,15 +661,12 @@ if(UNIX)
     target_link_libraries(mxnet PUBLIC gcov)
   endif()
   if(CMAKE_BUILD_TYPE STREQUAL "Distribution")
-    # TODO For handling mxnet's symbols the following can be replace by
-    # annotating symbol visibility in source code, specifying
-    # set(CMAKE_CXX_VISIBILITY_PRESET hidden) and
-    # set(CMAKE_VISIBILITY_INLINES_HIDDEN ON)
-    # Symbols from statically linked libraries can be discarded via -Wl,--exclude-libs,ALL
-    if(APPLE)
-      set_target_properties(mxnet PROPERTIES LINK_FLAGS "-Wl,-exported_symbols_list,${PROJECT_SOURCE_DIR}/cmake/libmxnet.sym")
+    if(NOT APPLE)
+      # Do not export symbols from statically linked dependencies such as zmq, opencv, etc.
+      set_target_properties(mxnet PROPERTIES LINK_FLAGS "-Wl,--exclude-libs,ALL")
     else()
-      set_target_properties(mxnet PROPERTIES LINK_FLAGS "-Wl,--version-script=${PROJECT_SOURCE_DIR}/cmake/libmxnet.ver")
+      # Apple does not support -Wl,--exclude-libs,ALL
+      set_target_properties(mxnet PROPERTIES LINK_FLAGS "-Wl,-exported_symbols_list,${PROJECT_SOURCE_DIR}/cmake/libmxnet.sym")

Review comment:
       by not using a whitelist for non-apple, I think the resulting exported symbols would be different.




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