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

util: Blacklist some session-specific variables #141

Merged
merged 2 commits into from
Apr 26, 2021

Conversation

leigh123linux
Copy link
Contributor

@leigh123linux leigh123linux commented Mar 29, 2021

@smcv
Copy link

smcv commented Mar 29, 2021

Thanks, this looks good to me. While you're here you might also want to sync up with later changes to the corresponding file in gnome-session, which fixes various bugs related to this code:

  • 9d8b0709 adds NOTIFY_SOCKET to this list (that variable should basically never propagate between processes)
  • b7b24627 uses systemd's UnsetAndSetEnvironment to actively unset some variables (instead of just not setting them, as done here)
  • 60e619b8 removes these variables from the environment of /etc/xdg/autostart apps, too
  • fe22c4ee fixes handling of rare whitespace characters like \r
  • 87d92fec avoids warnings when systemd is not managing the session
  • 3b57d117 fixes a stack overflow (session manager crash) if users have long environment variables
  • d44888fb is the opposite side of this: the change requested in Cinnamon session shouldn’t upload XDG_SESSION_ID to systemd —user #140 stops inappropriate variables from "leaking" out of a Cinnamon session into a later login session, and d44888fb stops those same variables from "leaking" into the later login session, on the principle that where there's a bad interaction between two components, the most robust thing to do is to fix it on both sides

Those all seem like good fixes to have.

@ItzSwirlz
Copy link
Contributor

Yeah, to be fair it's been a while since we've checked upstream for anything that may be useful for -session. Thanks @smcv :)

@ItzSwirlz
Copy link
Contributor

I'd love to implement GNOME/gnome-session@b7b24627 but we aren't sure what the idea with Wayland is.. I was asked if we would follow the proper XDG_DECORATION in muffin and stuff. Does any of the devs know what we want to do with Wayland or are we sticking with Xorg?

@smcv
Copy link

smcv commented Apr 6, 2021

GNOME can be run in either X11-only or Wayland mode (they appear as separate session types in gdm), and its handling of those environment variables is the same for both modes - so I expect that Cinnamon should probably do the same, regardless of whether Cinnamon is permanently X11-only, currently X11-only with plans to adopt Wayland in future (similar to old GNOME versions), or already supports Wayland (similar to current GNOME).

In an X11-only session, if a previous session (maybe GNOME or another desktop) has pushed WAYLAND_DISPLAY and WAYLAND_SOCKET into the systemd environment, you'll want to remove them from the environment so that they don't confuse apps into thinking they can still use Wayland. In a Wayland session, you'd want to remove those variables from the environment and replace them with your own values for them (if any). I think the gnome-session code does the right thing for both of those cases.

@smcv
Copy link

smcv commented Apr 6, 2021

If Cinnamon/muffin becomes a Wayland compositor in future, then it's up to the compositor developers whether it prefers client-side decoration (like Weston and mutter), server-side decoration (like traditional X11), or some combination of the two.

I don't think getting the environment variable handling right has any impact on that decision.

@ItzSwirlz
Copy link
Contributor

Implemented the commits above in leigh123linux#1 except for GNOME/gnome-session@60e619b8 because we need to add a few more commits to catch up to it.

@mtwebster mtwebster merged commit 753fa04 into linuxmint:master Apr 26, 2021
@mtwebster
Copy link
Member

mtwebster commented Apr 26, 2021

@ItzSwirlz please open a new pr for that other stuff. Also please be sure all of that even applies to us.
For instance, CINNAMON_SHELL_SESSION_MODE isn't a thing - I believe that had to do with gnome's short-lived 'classic mode'.

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

Successfully merging this pull request may close these issues.

Cinnamon session shouldn’t upload XDG_SESSION_ID to systemd —user
4 participants