bug#74252: 31.0.50; multi-tty;: init_tty using getenv for COLORTERM

bug#74252: 31.0.50; multi-tty;: init_tty using getenv for COLORTERM

[Gerd Möllmann]

Just copying parts of messages from emacs-devel.

----------

I was playing with multi-tty today, and I encountered strange behavior.
For example, using 2 terminals that have different capabilties, say
different TERM and/or COLORTERM, does not really work.

Informal example: I start an emacs server -nw in iTerm
(TERM=xterm-256color, COLORTERM=truecolor), then emacsclient --tty in a
Terminal window (TERM=xterm, COLORTERM not set) => The Emacs frame in
the Terminal window is obviously confused about the terminal's
color capabilties.

----------

Something is fishy here. AFAICS, emacsclient sends its environment to
the server before it opens a frame in the server, which is done with
server-create-tty-frame.

  (defun server-create-tty-frame (tty type proc &optional parameters)
    (unless tty
      (error "Invalid terminal device"))
    (unless type
      (error "Invalid terminal type"))
    (let ((frame
           (server-with-environment
               (process-get proc 'env)
               '("LANG" "LC_CTYPE" "LC_ALL"
                 ;; For tgetent(3); list according to ncurses(3).
                 "BAUDRATE" "COLUMNS" "ESCDELAY" "HOME" "LINES"
                 "NCURSES_ASSUMED_COLORS" "NCURSES_NO_PADDING"
                 "NCURSES_NO_SETBUF" "TERM" "TERMCAP" "TERMINFO"
                 "TERMINFO_DIRS" "TERMPATH"
                 ;; rxvt wants these
                 "COLORFGBG" "COLORTERM")
             (server--create-frame
              ;; Ignore nowait here; we always need to
              ;; clean up opened ttys when the client dies.
              nil proc
              `((window-system . nil)
                (tty . ,tty)
                (tty-type . ,type)
                ,@parameters)))))

      ;; ttys don't use the `display' parameter, but callproc.c does to set
      ;; the DISPLAY environment on subprocesses.
      (set-frame-parameter frame 'display
                           (getenv-internal "DISPLAY" (process-get proc 'env)))
      frame))

The (process-get proc 'env) should contain what emacsclient sent for the
enviroment. That apparently doesn't work as expected. Many calls stacks
down in make-frame -> ... -> make_terminal_frame -> init_tty Emacs uses
C getenv, and that doesn't return what I suspect server-with-environment
was intended for.

In the case I described, COLORTERM is still truecolor as it was when the
server was started. I wonder what is used when Emacs is used as a
daemon, hm.  Probably getenv returns NULL, at least on macOS/launchd.

----------

> They do, but then they change the settings in the per-terminal
> tty_display_info, no? Or am I misreading init_tty? Certainly
>
> TERM=xterm-mono emacsclient -tty
>
> gives me a mono emacs.

And TERN works because emacsclient picks it up and sends its value as
part of the frame creation command to the server. In the server, it is
then passed down to make_terminal_frame as a frame parameter which
itself uses it for init_tty.

A pretty easy fix would be to make the environment that we got from
emacsclient anyway another frame parameter. Then init_tty wouldn't have
to use getenv to get the value of COLORTERM.

bug#74252: 31.0.50; multi-tty;: init_tty using getenv for COLORTERM

[Gerd Möllmann]

[-- Attachment #1: Type: text/plain, Size: 124 bytes --]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

The following patch works perfectly for me (including daemon mode).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: init_tty --]
[-- Type: text/x-patch, Size: 1394 bytes --]

From 1d729a0bdeef0534c05b5f8358b19c04a814a308 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <gerd@gnu.org>
Date: Fri, 8 Nov 2024 11:05:58 +0100
Subject: [PATCH] multi-tty: fix wrong COLORTERM (bug#74252)

* src/term.c (init_tty): Use egetenv, not getenv, to pick up
emacsclient's environment.
---
 src/term.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/term.c b/src/term.c
index 66395ac6077..80a14fa1277 100644
--- a/src/term.c
+++ b/src/term.c
@@ -4358,7 +4358,7 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
 
       tty->TN_max_colors = tgetnum ("Co");
 
-#ifdef TERMINFO
+#  ifdef TERMINFO
       {
 	const char *fg = tigetstr ("setf24");
 	const char *bg = tigetstr ("setb24");
@@ -4383,7 +4383,9 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
 	   (de-facto standard introduced by tmux) or if	requested by
 	   the COLORTERM environment variable.  */
 	else if ((tigetflag ("Tc") > 0)
-		 || ((bg = getenv ("COLORTERM")) != NULL
+		 /* Use egetenv, not getenv, here so that we pick up
+		    environment variables sent to us by emacsclient.  */
+		 || ((bg = egetenv ("COLORTERM")) != NULL
 		     && strcasecmp (bg, "truecolor") == 0))
 	  {
 	    tty->TS_set_foreground = "\033[%?%p1%{8}%<%t3%p1%d%e38;2;%p1%{65536}%/%d;%p1%{256}%/%{255}%&%d;%p1%{255}%&%d%;m";
-- 
2.47.0

bug#74252: 31.0.50; multi-tty; : init_tty using getenv for COLORTERM

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Fri, 08 Nov 2024 11:22:18 +0100
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> The following patch works perfectly for me (including daemon mode).

Thanks, but I think this is wrong.  process-environment is not
supposed to affect how Emacs works, it's supposed to affect only the
sub-processes started by Emacs.

bug#74252: 31.0.50; multi-tty;: init_tty using getenv for COLORTERM

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Fri, 08 Nov 2024 11:22:18 +0100
>> 
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> 
>> The following patch works perfectly for me (including daemon mode).
>
> Thanks, but I think this is wrong.  process-environment is not
> supposed to affect how Emacs works, it's supposed to affect only the
> sub-processes started by Emacs.

I'll close this then (and used it in may Emacs :-)).