From: Mike Yuan Date: Wed, 28 May 2025 18:24:59 +0000 (+0200) Subject: core/exec-invoke: rework where to apply $TERM fallback logic X-Git-Tag: v258-rc1~443^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3235fae685c5f5996a06888b0b19645ebab1d2f0;p=thirdparty%2Fsystemd.git core/exec-invoke: rework where to apply $TERM fallback logic Follow-up for 728dbaeffb3e72872253c50ca5d1c100cc532634 and ad6ca4a6129fa0fb8e8c800d05cf2c7ed5d0bcbf This is inspired by #37538, see the discussion in https://github.com/systemd/systemd/pull/37538#discussion_r2110229075. 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. --- diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index e96a031c868..bfa21474e17 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -137,23 +137,6 @@ static bool is_kmsg_output(ExecOutput o) { EXEC_OUTPUT_KMSG_AND_CONSOLE); } -static bool exec_context_needs_term(const ExecContext *c) { - assert(c); - - /* Return true if the execution context suggests we should set $TERM to something useful. */ - - if (is_terminal_input(c->std_input)) - return true; - - if (is_terminal_output(c->std_output)) - return true; - - if (is_terminal_output(c->std_error)) - return true; - - return !!c->tty_path; -} - static int open_null_as(int flags, int nfd) { int fd; @@ -1932,8 +1915,8 @@ static int build_environment( assert(cgroup_context); assert(ret); -#define N_ENV_VARS 22 - our_env = new0(char*, N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX); +#define N_ENV_VARS 19 + our_env = new0(char*, N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX + 1); if (!our_env) return -ENOMEM; @@ -2037,69 +2020,6 @@ static int build_environment( our_env[n_env++] = x; } - if (exec_context_needs_term(c)) { - _cleanup_free_ char *cmdline = NULL, *dcs_term = NULL; - const char *tty_path, *term = NULL; - - tty_path = exec_context_tty_path(c); - - /* If we are forked off PID 1 and we are supposed to operate on /dev/console, then let's try - * to inherit the $TERM set for PID 1. This is useful for containers so that the $TERM the - * container manager passes to PID 1 ends up all the way in the console login shown. */ - - if (path_equal(tty_path, "/dev/console") && getppid() == 1) - term = getenv("TERM"); - else if (tty_path && in_charset(skip_dev_prefix(tty_path), ALPHANUMERICAL)) { - _cleanup_free_ char *key = NULL; - - key = strjoin("systemd.tty.term.", skip_dev_prefix(tty_path)); - if (!key) - return -ENOMEM; - - r = proc_cmdline_get_key(key, 0, &cmdline); - if (r < 0) - log_debug_errno(r, "Failed to read %s from kernel cmdline, ignoring: %m", key); - else if (r > 0) - term = cmdline; - } - - if (!term && tty_path) { - /* This handles real virtual terminals (returning "linux") and - * any terminals which support the DCS +q query sequence. */ - r = query_term_for_tty(tty_path, &dcs_term); - if (r >= 0) - term = dcs_term; - } - - if (!term) { - /* If $TERM is not known and we pick a fallback default, then let's also set - * $COLORTERM=truecolor. That's because our fallback default is vt220, which is - * generally a safe bet (as it supports PageUp/PageDown unlike vt100, and is quite - * universally available in terminfo/termcap), except for the fact that real DEC - * vt220 gear never actually supported color. Most tools these days generate color on - * vt220 anyway, ignoring the physical capabilities of the real hardware, but some - * tools actually believe in the historical truth. Which is unfortunate since *we* - * *don't* care about the historical truth, we just want sane defaults if nothing - * better is explicitly configured. It's 2025 after all, at the time of writing, - * pretty much all terminal emulators actually *do* support color, hence if we don't - * know any better let's explicitly claim color support via $COLORTERM. Or in other - * words: we now explicitly claim to be connected to a franken-vt220 with true color - * support. */ - x = strdup("COLORTERM=truecolor"); - if (!x) - return -ENOMEM; - - our_env[n_env++] = x; - - term = FALLBACK_TERM; - } - - x = strjoin("TERM=", term); - if (!x) - return -ENOMEM; - our_env[n_env++] = x; - } - if (journal_stream_dev != 0 && journal_stream_ino != 0) { if (asprintf(&x, "JOURNAL_STREAM=" DEV_FMT ":" INO_FMT, journal_stream_dev, journal_stream_ino) < 0) return -ENOMEM; @@ -2215,7 +2135,7 @@ static int build_environment( our_env[n_env++] = x; } - assert(n_env < N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX); + assert(n_env <= N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX); #undef N_ENV_VARS *ret = TAKE_PTR(our_env); @@ -4599,6 +4519,81 @@ static void prepare_terminal( (void) osc_context_open_service(p->unit_id, p->invocation_id, /* ret_seq= */ NULL); } +static int setup_term_environment(const ExecContext *context, char ***env) { + int r; + + assert(context); + assert(env); + + /* Already specified by user? */ + if (strv_env_get(*env, "TERM")) + return 0; + + /* Do we need $TERM at all? */ + if (!is_terminal_input(context->std_input) && + !is_terminal_output(context->std_output) && + !is_terminal_output(context->std_error) && + !context->tty_path) + return 0; + + const char *tty_path = exec_context_tty_path(context); + if (tty_path) { + /* If we are forked off PID 1 and we are supposed to operate on /dev/console, then let's try + * to inherit the $TERM set for PID 1. This is useful for containers so that the $TERM the + * container manager passes to PID 1 ends up all the way in the console login shown. */ + if (tty_is_console(tty_path) && getppid() == 1) { + const char *term = strv_find_prefix(environ, "TERM="); + if (term) { + r = strv_env_replace_strdup(env, term); + if (r < 0) + return r; + + return 1; + } + } + + if (in_charset(skip_dev_prefix(tty_path), ALPHANUMERICAL)) { + _cleanup_free_ char *key = NULL, *cmdline = NULL; + + key = strjoin("systemd.tty.term.", skip_dev_prefix(tty_path)); + if (!key) + return -ENOMEM; + + r = proc_cmdline_get_key(key, /* flags = */ 0, &cmdline); + if (r > 0) + return strv_env_assign(env, "TERM", cmdline); + if (r < 0) + log_debug_errno(r, "Failed to read '%s' from kernel cmdline, ignoring: %m", key); + } + + /* This handles real virtual terminals (returning "linux") and + * any terminals which support the DCS +q query sequence. */ + _cleanup_free_ char *dcs_term = NULL; + r = query_term_for_tty(tty_path, &dcs_term); + if (r >= 0) + return strv_env_assign(env, "TERM", dcs_term); + } + + /* If $TERM is not known and we pick a fallback default, then let's also set + * $COLORTERM=truecolor. That's because our fallback default is vt220, which is + * generally a safe bet (as it supports PageUp/PageDown unlike vt100, and is quite + * universally available in terminfo/termcap), except for the fact that real DEC + * vt220 gear never actually supported color. Most tools these days generate color on + * vt220 anyway, ignoring the physical capabilities of the real hardware, but some + * tools actually believe in the historical truth. Which is unfortunate since *we* + * *don't* care about the historical truth, we just want sane defaults if nothing + * better is explicitly configured. It's 2025 after all, at the time of writing, + * pretty much all terminal emulators actually *do* support color, hence if we don't + * know any better let's explicitly claim color support via $COLORTERM. Or in other + * words: we now explicitly claim to be connected to a franken-vt220 with true color + * support. */ + r = strv_env_replace_strdup(env, "COLORTERM=truecolor"); + if (r < 0) + return r; + + return strv_env_replace_strdup(env, "TERM=" FALLBACK_TERM); +} + int exec_invoke( const ExecCommand *command, const ExecContext *context, @@ -5258,6 +5253,12 @@ int exec_invoke( (void) umask(context->umask); + r = setup_term_environment(context, &accum_env); + if (r < 0) { + *exit_status = EXIT_MEMORY; + return log_error_errno(r, "Failed to construct $TERM: %m"); + } + r = setup_keyring(context, params, uid, gid); if (r < 0) { *exit_status = EXIT_KEYRING;