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

Optionally install wayland session files #12273

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SparkyBluefang
Copy link
Contributor

Add options to disable installing wayland session and xdg-portal files. Cinnamon shouldn't advertise having wayland sessions if the rest of the packages are built without wayland support.

I didn't see a good option for handling cinnamon-session-cinnamon without a more significant refactor. But fortunately cinnamon-session aborts with an obvious error message

$ cinnamon-session-cinnamon --wayland
cinnamon-session-binary[25984]: CRITICAL: t+0.00369s: Unable to start session: Failed to load session "cinnamon-wayland"

@Fantu
Copy link
Contributor

Fantu commented Jul 4, 2024

It seems clear to me that Wayland support is experimental and incomplete, but having the opportunity to try it can help you see the problems that will gradually be resolved.
Have you perhaps encountered problems with users who complained, or other problems?

@SparkyBluefang
Copy link
Contributor Author

For me, this is more about packaging consistency. All of the other cinnamon packages have optional wayland support, and if muffin (for example) is built without, it seems a little silly to be installing wayland session files.

I just checked and cinnamon-session --session cinnamon-wayland will happily launch a session even if muffin is built without. But that means it will be missing components like the screensaver daemon, which probably isn't ideal.

At least in Gentoo, wayland is not enabled by default unless you're specifically using a GNOME or KDE package manager profile. The xdg-portal support is also optional, though enabled by default. So it would be odd to install the config/session files if xdg-desktop-portal-xapp is not installed or wayland is disabled.

Side note - since cinnamon wayland support is experimental/incomplete, the Gentoo package does have an additional mask prevention users from enabling wayland builds. But a user is able to unmask and enable wayland, if they desire.

I'm not strongly pushing for this being accepted upstream, since this might be a bit specific to Gentoo packaging. If not, I'll just continue to conditionally remove these files in the package definition. But since I was going through and submitting a variety of PRs, I figured I'd give this one a chance too :)

@leigh123linux
Copy link
Contributor

I don't get the point to making xdg-portal conf optional, it is used in xorg session are well.
Are you going to deal with gentoo users who complain their apps (example gnome-online-accounts-gtk) don't have proper dark theme support.

@SparkyBluefang SparkyBluefang changed the title Optionally install wayland and xdg-portal files Optionally install wayland session files Jul 6, 2024
@SparkyBluefang
Copy link
Contributor Author

At one point, the xdg portal stuff was just targeting flatpack and other app sandboxes. But, as you point out, it's evidentially becoming increasingly prevalent in normal desktop apps/envs. Annoying, considering xdg-desktop-portal pulls in stuff like pipewire and fuse, but maybe better in the long run since it standardizes some of the settings rat's nest that DEs have to deal with.

libadwaita does use the portal by default - it does support uing gsettings if ADW_DISABLE_PORTAL=1 is in the environment, but that's hardly appropriate to set at the session/system level (and Cinnamon isn't copying the necessary gsettings keys anyways). More unexpectedly, it seems to break dark mode in Cinnamon itself, and I can't figure out where it's talking to either gsettings or the portal.

Regardless, the simple solution is to make the portal stuff required, since that seems to be the trend. I've dropped those changes from the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants