Skip to content

Conversation

@ChrisDryden
Copy link
Collaborator

@ChrisDryden ChrisDryden commented Dec 28, 2025

Detailed comments of how this came up in #8608

It turns out that the cause of a bunch of bugs that we have been seeing with stty compared to GNU have been caused by us defaulting to /dev/tty and stdout instead of stdin. This shows up in the stty -g output being different, differences in error messaging and differences related to how stty is able to parse piped input.

I added a bunch of integration test cases from the original errors: #8016 to make sure that we aren't regressing with this change and to address the reason this code was added in the first place.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/tty/tty-eof. tests/tty/tty-eof is passing on 'main'. Maybe you have to rebase?

@matthiasblaesing
Copy link

Thanks for working on this. I wanted to have a look at this myself and basicly came up with the same approach (drop opening /dev/tty and substitute stdout with stdin). The changes look sane to me.

I tested this with Ubuntu 25.10 live and the rebuild stty more or less works. More or less comes from the fact, that running stty fails with panics in many cases. What I did was start NetBeans on an Ubuntu 25.10 live run, replace stty and then try the terminal. I see error output from it, but terminal is better setup than with the base revision (col/row settings are updated now).

I think the problem I'm seeing is #8474 / nix-rust/nix#2672.

@ChrisDryden
Copy link
Collaborator Author

TIL that the error message in this case can be three separate things based on whether its musl, android:
https://android.googlesource.com/platform/bionic/%2B/50080a2/libc/bionic/strerror.cpp#72
https://github.com/runtimejs/musl-libc/blob/master/src/errno/__strerror.h#L11

It makes it look like I'm hardcoding the error message but actually its all based on the ENOTTY localization being so different

@ChrisDryden ChrisDryden marked this pull request as ready for review December 30, 2025 19:48
@sylvestre sylvestre merged commit c8790e6 into uutils:main Dec 31, 2025
128 of 129 checks passed
@matthiasblaesing
Copy link

@ChrisDryden thanks for taking care. I'll test again once a fix for #8474 is in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants