Jack Howarth via llvm-dev
2016-Feb-06 18:55 UTC
[llvm-dev] D16945: LLVM overhaul to avoid linking LLVM component libraries with libLLVM
Hans, I have posted a complete patch for solving the linkage issues with LLVM_LINK_LLVM_DYLIB on Phabricator at http://reviews.llvm.org/D16945. The bulk of the fix the simple changes of... Index: cmake/modules/AddLLVM.cmake ==================================================================--- cmake/modules/AddLLVM.cmake (revision 259743) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # 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) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + 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() endif() if(CMAKE_VERSION VERSION_LESS 2.8.12) Index: cmake/modules/LLVM-Config.cmake ==================================================================--- cmake/modules/LLVM-Config.cmake (revision 259743) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # 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() However the avoiding the accidental linkage of libLLVMSupport with libLLVM and libgtest for the unittests was really tricky as two different mechanisms to pass LLVMSupport are in play. The underlying problem was that the python based llvm-build tool was forcing a dependency on LLVMSupport for libgtest according to the required_libraries entry for the gtest library in utils/unittest/LLVMBuild.txt. Since the llvm-build tool has no access to the cmake defines, I had to solve this problem by adding a new --enable-llvm-link-llvm-dylib option in utils/llvm-build/llvmbuild/main.py to allow the LLVM_LINK_LLVM_DYLIB build to prune 'Support' from the the required library dependencies of the gtest library before the creation of tools/llvm-config/LibraryDependencies.inc. Now the LLVM_LINK_LLVM_DYLIB build contains... { "gtest", "libgtest.a", false, { } }, rather than { "gtest", "libgtest.a", false, { "support" } }, in that file. If there are specific maintainers for the llvm-build code, please add them to the review. Thanks in advance. Jack
Hans Wennborg via llvm-dev
2016-Feb-08 17:45 UTC
[llvm-dev] D16945: LLVM overhaul to avoid linking LLVM component libraries with libLLVM
Chris Bieneman is probably your best bet, and maybe also Dan Liew. On Sat, Feb 6, 2016 at 10:55 AM, Jack Howarth <howarth.mailing.lists at gmail.com> wrote:> Hans, > I have posted a complete patch for solving the linkage issues > with LLVM_LINK_LLVM_DYLIB on Phabricator at > http://reviews.llvm.org/D16945. The bulk of the fix the simple > changes of... > > Index: cmake/modules/AddLLVM.cmake > ==================================================================> --- cmake/modules/AddLLVM.cmake (revision 259743) > +++ cmake/modules/AddLLVM.cmake (working copy) > @@ -475,13 +475,15 @@ > # 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) > - else() > - llvm_map_components_to_libnames(llvm_libs > - ${ARG_LINK_COMPONENTS} > - ${LLVM_LINK_COMPONENTS} > - ) > + 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() > endif() > > if(CMAKE_VERSION VERSION_LESS 2.8.12) > Index: cmake/modules/LLVM-Config.cmake > ==================================================================> --- cmake/modules/LLVM-Config.cmake (revision 259743) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -40,10 +40,17 @@ > # 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() > > However the avoiding the accidental linkage of libLLVMSupport with > libLLVM and libgtest for the unittests was really tricky as two > different mechanisms to pass LLVMSupport are in play. The underlying > problem was that the python based llvm-build tool was forcing a > dependency on LLVMSupport for libgtest according to the > required_libraries entry for the gtest library in > utils/unittest/LLVMBuild.txt. Since the llvm-build tool has no access > to the cmake defines, I had to solve this problem by adding a new > --enable-llvm-link-llvm-dylib option in > utils/llvm-build/llvmbuild/main.py to allow the LLVM_LINK_LLVM_DYLIB > build to prune 'Support' from the the required library dependencies of > the gtest library before the creation of > tools/llvm-config/LibraryDependencies.inc. Now the > LLVM_LINK_LLVM_DYLIB build contains... > > { "gtest", "libgtest.a", false, { } }, > > rather than > > { "gtest", "libgtest.a", false, { "support" } }, > > in that file. If there are specific maintainers for the llvm-build > code, please add them to the review. > Thanks in advance. > Jack
Jack Howarth via llvm-dev
2016-Feb-09 20:41 UTC
[llvm-dev] D16945: LLVM overhaul to avoid linking LLVM component libraries with libLLVM
On Mon, Feb 8, 2016 at 12:45 PM, Hans Wennborg <hans at chromium.org> wrote:> Chris Bieneman is probably your best bet, and maybe also Dan Liew. >Hans, My current, and hopefully final, revision of the proposed patch is simplified and reworked to solve the problem entirely from cmake without touching the the llvm-build python scripts. Basically, the new fix for avoiding the Support library dependency set from the 'required_libraries = Support' in llvm/utils/unittest/LLVMBuild.txt is not read it for LLVM_LINK_LLVM_DYLIB. The new patch modifies llvm_add_library() in cmake/modules/AddLLVM.cmake to manually set the LLVMBUILD_LIB_DEPS_gtest property to LLVM when LLVM_LINK_LLVM_DYLIB is set and the library name is gtest rather than read from the tools/llvm-config/LibraryDependencies.inc generated by llvm-build. Jack ps As soon as this patch lands, I'll look at trying to rework the unittests handling of the LLVMSupport dependency and see if setting that explicitly can be dropped (as it should be pulled in from the LLVMBUILD_LIB_DEPS_gtest property setting of Support). However that really isn't suitable for back porting to 3.8 so I want to avoid conflating those changes with those required to solve the test suite failures in bug 26393. Index: cmake/modules/AddLLVM.cmake ==================================================================--- cmake/modules/AddLLVM.cmake (revision 260200) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -473,17 +473,23 @@ # 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 (LLVM_LINK_LLVM_DYLIB AND ${name} STREQUAL gtest) + set_property(GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_gtest LLVM) else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) endif() + 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() + endif() + if(CMAKE_VERSION VERSION_LESS 2.8.12) # Link libs w/o keywords, assuming PUBLIC. target_link_libraries(${name} @@ -885,11 +891,18 @@ 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}) + if (LLVM_LINK_LLVM_DYLIB) + target_link_libraries(${test_name} + gtest + gtest_main + ) + else() target_link_libraries(${test_name} - gtest - gtest_main - LLVMSupport # gtest needs it for raw_ostream. - ) + gtest + gtest_main + LLVMSupport # Depends on llvm::cl + ) + endif() add_dependencies(${test_suite} ${test_name}) get_target_property(test_suite_folder ${test_suite} FOLDER) Index: cmake/modules/LLVM-Config.cmake ==================================================================--- cmake/modules/LLVM-Config.cmake (revision 260200) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # 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() Index: utils/unittest/CMakeLists.txt ==================================================================--- utils/unittest/CMakeLists.txt (revision 260200) +++ utils/unittest/CMakeLists.txt (working copy) @@ -32,9 +32,15 @@ add_definitions( -DGTEST_HAS_PTHREAD=0 ) endif() -set(LIBS - LLVMSupport # Depends on llvm::raw_ostream -) +if (LLVM_LINK_LLVM_DYLIB) + set(LIBS + LLVM # Depends on llvm::raw_ostream + ) +else() + set(LIBS + LLVMSupport # Depends on llvm::raw_ostream + ) +endif() find_library(PTHREAD_LIBRARY_PATH pthread) if (PTHREAD_LIBRARY_PATH) Index: utils/unittest/UnitTestMain/CMakeLists.txt ==================================================================--- utils/unittest/UnitTestMain/CMakeLists.txt (revision 260200) +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) @@ -1,7 +1,17 @@ -add_llvm_library(gtest_main - TestMain.cpp +if (LLVM_LINK_LLVM_DYLIB) + add_llvm_library(gtest_main + TestMain.cpp - LINK_LIBS - gtest - LLVMSupport # Depends on llvm::cl - ) + LINK_LIBS + gtest + LLVM # Depends on llvm::cl + ) +else() + add_llvm_library(gtest_main + TestMain.cpp + + LINK_LIBS + gtest + LLVMSupport # Depends on llvm::cl + ) +endif()> On Sat, Feb 6, 2016 at 10:55 AM, Jack Howarth > <howarth.mailing.lists at gmail.com> wrote: >> Hans, >> I have posted a complete patch for solving the linkage issues >> with LLVM_LINK_LLVM_DYLIB on Phabricator at >> http://reviews.llvm.org/D16945. The bulk of the fix the simple >> changes of... >> >> Index: cmake/modules/AddLLVM.cmake >> ==================================================================>> --- cmake/modules/AddLLVM.cmake (revision 259743) >> +++ cmake/modules/AddLLVM.cmake (working copy) >> @@ -475,13 +475,15 @@ >> # 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) >> - else() >> - llvm_map_components_to_libnames(llvm_libs >> - ${ARG_LINK_COMPONENTS} >> - ${LLVM_LINK_COMPONENTS} >> - ) >> + 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() >> endif() >> >> if(CMAKE_VERSION VERSION_LESS 2.8.12) >> Index: cmake/modules/LLVM-Config.cmake >> ==================================================================>> --- cmake/modules/LLVM-Config.cmake (revision 259743) >> +++ cmake/modules/LLVM-Config.cmake (working copy) >> @@ -40,10 +40,17 @@ >> # 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() >> >> However the avoiding the accidental linkage of libLLVMSupport with >> libLLVM and libgtest for the unittests was really tricky as two >> different mechanisms to pass LLVMSupport are in play. The underlying >> problem was that the python based llvm-build tool was forcing a >> dependency on LLVMSupport for libgtest according to the >> required_libraries entry for the gtest library in >> utils/unittest/LLVMBuild.txt. Since the llvm-build tool has no access >> to the cmake defines, I had to solve this problem by adding a new >> --enable-llvm-link-llvm-dylib option in >> utils/llvm-build/llvmbuild/main.py to allow the LLVM_LINK_LLVM_DYLIB >> build to prune 'Support' from the the required library dependencies of >> the gtest library before the creation of >> tools/llvm-config/LibraryDependencies.inc. Now the >> LLVM_LINK_LLVM_DYLIB build contains... >> >> { "gtest", "libgtest.a", false, { } }, >> >> rather than >> >> { "gtest", "libgtest.a", false, { "support" } }, >> >> in that file. If there are specific maintainers for the llvm-build >> code, please add them to the review. >> Thanks in advance. >> Jack
Possibly Parallel Threads
- D16945: LLVM overhaul to avoid linking LLVM component libraries with libLLVM
- How to build only the necessary components with MSVC
- Windows vs Mac/Linux distribution discrepancy
- [LLVMdev] [PATCH] utils/unittest: Consistency of gtest and gtest_main libs.
- Building SVN head with CMake - shared libraries?