fastfetch-cli / fastfetch

An actively maintained, feature-rich and performance oriented, neofetch like system information tool.
MIT License
10.04k stars 399 forks source link

[BUG] Wrong WM/DE detection in TTY session #1110

Closed Dariqq closed 2 months ago

Dariqq commented 2 months ago

Hi,

General description of bug:

Connected to my laptop over ssh and noticed that the wm detection is wrong:

fastfetch -S LM:WM --format json
[
  {
    "type": "LM",
    "result": {
      "service": "sshd",
      "type": "TTY",
      "version": "9.8p1"
    }
  },
  {
    "type": "WM",
    "result": {
      "processName": "sway",
      "prettyName": "Sway",
      "protocolName": "X11",
      "pluginName": ""
    }
  }
]

It seems to detect sway from the other session and classify it as X11 which is incorrect. When I run from the laptop from within sway things seem correct.

fastfetch -S LM:WM --format json
[
  {
    "type": "LM",
    "result": {
      "service": "greetd",
      "type": "Wayland",
      "version": ""
    }
  },
  {
    "type": "WM",
    "result": {
      "processName": "sway",
      "prettyName": "Sway",
      "protocolName": "Wayland",
      "pluginName": ""
    }
  }
]

When I sshed into a machine with gnome fastfetch found gnome-shell/mutter and where something it is x11 in the ssh session and wayland normaly.

Dariqq commented 2 months ago

the sway part seems to come from getFromProcess invocation in wmde.c. Not sure where the X11 part comes from.

From the ssh session i see XDG_SESSION_TYPE=tty. Judging from a comment in wmde.c it should return nothing in this case and dont try to snoop into other sessions of the same user. Not sure why this is not happening in my case.

My guess is that something is setting x11 somewhere and thus the XDG_SESSION_TYPE is ignored.

Edit: Also occurs when logging in through a secondary tty when sway is running in another session

Dariqq commented 2 months ago

Changes to ds->wmProtocolName when in an ssh session on a gnome host:

ffdsConnectWayland: Sets to Wayland ffdsConnectXcbRandr : Sets to X11 ffdsConnectXrandr : Sets to nothing. ffdsConnectXcb : Sets to X11. ffdsConnectXlib : Sets to nothing. ffdsConnectDrm : Sets to nothing.

So culprit seems to be with the wayland/xcb/xcbrandr implementation finding things they are not supposed to.

CarterLi commented 2 months ago

ffdsConnectXcbRandr : Sets to X11 ffdsConnectXrandr : Sets to nothing. ffdsConnectXcb : Sets to X11. ffdsConnectXlib : Sets to nothing.

They all set ds->wmProtocolName to X11, don't they?

Dariqq commented 2 months ago

Probably right. i was removing previous calls and printing wmProtocolName afterwards. But things are different on my laptop with sway.

Dariqq commented 2 months ago

So all these will still set something even if the active session has no wm/de running.

disabling all the libx* and wayland backends fixes things for me. Unsure what a good fix would be

CarterLi commented 2 months ago

Fastfetch failed to connect wayland server, and couldn't find any environment variables about wayland, so fastfetch thought it was not wayland. However fastfetch connected x11 server successfully somehow.

Dariqq commented 2 months ago

maybe it finds the xwayland server from the other session (details on what connects may vary when other session is gnome/sway/something else?)?

CarterLi commented 2 months ago

If we can detect the x11 server is actually xwayland, we can fix this issue easily.

Dariqq commented 2 months ago

It would have to be xwayland for my case as i only have sway (and no X stuff) on my laptop. question is whether the xcb, xrandr,xlib libs can find that out that they found a a normal xserver or xwayland.

Maybe easier: check for XDG_SESSION_TYPE earlier (rather than only if previous things faill) and skip everything if if it is tty as it seems like this comment in wmde.c (ffdsDetectWMDE) is incorrect if somthing is running in a different sesstions.

    //We don't want to detect anything in TTY
    //This can't happen if a connection succeeded, so we don't need to clear wmProcessName
Dariqq commented 2 months ago

Maybe easier: check for XDG_SESSION_TYPE earlier (rather than only if previous things faill) and skip everything if if it is tty as it seems like this comment in wmde.c (ffdsDetectWMDE) is incorrect if somthing is running in a different sesstions.

Though this might still be incorrect if there are multiple sessions with different wm/de setups running but i think this is not as common as having WM + TTY/ WM + SSH session

Dariqq commented 2 months ago

Tried to look into rewiring the envvar check to quit detecting early if in a tty (as detected by getWMProtocolNameFromEnv). But i am bit confused how the new execution flow should be (in particular because ffdsConnectWayland also checks for XDG_SESSION_TYPE == wayland which it probably shouldnt do if XDG_SESSION_TYPE is checked before already.

Any ideas?

Dariqq commented 2 months ago

Currently have something similiar to this. Could probably be improved upon, seems to work for my case

diff --git a/src/detection/displayserver/linux/displayserver_linux.c b/src/detection/displayserver/linux/displayserver_linux.c
index e864bdc0..40a635f1 100644
--- a/src/detection/displayserver/linux/displayserver_linux.c
+++ b/src/detection/displayserver/linux/displayserver_linux.c
@@ -9,6 +9,11 @@

 void ffConnectDisplayServerImpl(FFDisplayServerResult* ds)
 {
+    getWMProtocolNameFromEnv(ds);
+    //We don't want to detect anything in TTY
+    if(ffStrbufIgnCaseCompS(&ds->wmProtocolName, FF_WM_PROTOCOL_TTY) == 0)
+        return;
+
     if (instance.config.general.dsForceDrm == FF_DS_FORCE_DRM_TYPE_FALSE)
     {
         //We try wayland as our preferred display server, as it supports the most features.
@@ -112,3 +117,37 @@ FFDisplayType ffdsGetDisplayType(const char* name)

     return FF_DISPLAY_TYPE_UNKNOWN;
 }
+
+
+void getWMProtocolNameFromEnv(FFDisplayServerResult* result)
+{
+
+    const char* env = getenv("XDG_SESSION_TYPE");
+    if(ffStrSet(env))
+    {
+        if(ffStrEqualsIgnCase(env, "x11"))
+            ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_X11);
+   else if(ffStrEqualsIgnCase(env, "wayland"))
+            ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_WAYLAND);
+        else if(ffStrEqualsIgnCase(env, "tty"))
+            ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_TTY);
+        else
+            ffStrbufSetS(&result->wmProtocolName, env);
+
+        return;
+    }
+
+    env = getenv("DISPLAY");
+    if(ffStrSet(env))
+    {
+        ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_X11);
+        return;
+    }
+
+    env = getenv("TERM");
+    if(ffStrSet(env) && ffStrEqualsIgnCase(env, "linux"))
+    {
+        ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_TTY);
+        return;
+    }
+}
diff --git a/src/detection/displayserver/linux/displayserver_linux.h b/src/detection/displayserver/linux/displayserver_linux.h
index c4937ee2..0a721cb3 100644
--- a/src/detection/displayserver/linux/displayserver_linux.h
+++ b/src/detection/displayserver/linux/displayserver_linux.h
@@ -19,6 +19,7 @@ void ffdsConnectXlib(FFDisplayServerResult* result);
 void ffdsConnectDrm(FFDisplayServerResult* result);

 void ffdsDetectWMDE(FFDisplayServerResult* result);
+void getWMProtocolNameFromEnv(FFDisplayServerResult* result);

 FFDisplayType ffdsGetDisplayType(const char* drmConnectorName);

diff --git a/src/detection/displayserver/linux/wayland/wayland.c b/src/detection/displayserver/linux/wayland/wayland.c
index 2bf03ebe..b87c9562 100644
--- a/src/detection/displayserver/linux/wayland/wayland.c
+++ b/src/detection/displayserver/linux/wayland/wayland.c
@@ -266,12 +266,7 @@ void ffdsConnectWayland(FFDisplayServerResult* result)
     if(getenv("XDG_RUNTIME_DIR") == NULL)
         return;

-    #ifdef FF_HAVE_WAYLAND
-        if(detectWayland(result))
-            return;
-    #endif
-
-    const char* xdgSessionType = getenv("XDG_SESSION_TYPE");
+    const char* xdgSessionType = result->wmProtocolName.chars;

     //If XDG_SESSION_TYPE is set, and doesn't contain "wayland", we are probably not running in a wayland session.
     if(xdgSessionType != NULL && !ffStrEqualsIgnCase(xdgSessionType, "wayland"))
@@ -282,6 +277,11 @@ void ffdsConnectWayland(FFDisplayServerResult* result)
     if(xdgSessionType == NULL && getenv("WAYLAND_DISPLAY") == NULL && getenv("WAYLAND_SOCKET") == NULL)
         return;

+    #ifdef FF_HAVE_WAYLAND
+        if(detectWayland(result))
+            return;
+    #endif
+
     //We are probably running a wayland compositor at this point,
     //but fastfetch was compiled without the required library, or loading the library failed.
     ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_WAYLAND);
diff --git a/src/detection/displayserver/linux/wmde.c b/src/detection/displayserver/linux/wmde.c
index 0319cdd4..99dcdc1a 100644
--- a/src/detection/displayserver/linux/wmde.c
+++ b/src/detection/displayserver/linux/wmde.c
@@ -240,38 +240,6 @@ static void applyPrettyNameIfDE(FFDisplayServerResult* result, const char* name)
     }
 }

-static void getWMProtocolNameFromEnv(FFDisplayServerResult* result)
-{
-    //This is only called if all connection attempts to a display server failed
-    //We don't need to check for wayland here, as the wayland code will always set the protocol name to wayland
-
-    const char* env = getenv("XDG_SESSION_TYPE");
-    if(ffStrSet(env))
-    {
-        if(ffStrEqualsIgnCase(env, "x11"))
-            ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_X11);
-        else if(ffStrEqualsIgnCase(env, "tty"))
-            ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_TTY);
-        else
-            ffStrbufSetS(&result->wmProtocolName, env);
-
-        return;
-    }
-
-    env = getenv("DISPLAY");
-    if(ffStrSet(env))
-    {
-        ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_X11);
-        return;
-    }
-
-    env = getenv("TERM");
-    if(ffStrSet(env) && ffStrEqualsIgnCase(env, "linux"))
-    {
-        ffStrbufSetS(&result->wmProtocolName, FF_WM_PROTOCOL_TTY);
-        return;
-    }
-}

 static const char* getFromProcesses(FFDisplayServerResult* result)
 {
@@ -396,14 +364,6 @@ static const char* getFromProcesses(FFDisplayServerResult* result)

 void ffdsDetectWMDE(FFDisplayServerResult* result)
 {
-    //If all connections failed, use the environment variables to detect protocol name
-    if(result->wmProtocolName.length == 0)
-        getWMProtocolNameFromEnv(result);
-
-    //We don't want to detect anything in TTY
-    //This can't happen if a connection succeeded, so we don't need to clear wmProcessName
-    if(ffStrbufIgnCaseCompS(&result->wmProtocolName, FF_WM_PROTOCOL_TTY) == 0)
-        return;

     const char* env = parseEnv();
CarterLi commented 2 months ago

Please test the dev build

Dariqq commented 2 months ago

Thanks.

Dariqq commented 1 month ago

Apologies for breaking things with this.

The problem is that the x* libraries might detect a xserver /xwayland in another session which then causes GetFromProcess to search for a WM/DE. Could we instead restrict to processes in the current session rather than all processes from the current user? I am not sure if that is possible though.