[PATCH] D42274: [Support] Remove the terminfo dependency and rely on TERM

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 23 16:06:19 PST 2018


This is awesome!

Petr Hosek via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> phosek created this revision.
> phosek added a reviewer: jyknight.
> Herald added subscribers: llvm-commits, mgorny.
>
> 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
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D42274
>
> Files:
>   CMakeLists.txt
>   cmake/config-ix.cmake
>   cmake/modules/LLVMConfig.cmake.in
>   include/llvm/Config/config.h.cmake
>   lib/Support/CMakeLists.txt
>   lib/Support/Unix/Process.inc
>
> Index: lib/Support/Unix/Process.inc
> ===================================================================
> --- lib/Support/Unix/Process.inc
> +++ lib/Support/Unix/Process.inc
> @@ -321,57 +321,13 @@
>    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;
>  }
>  
> Index: lib/Support/CMakeLists.txt
> ===================================================================
> --- lib/Support/CMakeLists.txt
> +++ lib/Support/CMakeLists.txt
> @@ -15,11 +15,6 @@
>    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()
> Index: include/llvm/Config/config.h.cmake
> ===================================================================
> --- include/llvm/Config/config.h.cmake
> +++ include/llvm/Config/config.h.cmake
> @@ -253,9 +253,6 @@
>  /* 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}
>  
> Index: cmake/modules/LLVMConfig.cmake.in
> ===================================================================
> --- cmake/modules/LLVMConfig.cmake.in
> +++ cmake/modules/LLVMConfig.cmake.in
> @@ -31,8 +31,6 @@
>  
>  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@)
> Index: cmake/config-ix.cmake
> ===================================================================
> --- cmake/config-ix.cmake
> +++ cmake/config-ix.cmake
> @@ -151,20 +151,6 @@
>      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)
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt
> +++ CMakeLists.txt
> @@ -349,8 +349,6 @@
>  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)
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list