Revert "Use find_library for ncurses"

[haampie]

Summary

The introduction of find_library for ncurses caused more issues than it solved problems. The current open issue is it makes the static build of LLVM fail. It's better to revert for now, and get back to it later.

Revert "[CMake] Fix an issue where get_system_libname creates an empty regex capture on windows"

This reverts commit 1ed1e16ab83f55d85c90ae43a05cbe08a00c20e0.

Revert "Fix msan build"

This reverts commit 34fe9613dda3c7d8665b609136a8c12deb122382.

Revert "[CMake] Always mark terminfo as unavailable on Windows"

This reverts commit 76bf26236f6fd453343666c3cd91de8f74ffd89d.

Revert "[CMake] Fix OCaml build failure because of absolute path in system libs"

This reverts commit 8e4acb82f71ad4effec8895b8fc957189ce95933.

Revert "[CMake] Don't look for terminfo libs when LLVM_ENABLE_TERMINFO=OFF"

This reverts commit 495f91fd33d492941c39424a32cf24bcfe192f35.

Revert "Use find_library for ncurses"

This reverts commit a52173a3e56553d7b795bcf3cdadcf6433117107.

[gkistanova]

I have checked this patch with http://lab.llvm.org:8011/builders/lld-perf-testsuite bot. It makes it green.

Thanks for reverting, Harmen!

[mati865]

FYI this doesn't fully fix MinGW issue:

$ cat tools/llvm-config/BuildVariables.inc | grep LLVM_SYSTEM_LIBS
#define LLVM_SYSTEM_LIBS "-lpsapi -lshell32 -lole32 -luuid -ladvapi32 -lz.dll"

It's because libraries are found by their import library which is libz.dll.a in this case.
libz.dll.a matches if in line 213 and gets stripped to z.dll which doesn't match shared library regex lib.*\.dll.

The easiest fix would be adding:

if(CMAKE_IMPORT_LIBRARY_PREFIX AND CMAKE_IMPORT_LIBRARY_SUFFIX AND
    zlib_library MATCHES "^${CMAKE_IMPORT_LIBRARY_PREFIX}.*${CMAKE_IMPORT_LIBRARY_SUFFIX }$")
  STRING(REGEX REPLACE "^${CMAKE_IMPORT_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
  STRING(REGEX REPLACE "${CMAKE_IMPORT_LIBRARY_SUFFIX }$" "" zlib_library ${zlib_library})
endif()

Should it fixed in separate diff given the fact this is a revert?

[gkistanova]

Hi Mateusz,
Yes, this looks like a separate patch on top of this revert.

By the way, this hasn't been merged yet. Are we waiting for anything? Harmen, do you need a help committing this patch?

[phosek]

I have suggested an alternative in https://reviews.llvm.org/D85820#2237357 to try and fix this forward and I was waiting for @haampie's response before landing this. I can implement that today if you're OK fixing this forward? I think it'd be easier to do that than reverting and then relanding all these changes again. @gkistanova how did you check the patch with http://lab.llvm.org:8011/builders/lld-perf-testsuite bot? Is there a way to do a presubmit check on that bot?

[gkistanova]

Hi Petr,

I don't have a strong opinion on the exact way of fixing the problem. But we should do this as soon as possible, as keeping the bots red for that long is not Ok.

Please send me your patch and I will check it on lld-perf-testsuite bot. If you do not have the patch ready yet, I'd say we shall commit this revert to buy more time.

[haampie]

@phosek it would be great to get that patch in, but the truth is it is difficult testing across multiple platforms for me; I can only test on Linux and macOS, not Windows (GNU). Since that patch would touch predefined, platform-dependent cmake variables, I would want to test it properly before submitting a patch, which might take a bit more time. So for me it would be best to temporarily accept this reverting patch, and submit a properly tested patch afterwards.

[gkistanova]

Fair enough. Reverted.
https://github.com/llvm/llvm-project/commit/cdcb9ab10e53ff08293915af3cd897c42112bcc5

Thanks, everyone!

Petr, please feel free to send me a patch you want me to check for you. Make sure I could apply it on top of the trunk.

compiler-rt/cmake/config-ix.cmake

 # Libraries.
 check_library_exists(dl dlopen "" COMPILER_RT_HAS_LIBDL)
 check_library_exists(rt shm_open "" COMPILER_RT_HAS_LIBRT)
 check_library_exists(m pow "" COMPILER_RT_HAS_LIBM)
 check_library_exists(pthread pthread_create "" COMPILER_RT_HAS_LIBPTHREAD)
 check_library_exists(execinfo backtrace "" COMPILER_RT_HAS_LIBEXECINFO)
 
 # Look for terminfo library, used in unittests that depend on LLVMSupport.
-if(LLVM_ENABLE_TERMINFO STREQUAL FORCE_ON)
-  set(MAYBE_REQUIRED REQUIRED)
-else()
-  set(MAYBE_REQUIRED)
-endif()
 if(LLVM_ENABLE_TERMINFO)
-  find_library(COMPILER_RT_TERMINFO_LIB NAMES terminfo tinfo curses ncurses ncursesw ${MAYBE_REQUIRED})
+  foreach(library terminfo tinfo curses ncurses ncursesw)
+    string(TOUPPER ${library} library_suffix)
+    check_library_exists(
+      ${library} setupterm "" COMPILER_RT_HAS_TERMINFO_${library_suffix})
+    if(COMPILER_RT_HAS_TERMINFO_${library_suffix})
+      set(COMPILER_RT_HAS_TERMINFO TRUE)
+      set(COMPILER_RT_TERMINFO_LIB "${library}")
+      break()
     endif()
-if(COMPILER_RT_TERMINFO_LIB)
-  set(LLVM_ENABLE_TERMINFO 1)
-else()
-  set(LLVM_ENABLE_TERMINFO 0)
+  endforeach()
 endif()
 
 if (ANDROID AND COMPILER_RT_HAS_LIBDL)
   # Android's libstdc++ has a dependency on libdl.
   list(APPEND CMAKE_REQUIRED_LIBRARIES dl)
 endif()
 check_library_exists(c++ __cxa_throw "" COMPILER_RT_HAS_LIBCXX)
 check_library_exists(stdc++ __cxa_throw "" COMPILER_RT_HAS_LIBSTDCXX)

compiler-rt/lib/xray/tests/CMakeLists.txt

 set(XRAY_UNITTEST_LINK_FLAGS
   ${COMPILER_RT_UNITTEST_LINK_FLAGS}
   ${CMAKE_THREAD_LIBS_INIT}
   )
 
 if (NOT APPLE)
   # Needed by LLVMSupport.
   append_list_if(
-    LLVM_ENABLE_TERMINFO
+    COMPILER_RT_HAS_TERMINFO
     -l${COMPILER_RT_TERMINFO_LIB} XRAY_UNITTEST_LINK_FLAGS)
 
   if (COMPILER_RT_STANDALONE_BUILD)
     append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
     append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LIBLIST} XRAY_UNITTEST_LINK_FLAGS)
     append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
       ${LLVM_TESTINGSUPPORT_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
     append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT

lldb/source/Core/CMakeLists.txt

 lldb_tablegen(CoreProperties.inc -gen-lldb-property-defs
   SOURCE CoreProperties.td
   TARGET LLDBCorePropertiesGen)
 
 lldb_tablegen(CorePropertiesEnum.inc -gen-lldb-property-enum-defs
   SOURCE CoreProperties.td
   TARGET LLDBCorePropertiesEnumGen)
 
 set(LLDB_CURSES_LIBS)
 set(LLDB_LIBEDIT_LIBS)
 
 if (LLDB_ENABLE_CURSES)
   list(APPEND LLDB_CURSES_LIBS ${CURSES_LIBRARIES} ${PANEL_LIBRARIES})
-  if(LLVM_ENABLE_TERMINFO)
-    list(APPEND LLDB_CURSES_LIBS ${TERMINFO_LIB})
+  if(LLVM_ENABLE_TERMINFO AND HAVE_TERMINFO)
+    list(APPEND LLDB_CURSES_LIBS ${TERMINFO_LIBS})
   endif()
   if (LLVM_BUILD_STATIC)
     list(APPEND LLDB_CURSES_LIBS gpm)
   endif()
 endif()
 
 add_lldb_library(lldbCore
   Address.cpp

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 STREQUAL FORCE_ON)
-      set(MAYBE_REQUIRED REQUIRED)
-    else()
-      set(MAYBE_REQUIRED)
-    endif()
     if(LLVM_ENABLE_TERMINFO)
-      find_library(TERMINFO_LIB NAMES terminfo tinfo curses ncurses ncursesw ${MAYBE_REQUIRED})
+      set(HAVE_TERMINFO 0)
+      foreach(library terminfo tinfo curses ncurses ncursesw)
+        string(TOUPPER ${library} library_suffix)
+        check_library_exists(${library} setupterm "" HAVE_TERMINFO_${library_suffix})
+        if(HAVE_TERMINFO_${library_suffix})
+          set(HAVE_TERMINFO 1)
+          set(TERMINFO_LIBS "${library}")
+          break()
         endif()
-    if(TERMINFO_LIB)
-      set(LLVM_ENABLE_TERMINFO 1)
+      endforeach()
     else()
-      set(LLVM_ENABLE_TERMINFO 0)
+      set(HAVE_TERMINFO 0)
     endif()
 
     find_library(ICONV_LIBRARY_PATH NAMES iconv libiconv libiconv-2 c)
     set(LLVM_LIBXML2_ENABLED 0)
     set(LIBXML2_FOUND 0)
     if((LLVM_ENABLE_LIBXML2) AND ((CMAKE_SYSTEM_NAME MATCHES "Linux") AND (ICONV_LIBRARY_PATH) OR APPLE))
       find_package(LibXml2)
       if (LIBXML2_FOUND)
         set(LLVM_LIBXML2_ENABLED 1)
         if ((CMAKE_OSX_SYSROOT) AND (EXISTS ${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR}))
           include_directories(${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR})
         else()
           include_directories(${LIBXML2_INCLUDE_DIR})
         endif()
       endif()
     endif()
-  else()
-    set(LLVM_ENABLE_TERMINFO 0)
   endif()
-else()
-  set(LLVM_ENABLE_TERMINFO 0)
 endif()
 
 if (LLVM_ENABLE_LIBXML2 STREQUAL "FORCE_ON" AND NOT LLVM_LIBXML2_ENABLED)
   message(FATAL_ERROR "Failed to congifure libxml2")
 endif()
 
 check_library_exists(xar xar_open "" HAVE_LIBXAR)
 if(HAVE_LIBXAR)

llvm/include/llvm/Config/config.h.cmake

 
 /* Define to 1 if you have the <sys/types.h> header file. */
 #cmakedefine HAVE_SYS_TYPES_H ${HAVE_SYS_TYPES_H}
 
 /* Define to 1 if you have the <sysexits.h> header file. */
 #cmakedefine HAVE_SYSEXITS_H ${HAVE_SYSEXITS_H}
 
 /* Define if the setupterm() function is supported this platform. */
-#cmakedefine LLVM_ENABLE_TERMINFO ${LLVM_ENABLE_TERMINFO}
+#cmakedefine HAVE_TERMINFO ${HAVE_TERMINFO}
 
 /* Define if the xar_open() function is supported this platform. */
 #cmakedefine HAVE_LIBXAR ${HAVE_LIBXAR}
 
 /* Define to 1 if you have the <termios.h> header file. */
 #cmakedefine HAVE_TERMIOS_H ${HAVE_TERMIOS_H}
 
 /* Define to 1 if you have the <unistd.h> header file. */

llvm/lib/Support/CMakeLists.txt

 if(LLVM_ENABLE_ZLIB)
   set(imported_libs ZLIB::ZLIB)
 endif()
 
-function(get_system_libname libpath libname)
-  get_filename_component(libpath ${libpath} NAME)
-  if( CMAKE_FIND_LIBRARY_PREFIXES )
-    string(REPLACE ";" "|" PREFIXES "${CMAKE_FIND_LIBRARY_PREFIXES}")
-    string(REGEX REPLACE "^(${PREFIXES})" "" libpath ${libpath})
-  endif()
-  if( CMAKE_FIND_LIBRARY_SUFFIXES )
-    string(REPLACE ";" "|" SUFFIXES "${CMAKE_FIND_LIBRARY_SUFFIXES}")
-    string(REGEX REPLACE "(${SUFFIXES})$" "" libpath ${libpath})
-  endif()
-  set(${libname} "${libpath}" PARENT_SCOPE)
-endfunction()
-
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
   set(system_libs ${system_libs} psapi shell32 ole32 uuid advapi32)
 elseif( CMAKE_HOST_UNIX )
   if( HAVE_LIBRT )
     set(system_libs ${system_libs} rt)
   endif()
   if( HAVE_LIBDL )
     set(system_libs ${system_libs} ${CMAKE_DL_LIBS})
   endif()
   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}")
+  if(LLVM_ENABLE_TERMINFO)
+    if(HAVE_TERMINFO)
+      set(system_libs ${system_libs} ${TERMINFO_LIBS})
+    endif()
   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)
   # CMAKE_BUILD_TYPE is only meaningful to single-configuration generators.
   if(CMAKE_BUILD_TYPE)
     string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
     get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
   endif()
   if(NOT zlib_library)
     get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
   endif()
-  get_system_libname(${zlib_library} zlib_library)
-  set(llvm_system_libs ${llvm_system_libs} "${zlib_library}")
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND CMAKE_STATIC_LIBRARY_SUFFIX AND
+      zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+    STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+    STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND CMAKE_SHARED_LIBRARY_SUFFIX AND
+      zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*${CMAKE_SHARED_LIBRARY_SUFFIX}$")
+    STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+    STRING(REGEX REPLACE "${CMAKE_SHARED_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
   endif()
-
-if(LLVM_ENABLE_TERMINFO)
-  get_system_libname(${TERMINFO_LIB} terminfo_library)
-  set(llvm_system_libs ${llvm_system_libs} "${terminfo_library}")
+  set(llvm_system_libs ${llvm_system_libs} "${zlib_library}")
 endif()
 
 set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${llvm_system_libs}")
 
 if(LLVM_WITH_Z3)
   target_include_directories(LLVMSupport SYSTEM
     PRIVATE
     ${Z3_INCLUDE_DIR}
     )
 endif()

llvm/lib/Support/Unix/Process.inc

 
 unsigned Process::StandardErrColumns() {
   if (!StandardErrIsDisplayed())
     return 0;
 
   return getColumns();
 }
 
-#ifdef LLVM_ENABLE_TERMINFO
+#ifdef HAVE_TERMINFO
 // We manually declare these extern functions because finding the correct
 // headers from various terminfo, curses, or other sources is harder than
 // writing their specs down.
 extern "C" int setupterm(char *term, int filedes, int *errret);
 extern "C" struct term *set_curterm(struct term *termp);
 extern "C" int del_curterm(struct term *termp);
 extern "C" int tigetnum(char *capname);
 #endif
 
-#ifdef LLVM_ENABLE_TERMINFO
+#ifdef HAVE_TERMINFO
 static ManagedStatic<std::mutex> TermColorMutex;
 #endif
 
 static bool terminalHasColors(int fd) {
-#ifdef LLVM_ENABLE_TERMINFO
+#ifdef HAVE_TERMINFO
   // First, acquire a global lock because these C routines are thread hostile.
   std::lock_guard<std::mutex> G(*TermColorMutex);
 
   int errret = 0;
   if (setupterm(nullptr, fd, &errret) != 0)
     // Regardless of why, if we can't get terminfo, we shouldn't try to print
     // colors.
     return false;

llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn

write_cmake_config("config") {
 
   if (llvm_enable_libxar) {
     values += [ "HAVE_LIBXAR=1" ]
   } else {
     values += [ "HAVE_LIBXAR=" ]
   }
 
   if (llvm_enable_terminfo) {
-    values += [ "LLVM_ENABLE_TERMINFO=1" ]
+    values += [ "HAVE_TERMINFO=1" ]
   } else {
-    values += [ "LLVM_ENABLE_TERMINFO=" ]
+    values += [ "HAVE_TERMINFO=" ]
   }
 
   if (llvm_enable_dia_sdk) {
     values += [ "LLVM_ENABLE_DIA_SDK=1" ]
   } else {
     values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }