-
Notifications
You must be signed in to change notification settings - Fork 236
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
User Services #573
base: master
Are you sure you want to change the base?
User Services #573
Conversation
c664da6
to
52a6b29
Compare
src/shared/misc.c
Outdated
char * | ||
rc_krunlevel_path_get() { | ||
if (!krunlevel_path) { | ||
krunlevel_path = xmalloc(sizeof(char) * PATH_MAX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @anna-cli. One major problem I see is that memory is allocated each time the function is called, but never freed. Or did I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @anna-cli. One major problem I see is that memory is allocated each time the function is called, but never freed. Or did I miss something?
Hello @andy5995! It shouldn't allocate more than once per function (since the
pointers are global, and there's the check before allocation). But you
made me realise that is probably better to just declare the string as a
static char array inside the function!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at #432 I don't see any of the maintainers making any comment on whether this feature is desired or not.
But just glancing over the diff, I see a decent amount of things which I do not like. I figured I'd leave my comments in here.
But keep in mind that I'm not a maintainer here, so for some of my "larger design related" comments, you probably should wait for a maintainer to comment before acting on it.
c89ba7d
to
8cf077a
Compare
7436c74
to
dec4e6a
Compare
Hello again! Last two days I rewrote the whole patchset, and I think it is ready for review! (although there's probably a few mistakes left still). |
f035ad9
to
b27b375
Compare
@dwfreed @williamh @vapier (CC @thesamesam), Hello! sam recommended I tag one of you, could you check this PR? If it is a desired feature, and if the code overall looks okay! I am currently daily driving the the patches and looking for bugs and if everything is working too. |
Easy way to test on gentoo:
|
114eea7
to
be062d4
Compare
Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev> Closes: OpenRC#573 Signed-off-by: Sam James <sam@gentoo.org>
This is great! However, have you had any problems with services using D-Bus at all? How would you handle environment exports like that? I am asking since stuff like PipeWire requires it to properly function. |
for the scripts i wrote so far, only wireplumber needs dbus, a hack to make it work is setting 'rc_env_allow="DBUS_SESSION_BUS_ADDRESS"' in .config/openrc/rc.conf, tho this won't really work when the runlevel is started via PAM (which would be the ideal way to handle that imo). one thing i'm thinking is making a user init script for a dbus user session, that could be a step to allow the services to see dbus, tho i'm still not sure how to get the var from that into the env of other scripts |
That's the problem. You should preemptively export Now, as for the how, it's up to you; I think you can place it in Also yes, an accompanying PAM module might be beneficial to the PR. I would suggest looking at https://github.com/chimera-linux/turnstile/blob/master/src/pam_turnstile.cc |
58a49f2
to
8d7290d
Compare
openrc --user 3.0 the patch is a lot simpler, does a lot less (and that's good) first all environment exporting/handling code got removed, instead we'll just pass that burden into the service package. /etc/profile.d already gives program a way to export variables into the user's environment, so a package can simply use that without openrc needing to do extra work. secondly the rc_*_dir api got simplified to two functions, that take a subpath. this was done because that was basically all they were used for, and imo, it's a nicer api and lastly, simpler openrc-pam, since no env code is needed. also instead of counting logins ourselves, this new code relies on utmpx. i am still unsure which is the better approach improved how openrc-pam handles xdg_rundir when it's not set as well. it was suggested using /etc/profile.d/00-openrc.sh or similar to launch openrc --user, while i like the idea, it lacks a way to stop openrc on log-out, as well as lacking a way to create the rundir properly (since it would require elevated privs) any suggestions or comments about those changes is appreciated! |
While I agree with this sentiment, /etc/profile.d/*.sh is usually sourced by /etc/profile, which only is sourced in login shells. Basically it all happens after PAM starts Also, |
/etc/init.d/user/dbus:
/etc/profile.d/50-dbus-openrc.sh:
why?
then they gotta do that themselves. many things relies on /etc/profile{,.d,.env}, if you're using fish as your login shell, you're gonna need to fix the system integrations yourself. distros can also just ship a fish version of the script snippet up there, it's their choice, the good thing about this is, openrc won't really care about how you export that. |
Honestly, it's pretty good and I didn't think of that initially. I wonder if there's a way to standardize an extra command, for example For that matter, a profile script that retrieves the list of user services that have to be ran before the login shell (equivalent to
Dumb brain loves static stuff.
Fair; though a lot of people use |
68c55b3
to
e5cc416
Compare
abc9636
to
175dbb8
Compare
What's the goal of using a pam module instead of a direct approach (like A pam module puts any form of reconfiguration outside of the reach of regular users. E.g.: it impossible for a user to run something before the service manager starts, temporarily disable the service manager, etc. |
.profile is sourced by the login shell, however, you don't know which login shell will be spawned, and you don't even know if that'll be a login shell. If it's a graphical login session, then stuff like dbus and pipewire have to necessarily be started before the login session. Of course, if you don't use a display manager you could put that in .profile, but that's not everyone's use case. |
with .profile, we can't ever stop services on the last log-out, as there's no system-wide "/etc/logout.profile" or something, we also can't set up xdg_runtime_dir for users without a setuid binary. i'm not super happy with pam either, to be honest. as is right now, it's tending to a bit of scope creep, and i'm unsure if openrc should be doing all that i'm still experimenting with alternatives, but the goals are:
there should be an option to disable auto-launching from the user-side, yeah. then everything is handed over to the user |
Well, fwiw, last year i created an ebuild for pam_xdg: https://bugs.gentoo.org/908431 |
add two api functions, `rc_service_dir` and `rc_sysconf_dir`, both are generate paths (and sub-paths) for resources, and meant to replace the hardcoded variables like `RC_SVCDIR`. those functions differ by dynamically switching between the system path, or the user path, set in their home folder or runtime directory. this lays out the intial support for user services. Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
Adds a set of apis to manipulate user options. Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
the module sets up XDG_RUNTIME_DIR if it's already not set, keeps a lock file in RC_SVCDIR/users/<username>/session_count, and then starts or stops openrc --user. Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
If services are started via pam, they will not inherit the environment defined in
If a user changes their shell to something that doesn't source
These services can be started by a program executed by the user, rather than by something managed by root and enforced via PAM. They do not need to start before the login shell; I personally run |
the pam module, as is right now, starts a login shell, with i'm also looking at instead of starting it directly, have a system wide service
the user will have an option to not auto-start services. saying that "not sourcing .profile" is the solution for that is silly, because, what if they want .profile, and just not openrc --user?
you then lose on the main point of a service manager, that is having it manage, auto-restart on failure, and more, your services that run as your user.
which does not work for DEs like kde plasma which need dbus running before it starts, and display managers like SDDM don't source .profile at all, those services need to start before the gui does. and another main issue with .profile for this, is still, we can't stop them. the user logs out of all their sessions, the services will keep running. and the user still has full control, by disabling autostart on their rc.conf (i'm still to implement this, but it'll be an option), or by adding services they want to start before login in the default runlevel, then stacking that into another runlevel with extra, post-login services, and calling that runlevel manually |
Fair enough, I you've address my points quite clearly :)
You're going to need some glue code for DEs that expect dbus to be running prior to them starting regardless of whether that glue is something that runs as part of the user session, or something that pam enforces.
Why implicitly enforce stopping of services instead of, again, letting users do it explicitly? Can users opt-out of all their processes being killed?
Nice. A bit tangential but: have you contemplated a runlevel that gets executed even when a user does not log in? (e.g.: for perhaps background services that they want permanently) |
both questions are kind of one in the same, about managing services outside of a session for the first, well, yes, because if you're not using pam to begin with, we can't stop the runlevel automatically the reason for pam is just, convenience for most users that just want, say, dbus and pipewire working when they're using their system. starts at login, stops at logout to preserver resources, and totally optional starting a runlevel at boot tho, is planned, but so far as a system-wide setting, aka, you'd need to be root to set it up. i'm unsure if we can do this in a sane way without root, nor know if it's even desirable to tho i'm thinking now, the bulk of user services is working, i wonder, i could drop the pam stuff from this pr, and ask for review/tests on the core functionality, maybe merge this and then work on pam/auto-start after? |
Hello! This is the start of a draft for supporting user services.
The current "design" works by basically mimicking the layout of the system services.
This commit is the start of the proof of concept, based on what was talked on #432.
I'm opening this draft PR early to ask for some comments and a bit of guidance (since is this my first patch contributing to any project!), and to track progress of the feature.
The current implementation for paths is not that great IMO but I failed to think of a better one.
This also only includes librc, some other utilities are accessing the define'd variables and thus are broken in this patch, but I was able to get rc-update and rc-status, and to an extend, rc-service too, working as things is (granted I created the files in .config manually).
Once a better implementation is decided (or if this one is deemed ideal) I'll patch every file needed and run the tests properly.
The current implementation checks for the callers
euid
in order to toggle to user mode. If this is not desirable, we could instead introduce subcommands or flags in the commands themselves (rc-update user add <service>
for example)I am also still not familiar with openrc-run, so more study on that and the possible syntax for user service files is needed.
In resume, the idea is to have user-specific runtimes, limited to the users home folder, and tracked by the user (either via PAM, by running the
openrc
command manually to set the runlevel or other method).If this feature is wanted, I'm willing to keep developing it and maintaining it for as long as necessary.
Signed-off-by: anna anna@navirc.com