[Support] Remove the terminfo dependency and rely on TERM

[phosek]

Summary

The current implementation of terminal color support that relies on
terminfo is broken and also introduces additional dependency. This
change removes this implementation and replaces it with a simpler
mechanism that detects the terminal color support by checking the value
of the TERM environment variable. This is what other GNU tools do as
well. If it's good enough for them, it's good enough for us too, and we
don't need to extra complexity and dependencies

[jyknight]

I'm totally in favor of this, it looks like a great improvement.

But of course nbjoerg will complain because it will break all the people who run clang outputting to Tektronix 4012 graphics terminals which can't parse vt100/ansi escape sequence syntax. And I don't have the will to argue the point.

cmake/config-ix.cmake

154

This stanza, and the option declaration above is still used by lldb's cmake file I believe.

[srhines]

https://github.com/android-ndk/ndk/issues/574 shows that this is becoming more of a problem for Android's users as well. I'm willing to +1 the ideas here too. It would be better to just not depend on these other libraries at all for general LLVM.

[rnk]

@thakis is disabling this logic in the Chromium build of Clang as well: https://chromium-review.googlesource.com/c/chromium/src/+/1025648

Maybe we should reconsider reverting this terminfo stuff and going back to TERM. @chandlerc?

[thakis]

Background: This was added in https://reviews.llvm.org/rL187874 ; from what I understand because someone google-internally who really doesn't like colored output complained that their existing setup for disabling colors in all programs wasn't honored by clang. It regressed color output without curses until r322962 finally restored that functionality. I agree that undoing this would be a good change in general, but said person might be unhappy about it. Maybe we can at least flip the default to false, but just removing it altogether if that's what gcc does these days seems very reasonable to me.

[mgorny]

In D42274#982263, @jyknight wrote:

But of course nbjoerg will complain because it will break all the people who run clang outputting to Tektronix 4012 graphics terminals which can't parse vt100/ansi escape sequence syntax. And I don't have the will to argue the point.

For the record, incompatible escape sequences are not limited to 'fringe hardware'. A commonly used software that doesn't exactly match others is GNU screen.

CMakeLists.txt

 
 option(LLVM_ENABLE_FFI "Use libffi to call external functions from the interpreter" OFF)
 set(FFI_LIBRARY_DIR "" CACHE PATH "Additional directory, where CMake should search for libffi.so")
 set(FFI_INCLUDE_DIR "" CACHE PATH "Additional directory, where CMake should search for ffi.h or ffi/ffi.h")
 
 set(LLVM_TARGET_ARCH "host"
   CACHE STRING "Set target to use for LLVM JIT or use \"host\" for automatic detection.")
 
-option(LLVM_ENABLE_TERMINFO "Use terminfo database if available." ON)
-
 set(LLVM_ENABLE_LIBXML2 "ON" CACHE STRING "Use libxml2 if available. Can be ON, OFF, or FORCE_ON")
 
 option(LLVM_ENABLE_LIBEDIT "Use libedit if available." ON)
 
 option(LLVM_ENABLE_THREADS "Use threads if available." ON)
 
 option(LLVM_ENABLE_ZLIB "Use zlib for compression/decompression if available." ON)
 

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)
-      set(HAVE_TERMINFO 0)
-      foreach(library tinfo terminfo 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()
-      endforeach()
-    else()
-      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)

cmake/modules/LLVMConfig.cmake.in

 set(LLVM_ABI_BREAKING_CHECKS @LLVM_ABI_BREAKING_CHECKS@)
 
 set(LLVM_ENABLE_ASSERTIONS @LLVM_ENABLE_ASSERTIONS@)
 
 set(LLVM_ENABLE_EH @LLVM_ENABLE_EH@)
 
 set(LLVM_ENABLE_RTTI @LLVM_ENABLE_RTTI@)
 
-set(LLVM_ENABLE_TERMINFO @LLVM_ENABLE_TERMINFO@)
-
 set(LLVM_ENABLE_THREADS @LLVM_ENABLE_THREADS@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
 
 set(LLVM_ENABLE_DIA_SDK @LLVM_ENABLE_DIA_SDK@)
 
 set(LLVM_NATIVE_ARCH @LLVM_NATIVE_ARCH@)
 

include/llvm/Config/config.h.cmake

 #cmakedefine HAVE_SYS_TIME_H ${HAVE_SYS_TIME_H}
 
 /* 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 <sys/uio.h> header file. */
 #cmakedefine HAVE_SYS_UIO_H ${HAVE_SYS_UIO_H}
 
-/* Define if the setupterm() function is supported this platform. */
-#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 the system has the type `uint64_t'. */
 #cmakedefine HAVE_UINT64_T ${HAVE_UINT64_T}

lib/Support/CMakeLists.txt

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 )
     set(system_libs ${system_libs} ${Backtrace_LIBRARIES})
   endif()
-  if(LLVM_ENABLE_TERMINFO)
-    if(HAVE_TERMINFO)
-      set(system_libs ${system_libs} ${TERMINFO_LIBS})
-    endif()
-  endif()
   if( LLVM_ENABLE_THREADS AND HAVE_LIBATOMIC )
     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()
 endif( MSVC OR MINGW )

lib/Support/Unix/Process.inc

 
 unsigned Process::StandardErrColumns() {
   if (!StandardErrIsDisplayed())
     return 0;
 
   return getColumns(2);
 }
 
-#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 HAVE_TERMINFO
-static ManagedStatic<sys::Mutex> TermColorMutex;
-#endif
-
 static bool terminalHasColors(int fd) {
-#ifdef HAVE_TERMINFO
-  // First, acquire a global lock because these C routines are thread hostile.
-  MutexGuard 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;
-
-  // Test whether the terminal as set up supports color output. How to do this
-  // isn't entirely obvious. We can use the curses routine 'has_colors' but it
-  // would be nice to avoid a dependency on curses proper when we can make do
-  // with a minimal terminfo parsing library. Also, we don't really care whether
-  // the terminal supports the curses-specific color changing routines, merely
-  // if it will interpret ANSI color escape codes in a reasonable way. Thus, the
-  // strategy here is just to query the baseline colors capability and if it
-  // supports colors at all to assume it will translate the escape codes into
-  // whatever range of colors it does support. We can add more detailed tests
-  // here if users report them as necessary.
-  //
-  // The 'tigetnum' routine returns -2 or -1 on errors, and might return 0 if
-  // the terminfo says that no colors are supported.
-  bool HasColors = tigetnum(const_cast<char *>("colors")) > 0;
-
-  // Now extract the structure allocated by setupterm and free its memory
-  // through a really silly dance.
-  struct term *termp = set_curterm(nullptr);
-  (void)del_curterm(termp); // Drop any errors here.
-
-  // Return true if we found a color capabilities for the current terminal.
-  if (HasColors)
-    return true;
-#endif
-
-  // Otherwise, be conservative.
+  if (const char *term = std::getenv("TERM")) {
+    // Most modern terminals support ANSI escape sequences for colors.
+    // The user can always ask for no colors by setting TERM to dumb, or
+    // using a commandline flag.
+    return strcmp(term, "dumb") != 0;
+  }
   return false;
 }
 
 bool Process::FileDescriptorHasColors(int fd) {
   // A file descriptor has colors if it is displayed and the terminal has
   // colors.
   return FileDescriptorIsDisplayed(fd) && terminalHasColors(fd);
 }