ahkok / user-session-units

A collection of units for the systemd user session.
GNU Lesser General Public License v2.1
72 stars 23 forks source link

include uid for env variables #5

Closed gtmanfred closed 11 years ago

gtmanfred commented 11 years ago

XDG_RUNTIME_DIR should have the uid instead of %I

gtmanfred commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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

sofar commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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

sofar commented 11 years ago

Merged the [Install] section snipplet posted above.

sofar commented 11 years ago

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.