-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
unix,tty: reopen /dev/tty for stdio fds #878
Conversation
if (uv__tty_is_slave(fd) && ttyname_r(fd, path, sizeof(path)) == 0) | ||
if (fd <= STDERR_FILENO) { | ||
r = uv__open_cloexec("/dev/tty", O_RDWR); | ||
} else if (uv__tty_is_slave(fd) && ttyname_r(fd, path, sizeof(path)) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename slave to follower so we won't get another shitstorm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Andrius, I'm not really in the mood for this crap. We use the terminology in man 4 pty
. End of the discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was merely a suggestion.
This will break writing to other ttys again:
I'm okay with that. Just be aware, that this adds another layer of unexpected behavior. |
Damn, we can't win here, can we? 😭 |
I think this fix is a good tradeoff. At least my usecases are fixed with libuv for now and you cannot break software in production. (Sorry, that I did) |
@Gottox It's not your fault in any way! OSX is really weird, /dev/tty cannot be used with kqueue whereas /dev/ttys* can, which creates some interesting side-effects. I'm not sure we can get away without any tradeoff, but this one seems reasonable to me. Maybe I should mention it in the docs too. @libuv/collaborators WDYT? |
This is an ugly hack. I think I caught a bug which was hiding in plain sight: #879 so basically any tty which was opened using the select() trick resulted in blocking writes, even if we could do them async anyway. With #879 the beahvior is consistent and we retain tty redirection working. I'd still like some more feedback on this whole thing, I might not be seeing the forest because of the threes. |
@saghul Do you intend to close this PR due to the findings in this discussion? |
Yep, let's close it now.
|
Refs: nodejs/node#6456 (comment)
R= @bnoordhuis, @Gottox