[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +27900
pull_request: https://github.com/python/cpython/pull/29658

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset 245f1f260577a005fd631144b4377febef0b47ed by Gregory P. Smith in 
branch 'main':
bpo-41818: ++ termios versionadded markers. (GH-27987)
https://github.com/python/cpython/commit/245f1f260577a005fd631144b4377febef0b47ed


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Gregory P. Smith]

Change by Gregory P. Smith :


--
pull_requests: +26434
pull_request: https://github.com/python/cpython/pull/27987

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset ae224bb566301d3602e9b090e37c1dcf5a48c914 by Soumendra Ganguly in 
branch 'main':
bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). (GH-23686)
https://github.com/python/cpython/commit/ae224bb566301d3602e9b090e37c1dcf5a48c914


--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Petr Viktorin]

Petr Viktorin  added the comment:


New changeset 65cf1ad6723b6b4489fa7dda04283bb2466be531 by Petr Viktorin in 
branch 'master':
bpo-41818: Close file descriptors in test_openpty (#GH-24119)
https://github.com/python/cpython/commit/65cf1ad6723b6b4489fa7dda04283bb2466be531


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Soumendra Ganguly  added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Petr Viktorin]

Petr Viktorin  added the comment:

I noticed an issue in one of the newly added tests; see GH-24119

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Petr Viktorin]

Change by Petr Viktorin :


--
nosy: +petr.viktorin
nosy_count: 11.0 -> 12.0
pull_requests: +22949
pull_request: https://github.com/python/cpython/pull/24119

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +22598
pull_request: https://github.com/python/cpython/pull/23740

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +22550
pull_request: https://github.com/python/cpython/pull/23686

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +22425
pull_request: https://github.com/python/cpython/pull/23546

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Andrew Svetlov]

Andrew Svetlov  added the comment:


New changeset 74311aeb45b52cc145d27d9fca99f01874d6882d 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)
https://github.com/python/cpython/commit/74311aeb45b52cc145d27d9fca99f01874d6882d


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Soumendra Ganguly  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +22418
pull_request: https://github.com/python/cpython/pull/23536

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Soumendra Ganguly  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +22415
pull_request: https://github.com/python/cpython/pull/23533

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Soumendra Ganguly  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Andrew Svetlov]

Andrew Svetlov  added the comment:


New changeset f5a19ead4ba8c81cc27d5a530f830f4709ce240e 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)
https://github.com/python/cpython/commit/f5a19ead4ba8c81cc27d5a530f830f4709ce240e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Jakub Kulik]

Jakub Kulik  added the comment:

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.

--
nosy: +kulikjak

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
pull_requests: +22408
pull_request: https://github.com/python/cpython/pull/23526

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Soumendra Ganguly  added the comment:

The reproducer was helpful. https://github.com/python/cpython/pull/23526 should 
fix this issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Zachary Ware]

Zachary Ware  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Andrew Svetlov]

Andrew Svetlov  added the comment:


New changeset 87f7ab5359bc12eeb858272b7bd58e132cb9c176 by Andrew Svetlov in 
branch 'master':
bpo-41818: test_openpty succeed on Gentoo, don't expect to fail on this 
platform (GH-23514)
https://github.com/python/cpython/commit/87f7ab5359bc12eeb858272b7bd58e132cb9c176


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[STINNER Victor]

STINNER Victor  added the comment:

> https://github.com/python/cpython/pull/23514 has the fix, waiting for all 
> buildbots finish before pressing "Merge" button.

Thanks for working on a fix :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Andrew Svetlov]

Andrew Svetlov  added the comment:

https://github.com/python/cpython/pull/23514 has the fix, waiting for all 
buildbots finish before pressing "Merge" button.
Gentoo bots are green.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[STINNER Victor]

STINNER Victor  added the comment:

This change broke x86 Gentoo buildbots: bpo-42463.

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Andrew Svetlov]

Change by Andrew Svetlov :


--
pull_requests: +22401
pull_request: https://github.com/python/cpython/pull/23514

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Andrew Svetlov]

Andrew Svetlov  added the comment:


New changeset c13d89955d9a2942c6355d6839d7096323244136 by Soumendra Ganguly in 
branch 'master':
bpo-41818: Updated tests for the standard pty library (GH-22962)
https://github.com/python/cpython/commit/c13d89955d9a2942c6355d6839d7096323244136


--
nosy: +asvetlov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Change by Soumendra Ganguly :


--
keywords: +patch
pull_requests: +21878
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/22962

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

Soumendra Ganguly  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Kyle Stanley]

Kyle Stanley  added the comment:

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 
(https://github.com/python/devguide/issues/605) to more explicitly state terms 
to avoid, and support for avoiding usage of "slave/master" seems 
uncontroversial (especially in any new code).

--
nosy: +aeros

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41818] Lib/pty.py major revision

[Soumendra Ganguly]

New submission from Soumendra Ganguly :

The current pty library has the following issues:

  1. Does not set slave termios. Documented in the source.

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

  3. Does not perform signal handling. Signals must be blocked during sensitive 
portions of code.

  4. Hangs on FreeBSD. See bpo-26228.

  5. Includes deprecated functions pty.master_open(), pty.slave_open().

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

  7. tty.setraw() is called right after tty.tcgetattr(). This increases 
redundancy of code because tty.setraw() itself makes an identical 
tty.tcgetattr() call.

  8. Requires testing/porting to more platforms. Solaris, Illumos, macOS, 
Cygwin, etc. Windows ConPTY?

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

  10. Tests are incomplete. Tests consider OSes such as Tru64 but not 
{Free/Net/Open/...}BSD.

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

--
components: FreeBSD, Library (Lib), Tests, Windows, macOS
messages: 377195
nosy: koobs, ned.deily, paul.moore, ronaldoussoren, soumendra, steve.dower, 
tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: Lib/pty.py major revision
versions: Python 3.10, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com