| Issue |
139499
|
| Summary |
[Support] Querying the terminal width on Linux is broken
|
| Labels |
llvm:support
|
| Assignees |
|
| Reporter |
Sirraide
|
The Linux implementation of `Process::StandardOutColumns()`/`Process::StandardErrColumns()` (or, really, the underlying implementation of `getColumns()` in `Support/Unix/Process.inc`) seems wrong to me and doesn’t work on my system (Fedora 42) at least: both functions always return `0`. >From looking at the commit history around those functions, we used to query this via `ioctl()` with `TIOCGWINSZ`, but according to a3eb3d3d, this was ‘essentially disabled’ in https://reviews.llvm.org/D61326, and a3eb3d3d then removed the functionality entirely. Now, we rely exclusively on the ‘environment variable’ `COLUMNS` for this.
The problem with this is that `COLUMNS` is apparently not really an ‘environment variable’ but rather a shell variable, i.e. while accessible inside the shell (e.g. `echo $COLUMNS` works), it is not exported to any child processes spawned by that shell (at least not consistently on all systems). This is the case for instance on my system (Fedora 42) and I also tested it on a Debian 12 server; in both cases, running `std::getenv("COLUMNS")` always returns `nullptr`, for every combination of shell+terminal available to me.
As an aside, while a3eb3d3d also mentions that ‘Nobody has complained for one year’ wrt us relying on `COLUMNS` for this, there is exactly 1 combined use of these functions in the entire monorepo (and no tests for that matter but this also seems hard to test): the Clang driver calls `StandardErrColumns()` to try and determine a sensible error message length if `-fmessage-length` isn’t passed, which as a result is also very noticeably broken on my system. Interestingly, LLDB on Linux actually just uses `ioctl()` to figure out the terminal width and doesn’t bother calling either of these helpers.
I think we should undo these changes on some capacity and fall back on `ioctl()` if it is available and if `COLUMNS` is not defined. https://reviews.llvm.org/D61326 mentions that this functionality was disabled ‘for POSIX compatibility’, but I think we should still be able to make use of this on *some* systems (termios.h is part of the POSIX standard, and afaik `sys/ioctl.h` is always available on Linux at least). That said, I’m not an export on platform-dependent stuff like this so maybe there are issues with that, but the fact remains that these functions are just broken on some systems...
CC @MaskRay, @Endilll because iirc you’re familiar with this topic and @hubert-reinterpretcast because you reviewed D61326, but admittedly that was 6 years ago so I wouldn’t be surprised if you don’t remember much about this ;Þ
![]()
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
