Lib/pty.py major revision #85984

8vasu · 2020-09-19T23:19:34Z

BPO	41818
Nosy	@gpshead, @pfmoore, @ronaldoussoren, @tjguk, @ned-deily, @encukou, @asvetlov, @zware, @koobs, @zooba, @kulikjak, @aeros, @8vasu
PRs	bpo-41818: Updated tests for the standard pty library #22962 bpo-41818: test_openpty succeed on Gentoo, don't expect to fail on this platform #23514 bpo-41818: Make test_openpty() avoid unexpected success due to number of rows and/or number of columns being == 0. #23526 bpo-41818: test_master_read() in test_pty.py should be expected to fail on every platform that is not Linux; that includes Solaris. #23533 bpo-41818: Fix test_master_read() so that it succeeds on all platforms that either raise OSError or return b"" upon reading from master #23536 bpo-41818: Make additions to the tty module #23546 bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686 bpo-41818: Add os.login_tty() for nix. #23740 bpo-41818: Close file descriptors in test_openpty #24119 bpo-41818: ++ termios versionadded markers. #27987 bpo-41818: Add os.login_tty() for nix. #29658

^{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:

assignee = None
closed_at = None
created_at = <Date 2020-09-19.23:19:34.173>
labels = ['OS-mac', '3.8', '3.9', '3.10', 'library', 'tests', 'OS-windows']
title = 'Lib/pty.py major revision'
updated_at = <Date 2021-11-20.10:06:35.452>
user = 'https://github.com/8vasu'

bugs.python.org fields:

activity = <Date 2021-11-20.10:06:35.452>
actor = 'soumendra'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'macOS', 'Tests', 'Windows', 'FreeBSD']
creation = <Date 2020-09-19.23:19:34.173>
creator = 'soumendra'
dependencies = []
files = []
hgrepos = []
issue_num = 41818
keywords = ['patch']
message_count = 22.0
messages = ['377195', '377198', '377202', '381830', '381839', '381843', '381845', '381847', '381852', '381870', '381922', '381930', '381941', '381965', '381968', '381980', '382016', '384408', '384413', '385257', '400389', '400393']
nosy_count = 13.0
nosy_names = ['gregory.p.smith', 'paul.moore', 'ronaldoussoren', 'tim.golden', 'ned.deily', 'petr.viktorin', 'asvetlov', 'zach.ware', 'koobs', 'steve.dower', 'kulikjak', 'aeros', 'soumendra']
pr_nums = ['22962', '23514', '23526', '23533', '23536', '23546', '23686', '23740', '24119', '27987', '29658']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue41818'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

Linked PRs

8vasu · 2020-09-19T23:19:33Z

The current pty library has the following issues:

Does not set slave termios. Documented in the source.
Does not set initial slave window size. Documented in the source. Does not handle SIGWINCH. See bpo-41494, bpo-41541. This is essential in the following practical scenarios: i. creating split windows/panes while using a terminal multiplexer; ii. when resizing GUI terminal emulator window, especially relevant when using tiling window managers; iii. resizing an ansi-term window created inside a GNU Emacs frame.
Does not perform signal handling. Signals must be blocked during sensitive portions of code.
Hangs on FreeBSD. See bpo-26228.
Includes deprecated functions pty.master_open(), pty.slave_open().
In pty.fork(), the fallback code should try using TIOCSCTTY first. It is still using the old method of opening a tty to make it the controlling tty. Currently even SysV based systems provide TIOCSCTTY. See https://stackoverflow.com/questions/51593530/code-explanation-for-glibc-login-tty-function-openttyname-immediately-f

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).

tty.setraw() is called right after tty.tcgetattr(). This increases redundancy of code because tty.setraw() itself makes an identical tty.tcgetattr() call.
Requires testing/porting to more platforms. Solaris, Illumos, macOS, Cygwin, etc. Windows ConPTY?
There should be an option in pty.spawn() to turn off slave's ECHO flag. For example, when it is being used in a pipe. See https://github.com/karelzak/util-linux/commit/1eee1acb245a8b724e441778dfa9b858465bf7e5 and https://github.com/karelzak/util-linux/commit/75ccd75a2fa1194c6415c47b0024a438e26f1ad7#diff-3834a3d25eeaf20d9d0dcb05a46995f6
Tests are incomplete. Tests consider OSes such as Tru64 but not {Free/Net/Open/...}BSD.

Find ongoing work here: https://github.com/8vasu/pypty2

aeros · 2020-09-20T00:51:33Z

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).

8vasu · 2020-09-20T02:51:39Z

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
terminology will find this easy to remember. Terminology such as parent/child and server/client might make it a little confusing.

asvetlov · 2020-11-25T13:41:29Z

New changeset c13d899 by Soumendra Ganguly in branch 'master':
bpo-41818: Updated tests for the standard pty library (GH-22962)
c13d899

vstinner · 2020-11-25T15:29:46Z

This change broke x86 Gentoo buildbots: bpo-42463.

asvetlov · 2020-11-25T16:05:33Z

#23514 has the fix, waiting for all buildbots finish before pressing "Merge" button.
Gentoo bots are green.

vstinner · 2020-11-25T16:24:32Z

#23514 has the fix, waiting for all buildbots finish before pressing "Merge" button.

Thanks for working on a fix :-)

vstinner · 2020-11-25T16:32:26Z

In addition to the above, if a major revision is made to pty, I'd suggest also addressing the issue of "master/slave" terminology

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.

asvetlov · 2020-11-25T17:06:20Z

New changeset 87f7ab5 by Andrew Svetlov in branch 'master':
bpo-41818: test_openpty succeed on Gentoo, don't expect to fail on this platform (GH-23514)
87f7ab5

zware · 2020-11-25T23:45:29Z

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())"

8vasu · 2020-11-26T23:49:47Z

The reproducer was helpful. #23526 should fix this issue.

kulikjak · 2020-11-27T08:36:20Z

This change also broke Solaris (SunOS), where (similarly to BSDs and Darwin) OSError is not raised in the new test_master_read() test.

Adding or PLATFORM == "SunOS" into the expectedFailureOnBSD function fixes this issue, but it's no longer just BSDs and it's derivates.

asvetlov · 2020-11-27T10:16:45Z

New changeset f5a19ea by Soumendra Ganguly in branch 'master':
bpo-41818: Make test_openpty() avoid unexpected success due to number of rows and/or number of columns being == 0. (GH-23526)
f5a19ea

8vasu · 2020-11-27T20:50:48Z

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.

8vasu · 2020-11-27T22:01:04Z

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.

8vasu · 2020-11-28T06:45:10Z

Update: I closed PR 23533. PR 23536 is much better. It will help us detect exact behavior on each platform, which is necessary for making pty.spawn() successfully exit its copy loop.

asvetlov · 2020-11-28T21:04:29Z

New changeset 74311ae by Soumendra Ganguly in branch 'master':
bpo-41818: Fix test_master_read() so that it succeeds on all platforms that either raise OSError or return b"" upon reading from master (GH-23536)
74311ae

encukou · 2021-01-05T15:34:39Z

I noticed an issue in one of the newly added tests; see #68307

8vasu · 2021-01-05T16:28:34Z

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?

encukou · 2021-01-19T13:03:20Z

New changeset 65cf1ad by Petr Viktorin in branch 'master':
bpo-41818: Close file descriptors in test_openpty (#GH-24119)
65cf1ad

gpshead · 2021-08-27T02:56:33Z

New changeset ae224bb by Soumendra Ganguly in branch 'main':
bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). (GH-23686)
ae224bb

gpshead · 2021-08-27T03:29:38Z

New changeset 245f1f2 by Gregory P. Smith in branch 'main':
bpo-41818: ++ termios versionadded markers. (GH-27987)
245f1f2

gpshead · 2022-05-05T16:05:43Z

#29658 merged to add os.login_tty(fd) in 3.11.

Refactored the implementation of pty.fork to use os.login_tty. A DeprecationWarning is now raised by pty.master_open() and pty.slave_open(). They were undocumented and deprecated long long ago in the docstring in favor of pty.openpty. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>

* main: (82 commits) pythongh-101670: typo fix in PyImport_ExtendInittab() (python#101723) pythonGH-99293: Document that `Py_TPFLAGS_VALID_VERSION_TAG` shouldn't be used. (#pythonGH-101736) no-issue: Add Dong-hee Na as the cjkcodecs codeowner (pythongh-101731) pythongh-101678: Merge math_1_to_whatever() and math_1() (python#101730) pythongh-101678: refactor the math module to use special functions from c11 (pythonGH-101679) pythongh-85984: Remove legacy Lib/pty.py code. (python#92365) pythongh-98831: Use opcode metadata for stack_effect() (python#101704) pythongh-101283: Version was just released, so should be changed in 3.11.3 (pythonGH-101719) pythongh-101283: Fix use of unbound variable (pythonGH-101712) pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286) pythongh-101277: Port more itertools static types to heap types (python#101304) pythongh-98831: Modernize CALL and family (python#101508) pythonGH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (python#101697) pythongh-100221: Fix creating dirs in `make sharedinstall` (pythonGH-100329) pythongh-101670: typo fix in PyImport_AppendInittab() (pythonGH-101672) pythongh-101196: Make isdir/isfile/exists faster on Windows (pythonGH-101324) pythongh-101614: Don't treat python3_d.dll as a Python DLL when checking extension modules for incompatibility (pythonGH-101615) pythongh-100933: Improve `check_element` helper in `test_xml_etree` (python#100934) pythonGH-101578: Normalize the current exception (pythonGH-101607) pythongh-47937: Note that Popen attributes are read-only (python#93070) ...

…101831) Utilize new functions termios.tcgetwinsize() and termios.tcsetwinsize in test_pty.py. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>

* main: pythongh-101810: Remove duplicated st_ino calculation (pythonGH-101811) pythongh-92547: Purge sqlite3_enable_shared_cache() detection from configure (python#101873) pythonGH-100987: Refactor `_PyInterpreterFrame` a bit, to assist generator improvement. (pythonGH-100988) pythonGH-87849: Simplify stack effect of SEND and specialize it for generators and coroutines. (pythonGH-101788) Correct trivial grammar in reset_mock docs (python#101861) pythongh-101845: pyspecific: Fix i18n for availability directive (pythonGH-101846) pythongh-89792: Limit test_tools freeze test build parallelism based on the number of cores (python#101841) pythongh-85984: Utilize new "winsize" functions from termios in pty tests. (python#101831) pythongh-89792: Prevent test_tools from copying 1000M of "source" in freeze test (python#101837) Fix typo in test_fstring.py (python#101823) pythonGH-101797: allocate `PyExpat_CAPI` capsule on heap (python#101798) pythongh-101390: Fix docs for `imporlib.util.LazyLoader.factory` to properly call it a class method (pythonGH-101391)

8vasu mannequin added 3.8 only security fixes 3.9 only security fixes 3.10 bug and security fixes stdlib Python modules in the Lib dir OS-mac tests Tests in the Lib/test dir OS-windows labels Sep 19, 2020

ezio-melotti transferred this issue from another repository Apr 10, 2022

bedevere-bot mentioned this issue Oct 7, 2022

gh-85984: Remove legacy Lib/pty.py code. #92365

Merged

This was referenced Feb 11, 2023

gh-85984: Utilize new "winsize" functions from termios in pty tests. #101831

Merged

gh-85984: New additions and improvements to the tty library. #101832

Draft

gh-85984: Major revision of the pty library. #101833

Draft

bedevere-bot mentioned this issue Mar 4, 2023

gh-85984: Add POSIX pseudo-terminal functions. #102413

Open

8vasu mentioned this issue Mar 7, 2023

[PATCH] Make pty.spawn set window size #85713

Open

Lib/pty.py major revision #85984

Lib/pty.py major revision #85984

8vasu mannequin commented Sep 19, 2020 •

edited by bedevere-bot

8vasu mannequin commented Sep 19, 2020

aeros commented Sep 20, 2020

8vasu mannequin commented Sep 20, 2020

asvetlov commented Nov 25, 2020

vstinner commented Nov 25, 2020

asvetlov commented Nov 25, 2020

vstinner commented Nov 25, 2020

vstinner commented Nov 25, 2020

asvetlov commented Nov 25, 2020

zware commented Nov 25, 2020

8vasu mannequin commented Nov 26, 2020

kulikjak mannequin commented Nov 27, 2020

asvetlov commented Nov 27, 2020

8vasu mannequin commented Nov 27, 2020

8vasu mannequin commented Nov 27, 2020

8vasu mannequin commented Nov 28, 2020

asvetlov commented Nov 28, 2020

encukou commented Jan 5, 2021

8vasu mannequin commented Jan 5, 2021

encukou commented Jan 19, 2021

gpshead commented Aug 27, 2021

gpshead commented Aug 27, 2021

gpshead commented May 5, 2022

Lib/pty.py major revision #85984

Lib/pty.py major revision #85984

Comments

8vasu mannequin commented Sep 19, 2020 • edited by bedevere-bot

Linked PRs

8vasu mannequin commented Sep 19, 2020

aeros commented Sep 20, 2020

8vasu mannequin commented Sep 20, 2020

asvetlov commented Nov 25, 2020

vstinner commented Nov 25, 2020

asvetlov commented Nov 25, 2020

vstinner commented Nov 25, 2020

vstinner commented Nov 25, 2020

asvetlov commented Nov 25, 2020

zware commented Nov 25, 2020

8vasu mannequin commented Nov 26, 2020

kulikjak mannequin commented Nov 27, 2020

asvetlov commented Nov 27, 2020

8vasu mannequin commented Nov 27, 2020

8vasu mannequin commented Nov 27, 2020

8vasu mannequin commented Nov 28, 2020

asvetlov commented Nov 28, 2020

encukou commented Jan 5, 2021

8vasu mannequin commented Jan 5, 2021

encukou commented Jan 19, 2021

gpshead commented Aug 27, 2021

gpshead commented Aug 27, 2021

gpshead commented May 5, 2022

8vasu mannequin commented Sep 19, 2020 •

edited by bedevere-bot