[CMake] Installable find modules for terminfo and libffi

[jackoalan]

Summary

Improves cross-distro portability of LLVM cmake package by resolving paths for
terminfo and libffi via import targets.

When LLVMExports.cmake is generated for installation, it contains absolute
library paths which are likely to be a common cause of portability issues. To
mitigate this, the discovery logic for these dependencies is refactored into
find modules which get installed alongside LLVMConfig.cmake. The result is
cleaner, cmake-friendly management of these dependencies that respect the
environment of the LLVM package importer.

[JDevlieghere]

I'm assuming the logic to find the actual packages is the same (I didn't check in detail). Splitting this into separate Find-modules is definitely the way to go and that part of this patch LGTM.

llvm/cmake/modules/FindFFI.cmake

59

In LLDB we format this call as shown below to help with readability:

find_package_handle_standard_args(LibEdit
                                  FOUND_VAR
                                    LibEdit_FOUND
                                  REQUIRED_VARS
                                    LibEdit_INCLUDE_DIRS
                                    LibEdit_LIBRARIES
                                  VERSION_VAR
                                    LibEdit_VERSION_STRING)

[jackoalan]

Use LLDB's find module naming conventions and style.

[jackoalan]

In D114327#3158666, @JDevlieghere wrote:

I'm assuming the logic to find the actual packages is the same

Yes, mostly existing code refactored into find modules.

One minor difference is libffi is checked using check_c_source_compiles linking to symbol declarations rather than check_symbol_exists, so it is only concerned with the ability to successfully link. This is useful for distributions that ship headers in separate packages that the user may not have installed (and technically should not have to as a pre-built LLVM importer).

[jackoalan]

I should also note that I tested this change on Windows and this does not interfere with package importing there. (LLVM_ENABLE_TERMINFO and LLVM_ENABLE_FFI are set false in LLVMConfig.cmake)

[jackoalan]

I also do not have commit access yet. Would you mind committing this for me?
--author="Jack Andersen <[e-mail]>"

llvm/cmake/config-ix.cmake

if(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
   # Don't look for these libraries on Windows.
   if (NOT PURE_WINDOWS)
     # Skip libedit if using ASan as it contains memory leaks.
     if (LLVM_ENABLE_LIBEDIT AND HAVE_HISTEDIT_H AND NOT LLVM_USE_SANITIZER MATCHES ".*Address.*")
       check_library_exists(edit el_init "" HAVE_LIBEDIT)
     else()
       set(HAVE_LIBEDIT 0)
     endif()
+    if(LLVM_ENABLE_TERMINFO)
       if(LLVM_ENABLE_TERMINFO STREQUAL FORCE_ON)
-      set(MAYBE_REQUIRED REQUIRED)
+        find_package(Terminfo REQUIRED)
       else()
-      set(MAYBE_REQUIRED)
-    endif()
-    if(LLVM_ENABLE_TERMINFO)
-      find_library(TERMINFO_LIB NAMES terminfo tinfo curses ncurses ncursesw ${MAYBE_REQUIRED})
+        find_package(Terminfo)
       endif()
-    if(TERMINFO_LIB)
-      set(LLVM_ENABLE_TERMINFO 1)
-    else()
-      set(LLVM_ENABLE_TERMINFO 0)
+      set(LLVM_ENABLE_TERMINFO "${TERMINFO_FOUND}")
     endif()
   else()
     set(LLVM_ENABLE_TERMINFO 0)
   endif()
 else()
   set(LLVM_ENABLE_TERMINFO 0)
 endif()
 
else(LLVM_PATH_${NAME})
     set(HAVE_${NAME} "" CACHE INTERNAL "Is ${name} available ?")
   endif(LLVM_PATH_${NAME})
 endfunction()
 
 if (LLVM_ENABLE_DOXYGEN)
   llvm_find_program(dot)
 endif ()
 
-if( LLVM_ENABLE_FFI )
-  find_path(FFI_INCLUDE_PATH ffi.h PATHS ${FFI_INCLUDE_DIR})
-  if( EXISTS "${FFI_INCLUDE_PATH}/ffi.h" )
-    set(FFI_HEADER ffi.h CACHE INTERNAL "")
-    set(HAVE_FFI_H 1 CACHE INTERNAL "")
+if(LLVM_ENABLE_FFI)
+  set(FFI_REQUIRE_INCLUDE On)
+  if(LLVM_ENABLE_FFI STREQUAL FORCE_ON)
+    find_package(FFI REQUIRED)
   else()
-    find_path(FFI_INCLUDE_PATH ffi/ffi.h PATHS ${FFI_INCLUDE_DIR})
-    if( EXISTS "${FFI_INCLUDE_PATH}/ffi/ffi.h" )
-      set(FFI_HEADER ffi/ffi.h CACHE INTERNAL "")
-      set(HAVE_FFI_FFI_H 1 CACHE INTERNAL "")
-    endif()
+    find_package(FFI)
   endif()
-
-  if( NOT FFI_HEADER )
-    message(FATAL_ERROR "libffi includes are not found.")
-  endif()
-
-  find_library(FFI_LIBRARY_PATH ffi PATHS ${FFI_LIBRARY_DIR})
-  if( NOT FFI_LIBRARY_PATH )
-    message(FATAL_ERROR "libffi is not found.")
-  endif()
-
-  list(APPEND CMAKE_REQUIRED_LIBRARIES ${FFI_LIBRARY_PATH})
-  list(APPEND CMAKE_REQUIRED_INCLUDES ${FFI_INCLUDE_PATH})
-  check_symbol_exists(ffi_call ${FFI_HEADER} HAVE_FFI_CALL)
-  list(REMOVE_ITEM CMAKE_REQUIRED_INCLUDES ${FFI_INCLUDE_PATH})
-  list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES ${FFI_LIBRARY_PATH})
+  set(LLVM_ENABLE_FFI "${FFI_FOUND}")
 else()
   unset(HAVE_FFI_FFI_H CACHE)
   unset(HAVE_FFI_H CACHE)
   unset(HAVE_FFI_CALL CACHE)
-endif( LLVM_ENABLE_FFI )
+endif()
 
 check_symbol_exists(proc_pid_rusage "libproc.h" HAVE_PROC_PID_RUSAGE)
 
 # Whether we can use std::is_trivially_copyable to verify llvm::is_trivially_copyable.
 CHECK_CXX_SOURCE_COMPILES("
 #include <type_traits>
 struct T { int val; };
 static_assert(std::is_trivially_copyable<T>::value, \"ok\");

llvm/cmake/modules/FindFFI.cmake

+# Attempts to discover ffi library with a linkable ffi_call function.
+#
+# Example usage:
+#
+# find_package(FFI)
+#
+# FFI_REQUIRE_INCLUDE may be set to consider ffi found if the includes
+# are present in addition to the library. This is useful to keep off
+# for the imported package on platforms that install the library but
+# not the headers.
+#
+# FFI_LIBRARY_DIR may be set to define search paths for the ffi library.
+#
+# If successful, the following variables will be defined:
+# FFI_FOUND
+# FFI_INCLUDE_PATH
+# FFI_LIBRARY_PATH
+# HAVE_FFI_CALL
+#
+# HAVE_FFI_H or HAVE_FFI_FFI_H is defined depending on the ffi.h include path.
+#
+# Additionally, the following import target will be defined:
+# FFI::ffi
+
+find_path(FFI_INCLUDE_PATH ffi.h PATHS ${FFI_INCLUDE_DIR})
+if( EXISTS "${FFI_INCLUDE_PATH}/ffi.h" )
+  set(FFI_HEADER ffi.h CACHE INTERNAL "")
+  set(HAVE_FFI_H 1 CACHE INTERNAL "")
+else()
+  find_path(FFI_INCLUDE_PATH ffi/ffi.h PATHS ${FFI_INCLUDE_DIR})
+  if( EXISTS "${FFI_INCLUDE_PATH}/ffi/ffi.h" )
+    set(FFI_HEADER ffi/ffi.h CACHE INTERNAL "")
+    set(HAVE_FFI_FFI_H 1 CACHE INTERNAL "")
+  endif()
+endif()
+
+find_library(FFI_LIBRARY_PATH ffi PATHS ${FFI_LIBRARY_DIR})
+
+if(FFI_LIBRARY_PATH)
+  include(CMakePushCheckState)
+  include(CheckCSourceCompiles)
+  cmake_push_check_state()
+  list(APPEND CMAKE_REQUIRED_LIBRARIES ${FFI_LIBRARY_PATH})
+  check_c_source_compiles("
+    struct ffi_cif;
+    typedef struct ffi_cif ffi_cif;
+    void ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue);
+    int main() { ffi_call(0, 0, 0, 0); }"
+    HAVE_FFI_CALL)
+  cmake_pop_check_state()
+endif()
+
+unset(required_includes)
+if(FFI_REQUIRE_INCLUDE)
+  set(required_includes FFI_INCLUDE_PATH)
+endif()
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(FFI DEFAULT_MSG FFI_LIBRARY_PATH ${required_includes} HAVE_FFI_CALL)
+mark_as_advanced(FFI_LIBRARY_PATH FFI_INCLUDE_PATH HAVE_FFI_CALL FFI_HEADER HAVE_FFI_H HAVE_FFI_FFI_H)
+
+if(FFI_FOUND)
+  if(NOT TARGET FFI::ffi)
+    add_library(FFI::ffi UNKNOWN IMPORTED)
+    set_target_properties(FFI::ffi PROPERTIES IMPORTED_LOCATION "${FFI_LIBRARY_PATH}")
+    if(FFI_INCLUDE_PATH)
+      set_target_properties(FFI::ffi PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${FFI_INCLUDE_PATH}")
+    endif()
+  endif()
+endif()

llvm/cmake/modules/FindTerminfo.cmake

+# Attempts to discover terminfo library with a linkable setupterm function.
+#
+# Example usage:
+#
+# find_package(Terminfo)
+#
+# If successful, the following variables will be defined:
+# TERMINFO_FOUND
+# TERMINFO_LIB
+#
+# Additionally, the following import target will be defined:
+# Terminfo::terminfo
+
+find_library(TERMINFO_LIB NAMES terminfo tinfo curses ncurses ncursesw)
+
+if(TERMINFO_LIB)
+  include(CMakePushCheckState)
+  include(CheckCSourceCompiles)
+  cmake_push_check_state()
+  list(APPEND CMAKE_REQUIRED_LIBRARIES ${TERMINFO_LIB})
+  check_c_source_compiles("
+    int setupterm(char *term, int filedes, int *errret);
+    int main() { return setupterm(0, 0, 0); }"
+    HAVE_TERMINFO)
+  cmake_pop_check_state()
+endif()
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(Terminfo DEFAULT_MSG TERMINFO_LIB HAVE_TERMINFO)
+mark_as_advanced(TERMINFO_LIB HAVE_TERMINFO)
+
+if(TERMINFO_FOUND)
+  if(NOT TARGET Terminfo::terminfo)
+    add_library(Terminfo::terminfo UNKNOWN IMPORTED)
+    set_target_properties(Terminfo::terminfo PROPERTIES IMPORTED_LOCATION "${TERMINFO_LIB}")
+  endif()
+endif()

llvm/cmake/modules/LLVMConfig.cmake.in

 # This file provides information and services to the final user.
 
 @LLVM_CONFIG_CODE@
 
+# For finding self-installed Find*.cmake packages.
+list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
 set(LLVM_VERSION_MAJOR @LLVM_VERSION_MAJOR@)
 set(LLVM_VERSION_MINOR @LLVM_VERSION_MINOR@)
 set(LLVM_VERSION_PATCH @LLVM_VERSION_PATCH@)
 set(LLVM_VERSION_SUFFIX @LLVM_VERSION_SUFFIX@)
 set(LLVM_PACKAGE_VERSION @PACKAGE_VERSION@)
 set(LLVM_PACKAGE_BUGREPORT @PACKAGE_BUGREPORT@)
 
 set(LLVM_BUILD_TYPE @CMAKE_BUILD_TYPE@)

 set(LLVM_ENABLE_WARNINGS @LLVM_ENABLE_WARNINGS@)
 
 set(LLVM_ENABLE_EXPENSIVE_CHECKS @LLVM_ENABLE_EXPENSIVE_CHECKS@)
 
 set(LLVM_ENABLE_ASSERTIONS @LLVM_ENABLE_ASSERTIONS@)
 
 set(LLVM_ENABLE_EH @LLVM_ENABLE_EH@)
 
+set(LLVM_ENABLE_FFI @LLVM_ENABLE_FFI@)
+if(LLVM_ENABLE_FFI)
+  find_package(FFI)
+endif()
+
 set(LLVM_ENABLE_RTTI @LLVM_ENABLE_RTTI@)
 
 set(LLVM_ENABLE_TERMINFO @LLVM_ENABLE_TERMINFO@)
+if(LLVM_ENABLE_TERMINFO)
+  find_package(Terminfo)
+endif()
 
 set(LLVM_ENABLE_THREADS @LLVM_ENABLE_THREADS@)
 
 set(LLVM_ENABLE_UNWIND_TABLES @LLVM_ENABLE_UNWIND_TABLES@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
 if(LLVM_ENABLE_ZLIB)
   find_package(ZLIB)

llvm/lib/ExecutionEngine/Interpreter/CMakeLists.txt

-# Make sure that the path to libffi headers is on the command
-# line. That path can be a compiler's non-default path even when
-# FFI_INCLUDE_DIR was not used, because cmake has its own paths for
-# searching for headers (CMAKE_SYSTEM_INCLUDE_PATH, for instance):
-if( FFI_INCLUDE_PATH )
-  include_directories( ${FFI_INCLUDE_PATH} )
-endif()
-
 add_llvm_component_library(LLVMInterpreter
   Execution.cpp
   ExternalFunctions.cpp
   Interpreter.cpp
 
   DEPENDS
   intrinsics_gen
 
   LINK_COMPONENTS
   CodeGen
   Core
   ExecutionEngine
   Support
   )
 
 if( LLVM_ENABLE_FFI )
-  target_link_libraries( LLVMInterpreter PRIVATE ${FFI_LIBRARY_PATH} )
+  target_link_libraries( LLVMInterpreter PRIVATE FFI::ffi )
 endif()

llvm/lib/Support/CMakeLists.txt

if( HAVE_BACKTRACE AND NOT "${Backtrace_LIBRARIES}" STREQUAL "" )
     # On BSDs, CMake returns a fully qualified path to the backtrace library.
     # We need to remove the path and the 'lib' prefix, to make it look like a
     # regular short library name, suitable for appending to a -l link flag.
     get_filename_component(Backtrace_LIBFILE ${Backtrace_LIBRARIES} NAME_WE)
     STRING(REGEX REPLACE "^lib" "" Backtrace_LIBFILE ${Backtrace_LIBFILE})
     set(system_libs ${system_libs} ${Backtrace_LIBFILE})
   endif()
   if( LLVM_ENABLE_TERMINFO )
-    set(imported_libs ${imported_libs} "${TERMINFO_LIB}")
+    set(imported_libs ${imported_libs} Terminfo::terminfo)
   endif()
   if( LLVM_ENABLE_THREADS AND (HAVE_LIBATOMIC OR HAVE_CXX_LIBATOMICS64) )
     set(system_libs ${system_libs} atomic)
   endif()
   set(system_libs ${system_libs} ${LLVM_PTHREAD_LIB})
   if( UNIX AND NOT (BEOS OR HAIKU) )
     set(system_libs ${system_libs} m)
   endif()
if(LLVM_ENABLE_ZLIB)
   if(NOT zlib_library)
     get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
   endif()
   get_library_name(${zlib_library} zlib_library)
   set(llvm_system_libs ${llvm_system_libs} "${zlib_library}")
 endif()
 
 if(LLVM_ENABLE_TERMINFO)
-  get_library_name(${TERMINFO_LIB} terminfo_library)
+  if(NOT terminfo_library)
+    get_property(terminfo_library TARGET Terminfo::terminfo PROPERTY LOCATION)
+  endif()
+  get_library_name(${terminfo_library} terminfo_library)
   set(llvm_system_libs ${llvm_system_libs} "${terminfo_library}")
 endif()
 
 set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${llvm_system_libs}")
 
 
 if(LLVM_INTEGRATED_CRT_ALLOC)
   if(LLVM_INTEGRATED_CRT_ALLOC MATCHES "snmalloc$")