Created attachment 128595 [details] [review] the patch With the attached patch dbus can boot up without any support in /etc or /var correctly, on systemd-based systems. The patch adds a small "sysusers.d" snippet that creates the "dbus" system user early at boot if the system boots up with an empty /etc directory. This only has an effect on systemd-based systems (and the snippet is only installed on them), and improves stateless system support of dbus.
Comment on attachment 128595 [details] [review] the patch Review of attachment 128595 [details] [review]: ----------------------------------------------------------------- ::: bus/dbus.conf @@ +1,5 @@ > +# sysusers.d snippet for creating the D-Bus system user automatically > +# at boot on systemd-based systems that ship with an unpopulated > +# /etc. See sysusers.d(5) for details. > + > +u dbus - "System Message Bus" Instead of hardcoding `dbus`, this needs to use the value from configure’s --with-dbus-user.
ah, indeed. I wasn't aware this was configurable.
Created attachment 128641 [details] [review] new patch, this time honours the user name configured at configure time
Comment on attachment 128641 [details] [review] new patch, this time honours the user name configured at configure time Review of attachment 128641 [details] [review]: ----------------------------------------------------------------- Looks like it distributes dbus.conf and dbus.conf.in? ::: bus/Makefile.am @@ +318,4 @@ > dbus.socket.in \ > systemd-user/dbus.service.in \ > systemd-user/dbus.socket.in \ > + dbus.conf \ I think this should probably be `dbus.conf.in`?
(In reply to Philip Withnall from comment #4) > Comment on attachment 128641 [details] [review] [review] > new patch, this time honours the user name configured at configure time > > Review of attachment 128641 [details] [review] [review]: > ----------------------------------------------------------------- > > Looks like it distributes dbus.conf and dbus.conf.in? > > ::: bus/Makefile.am > @@ +318,4 @@ > > dbus.socket.in \ > > systemd-user/dbus.service.in \ > > systemd-user/dbus.socket.in \ > > + dbus.conf \ > > I think this should probably be `dbus.conf.in`? True. And worse: the dbus.conf.in shouldn't have been listed in CONFIG_IN_FILES=!
Created attachment 128846 [details] [review] third version of this patch fix the .in confusion
Comment on attachment 128846 [details] [review] third version of this patch Review of attachment 128846 [details] [review]: ----------------------------------------------------------------- Looks good to me, thanks! Not sure I can push it, so this might have to wait for smcv to ack it, but from my pov this is review+.
Comment on attachment 128846 [details] [review] third version of this patch Review of attachment 128846 [details] [review]: ----------------------------------------------------------------- ::: bus/Makefile.am @@ +1,4 @@ > dbusdatadir=$(datadir)/dbus-1 > legacydbusdatadir=$(sysconfdir)/dbus-1 > dbus_daemon_execdir = $(DBUS_DAEMONDIR) > +systemdsysusersdir = $(libdir)/sysusers.d On multilib systems (those with lib64) and Debian-style multiarch systems (those with lib/x86_64-linux-gnu or whatever) this is not going to be where systemd looks for the files, I don't think? Shouldn't it be more like this? # Always "lib", even on systems with lib64 or multiarch systemdsysusersdir = $(prefix)/lib/sysusers.d @@ +324,5 @@ > dbus.service \ > dbus.socket > + > +nodist_systemdsysusers_DATA = \ > + dbus.conf I'd slightly prefer to use bus/sysusers.d/dbus.conf{,.in}. Otherwise this file would collide with any other configuration drop-in that is owned by dbus and happens to also use the ".conf" extension.
(In reply to Simon McVittie from comment #8) > @@ +1,4 @@ > > dbusdatadir=$(datadir)/dbus-1 > > legacydbusdatadir=$(sysconfdir)/dbus-1 > > dbus_daemon_execdir = $(DBUS_DAEMONDIR) > > +systemdsysusersdir = $(libdir)/sysusers.d > > On multilib systems (those with lib64) and Debian-style multiarch systems > (those with lib/x86_64-linux-gnu or whatever) this is not going to be where > systemd looks for the files, I don't think? Indeed, of course! > I'd slightly prefer to use bus/sysusers.d/dbus.conf{,.in}. Otherwise this > file would collide with any other configuration drop-in that is owned by > dbus and happens to also use the ".conf" extension. OK, will fix.
Created attachment 129439 [details] [review] fourth version of the patch This time with the path, and moved down one dir
Comment on attachment 129439 [details] [review] fourth version of the patch Review of attachment 129439 [details] [review]: ----------------------------------------------------------------- ::: bus/Makefile.am @@ +1,4 @@ > dbusdatadir=$(datadir)/dbus-1 > legacydbusdatadir=$(sysconfdir)/dbus-1 > dbus_daemon_execdir = $(DBUS_DAEMONDIR) > +systemdsysusersdir = $(prefix)/lib/sysusers.d I'd like a comment above this so that future maintainers don't "correct" this to what you initially said: # Always .../lib, even on systems where ${libdir} is .../lib64 or .../lib/x86_64-linux-gnu or something like that.
Looks good to merge. I'd prefer it with that comment but it isn't required. (If you don't get there first, I'll add the suggested comment and apply this at some point.)
Fixed in git for 1.11.10, thanks
Excellent! Thanks for merging!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.