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

Add gnome wayland support #44497

Merged
merged 7 commits into from
Dec 10, 2018
Merged

Conversation

hedning
Copy link
Contributor

@hedning hedning commented Aug 5, 2018

Motivation for this change

This adds support for launching gnome wayland sessions from GDM and SDDM, using the upstream session file. See #39871 and #32806 for some more discussion.

Edit: Updated against master now that gnome 3.30 is merged.

sessionCommand support

This should work quite well, but one remaining issue is the lack of support for displayManager.sessionCommands and therefore also gnome3.sessionPath. Some options here:

  • We could patch GDM, gnome-session, to run the wrapper for wayland-sessions too, not super happy about it, but might be good as it makes GDM act more like SDDM and LightDM.
  • Or we could move sessionCommands into /etc/profile checking for eg. XDG_SESSION_TYPE, but that seems really ugly.

Edit: We landed on simply not supporting sessionCommands on wayland for now.

patching gnome-session more?

6171bc0 might be better expressed by patching gnome-session to always look for session files in its own share/ folder.

Writing a test

Edit: gnome3-gdm now runs a wayland gnome session (GDM prefers the wayland session and we don't have a good way of telling it otherwise), so I ported it to work with wayland.

cc @jtojnar

Things done

Edit:

  • Tested succesfully in a VM (at a6c1ffe).

  • Ran gnome3, gnome3-gdm and sddm tests succesfully.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)

  • Built on platform(s)

    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)

  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"

  • Tested execution of all binary files (usually in ./result/bin/)

  • Determined the impact on package closure size (by running nix path-info -S before and after)

  • Fits CONTRIBUTING.md.


@GrahamcOfBorg GrahamcOfBorg added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) labels Aug 5, 2018
jtojnar
jtojnar previously requested changes Aug 5, 2018
pkgs/desktops/gnome-3/core/gnome-session/fix-paths.patch Outdated Show resolved Hide resolved
@hedning
Copy link
Contributor Author

hedning commented Aug 6, 2018

Replying to #39871 (comment) here instead.

I would not call the xsessionWrapper particularly elegant solution either. I would like to use the Xsession provided by GDM in the end.

Agreed, and in Gnome's case it seems the session is set up correctly with eg. pulseaudio, journal logging and dbus without the wrapper, mostly leaving /etc/profile which is sourced correctly. The only missing things seems to be sessionsCommands and graphical-session.target I think.

Any idea how to get the graphical-session.target running?

For what is worth, sessionPath might actually make sense outside of GNOME (e.g. for elementary), so moving it to /etc/profile should work.

True, there's not really any reason to switch on XDG_CURRENT_SESSION, users probably want functional software if they happen to currently run something else than Gnome. I'm guessing it even might make sense in a console session, as there could be cli tools requiring something exposed in these variables (probably not likely, but it shouldn't hurt).

As for sessionCommands, I would just add a comment it is not supported for sessions created from extraSessionFilePackages.

I'm not opposed to that. It should be noted that sessionCommands are currently sourced for X sessions from extraSessionFilePackages, due to the xsessionWrapper, so it's only wayland sessions that won't run it, which is even less breakage :)

Default session

One more problem, the wayland session have the same name as the xorg session (there's an extra session file that's explicitly for xorg) and seem to take priority. So people might end up «upgrading» to wayland without any warning which could be jarring.

@jtojnar
Copy link
Member

jtojnar commented Aug 6, 2018

Any idea how to get the graphical-session.target running?

Not sure. These might be relevant:

The following presentation looks extra interesting.

https://lists.freedesktop.org/archives/systemd-devel/2018-June/040952.html

True, there's not really any reason to switch on XDG_CURRENT_SESSION, users probably want functional software if they happen to currently run something else than Gnome.

For GNOME Flashback and Elementary, I have changed it to jtojnar@b5b2d8d but removal of the condition might make sense too.

I'm not opposed to that. It should be noted that sessionCommands are currently sourced for X sessions from extraSessionFilePackages, due to the xsessionWrapper.

Right. I would actually expect most of the things in the xsessionWrapper to be in the xsession instead. After all, upstream sessions should be launching most of these themselves, there should only be the Xorg things, see ${gnome3.gdm}/etc/gdm/Xsession.

So people might end up «upgrading» to wayland without any warning which could be jarring.

I would not worry about that. If the Wayland session fails to start for some reason, gnome-xorg will be selected automatically.

Actually, I am not sure GDM even respects default DE nixos option. I think it uses the user’s last selected one.

@hedning
Copy link
Contributor Author

hedning commented Aug 6, 2018

Hmm, so looks like we're abusing graphical-session.target. In that case I'm at least fine with breaking that in the wayland session.

Right. I would actually expect most of the things in the xsessionWrapper to be in the xsession instead. After all, upstream sessions should be launching most of these themselves, there should only be the Xorg things, see ${gnome3.gdm}/etc/gdm/Xsession.

Hmm, true, I didn't think that through fully when I wrote it. The wrapper should model gdm's Xsession more closely, though ${sessionCommands} will need to go there too. Perhaps out of scope of this PR, though could of course put it here anyway if it's wanted?

@hedning
Copy link
Contributor Author

hedning commented Aug 6, 2018

Another issue I bumped into (dogfooding the PR at the moment):

gnome-session launches a login shell using the user's preferred shell (as opposed to xsessionWrapper sourcing /etc/profile). In my case ZSH.

This results in any new interactive bash shells to source /etc/profile. In particular running nix run nixpkgs.some-package will result in an environment with a PATH that doesn't include the package. nix-shell is unaffected, so this might be a bug in nix, but it's never the less very sub-optimal. We're basically hitting a strange variant of #33219

The problem boils down to $__ETC_PROFILE_DONE never being set.

In lieu of /etc/environment we would probably need some sort of shell agnostic variant of __ETC_PROFILE_DONE that guards against sourcing the environment again for interactive shells.

@jtojnar jtojnar mentioned this pull request Aug 15, 2018
9 tasks
@hedning hedning mentioned this pull request Oct 7, 2018
13 tasks
jtojnar referenced this pull request Oct 15, 2018
We are patching GDM to respect GDM_SESSIONS_DIR environment
variable, which we are setting in the GDM module. Previously, we
only took care of a single code path, the one that handled session
start-up; missing the one obtaining the list of sessions.

This commit patches the second code path, and also whitelists the
GDM_SESSIONS_DIR so that it can be passed to the greeter.

Fixes #34101
@bkchr
Copy link
Contributor

bkchr commented Nov 11, 2018

Any update on this pull request?

@jtojnar
Copy link
Member

jtojnar commented Nov 11, 2018

This is blocked by #45950 which is blocked by #46020

@nathyong
Copy link
Contributor

nathyong commented Dec 4, 2018

What's the status on this now that gnome3 has been updated?

@hedning
Copy link
Contributor Author

hedning commented Dec 5, 2018

@nathyong gnome-3.30 is in staging-next, I'll fix up this PR when it hits master, then we should have wayland support pretty soon :)

@hedning
Copy link
Contributor Author

hedning commented Dec 9, 2018

Ported the gnome3-gdm test to wayland (it broke since gdm prefers the wayland session. It's not super pretty, but should work, so we'll have test too :)

I think this should be ready now 🎉

@hedning hedning changed the title [WIP] Add gnome wayland support Add gnome wayland support Dec 9, 2018
gnome-session inherits GDMS PATH, which is at the moment non-functional. In X11
this didn't matter as the `Xsession` wrapper would populate the environment
beforehand. Wayland sessions doesn't source `Xesssion` (duh), so we patch
`bin/gnome-session` to use absolute paths for `grep` and `bash`.

In addition `bin/gnome-session` is a simple wrapper around
`libexec/gnome-session-binary` mostly responsible for sourcing the users profile
before launching the binary. This made our wrapping of `bin/gnome-session`
ineffective on wayland as the profile would reset the environment. Simply wrap
`libexec/gnome-session-binary` instead.
LightDM is unable to separate between `wayland-sessions/gnome.desktop` and
`xsessions/gnome.desktop` so I ommitted adding this to LightDM.
This will simply make the `sessionPath` more likely to work.
This isn't strictly necessary yet as LightDM doesn't read the wayland sessions,
but there's no harm in being explicit.
The test now runs wayland, which means we can no longer use X11 style testing.
Instead we get gnome shell to execute javascript through its dbus interface.
@hedning
Copy link
Contributor Author

hedning commented Dec 10, 2018

Found a much simpler fix for gnome-session: simply wrap libexec/gnome-session-binary instead of bin/gnome-session, that way sourcing the profile in gnome-session won't clobber the wrapping environment.

Reran the tests, will merge later today if no one has any objections.

@hedning
Copy link
Contributor Author

hedning commented Dec 10, 2018

@GrahamcOfBorg build nixosTests.gnome3-gdm

@hedning hedning merged commit 59d1fb6 into NixOS:master Dec 10, 2018
@hedning
Copy link
Contributor Author

hedning commented Dec 10, 2018

The test timed out on ofborg, hopefully not due to testing for active gnome shell too early, putting too much pressure on the machine. Lets see how it dose on hydra.

@jtojnar jtojnar mentioned this pull request Jan 18, 2019
6 tasks
@hedning hedning deleted the gnome-upstream-wayland branch October 15, 2019 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants