classification
Title: [PATCH] Make pty.spawn set window size
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ethan.furman, soumendra, twouters
Priority: normal Keywords: patch

Created on 2020-08-13 17:37 by soumendra, last changed 2020-09-15 01:08 by soumendra.

Files
File name Uploaded Description Edit
pty.diff soumendra, 2020-08-19 02:38 v0.6
before.png soumendra, 2020-09-15 01:04
after.png soumendra, 2020-09-15 01:04
script.py soumendra, 2020-09-15 01:08
Pull Requests
URL Status Linked Edit
PR 21861 open soumendra, 2020-08-13 17:44
Messages (15)
msg375326 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-13 17:36
The example in https://docs.python.org/3/library/pty.html that mimics script(1) can be used to reproduce the problem: since window size is not set by pty.spawn, output of "ls" becomes scattered and hard to visually parse if xterm window is not in fullscreen mode.

To fix the above issue, this patch makes pty.spawn set window size ( TIOCSWINSZ ). Also, this patch makes the code of pty.fork() more readable by defining _login_pty(); the latter is reused in spawn().
msg375366 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-14 04:23
OS: Linux 4.19.0-9-amd64 Debian 10 GNU/Linux
msg375396 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-14 11:36
Note that defining _login_pty() was not a cosmetic change; it is reused in spawn().
msg375433 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-14 21:25
v0.2 moves _setwinsz block to parent after fork.
msg375438 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-14 23:18
Screenshot: output of "ls" before the patch is applied.
msg375439 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-14 23:19
Screenshot: output of "ls" after the patch is applied.
msg375443 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-15 02:22
Adding the test program [ test.py ] as an attachment. It was taken from https://docs.python.org/3/library/pty.html.

How to reproduce issue:

1. Notice that the xterm window in before.png is not too wide; this makes the output of "ls" wrap around the xterm window.
2. Run test.py.
3. Run "ls".

Solution:

1. Apply latest version of the patch [ pty.diff ].
2. Run test.pty and run "ls".
3. Run "ls".
msg375445 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-15 03:17
I am new to BPO. Just learned how to make someone nosy.

@twouters, I have attached all resources. This is ready for a review.

Thank you.
msg375446 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-15 03:57
Additional note: I am using the i3wm window manager. No desktop environment.
msg375452 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-15 08:59
v0.3 removes _login_pty() and defines _login_tty() instead; the latter is based on login_tty(3) from glibc.
msg375461 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-15 12:45
v0.4 puts try-except guards around imports so that existing code does not break.
msg375513 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-16 16:14
Further proposal: Rename my _login_tty to login_tty and make it available as a part of the pty library. Note that usually login_tty accompanies openpty and forkpty on a system; for example, see

https://www.man7.org/linux/man-pages/man3/login_tty.3.html
https://man.openbsd.org/login_tty
https://netbsd.gw.com/cgi-bin/man-cgi?login_tty++NetBSD-current

However, python's pty only offers openpty and forkpty in the form of pty.openpty and pty.fork respectively. While it is true that forkpty [ pty.fork ] combines openpty, fork, and login_tty, it also closes the slave end of the pty, making it unsuitable for situations where the slave end needs to be kept open; for example, in my patch, the slave end is used to set the window size; or, in case someone wants to do even better and register a SIGWINCH handler for situations in which the window size can change.
msg375582 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-18 00:49
Further note: login_tty will also enable us to set slave termios from the parent process in pty.spawn.

Due to the fact that reviewing patches can be overwhelming, v0.5 removes a lot of stuff and instead simply performs window resize by calling ioctl TIOCSWINSZ on the master end of the pty. Still works as expected.
msg375631 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-19 02:38
v0.5 had introduced minor mistakes + one hack [ was using master instead of slave to set window size ]. v0.6 removes all such mistakes.
msg375774 - (view) Author: Soumendra Ganguly (soumendra) * Date: 2020-08-21 22:42
All images, test programs, and old patches have been removed.

window resize test is now being performed using stty.

On linux:
stty -F <terminal-device-file> rows x cols y

On BSDs:
stty -f <terminal-device-file> rows x cols y

to change window size.
History
Date User Action Args
2020-09-15 01:08:45soumendrasetfiles: + script.py
2020-09-15 01:07:22soumendrasetfiles: - script.py
2020-09-15 01:06:35soumendrasetfiles: + script.py
2020-09-15 01:04:58soumendrasetfiles: + after.png
2020-09-15 01:04:51soumendrasetfiles: + before.png
2020-09-14 21:45:15ethan.furmansetnosy: + ethan.furman
2020-08-21 22:42:29soumendrasetmessages: + msg375774
2020-08-21 22:39:19soumendrasetfiles: - test.py
2020-08-21 22:39:07soumendrasetfiles: - pty.diff
2020-08-21 22:39:02soumendrasetfiles: - pty.diff
2020-08-21 22:38:55soumendrasetfiles: - after.png
2020-08-21 22:38:49soumendrasetfiles: - before.png
2020-08-21 22:38:41soumendrasetfiles: - pty.diff
2020-08-21 22:38:32soumendrasetfiles: - pty.diff
2020-08-21 22:38:24soumendrasetfiles: - pty.diff
2020-08-19 02:38:35soumendrasetfiles: + pty.diff

messages: + msg375631
2020-08-18 00:49:34soumendrasetfiles: + pty.diff

messages: + msg375582
2020-08-16 16:14:08soumendrasetmessages: + msg375513
2020-08-15 12:45:08soumendrasetfiles: + pty.diff

messages: + msg375461
2020-08-15 08:59:51soumendrasetfiles: + pty.diff

messages: + msg375452
2020-08-15 04:04:39soumendrasettitle: Make pty.spawn set window size [ patch + before, after screenshots ] -> [PATCH] Make pty.spawn set window size
2020-08-15 03:57:02soumendrasetnosy: - mark.dickinson, meador.inge
messages: + msg375446
2020-08-15 03:43:23soumendrasetnosy: + mark.dickinson, meador.inge
2020-08-15 03:17:42soumendrasetnosy: + twouters
messages: + msg375445
2020-08-15 02:22:04soumendrasetfiles: + test.py

messages: + msg375443
2020-08-14 23:23:48soumendrasettitle: Make pty.spawn set window size [ + before, after screenshots ] -> Make pty.spawn set window size [ patch + before, after screenshots ]
2020-08-14 23:19:04soumendrasetfiles: + after.png

messages: + msg375439
2020-08-14 23:18:24soumendrasetfiles: + before.png

messages: + msg375438
title: Make pty.spawn set window size -> Make pty.spawn set window size [ + before, after screenshots ]
2020-08-14 21:25:54soumendrasetfiles: + pty.diff

messages: + msg375433
2020-08-14 11:36:35soumendrasetmessages: + msg375396
2020-08-14 04:23:10soumendrasetmessages: + msg375366
2020-08-13 17:44:23soumendrasetstage: patch review
pull_requests: + pull_request20988
2020-08-13 17:43:34soumendrasettitle: Make pty.spawn set window size, make code more readable -> Make pty.spawn set window size
2020-08-13 17:37:00soumendracreate