[llvm] r260641 - Avoid linking LLVM component libraries with libLLVM

Fri Feb 12 09:52:17 PST 2016

Hans,

I think this patch came out really great. It is minimally invasive and fixes a really important issue. I think it is a perfect candidate for 3.8.

-Chris

> On Feb 12, 2016, at 8:27 AM, Hans Wennborg <hans at chromium.org> wrote:
> 
> Chris, do you think this can be merged to 3.8?
> 
> Thanks,
> Hans
> 
> On Thu, Feb 11, 2016 at 5:42 PM, Andrew Wilkins via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: axw
>> Date: Thu Feb 11 19:42:43 2016
>> New Revision: 260641
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=260641&view=rev
>> Log:
>> Avoid linking LLVM component libraries with libLLVM
>> 
>> Patch by Jack Howarth.
>> 
>> When linking to libLLVM, don't also link to the component
>> libraries that constitute libLLVM.
>> 
>> Differential Revision: http://reviews.llvm.org/D16945
>> 
>> 
>> Modified:
>>    llvm/trunk/cmake/modules/AddLLVM.cmake
>>    llvm/trunk/cmake/modules/LLVM-Config.cmake
>>    llvm/trunk/utils/unittest/CMakeLists.txt
>>    llvm/trunk/utils/unittest/UnitTestMain/CMakeLists.txt
>> 
>> Modified: llvm/trunk/cmake/modules/AddLLVM.cmake
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/AddLLVM.cmake?rev=260641&r1=260640&r2=260641&view=diff
>> ==============================================================================
>> --- llvm/trunk/cmake/modules/AddLLVM.cmake (original)
>> +++ llvm/trunk/cmake/modules/AddLLVM.cmake Thu Feb 11 19:42:43 2016
>> @@ -468,20 +468,23 @@ function(llvm_add_library name)
>>     endif()
>>   endif()
>> 
>> -  # Add the explicit dependency information for this library.
>> -  #
>> -  # It would be nice to verify that we have the dependencies for this library
>> -  # name, but using get_property(... SET) doesn't suffice to determine if a
>> -  # property has been set to an empty value.
>> -  get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name})
>> -
>> -  if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB)
>> -    set(llvm_libs LLVM)
>> +  if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS)
>> +    if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB)
>> +      set(llvm_libs LLVM)
>> +    else()
>> +      llvm_map_components_to_libnames(llvm_libs
>> +       ${ARG_LINK_COMPONENTS}
>> +       ${LLVM_LINK_COMPONENTS}
>> +       )
>> +    endif()
>>   else()
>> -    llvm_map_components_to_libnames(llvm_libs
>> -      ${ARG_LINK_COMPONENTS}
>> -      ${LLVM_LINK_COMPONENTS}
>> -      )
>> +    # Components have not been defined explicitly in CMake, so add the
>> +    # dependency information for this library as defined by LLVMBuild.
>> +    #
>> +    # It would be nice to verify that we have the dependencies for this library
>> +    # name, but using get_property(... SET) doesn't suffice to determine if a
>> +    # property has been set to an empty value.
>> +    get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name})
>>   endif()
>> 
>>   if(CMAKE_VERSION VERSION_LESS 2.8.12)
>> @@ -882,14 +885,11 @@ function(add_unittest test_suite test_na
>> 
>>   set(LLVM_REQUIRES_RTTI OFF)
>> 
>> +  list(APPEND LLVM_LINK_COMPONENTS Support) # gtest needs it for raw_ostream
>>   add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO ${ARGN})
>>   set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
>>   set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir})
>> -  target_link_libraries(${test_name}
>> -    gtest
>> -    gtest_main
>> -    LLVMSupport # gtest needs it for raw_ostream.
>> -    )
>> +  target_link_libraries(${test_name} gtest_main gtest)
>> 
>>   add_dependencies(${test_suite} ${test_name})
>>   get_target_property(test_suite_folder ${test_suite} FOLDER)
>> 
>> Modified: llvm/trunk/cmake/modules/LLVM-Config.cmake
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/LLVM-Config.cmake?rev=260641&r1=260640&r2=260641&view=diff
>> ==============================================================================
>> --- llvm/trunk/cmake/modules/LLVM-Config.cmake (original)
>> +++ llvm/trunk/cmake/modules/LLVM-Config.cmake Thu Feb 11 19:42:43 2016
>> @@ -40,10 +40,19 @@ macro(llvm_config executable)
>>     # done in case libLLVM does not contain all of the components
>>     # the target requires.
>>     #
>> -    # TODO strip LLVM_DYLIB_COMPONENTS out of link_components.
>> +    # Strip LLVM_DYLIB_COMPONENTS out of link_components.
>>     # To do this, we need special handling for "all", since that
>>     # may imply linking to libraries that are not included in
>>     # libLLVM.
>> +
>> +    if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS)
>> +      if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all")
>> +        set(link_components "")
>> +      else()
>> +        list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS})
>> +      endif()
>> +    endif()
>> +
>>     target_link_libraries(${executable} LLVM)
>>   endif()
>> 
>> 
>> Modified: llvm/trunk/utils/unittest/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/CMakeLists.txt?rev=260641&r1=260640&r2=260641&view=diff
>> ==============================================================================
>> --- llvm/trunk/utils/unittest/CMakeLists.txt (original)
>> +++ llvm/trunk/utils/unittest/CMakeLists.txt Thu Feb 11 19:42:43 2016
>> @@ -32,10 +32,6 @@ if (NOT LLVM_ENABLE_THREADS)
>>   add_definitions( -DGTEST_HAS_PTHREAD=0 )
>> endif()
>> 
>> -set(LIBS
>> -  LLVMSupport # Depends on llvm::raw_ostream
>> -)
>> -
>> find_library(PTHREAD_LIBRARY_PATH pthread)
>> if (PTHREAD_LIBRARY_PATH)
>>   list(APPEND LIBS pthread)
>> @@ -46,6 +42,9 @@ add_llvm_library(gtest
>> 
>>   LINK_LIBS
>>   ${LIBS}
>> +
>> +  LINK_COMPONENTS
>> +  Support # Depends on llvm::raw_ostream
>> )
>> 
>> add_subdirectory(UnitTestMain)
>> 
>> Modified: llvm/trunk/utils/unittest/UnitTestMain/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/UnitTestMain/CMakeLists.txt?rev=260641&r1=260640&r2=260641&view=diff
>> ==============================================================================
>> --- llvm/trunk/utils/unittest/UnitTestMain/CMakeLists.txt (original)
>> +++ llvm/trunk/utils/unittest/UnitTestMain/CMakeLists.txt Thu Feb 11 19:42:43 2016
>> @@ -3,5 +3,7 @@ add_llvm_library(gtest_main
>> 
>>   LINK_LIBS
>>   gtest
>> -  LLVMSupport # Depends on llvm::cl
>> +
>> +  LINK_COMPONENTS
>> +  Support # Depends on llvm::cl
>>   )
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits