[Support][CMake] Fix exposed absolute path dependency of terminfo library

[xndcn]

Summary

terminfo library dependency is exposed as absolute path in "LLVMExports.cmake".

clang+llvm-13.0.0-x86_64-apple-darwin.tar.xz/lib/cmake/llvm/LLVMExports.cmake:

set_target_properties(LLVMSupport PROPERTIES
  INTERFACE_LINK_LIBRARIES "m;ZLIB::ZLIB;/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/lib/libcurses.tbd;LLVMDemangle"
)

clang+llvm-13.0.0-aarch64-linux-gnu.tar.xz/lib/cmake/llvm/LLVMExports.cmake:

set_target_properties(LLVMSupport PROPERTIES
  INTERFACE_LINK_LIBRARIES "rt;dl;-lpthread;m;ZLIB::ZLIB;/usr/lib/aarch64-linux-gnu/libtinfo.so;LLVMDemangle"
)

[compnerd]

I'm not sure that this is a good idea. This now means that you can link against two different versions of this library - one when building LLVMSupport and one that any user linking against LLVMSupport happens to find in the library search path. Consider someone explicitly providing a path to the terminfo library when configuring. At the very least, the library search path should be propagated to ensure that the right version is used.

[xndcn]

In D114437#3157211, @compnerd wrote:

I'm not sure that this is a good idea. This now means that you can link against two different versions of this library - one when building LLVMSupport and one that any user linking against LLVMSupport happens to find in the library search path. Consider someone explicitly providing a path to the terminfo library when configuring. At the very least, the library search path should be propagated to ensure that the right version is used.

Thanks! I have updated the patch. In fact the library like "ZLIB::ZLIB" or "LibXML2" also dose not use absolute path, so I think we can use the same idea to wrap the libtinfo into an IMPORTED library "LibTerminfo" in CMake. In LLVMConfig.cmake, we can try to find LibTerminfo from the specific path as a "HINTS".

[compnerd]

I think that this is almost correct now. While this will generate the binaries correctly, this is still a problem for tests. Without the absolute path, we now run the risk of the DT_NEEDED being correct but without a DT_RPATH/DR_RUNPATH, this could pick up the wrong library still at runtime. While the link time issue is resolved, the runtime portion is still a problem.

[xndcn]

In D114437#3171687, @compnerd wrote:

I think that this is almost correct now. While this will generate the binaries correctly, this is still a problem for tests. Without the absolute path, we now run the risk of the DT_NEEDED being correct but without a DT_RPATH/DR_RUNPATH, this could pick up the wrong library still at runtime. While the link time issue is resolved, the runtime portion is still a problem.

Thanks! Just found a similar but more elegant commit has been merged yesterday: https://reviews.llvm.org/D114327
So I will close it now.

llvm/cmake/config-ix.cmake

 
 include(CheckCompilerVersion)
 include(HandleLLVMStdlib)
+include(GetLibraryName)
 
 if( UNIX AND NOT (APPLE OR BEOS OR HAIKU) )
   # Used by check_symbol_exists:

     endif()
     if(LLVM_ENABLE_TERMINFO)
       find_library(TERMINFO_LIB NAMES terminfo tinfo curses ncurses ncursesw ${MAYBE_REQUIRED})
+      get_library_name(${TERMINFO_LIB} TERMINFO_LIB_NAME)
+      get_filename_component(TERMINFO_LIB_DIR ${TERMINFO_LIB} DIRECTORY)
+      add_library(LibTerminfo INTERFACE IMPORTED)
+      target_link_libraries(LibTerminfo INTERFACE ${TERMINFO_LIB})
     endif()
     if(TERMINFO_LIB)
       set(LLVM_ENABLE_TERMINFO 1)

llvm/cmake/modules/LLVMConfig.cmake.in

 set(LLVM_ENABLE_RTTI @LLVM_ENABLE_RTTI@)
 
 set(LLVM_ENABLE_TERMINFO @LLVM_ENABLE_TERMINFO@)
+if(LLVM_ENABLE_TERMINFO)
+  find_library(TERMINFO_LIB @TERMINFO_LIB_NAME@ HINTS @TERMINFO_LIB_DIR@ REQUIRED)
+  add_library(LibTerminfo INTERFACE IMPORTED)
+  target_link_libraries(LibTerminfo INTERFACE ${TERMINFO_LIB})
+endif()
 
 set(LLVM_ENABLE_THREADS @LLVM_ENABLE_THREADS@)
 

llvm/lib/Support/CMakeLists.txt

     set(system_libs ${system_libs} ${Backtrace_LIBFILE})
   endif()
   if( LLVM_ENABLE_TERMINFO )
-    set(imported_libs ${imported_libs} "${TERMINFO_LIB}")
+    set(imported_libs ${imported_libs} LibTerminfo)
   endif()
   if( LLVM_ENABLE_THREADS AND (HAVE_LIBATOMIC OR HAVE_CXX_LIBATOMICS64) )
     set(system_libs ${system_libs} atomic)