Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Fix incorrect wayland session detection faq #64

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

FinnPerry
Copy link

The current suggested fix doesn't work as I believe :0 is a X11 thing. For wayland it should be wayland-0 (or a number other than 0).

I've used ExecStart instead of Environment because that's the only way to get systemd to expand the variable.

@@ -226,7 +226,8 @@
Add this block below the first 2 lines of the file, then save and exit
<codeblock class="mt-2" language="ini">
[Service]
Environment=WAYLAND_DISPLAY=:0
ExecStart=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for this empty extra ExecStart?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty ExecStart is to clear the value. Otherwise when you run it, it will run both the default start command and the custom one which makes it throw an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's pretty weird behavior, makes me wonder if that's what it's supposed to do.

@X9VoiD
Copy link
Member

X9VoiD commented Feb 26, 2022

Should probably change it to ExecStart=/usr/bin/env sh -c "WAYLAND_DISPLAY=$WAYLAND_DISPLAY /usr/bin/otd".

Not sure if doing the same for invoking otd is better but this thing above is at least an improvement as it removes explicit /bin/bash dependency.

@cidkidnix
Copy link

Ideally OTD should detect the socket by itself. My issue with this PR is that it assumes that every user-unit in systemd inherits the user environment.

@cidkidnix
Copy link

Also could stick with the original method of doing it but telling users to run ls /run/user/$UID/ | grep wayland and taking the wayland socket name from there

@FinnPerry
Copy link
Author

Changed to use /usr/bin/env. I tested and if you run it like that it can get the env var normally so I've removed the manual setting.

@FinnPerry
Copy link
Author

Also could stick with the original method of doing it but telling users to run ls /run/user/$UID/ | grep wayland and taking the wayland socket name from there

Wouldn't they have to do this again if the socket changes though? The reason I'm making this pr is because I originally had mine set to wayland-0 then my pc changed it to wayland-1 at some point so the config broke.

@gonX
Copy link
Member

gonX commented Feb 26, 2022

#43 ?

@cidkidnix
Copy link

Also could stick with the original method of doing it but telling users to run ls /run/user/$UID/ | grep wayland and taking the wayland socket name from there

Wouldn't they have to do this again if the socket changes though? The reason I'm making this pr is because I originally had mine set to wayland-0 then my pc changed it to wayland-1 at some point so the config broke.

This was probably due to wlroots switching from wayland-0 to wayland-1 assuming you're on a wlroots-based compositor

@FinnPerry
Copy link
Author

#43 ?

Yeah the graphical-session dependency seems like a better long term solution than my thing.
Will try it out when I have free time.

@InfinityGhost InfinityGhost added bug Something isn't working documentation Improvements or additions to documentation labels Mar 3, 2022
@InfinityGhost InfinityGhost marked this pull request as draft March 3, 2022 21:35
@InfinityGhost
Copy link
Member

Marked as draft due to a potentially better solution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants