[GitHub] de-vri-es commented on a change in pull request #6482: Use GNUInstallDirs and improve CMake handling of shared/static mxnet library.

[gi...@git.apache.org]

de-vri-es commented on a change in pull request #6482: Use GNUInstallDirs and improve CMake handling of shared/static mxnet library.
URL: https://github.com/apache/incubator-mxnet/pull/6482#discussion_r135191591
 
 

 ##########
 File path: CMakeLists.txt
 ##########
 @@ -372,35 +373,30 @@ else()
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
 endif()
 
+set(MXNET_INSTALL_TARGETS mxnet)
 if(${CMAKE_SYSTEM_NAME} STREQUAL "Darwin" AND USE_MXNET_LIB_NAMING)
   add_library(mxnet MODULE ${SOURCE})
 else()
   if(UNIX)
+    list(APPEND MXNET_INSTALL_TARGETS mxnet_static)
     add_library(mxnet_static STATIC ${SOURCE})
     # Need an arbitrary source file to trigger CMake to build the library
     add_library(mxnet SHARED)
-    # This has prolems, as it adds libmxnet_static to INTERFACE_LINK_LIBRARIES
-    target_link_libraries(mxnet "-Wl,--whole-archive $<TARGET_FILE:mxnet_static> -Wl,--no-whole-archive")
-    target_link_libraries(mxnet mxnet_static) # Let cmake understand the dependency
-    add_custom_target(
-        StaticallyLinkStaticMXNetLibrary ALL
-        BYPRODUCTS ${CMAKE_ARCHIVE_OUTPUT_DIRECTORY}/libmxnet.a
-        WORKING_DIRECTORY ${CMAKE_ARCHIVE_OUTPUT_DIRECTORY}
-        COMMAND ln -sf libmxnet_static.a libmxnet.a
-        DEPENDS mxnet_static
-    )
+    set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+    target_link_libraries(mxnet PRIVATE "-Wl,--whole-archive $<TARGET_FILE:mxnet_static> -Wl,--no-whole-archive")
+    target_link_libraries(mxnet PRIVATE mxnet_static) # Let cmake understand the dependency
 
 Review comment:
   With the old `target_link_libraries(mxnet mxnet_static)`, doing `target_link_libraries(my_target mxnet)` will link both `mxnet` and `mxnet_static` into `my_target`. That is how cmake treats PUBLIC and INTERFACE link libraries of a target. By marking it PRIVATE this is no longer the case. You can still link `mxnet_static` explicitly, but it will not be added simply by linking `mxnet`.
   
   For more information see also:
   https://cmake.org/cmake/help/v3.0/command/target_link_libraries.html
   https://cmake.org/cmake/help/v3.0/prop_tgt/INTERFACE_LINK_LIBRARIES.html#prop_tgt:INTERFACE_LINK_LIBRARIES
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services