-
-
Notifications
You must be signed in to change notification settings - Fork 256
Add StartWithAttrs to allow bypassing setsid/setctty #97
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
Conversation
Signed-off-by: Guillaume J. Charmes <git+guillaume@charmes.net>
How would aerc need to use this? |
Thanks - 943a9a2 seems to fix the problem I was seeing. Tested against Go |
@ddevault I submitted a patch here https://lists.sr.ht/~sircmpwn/aerc/patches/10481 it fixes the pty-example case. Please let me know if you find more issues. |
Thanks for the patch! |
Is the plan to get this merged soon-ish? |
Will merge and publish new tag tomorrow. |
Thanks. Just noting Ian's reply in #96 (comment) |
I considered passing the tty as ExtraFiles, or even setting Ctty to stdout when stdin is overridden, but as the change is relatively recent and splitting a tty in part if usually a bad idea, I think it is better to revert the main helper to the original behavior. Edge cases needing to do that can use the new helper or directly call |
Soves an issue with go1.15 not letting ctty be a parent. See creack/pty#97 for more details. Signed-off-by: Guillaume J. Charmes <git+guillaume@charmes.net>
Starting with golang 1.5, setting Ctty value result in `Setctty set but Ctty not valid in child` error, as part of golang/go#29458 . This commit lifts the fix in creack/pty#97 .
Upgrade to golang 1.15 Starting with golang 1.5, setting Ctty value result in `Setctty set but Ctty not valid in child` error, as part of golang/go#29458 . This commit lifts the fix in creack/pty#97 .
Revert #75 but don't change the "old" behavior of
pty.Start
andpty.StartWithSize
.Introduce
pty.StartWithAttr
to explicitly set process attributes, which allow for pty to be created without setsid/setctty, fixing the issue faced by aerc.Fixes error with the new go1.15 behavior introduced in https://go-review.googlesource.com/c/go/+/231638/ ( golang/go#29458 ).
Fixes #96
cc @kr @ddevault @myitcv