dbus uses the sd_booted() check for determining whether to talk to logind (in _dbus_is_console_user()) and whether journald is available (in _dbus_init_system_log()). Both of these currently don't do the right thing, as explained in detail here: https://mail.gnome.org/archives/desktop-devel-list/2013-March/msg00092.html So we need to fix _dbus_is_console_user() to actually check for logind (to avoid failing if systemd was built with --disable-logind), and sd_booted() to only succeed if systemd was actually booted, to avoid failing if running logind without the systemd init parts. The latter has been fixed upstream already, and just needs to update the local copy of sd-daemon.c.
Created attachment 76858 [details] [review] Fix test for logind availability This fixes the first part, for _dbus_is_console_user().
Created attachment 76859 [details] [review] Update sd-daemon.[hc] from upstream This fixes the second part. I simply ran "make update-systemd" for this.
Comment on attachment 76858 [details] [review] Fix test for logind availability Review of attachment 76858 [details] [review]: ----------------------------------------------------------------- Looks fine apart from whitespace trivia. Do you need this in 1.6.x or just in 1.7? I suspect 1.6.x is the version we ought to be recommending for GNOME 3.8, since nobody's had time to do much with 1.7 - unless you have been testing with 1.7 or master all along. ::: dbus/dbus-userdb-util.c @@ +21,4 @@ > * > */ > #include <config.h> > +#include <unistd.h> For information, this would not be OK in most source files; but dbus-userdb*.* are only used on Unix, so it's fine. @@ +55,5 @@ > dbus_bool_t result = FALSE; > > #ifdef HAVE_SYSTEMD > + /* check if we have logind */ > + if (access("/run/systemd/seats/", F_OK) >= 0) Space before parenthesis please. (But I'll fix that when I commit it.)
Comment on attachment 76859 [details] [review] Update sd-daemon.[hc] from upstream Review of attachment 76859 [details] [review]: ----------------------------------------------------------------- Yeah, looks OK.
For Debian we'll need to apply it to both the unstable (1.6.8) and the experimental (1.7.0) version, but backporting this seems easy enough. So as long as it hits upstream master, we won't have a permanent delta and the "systemd init without logind" case will be fixed for other folks as well. I don't know about your rules for stable branches; I believe this is perfectly safe and backwards compatible, but it's not a major bug either. > > +#include <unistd.h> > For information, this would not be OK in most source files; but dbus-userdb*.* > are only used on Unix, so it's fine. Right, logind is Linux only, that constrains it even further. Out of interest, what is being used on Windows?
(In reply to comment #5) > For Debian we'll need to apply it to both the unstable (1.6.8) and the > experimental (1.7.0) version, but backporting this seems easy enough. (I am also a Debian maintainer of dbus. Please respect the freeze and do not upload non-RT-approved changes to unstable at this point.) (In reply to comment #5) > I don't know about your rules for stable branches; I believe this is > perfectly safe and backwards compatible, but it's not a major bug either. The policy I apply to stable-branches is roughly "the Debian release team would normally consider it for a freeze exception" :-) (Or: "non-intrusive fixes for important bugs that affect non-hypothetical users".) If a distribution using D-Bus 1.6 needs to support systemd-logind without systemd-as-pid-1, I'd be willing to backport the userdb part, and the parts of sd-daemon.c that are not whitespace-damage or other unrelated changes. However, Debian unstable doesn't support systemd-logind unless you also have systemd as pid 1 (which won't change until at least the Debian 7 release), Ubuntu patches dbus for Upstart support anyway, and most other Linux distributions appear to be using systemd, so I don't think this is necessary at this stage. Let us know if it becomes a problem.
(In reply to comment #6) > Please respect the freeze and do not upload non-RT-approved changes to > unstable at this point. Oh dear, no. Sorry if that was unclear, but all of this is planning ahead for jessy. But as "1.7.x" sounds like developer releases which we don't usually put into unstable, so if we keep 1.6.8 in unstable after wheezy's release, we should apply the patch there, too. I'll file Debian bugs about all affected packages in due time. > The policy I apply to stable-branches is roughly "the Debian release team > would normally consider it for a freeze exception" :-) I guess that means "only apply it to master" then. Sounds fine to me. Thanks!
(In reply to comment #5) > Out of interest, what is being used on Windows? The build system (Makefile.am or cmake/*/CMakeLists.txt) would tell you. A few .c files (including dbus-sysdeps*, dbus-userdb*, dbus-spawn*, *-unix, *-win) are allowed to use OS-specific headers and functions. The rest is meant to be a portable subset of C89 (sadly, Microsoft C compilers are stuck in the 80s) with, in principle, no system headers in header files.
Fixed in git for 1.7.2. Backports can be considered later if needed (but I would hope that by the time we get to Debian 8 we'll be using D-Bus 1.8).
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.