Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

include uid for env variables #5

Closed
wants to merge 1 commit into from

4 participants

@gtmanfred

XDG_RUNTIME_DIR should have the uid instead of %I

@sofar
Owner

Technically I think this is incorrect - it's explained in the README. After systemd version 186, you must use the uid to specify the instance of user-session@.service.

Since the instance already is the UID, %I is properly set this way to make /run/user/%I point to the right location.

Unless you use a login manager which is starting the user session with a name instead of a uid. This is probably a bug, since it is valid to have 2 uid's with the same username. right? What do you use to start the user-session?

The rest of the patch seems appropriate though, and I would not have a problem with a separate patch.

Owner

BTW we really should have the XDG stuff become part or being set through pam_env instead...

@gtmanfred

So here is the other problem, you have to set all these values so that things like urxvt get SHELL set, because urxvt uses $SHELL instead of looking at passwd, it falls back to sh.

The final solution might be to just create a way for xorg-helper-launch to set everything that startx would set, so you also don't have to have this long string of environment variables.

Also, how is it going to work if USER is set to 1000? I hadn't tried that yet but it just seems odd

@henryptung

It seems like this works overall, except for the small problem that the referenced cgroup is based on the UID, not the username anymore, which contradicts systemd; the relevant part of src/login/logind_user.c sets the default cgroup path based on the username:

if (!u->cgroup_path) {

if (asprintf(&p, "%s/%s", u->manager->cgroup_path, u->name) < 0)

return log_oom();

} else

p = u->cgroup_path;

If you think this too is a systemd bug, it should probably be filed on Bugzilla. In the meantime, maybe user-session@.service can be changed to use %u instead of %I for consistency (otherwise, starting up user-session@UID.service after initial login creates two different cgroups for the user, as the original login will use /systemd/user/$USER).

@sofar
Owner

systemd creating both /sys/fs/cgroup/systemd/user/1000 and /sys/fs/cgroup/systemd/user/ahkok seems like a total bug, everything should go to UID instead of NAME instead.

I'll look into that first - that will resolve the isse.

As for $USER=1000, that would be a mistake, unless there is a user with userNAME "1000".

@gtmanfred

is there any way to get this to get all the environment variables from /etc/profile ?

@sofar
Owner

technically, yes, but it requires you to execute a full shell and evaluate all the startup scripts, and then re-import all the settings one-by-one using systemctl environment....

not something I actually support.

I'm thoroughly convinced that getenv() should be a wrapper to some db function ... or just killed.

@gtmanfred

What if it could just use EnvironmentFile=/etc/profile.d/*.sh

this is my current problem right now, it doesn't pick up PATH additions or changes in those files, even if i set that environment, not sure if it has to do with ZSH not inheriting from that environment dir stuff or if it is something else now.

@sofar
Owner

gtmanfred: it will only work if your *.sh files are strictly formatted VAR=VAL only. You can't do anything else like if... then.

@Bruners

enabling user session now seems to work now as intended adding

[Install]
Alias=user-session@%i.service

Tested with systemd 198 and yes that is a lowercase i

@gtmanfred

Is there any arguement against using %U instead of %I everywhere that the uid is needed so that it could be enabled using user-session@$USER or user-session@$UID?

@Bruners

Don't think it matters what you use for enabling as long as the variables that need the UUID gets it, my comment was just to update the "need to manually symlink to activate"

@sofar
Owner

No argument against using %U, especially since we should move away from supporting old systemd versions where %U is not available.

@sofar
Owner

Merged the [Install] section snipplet posted above.

@sofar
Owner

Actually, while there technically isn't any reason that %U wouldn't work, I fail to see why we should change %I to %U. Note that we should never use names here anyway (imagine the problems with utf8 user names or names containing odd characters), and changing it to %U would just lead to unintended consequences later on.

So, let's keep the code using %I... it will make things easier to debug later on.

@sofar sofar closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 2 deletions.
  1. +7 −2 units/system/user-session@.service.in
View
9 units/system/user-session@.service.in
@@ -18,5 +18,10 @@ Type=notify
TTYPath=/dev/tty1
ExecStart=-@SYSTEMDUTILDIR@/systemd --user
Environment=DISPLAY=:0
-Environment=XDG_RUNTIME_DIR=/run/user/%I
-Environment=DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/%I/dbus/user_bus_socket
+Environment=XDG_CACHE_HOME=%h/.cache
+Environment=XDG_CONFIG_DIRS=/etc/xdg
+Environment=XDG_CONFIG_HOME=%h/.config
+Environment=XDG_DATA_DIRS=/usr/local/share/:/usr/share/
+Environment=XDG_DATA_HOME=%h/.local/share
+Environment=XDG_RUNTIME_DIR=/run/user/%U
+Environment=DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/%U/dbus/user_bus_socket
Something went wrong with that request. Please try again.