[llvm] [LLVM] [Support] Query the terminal width using the termios API (PR #143514)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: None (Sirraide)

<details>
<summary>Changes</summary>

On unix systems, we were trying to determine the terminal width using the `COULMNS` environment variable. Unfortunately, `COLUMNS` is not a proper environment variable, but rather a shell variable and a such is not exported to actual processes (even though it *is* available in e.g. shell scripts) on some systems, including mine.

We were previously using the termios API for this, but we stopped doing that, apparently because it wasn’t fully cross-platform. As for what systems that are, I’m not sure. I’d expect it to be available on every system that provides both `<sys/ioctl.h>` and `<termios.h>`, but if that isn’t the case, then we can also just add e.g. `&& defined(__linux__)` because that’d fix this at least for some systems.

This essentially reverts a3eb3d3d92d037fe3c9deaad87f6fc42fe9ea766 and parts of https://reviews.llvm.org/D61326.

For more information, see #<!-- -->139499.

Fixes #<!-- -->139499.

---
Full diff: https://github.com/llvm/llvm-project/pull/143514.diff


3 Files Affected:

- (modified) llvm/cmake/config-ix.cmake (+10) 
- (modified) llvm/include/llvm/Config/config.h.cmake (+6) 
- (modified) llvm/lib/Support/Unix/Process.inc (+22-6) 


``````````diff
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index 687f5077cbfd2..319b0c0267005 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -24,6 +24,8 @@ if (ANDROID OR CYGWIN OR CMAKE_SYSTEM_NAME MATCHES "AIX|DragonFly|FreeBSD|Haiku|
   set(HAVE_SYS_MMAN_H 1)
   set(HAVE_SYSEXITS_H 1)
   set(HAVE_UNISTD_H 1)
+  set(HAVE_SYS_IOCTL_H 1)
+  set(HAVE_TERMIOS_H 1)
 elseif (APPLE)
   set(HAVE_MACH_MACH_H 1)
   set(HAVE_MALLOC_MALLOC_H 1)
@@ -31,6 +33,8 @@ elseif (APPLE)
   set(HAVE_SYS_MMAN_H 1)
   set(HAVE_SYSEXITS_H 1)
   set(HAVE_UNISTD_H 1)
+  set(HAVE_SYS_IOCTL_H 1)
+  set(HAVE_TERMIOS_H 1)
 elseif (PURE_WINDOWS)
   set(HAVE_MACH_MACH_H 0)
   set(HAVE_MALLOC_MALLOC_H 0)
@@ -38,6 +42,8 @@ elseif (PURE_WINDOWS)
   set(HAVE_SYS_MMAN_H 0)
   set(HAVE_SYSEXITS_H 0)
   set(HAVE_UNISTD_H 0)
+  set(HAVE_SYS_IOCTL_H 0)
+  set(HAVE_TERMIOS_H 0)
 elseif (ZOS)
   # Confirmed in
   # https://github.com/llvm/llvm-project/pull/104706#issuecomment-2297109613
@@ -47,6 +53,8 @@ elseif (ZOS)
   set(HAVE_SYS_MMAN_H 1)
   set(HAVE_SYSEXITS_H 0)
   set(HAVE_UNISTD_H 1)
+  set(HAVE_SYS_IOCTL_H 1)
+  set(HAVE_TERMIOS_H 1)
 else()
   # Other platforms that we don't promise support for.
   check_include_file(mach/mach.h HAVE_MACH_MACH_H)
@@ -55,6 +63,8 @@ else()
   check_include_file(sys/mman.h HAVE_SYS_MMAN_H)
   check_include_file(sysexits.h HAVE_SYSEXITS_H)
   check_include_file(unistd.h HAVE_UNISTD_H)
+  check_include_file(sys/ioctl.h HAVE_SYS_IOCTL_H)
+  check_include_file(termios.h HAVE_TERMIOS_H)
 endif()
 
 if( UNIX AND NOT (APPLE OR BEOS OR HAIKU) )
diff --git a/llvm/include/llvm/Config/config.h.cmake b/llvm/include/llvm/Config/config.h.cmake
index 06d4756397911..3f0fa843974d2 100644
--- a/llvm/include/llvm/Config/config.h.cmake
+++ b/llvm/include/llvm/Config/config.h.cmake
@@ -164,6 +164,12 @@
 /* Define to 1 if you have the <sys/mman.h> header file. */
 #cmakedefine HAVE_SYS_MMAN_H ${HAVE_SYS_MMAN_H}
 
+/* Define to 1 if you have the <sys/ioctl.h> header file. */
+#cmakedefine HAVE_SYS_IOCTL_H ${HAVE_SYS_IOCTL_H}
+
+/* Define to 1 if you have the <termios.h> header file. */
+#cmakedefine HAVE_TERMIOS_H ${HAVE_TERMIOS_H}
+
 /* Define to 1 if stat struct has st_mtimespec member .*/
 #cmakedefine HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC ${HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC}
 
diff --git a/llvm/lib/Support/Unix/Process.inc b/llvm/lib/Support/Unix/Process.inc
index b5c3719f57963..c4ffa5b2749bd 100644
--- a/llvm/lib/Support/Unix/Process.inc
+++ b/llvm/lib/Support/Unix/Process.inc
@@ -34,6 +34,12 @@
 #ifdef HAVE_GETAUXVAL
 #include <sys/auxv.h>
 #endif
+#ifdef HAVE_SYS_IOCTL_H
+#include <sys/ioctl.h>
+#endif
+#ifdef HAVE_TERMIOS_H
+#include <termios.h>
+#endif
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only generic UNIX code that
@@ -304,7 +310,7 @@ bool Process::FileDescriptorIsDisplayed(int fd) {
 #endif
 }
 
-static unsigned getColumns() {
+static unsigned getColumns([[maybe_unused]] int FileID) {
   // If COLUMNS is defined in the environment, wrap to that many columns.
   if (const char *ColumnsStr = std::getenv("COLUMNS")) {
     int Columns = std::atoi(ColumnsStr);
@@ -312,23 +318,33 @@ static unsigned getColumns() {
       return Columns;
   }
 
-  // We used to call ioctl TIOCGWINSZ to determine the width. It is considered
-  // unuseful.
-  return 0;
+  // Unfortunately, COLUMNS is a shell variable, not a proper environment
+  // variable, so it is often not available; query the column count via
+  // the termios API instead if it isn't.
+  unsigned Columns = 0;
+
+#if defined(HAVE_SYS_IOCTL_H) && defined(HAVE_TERMIOS_H)
+  // Try to determine the width of the terminal.
+  struct winsize ws;
+  if (ioctl(FileID, TIOCGWINSZ, &ws) == 0)
+    Columns = ws.ws_col;
+#endif
+
+  return Columns;
 }
 
 unsigned Process::StandardOutColumns() {
   if (!StandardOutIsDisplayed())
     return 0;
 
-  return getColumns();
+  return getColumns(0);
 }
 
 unsigned Process::StandardErrColumns() {
   if (!StandardErrIsDisplayed())
     return 0;
 
-  return getColumns();
+  return getColumns(1);
 }
 
 static bool terminalHasColors() {

``````````

</details>


https://github.com/llvm/llvm-project/pull/143514