Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Dariqq opened this issue Jul 22, 2024 · 15 comments
Closed

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

Dariqq opened this issue Jul 22, 2024 · 15 comments
Labels
bug Something isn't working

Comments

@Dariqq
Copy link
Contributor

Dariqq commented Jul 22, 2024

Hi,

General description of bug:

  • What happened:

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.

  • What should happen: I would expect either one of
    • Detect Sway correctly as Wayland in this case
    • Dont detect WM/DE from another session
  • Fastfetch version used: 2.19.0
  • Did it work in an older version: idk
  • Where did you get the binary: Build from source directly.
  • Does this issue still occur in the latest dev build? Yes
@Dariqq Dariqq added the bug Something isn't working label Jul 22, 2024
@Dariqq
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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 Dariqq changed the title [BUG] Wrong WM/DE detection in SSH session [BUG] Wrong WM/DE detection in TTY session Jul 22, 2024
@Dariqq
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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
Copy link
Member

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
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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

@Dariqq
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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
Copy link
Member

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
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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
Copy link
Member

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

@Dariqq
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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
Copy link
Contributor Author

Dariqq commented Jul 22, 2024

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
Copy link
Contributor Author

Dariqq commented Jul 26, 2024

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
Copy link
Contributor Author

Dariqq commented Jul 26, 2024

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
Copy link
Member

Please test the dev build

@Dariqq
Copy link
Contributor Author

Dariqq commented Jul 26, 2024

Thanks.

@Dariqq
Copy link
Contributor Author

Dariqq commented Aug 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants