[Support] Querying the terminal width on Linux is broken

[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 a3eb3d3, this was ‘essentially disabled’ in https://reviews.llvm.org/D61326, and a3eb3d3 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 a3eb3d3 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 ;Þ

[Sirraide]

I’m currently working on something diagnostics-related in Clang; the fix for this issue is currently on the branch for that too but I’ll split it out into a separate patch later.

[MaskRay]

When -fmessage-length is unspecified, the default behavior is derived from llvm::sys::Process::StandardErrColumns(), which reads the environment variable COLUMNS. COLUMNS is a variable in bash/zsh that is not exported. GCC uses getenv("COLUMNS") as well. I wonder whether reading the terminal width really makes sense. Perhaps we should just default to -fmessage-length=0 and delete the terminal width query? For most non-interative programs, their behaviors don't change by the terminal width

[Sirraide]

Perhaps we should just default to -fmessage-length=0 and delete the terminal width query? For most non-interative programs, their behaviors don't change by the terminal width

Actually knowing the terminal width is quite useful because it allows us to wrap long diagnostics in a sensible manner (by indenting continuation lines), which makes them quite a bit more readable; I’m currently working on a patch that adds support for visually nesting diagnotics, and poor line wrapping would be even worse for those than it is for our current diagnostics.

[MaskRay]

I've checked what rust does. Its --diagnostic-width has a default value from termize::dimensions https://docs.rs/termize/latest/src/termize/platform/unix.rs.html#86-94 Restoring the ioctl call LG.