SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core/exec-invoke: rework where to apply $TERM fallback logic

Browse Source 
Follow-up for 728dbaeffb
and ad6ca4a612

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.
This commit is contained in:
Mike Yuan
2025-05-29 21:16:25 +02:00
parent 94f533e305
commit 3235fae685
1 changed files with 84 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/exec-invoke.c
+84 -83
View File
@@ -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;