[PATCH] Make pty.spawn set window size

[8vasu]

BPO	41541
Nosy	@Yhg1s, @ethanfurman, @8vasu
PRs	bpo-41541: Make pty.spawn set window size #21861
Files	pty.diff: v0.6 before.png after.png script.py

^{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-08-13.17:37:00.291>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = '[PATCH] Make pty.spawn set window size'
updated_at = <Date 2020-09-15.01:08:45.199>
user = 'https://github.com/8vasu'

bugs.python.org fields:

activity = <Date 2020-09-15.01:08:45.199>
actor = 'soumendra'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-08-13.17:37:00.291>
creator = 'soumendra'
dependencies = []
files = ['49404', '49455', '49456', '49458']
hgrepos = []
issue_num = 41541
keywords = ['patch']
message_count = 15.0
messages = ['375326', '375366', '375396', '375433', '375438', '375439', '375443', '375445', '375446', '375452', '375461', '375513', '375582', '375631', '375774']
nosy_count = 3.0
nosy_names = ['twouters', 'ethan.furman', 'soumendra']
pr_nums = ['21861']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41541'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[8vasu]

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

[8vasu]

OS: Linux 4.19.0-9-amd64 Debian 10 GNU/Linux

[8vasu]

Note that defining _login_pty() was not a cosmetic change; it is reused in spawn().

[8vasu]

v0.2 moves _setwinsz block to parent after fork.

[8vasu]

Screenshot: output of "ls" before the patch is applied.

[8vasu]

Screenshot: output of "ls" after the patch is applied.

[8vasu]

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

[8vasu]

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.

[8vasu]

Additional note: I am using the i3wm window manager. No desktop environment.

[8vasu]

v0.3 removes _login_pty() and defines _login_tty() instead; the latter is based on login_tty(3) from glibc.

[8vasu]

v0.4 puts try-except guards around imports so that existing code does not break.

[8vasu]

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.

[8vasu]

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.

[8vasu]

v0.5 had introduced minor mistakes + one hack [ was using master instead of slave to set window size ]. v0.6 removes all such mistakes.

[8vasu]

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.

[kjoyce77]

Any reason this was dropped? Anyway the patch was useful for me.

[8vasu]

@kjoyce77 This was opened as "proof of concept" and was a dirty quick fix (not supposed to be merged). Please comment here instead: #85984. The core developers are very busy (I am not a core developer), and they do the best they can when they get time to review our pull requests. Please help them review my pull requests on that thread if you have time!

[encukou]

Superseded by #85984. Let's get that one in :)