search

pharo-project / pharo-vm

This is the VM used by Pharo
http://pharo.org
Other
115 stars 71 forks source link

Add library with function to spawn a process connected to a pseudo-terminal #742

Closed Rinzwind closed 8 months ago

Rinzwind commented 9 months ago

This pull request adds a small library with a function to spawn a process connected to a pseudo-terminal. As discussed in pavel-krivanek/terminal issue #2 and PTerm issue #58, it’s not entirely possible to replicate this function in-image using posix_spawn. The function calls setsid and ioctl with TIOCSCTTY in the spawned process. For the setsid call, the attribute flag POSIX_SPAWN_SETSID can be passed to posix_spawn (nonstandard but supported by the GNU C Library and macOS). But for the ioctl call there is, as far as I know, no corresponding attribute or file action that can be passed.

The new File Browser that is being adopted (see: Pharo pull request #16084) offers a context menu item ‘Open terminal here’. That currently does not allow using PTerm (PTerm issue #62), but it would be nice if it did and if the VM made that convenient to use by providing the necessary supporting library.

guillep commented 9 months ago

Hi @Rinzwind, @pavel-krivanek nice!

The build had a hiccup, I relaunched it. I see no objections to this.

It would be nice however to see some comments in the code. Maybe copy-pasting part of the PR description is enough?

Rinzwind commented 9 months ago

I added a comment, fixed the compilation error on Linux (“error: implicit declaration of function 'ptsname'”), and improved the error checks in ‘tty_spawn’.

I’m not sure whether the library should be given a more distinct name. There’s an existing ‘libtty’.

Rinzwind commented 8 months ago

Can this be merged? I had to make one more change (commit 6fa0c9e28884ae95) but I think it’s ready now. I wrote some tests for which I will open a Pharo pull request once ‘libtty’ is available in the VM.