flatpak / flatpak-xdg-utils

Simple portal-based commandline tools for use inside flatpak sandboxes
GNU Lesser General Public License v2.1
32 stars 14 forks source link

flatpak-spawn --host doesn't work with apps requiring tty/pty #57

Open JayDoubleu opened 2 years ago

JayDoubleu commented 2 years ago
flatpak-spawn --host sudo echo
sudo: a terminal is required to read the password; either use the -S option to read from standard input or configure an askpass helper
sudo: a password is required

I've also noticed some apps like neovim are not rendering properly and glitch when being resized. It feels like working in serial console in general.

TingPing commented 2 years ago

I'm not sure if flatpak-spawn should be changed but here is an example project and some documentation about properly running a PTY on the host:

https://gitlab.gnome.org/chergert/flatterm/-/blob/master/fp-vte-util.c#L33-56

JayDoubleu commented 2 years ago

I see. Is this also related to https://github.com/flatpak/flatpak/issues/3697 then ? flatpak-spawn is heavily used by fedora toolbx users.

TingPing commented 2 years ago

It looks the same yes.

JayDoubleu commented 2 years ago

IMHO If would be great if flatpak-spawn could be changed to remediate this. I find myself running quite a few programs using it.

TingPing commented 2 years ago

I'm not familiar enough with it to do it atm, but I'll review a patch adapting those solutions, perhaps with a new flag to avoid breaking anything.

1player commented 2 years ago

I have been working on an alternative reimplementation of flatpak-spawn --host: https://github.com/1player/host-spawn

Improvements over flatpak-spawn:

It's a reimplementation because I was more comfortable with exploring this problem in Go instead of hacking the original C code base, but it's in the public domain so feel free to use my code to fix flatpak-spawn.

smcv commented 2 years ago

I've implemented a similar pseudo-terminal bridge in GObject-flavoured C as part of steam-runtime-launch-client, a development/debugging tool shipped as part of Steam, which is derived from flatpak-spawn. My plan was to test it in Steam, and then contribute it (and other improvements) to flatpak-spawn when it's better-tested.

(Due credit: the pseudo-terminal juggling is derived from similar code in systemd.)

Allocates a pty for the spawned process, which fixes this issue; Passes through SIGWINCH signals, to handle terminal resizing

Those are implemented.

Passes through some env variables (such as TERM)

I'm unsure about this: it might be a good enhancement, but I'd want to provide a way to not do this, because if we unconditionally overwrite an environment variable that was inherited from the server (flatpak-session-helper or flatpak-portal), there's usually no good way to get it back.

smcv commented 2 years ago

Passes through some env variables (such as TERM)

I'm unsure about this

The way I ended up doing this for steam-runtime-launch-client is that it automatically passes through $TERM, but only if at least one of stdin, stdout or stderr is a terminal, and you can disable this with --inherit-env=TERM (part of a set of options for finer control of the environment, as mentioned in #28).

1player commented 2 years ago

Not a bad idea, though I'm not sure which programs benefit from not having $TERM set. AFAIK if you don't set it manually, it's not set on the host side, which often causes issues.

Looking forward to see your changes merged with flatpak-spawn. Do you have a standalone repo for steam-runtime-launch-client where I can play with it?

sebastian-de commented 1 year ago

My plan was to test it in Steam, and then contribute it (and other improvements) to flatpak-spawn when it's better-tested.

Hey @smcv, gentle ping on this - do you still have plans to upstream the mentioned changes? That would be much appreciated!

cuviper commented 10 months ago

Do you have a standalone repo for steam-runtime-launch-client where I can play with it?

FWIW, I found it here:

https://gitlab.steamos.cloud/steamrt/steam-runtime-tools/-/blob/main/bin/launch-client.c

smcv commented 10 months ago

do you still have plans to upstream the mentioned changes? That would be much appreciated!

This is still on my list, but my list is very long.

There is a bug somewhere in the implementation used in steam-runtime-tools where it will sometimes go into a busy-loop when using this interface to inject commands into a Flatpak app, probably because end-of-file works oddly for terminals. I think solving that is likely to be a prerequisite for upstreaming it into flatpak-spawn.

geekley commented 8 months ago

Currently, is there a way to kill a (frozen) process started via flatpak-spawn --host ...?

I need to kill Krita from my code (because it freezes when you try to run a second instance of the program) but killing the process resulting from flatpak-spawn --host krita ... doesn't end the one under flatpak-session-helper. Which means closing the first instance of krita will still make the killed command run.

geekley commented 8 months ago

Ah I see. Interesting. So it seems that if send SIGINT or SIGTERM to the process of flatpak-spawn then it terminates the other process under flatpak-session-helper. However, if I send SIGKILL, then that process doesn't kill the other process. Because presumably it would be impossible in this case to re-pass the signal?

1player commented 8 months ago

SIGKILL cannot be caught, which means cannot be passed through via DBus to the other process. The kernel will just terminate the flatpak-spawn process with no questions asked.

On Mon, 29 Jan 2024, at 18:57, geekley wrote:

Ah I see. Interesting. So it seems that if send SIGINT or SIGTERM to the process of flatpak-spawn then it terminates the other process under flatpak-session-helper. However, if I send SIGKILL, then that process doesn't kill the other process. Because presumably it would be impossible in this case to re-pass the signal?

— Reply to this email directly, view it on GitHub https://github.com/flatpak/flatpak-xdg-utils/issues/57#issuecomment-1915365563, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFIPSHTVQNEVTOMI7VAHNTYQ7WHZAVCNFSM5KG3S4VKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJRGUZTMNJVGYZQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>