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
Lib/pty.py major revision #85984
Comments
The current pty library has the following issues:
The current version of pty.spawn() uses pty.fork() internally. However, pty.fork() closes slave and only returns (pid, master_fd). To update winsize, access to slave is necessary. Further, slave termios must be properly set. The proposed modifications do this by implementing a login_tty(3) based function ( tty.login() ), and using that in pty.spawn() instead of pty.fork(). tty.login() tries TIOCSCTTY before falling back to the old SysV method because Python currently does not provide an interface to the native login_tty(3).
Find ongoing work here: https://github.com/8vasu/pypty2 |
In addition to the above, if a major revision is made to pty, I'd suggest also addressing the issue of "master/slave" terminology, and replace it with something comparable like "parent/child". There's an open devguide issue (python/devguide#605) to more explicitly state terms to avoid, and support for avoiding usage of "slave/master" seems uncontroversial (especially in any new code). |
Makes sense. I will happily make a change of terminology in the pypty2 repository after the most desirable alternative is determined based on the choice of the majority. I think 'mother/son' sounds cute while still retaining the same initials as before; people used to the older |
This change broke x86 Gentoo buildbots: bpo-42463. |
#23514 has the fix, waiting for all buildbots finish before pressing "Merge" button. |
Thanks for working on a fix :-) |
In bpo-34605, I chose to leave the pty module unchange since it *seems* like "master_fd" and "slave_fd" is part of the API. See my rejected PR 9100. More recently, I discussed with a glibc maintainer who is open to change the openpty() manual page to avoid "master" and "slave" terms. In fact, these terms are not part of the API. They are just variable names in a manual page. "parent" and "child" terms would work here. I'm not sure of the exact relationship between the two file descriptors. |
Moving my notes from PR23514 to here, the issue that that PR addressed is not Gentoo-specific; here's a simple reproducer on Ubuntu: ./python -c "import pty;pty.spawn('./python -m test -v test_pty'.split())" |
The reproducer was helpful. #23526 should fix this issue. |
This change also broke Solaris (SunOS), where (similarly to BSDs and Darwin) OSError is not raised in the Adding |
This is actually good news. I had not tested the code on Solaris; this confirms my suspicion that Linux is the only platform that "behaves differently". Sadly, the current Lib/pty.py code depends on such "different behavior" to exit from pty.spawn()'s copy loop, which is why it hangs on the BSDs, macOS, and now we know that it (probably) also hangs on Solaris. Adding 'or PLATFORM == "SunOS"' is the correct thing to do. I am working on this now. |
PR-23533 should fix the test_master_read() issue on Solaris. Actually, instead of adding 'or PLATFORM == "SunOS"', I ended up removing the whole line and replaced it with 'if platform.system() != "Linux"' because I expect that test to fail on every platform that is not Linux. |
I noticed an issue in one of the newly added tests; see #68307 |
Thank you for the fix. That test was created by modifying an existing test which already had that issue; it is documented in a comment by user nnorwitz in the file. If your solution resolves the problem for all the tests in "class PtyTest", then can you please also remove that comment? |
#29658 merged to add |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: