Use DCS sequence to query terminal name and set $TERM automatically by keszybz · Pull Request #37538 · systemd/systemd

keszybz · 2025-05-21T12:18:25Z

This code seems to work quickly and nicely for a bunch of modern terminals. Setting $TERM automatically removes an common annoyance for users. This code will not work for all terminal emulators, but by adding it in systemd we'll entice maintainers of those terminals to add support for the sequences. For the terminals that don't support the sequence, we get a bit of a slowdown of < 1 ms, which seems hardly noticeable. The user can always set TERM explicitly to avoid this if upgrading to a newer terminal emulator is not possible.

Closes #36994.

src/core/exec-invoke.c

src/basic/terminal-util.c

src/test/test-terminal-util.c

src/basic/terminal-util.c

src/core/exec-invoke.c

keszybz · 2025-05-28T11:25:10Z

mkosi / ci (ubuntu, noble): 95/95 systemd:integration-tests / TEST-24-CRYPTSETUP TIMEOUT 1800.32s (exit status 254 or signal 126 SIGinvalid)

noble/s390x: 1474s autopkgtest [11:02:49]: ERROR: testbed failure: unexpected eof from the testbed

src/basic/terminal-util.c

src/basic/terminal-util.h

src/core/exec-invoke.c

keszybz · 2025-05-29T16:16:24Z

Updated with the requested changes, except for a few cases where I replied above.

keszybz · 2025-05-29T16:18:40Z

So what happens on VTE right now? if i spawn a vm with serial console I'll have to wait 333ms now so that the query times out?

Usually, there are plenty of time between systemd first starts and login prompt shows up. To mitigate the effective delay, could we handle the sequence asynchronously or something like that?

Theoretically, it'd be possible. But in practice pretty annoying, since we'd like to start outputting stuff as soon as possible. So if this was asynchronous, we might end up printing some stuff assuming that color is not available, and then switch, or vice versa. It'd also complicate the code quite a bit. A much better solution is to just enable support in terminals and have the reply quickly.

src/basic/terminal-util.c

YHNdnzj · 2025-05-29T16:45:00Z

LGTM otherwise

As requested in systemd#36994, use DCS + q name ST. This works, but has limited terminal support: xterm, foot, kitty.

query_term_for_tty() is used in two places: in fixup_environment(), which affects PID1 itself, and in build_environment(), which affects spawned services. There is obviously some cost to the extra call, but I think it's worthwhile to do it. When $TERM is set incorrectly, basic output works OK, but then there are various annoying corner cases. In particular, we get the support for color (or lack of it) wrong, and when output is garbled, users are annoyed. Things like text editors are almost certain to behave incorrectly. Testing in test-terminal-util indicates that the time required to make a successful query is on the order of a dozen microseconds, and an unsuccessful query costs as much as our timeout, i.e. currently 1/3 ms. I think this is an acceptable tradeoff. No caching is used, because fixup_environment() is only called once, and the other place in build_environment(), only affects services which are connected to a tty, which is only a handful of services, and often only started in special circumstances. Fixes systemd#36994.

keszybz · 2025-05-29T17:22:42Z

Updated. Phew.

Follow-up for 728dbae and ad6ca4a This is inspired by systemd#37538, see the discussion in systemd#37538 (comment). If the user already specifies $TERM (which is actually quite common if you look at run0), we'd needlessly invoke the "fallback" logic and a) possibly issue a DCS query whose result we end up simply discarding in strv_env_merge() b) set $COLORTERM to "truecolor" unconditionally, whereas the explicit $TERM value might intend to disable the color output To address this, the logic of setting fallback $TERM and friends has been split out of build_environment(), and we'd call into it only after all envvars have been collected.

bam80 · 2025-05-29T19:39:38Z

I would like to thank everyone involved here, especially @keszybz. Good job guys!

I'll report to VTE the patch is merged, with the relevant implications.

Inspired by #37538, see a detailed rationale in #37538 (comment).

bluca reviewed May 21, 2025

View reviewed changes

src/core/exec-invoke.c Outdated Show resolved Hide resolved

YHNdnzj reviewed May 21, 2025

View reviewed changes

src/basic/terminal-util.c Outdated Show resolved Hide resolved

github-actions bot added util-lib tests please-review PR is ready for (re-)review by a maintainer labels May 21, 2025