[cmake] Allow multiple LibEdit_LIBRARIES

[mysterymath]

If built statically, libedit may have a private static library dependency on a provider of the terminfo API (e.g., ncurses). This means that multiple libraries would need to be provided as the value for LibEdit_LIBRARIES, but the current implementation of FindLibEdit precludes this. This PR allows a list to be passed to LibEdit_LIBRARIES.

[Romain-Geissler-1A]

Hi,

I fail to understand how with the current code LibEdit_LIBRARIES can be a list. At the beginning of the file, it seems LibEdit_LIBRARIES is always defined to be a single string, not a list, with:

find_path(LibEdit_INCLUDE_DIRS NAMES histedit.h HINTS ${PC_LIBEDIT_INCLUDE_DIRS})
find_library(LibEdit_LIBRARIES NAMES edit HINTS ${PC_LIBEDIT_LIBRARY_DIRS})

It seems we totally loose the list part, which is stored in PC_LIBEDIT_LIBRARIES. Shouldn't we reset LibEdit_LIBRARIES = "${PC_LIBEDIT_LIBRARIES}" if all check pass (and similarly for include dirs) ? Or maybe concat PC_LIBEDIT_LIBRARIES & LibEdit_LIBRARIES so that it will work even when nothing is found from pkg-file but found from manually added -I/-L flags ?

[mysterymath]

LibEdit_LIBRARIES is a cache variable, so it's legitimate for a user to specify it explicitly. I do think ideally there would be a way to avoid the manual work we needed to compute that, perhaps using pkg-config. But IIRC, find_library wasn't itself aware of the dependencies of a statically-linked libedit.

[Romain-Geissler-1A]

In my case with a statically linked libedit (& ncurses/libtinfo) pkg-config (in static mode, with --static) is aware of the -ltinfo dependency. I was able to make my build work, without forcing LibEdit_LIBRARIES to any value, with a patch like this:

Author: Romain Geissler <[email protected]>
Date:   Tue Aug 13 02:22:31 2024 +0000
    
    Use include and libraries from libedit's pkg-config, which can be a list.

diff --git a/llvm/cmake/modules/FindLibEdit.cmake b/llvm/cmake/modules/FindLibEdit.cmake
index b3f49e1477f0..06266d05a24d 100644
--- a/llvm/cmake/modules/FindLibEdit.cmake
+++ b/llvm/cmake/modules/FindLibEdit.cmake
@@ -54,12 +54,16 @@ find_package_handle_standard_args(LibEdit
                                   REQUIRED_VARS
                                     LibEdit_INCLUDE_DIRS
                                     LibEdit_LIBRARIES
+                                    PC_LIBEDIT_LIBRARIES
+                                    PC_LIBEDIT_LIBDIR
+                                    PC_LIBEDIT_INCLUDE_DIRS
                                   VERSION_VAR
                                     LibEdit_VERSION_STRING)
 mark_as_advanced(LibEdit_INCLUDE_DIRS LibEdit_LIBRARIES)

 if (LibEdit_FOUND AND NOT TARGET LibEdit::LibEdit)
   add_library(LibEdit::LibEdit INTERFACE IMPORTED)
-  target_link_libraries(LibEdit::LibEdit INTERFACE ${LibEdit_LIBRARIES})
-  target_include_directories(LibEdit::LibEdit INTERFACE ${LibEdit_INCLUDE_DIRS})
+  target_link_libraries(LibEdit::LibEdit INTERFACE ${PC_LIBEDIT_LIBRARIES})
+  target_link_directories(LibEdit::LibEdit INTERFACE ${PC_LIBEDIT_LIBDIR})
+  target_include_directories(LibEdit::LibEdit INTERFACE ${PC_LIBEDIT_INCLUDE_DIRS})
 endif()

However if I understand correctly your usage (forcing manually LibEdit_LIBRARIES) it would break you if something like was merged. I wonder what to do here (I don't really like the idea of manually providing LibEdit_LIBRARIES myself while it seems at least in some cases pkg-config is good enough to find the necessary libraries.

[mysterymath]

In my case with a statically linked libedit (& ncurses/libtinfo) pkg-config (in static mode, with --static) is aware of the -ltinfo dependency. I was able to make my build work, without forcing LibEdit_LIBRARIES to any value, with a patch like this:

Author: Romain Geissler <[email protected]>
Date:   Tue Aug 13 02:22:31 2024 +0000
    
    Use include and libraries from libedit's pkg-config, which can be a list.

diff --git a/llvm/cmake/modules/FindLibEdit.cmake b/llvm/cmake/modules/FindLibEdit.cmake
index b3f49e1477f0..06266d05a24d 100644
--- a/llvm/cmake/modules/FindLibEdit.cmake
+++ b/llvm/cmake/modules/FindLibEdit.cmake
@@ -54,12 +54,16 @@ find_package_handle_standard_args(LibEdit
                                   REQUIRED_VARS
                                     LibEdit_INCLUDE_DIRS
                                     LibEdit_LIBRARIES
+                                    PC_LIBEDIT_LIBRARIES
+                                    PC_LIBEDIT_LIBDIR
+                                    PC_LIBEDIT_INCLUDE_DIRS
                                   VERSION_VAR
                                     LibEdit_VERSION_STRING)
 mark_as_advanced(LibEdit_INCLUDE_DIRS LibEdit_LIBRARIES)

 if (LibEdit_FOUND AND NOT TARGET LibEdit::LibEdit)
   add_library(LibEdit::LibEdit INTERFACE IMPORTED)
-  target_link_libraries(LibEdit::LibEdit INTERFACE ${LibEdit_LIBRARIES})
-  target_include_directories(LibEdit::LibEdit INTERFACE ${LibEdit_INCLUDE_DIRS})
+  target_link_libraries(LibEdit::LibEdit INTERFACE ${PC_LIBEDIT_LIBRARIES})
+  target_link_directories(LibEdit::LibEdit INTERFACE ${PC_LIBEDIT_LIBDIR})
+  target_include_directories(LibEdit::LibEdit INTERFACE ${PC_LIBEDIT_INCLUDE_DIRS})
 endif()

However if I understand correctly your usage (forcing manually LibEdit_LIBRARIES) it would break you if something like was merged. I wonder what to do here (I don't really like the idea of manually providing LibEdit_LIBRARIES myself while it seems at least in some cases pkg-config is good enough to find the necessary libraries.

Would it be possible to set LibEdit_LIBRARIES using the information from pkg-config?