Page MenuHomePhabricator

[CMake] Installable find modules for terminfo and libffi
ClosedPublic

Authored by jackoalan on Nov 20 2021, 2:58 PM.

Details

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.

Diff Detail

Event Timeline

jackoalan created this revision.Nov 20 2021, 2:58 PM
jackoalan requested review of this revision.Nov 20 2021, 2:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2021, 2:58 PM
jackoalan edited the summary of this revision. (Show Details)Nov 20 2021, 2:59 PM
jackoalan edited the summary of this revision. (Show Details)Nov 21 2021, 6:51 AM
jackoalan edited the summary of this revision. (Show Details)
JDevlieghere accepted this revision.Nov 29 2021, 9:28 AM

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
60

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)
This revision is now accepted and ready to land.Nov 29 2021, 9:28 AM
jackoalan updated this revision to Diff 390481.Nov 29 2021, 3:05 PM

Use LLDB's find module naming conventions and style.

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).

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 marked an inline comment as done.Nov 29 2021, 3:20 PM

I also do not have commit access yet. Would you mind committing this for me?
--author="Jack Andersen <jackoalan@gmail.com>"