See the thread starting at <http://lists.freedesktop.org/archives/systemd-devel/2013-February/009119.html> for discussion.
Created attachment 75344 [details] [review] Install systemd user units for a per-user bus When systemd gains support for activating a per-user systemd for each set of overlapping login sessions, this is what it'll use. The socket path used here, dbus/user_bus_socket, is what's assumed by systemd's user@.service and by user-session-units. This obsoletes the dbus.service, dbus.socket units found in user-session-units. --- I would personally have avoided the subdirectory, but everyone who's experimenting with user-session units seems to be using this path so let's go with it. To be useful, this patch requires a systemd with one of the patches from Bug #61129. Lennart, do I need any extra stuff in [Install], like making dbus.service "wanted by" default.target or something?
Created attachment 75345 [details] [review] Pick up an up-to-date DISPLAY from logind if possible In the systemd user bus design, a user bus can span across more than one login session, so the most appropriate DISPLAY to use can change. --- Sorry, when I said "needs one of the patches from Bug #61129" I was thinking about this patch, not the one before... This will become unnecessary if every D-Bus session service becomes a systemd user service (perhaps via a user unit generator).
Created attachment 75346 [details] [review] Make sure tests run with a temporary XDG_RUNTIME_DIR We don't want the regression tests' "session" getting mixed up in system-wide "sessions". This doesn't actually matter yet, but it is likely to matter in future.
Comment on attachment 75344 [details] [review] Install systemd user units for a per-user bus Review of attachment 75344 [details] [review]: ----------------------------------------------------------------- Yes, this is great, thanks for writing this!
(In reply to comment #3) > Created attachment 75346 [details] [review] > Make sure tests run with a temporary XDG_RUNTIME_DIR This part can be merged at any time, if someone reviews it. (In reply to comment #1) > Created attachment 75344 [details] [review] > Install systemd user units for a per-user bus ... > Lennart, do I need any extra stuff in [Install], like making dbus.service > "wanted by" default.target or something? This part needs review from Lennart or another systemd committer. (In reply to comment #2) > Created attachment 75345 [details] [review] > Pick up an up-to-date DISPLAY from logind if possible ... > needs one of the patches from Bug #61129 This part is blocked by Bug #61129.
See also https://people.gnome.org/~walters/user-session-patches/dbus/0001-Install-systemd-user-units-support-per-user-bus-by-d.patch I think I was unaware of these patches at the time I wrote that one. These may be better.
(In reply to comment #6) > See also > https://people.gnome.org/~walters/user-session-patches/dbus/0001-Install- > systemd-user-units-support-per-user-bus-by-d.patch > > I think I was unaware of these patches at the time I wrote that one. These > may be better. + if (!_dbus_string_append (address, "unix:path=")) + goto out; + if (!_dbus_string_append (address, _dbus_string_get_const_data (&user_bus_path))) + goto out; Needs escaping; and I think I prefer my approach on Bug #61303, in which xdg-runtime: is a transport in its own right, like autolaunch:. +ExecReload=@EXPANDED_BINDIR@/dbus-send --print-reply --system --type=method_call --dest=org.freedesktop.DBus / org.freedesktop.DBus.ReloadConfig Should be --session
Comment on attachment 75345 [details] [review] Pick up an up-to-date DISPLAY from logind if possible Superseded by Bug #61303, I think.
Comment on attachment 75344 [details] [review] Install systemd user units for a per-user bus Review of attachment 75344 [details] [review]: ----------------------------------------------------------------- ::: bus/systemd-user/dbus.socket.in @@ +4,5 @@ > +[Socket] > +ListenStream=%t/dbus/user_bus_socket > + > +[Install] > +WantedBy=sockets.target How about install it into sockets.target.wants in bus/Makefile.am? If these user unit is installed, I think it's expected that per-user session dbus-daemon will be activated by systemd.
Another problem here is that the default session bus connect address in libdbus is still set to "autolaunch:" in this case. Apparently it's not correct since the per-user session bus wasn't started with "--autolaunch". So we need setup DBUS_SESSION_BUS_ADDRESS at many places. 1. For the whole user session services, it's fine to set DBUS_SESSION_BUS_ADDRESS in the systemd user instance (user@.service). 2. For the others, we need setup DBUS_SESSION_BUS_ADDRESS one by one. The second case makes things very trivial and messed up. For example, https://review.tizen.org/gerrit/#/c/11933/3/bt-service/org.projectx.bt.service which provides dbus service on system-wide bus while connect to user-session bus to do some things. This is quite *ugly*, program running with login user permission but provides system-wide dbus service. I think we also need similar way to get current session bus connect address in libdbus, for example, _dbus_getenv ("XDG_RUNTIME_DIR")/dbus/user_bus_socket.
Comment on attachment 75346 [details] [review] Make sure tests run with a temporary XDG_RUNTIME_DIR Review of attachment 75346 [details] [review]: ----------------------------------------------------------------- Looks good though I was curious why it's necessary since no one use it currently. However #c3 answered my curious.
(In reply to comment #10) > Another problem here is that the default session bus connect address in > libdbus is still set to "autolaunch:" in this case. Apparently it's not > correct since the per-user session bus wasn't started with "--autolaunch". > So we need setup DBUS_SESSION_BUS_ADDRESS at many places. > > 1. For the whole user session services, it's fine to set > DBUS_SESSION_BUS_ADDRESS in the systemd user instance (user@.service). > > 2. For the others, we need setup DBUS_SESSION_BUS_ADDRESS one by one. > > The second case makes things very trivial and messed up. For example, > https://review.tizen.org/gerrit/#/c/11933/3/bt-service/org.projectx.bt. > service which provides dbus service on system-wide bus while connect to > user-session bus to do some things. This is quite *ugly*, program running > with login user permission but provides system-wide dbus service. > > I think we also need similar way to get current session bus connect address > in libdbus, for example, _dbus_getenv > ("XDG_RUNTIME_DIR")/dbus/user_bus_socket. OK, seems #bug61303 is target for this issue.
(In reply to comment #10) > The second case makes things very trivial and messed up. For example, > https://review.tizen.org/gerrit/#/c/11933/3/bt-service/org.projectx.bt. > service which provides dbus service on system-wide bus while connect to > user-session bus to do some things. I think that's wrong. While it might not matter for your OS, what happens if the user hasn't logged in yet and there's no session around? What if they log out? Even if we're making it harder to support multiple sessions with the per-user bus movement, I don't think we should support "upcalls" from the system to the session. The canonical model for this now is that used by Bluez where a user program connects to the system bus, and then makes a call to the system to register itself as an agent, using its unique bus name. When that unique bus name goes away, it's unregistered. This model is harder to write, but a lot more architecturally sound, I believe.
(In reply to comment #13) > (In reply to comment #10) > > > The second case makes things very trivial and messed up. For example, > > https://review.tizen.org/gerrit/#/c/11933/3/bt-service/org.projectx.bt. > > service which provides dbus service on system-wide bus while connect to > > user-session bus to do some things. > > I think that's wrong. While it might not matter for your OS, what happens > if the user hasn't logged in yet and there's no session around? What if > they log out? > Yes, agreed, however, it's currently so Tizen specific. And this is not the only case, I tracked an issue for tizen at https://bugs.tizen.org/jira/browse/PTREL-118, your comments are welcome. > Even if we're making it harder to support multiple sessions with the > per-user bus movement, I don't think we should support "upcalls" from the > system to the session. Yes, this is not the recommended practice, but as you know, there are some real cases in Tizen. :-(. > > The canonical model for this now is that used by Bluez where a user program > connects to the system bus, and then makes a call to the system to register > itself as an agent, using its unique bus name. I'm totally not a bluetooth guy, not sure how the whole bluetooth stack implemented in Tizen. > > When that unique bus name goes away, it's unregistered. > > This model is harder to write, but a lot more architecturally sound, I > believe.
(In reply to comment #10) > 1. For the whole user session services, it's fine to set > DBUS_SESSION_BUS_ADDRESS in the systemd user instance (user@.service). Changing the default to xdg-runtime:,autolaunch: addresses this, without needing configuration. > The second case makes things very trivial and messed up. For example, > https://review.tizen.org/gerrit/#/c/11933/3/bt-service/org.projectx.bt. > service which provides dbus service on system-wide bus while connect to > user-session bus to do some things. xdg-runtime: would also make this easier. "Makes a wrong usage easier" isn't a reason to prefer xdg-runtime:, but I don't think it's a reason to reject it either. I agree with Colin that it's incorrect to hard-code GUI login session users' names (and runtime directories) in system services. For instance, that service appears to be hard-coding "app". The model used by BlueZ, NetworkManager, and systemd password agents (user-session apps that register with a system service) is much more appropriate.
(In reply to comment #15) > (In reply to comment #10) > > 1. For the whole user session services, it's fine to set > > DBUS_SESSION_BUS_ADDRESS in the systemd user instance (user@.service). > > Changing the default to xdg-runtime:,autolaunch: addresses this, without > needing configuration. Yes, indeed, I think tizen will be the first one to make use of xdg-runtime: > > > The second case makes things very trivial and messed up. For example, > > https://review.tizen.org/gerrit/#/c/11933/3/bt-service/org.projectx.bt. > > service which provides dbus service on system-wide bus while connect to > > user-session bus to do some things. > > xdg-runtime: would also make this easier. "Makes a wrong usage easier" isn't > a reason to prefer xdg-runtime:, but I don't think it's a reason to reject > it either. > > I agree with Colin that it's incorrect to hard-code GUI login session users' > names (and runtime directories) in system services. For instance, that > service appears to be hard-coding "app". The model used by BlueZ, Yes, apparently, if it can running without root privileges, then it should running with its own non-loginable user, just like dbus-daemon running with "messagebus", I did already suggested one Tizen bluetooth guy around me. There are a lot of bad use cases of DBus in Tizen as you know. Hope the components maintainer can fix them in Tizen 3.0 > NetworkManager, and systemd password agents (user-session apps that register > with a system service) is much more appropriate.
Comment on attachment 75344 [details] [review] Install systemd user units for a per-user bus Review of attachment 75344 [details] [review]: ----------------------------------------------------------------- ::: bus/systemd-user/dbus.socket.in @@ +1,5 @@ > +[Unit] > +Description=D-Bus User Message Bus Socket > + > +[Socket] > +ListenStream=%t/dbus/user_bus_socket According to another bug, Lennart apparently wants this to be just "%t/bus".
Comment on attachment 75346 [details] [review] Make sure tests run with a temporary XDG_RUNTIME_DIR Applied this one for 1.7.10
Created attachment 91549 [details] [review] Install systemd user units for a per-user bus When systemd gains support for activating a per-user systemd for each set of overlapping login sessions, this is what it'll use. The socket path used here, $XDG_RUNTIME_DIR/bus, does not match what was used in user-session-units, but is what Lennart recommended on fd.o #61303. [Patch v2: replaced XDG_RUNTIME_DIR/dbus/user_bus_socket with XDG_RUNTIME_DIR/bus, added sockets.target.wants/dbus.socket link, added Documentation key.]
Colin/Lennart, any thoughts on this for 1.9?
From upstream systemd we have no plans in support non-kdbus user-mode systemd. While we could open this up and allow user-mode systemd with classic dbus1 (and people are hacking things into place to make this work locally), I think it makes sense to go all the way in one step. Hence, I'd not apply this to dbus1 at this point, we wouldn't support the counterpart in systemd...
(In reply to Lennart Poettering from comment #21) > From upstream systemd we have no plans in support non-kdbus user-mode > systemd. Is there a graceful way in which the dbus.socket installed by classic D-Bus can detect that kdbus is supported, and disable itself? ConditionPathExists=!/sys/fs/kdbus or something?
(In reply to Lennart Poettering from comment #21) > we could open this up and allow user-mode systemd with > classic dbus1 (and people are hacking things into place to make this work > locally) AFAICS, the only requirement other than making dbus-daemon listen on %t/bus is that either the predictable DBUS_SESSION_BUS_ADDRESS needs to be given to session and user-service processes in their environment, or D-Bus implementations need to learn to try XDG_RUNTIME_DIR/bus before resorting to autolaunch. Does either of those actually need systemd changes? systemd already has facilities for shuffling environment variables around in a configurable way, and sd-bus already looks for unix:path=XDG_RUNTIME_DIR/bus, subject to the assumption that XDG_RUNTIME_DIR is something that doesn't need escaping. That property is true in practice for runtime directories made by systemd-logind; presumably sd-bus does not support being used on systems where something that isn't systemd-logind is responsible for XDG_RUNTIME_DIR.
(In reply to Simon McVittie from comment #23) > AFAICS, the only requirement other than making dbus-daemon listen on %t/bus > is that either the predictable DBUS_SESSION_BUS_ADDRESS needs to be given to > session and user-service processes in their environment, or D-Bus > implementations need to learn to try XDG_RUNTIME_DIR/bus before resorting to > autolaunch. Remember that the autolaunch mechanism is there for compatibility with systems that didn't start D-Bus in the first place. We can expect systemd to do it, so we should simply require that the proper environment variables be set.
Created attachment 113368 [details] [review] [1/4] On Unix platforms, try $XDG_RUNTIME_DIR/bus before default address This is safe to do even on systems where there is a per-login-session bus: the $XDG_RUNTIME_DIR/bus would just not exist there. This means that OS builders can enable a per-user-session bus by merely providing configuration to start it, without needing to rebuild the client library. Based on a patch by Colin Walters, with these changes: - factor out the actual XDG_RUNTIME_DIR bit into a function - set error correctly on OOM - do not try to use an XDG_RUNTIME_DIR/bus that belongs to a different uid or is not a socket - escape the path if it contains inconvenient characters - coding style adjustments --- This is the second half of what I originally did on Bug #61303, but as extra code in _dbus_lookup_user_bus(), rather than an explicit name "xdg-runtime:" for this transport.
Created attachment 113369 [details] [review] [2/4] Add a regression test for connecting to XDG_RUNTIME_DIR/bus by default This test requires the unix:runtime=yes sub-transport from Bug #61303. --- The actual implementation does not require that sub-transport; it's only for the listening address in the test's configuration file. However, I would strongly prefer to have regression tests for this feature (and all new features, really), so I've marked this bug as depending on #61303.
Created attachment 113370 [details] [review] [3/4] Optionally install systemd user units for a per-user bus The socket path used here, $XDG_RUNTIME_DIR/bus, does not match what was used in user-session-units, but is what Lennart recommended on fd.o #61303, and is also what kdbus will use for its bus proxy. Installation of these units switches D-Bus to a different model of the system: instead of considering each login session (approximately, each password typed in) to be its own session, the user-session model is that all concurrent logins by the same user form one large session. This allows the same bus to be shared by a graphical session, cron jobs, tty/ssh sessions, screen/tmux sessions and so on. Because this is a different world-view, it is compile-time optional: OS builders can choose which world their OS will live in. The default is still the login-session model used in earlier D-Bus releases, but might change to the user-session model in future. Explicit configuration is recommended. In OSs that support both models (either for sysadmin flexibility or as a transitional measure), the OS builder should enable the user bus units, but split them off into a dpkg binary package, RPM subpackage etc.; the sysadmin can choose whether to enable the user-session model by choosing whether to install that package. --- At this point I do not consider "wait for kdbus" to be a good objection - kdbus is already looking like a difficult flag-day, and if it's also going to force distributions to move from D-Bus' decade-old session model to a better one, we'd better get some implementation experience with that better one. Also, this bug has had a basically-functional implementation of this model for 2 years now, and people keep pasting the units into their OSs. It's about time it went upstream.
Created attachment 113371 [details] [review] [4/4] Add dbus-update-activation-environment tool If OS builders (distributions) have chosen to use the per-user bus, this provides two possible modes of operation for compatibility with existing X session startup hooks. A legacy-free system can just upload DISPLAY, XAUTHORITY and possibly DBUS_SESSION_BUS_ADDRESS into dbus-daemon's and systemd's activation environments, similar to http://cgit.freedesktop.org/systemd/systemd/tree/xorg/50-systemd-user.sh installed by systemd (but unlike systemctl, dbus-update-activation-environment works for traditional D-Bus-activated services, not just for systemd services). A system where compatibility is required for environment variables exported by snippets in /etc/X11/xinit/xinitrc.d (in Red Hat derivatives, Gentoo, etc.) or /etc/X11/Xsession.d (Debian derivatives) can upload the entire environment of the X session, minus some selected environment variables which are specific to a login session (notably XDG_SESSION_ID). In Debian, I plan to put the former in a new dbus-user-session package that enables a user-session-centric mode of operation for D-Bus, and the latter in the existing dbus-x11 package, with the intention that dbus-x11 eventually becomes a tool for change-averse setups or goes away entirely. --- This is the compatibility glue that lets us get from here to there without regressions. Legacy-free distributions don't have to use it if they don't want to, but I need it for Debian, because "legacy-free" and "has 74GB of source code" are not concepts that fit together well in practice. I'll link to the Xsession.d snippets that I plan to use in Debian shortly. This patch assumes that the patch from Bug #89086 has been applied. If not, there are trivial conflicts in configure.ac and doc/Makefile.am.
(In reply to Simon McVittie from comment #28) > I'll link to the Xsession.d snippets that I plan to use in Debian shortly. Early in Xsession.d, if dbus-user-session is installed: https://anonscm.debian.org/cgit/users/smcv/dbus.git/tree/debian/20dbus_xdg-runtime?h=user-session Later in Xsession.d, if dbus-x11 (dbus-launch) is installed: https://anonscm.debian.org/cgit/users/smcv/dbus.git/tree/debian/75dbus_dbus-launch?h=user-session (this is skipped if dbus-user-session set the D_S_B_A) Right at the end of Xsession.d before executing gnome-session or whatever, if dbus-x11 (dbus-launch) is installed (i.e. this is the "legacy" mode, which, realistically, will probably have to be present in Debian 9): https://anonscm.debian.org/cgit/users/smcv/dbus.git/tree/debian/95dbus_update-activation-env?h=user-session
(In reply to Thiago Macieira from comment #24) > Remember that the autolaunch mechanism is there for compatibility with > systems that didn't start D-Bus in the first place. We can expect systemd to > do it, so we should simply require that the proper environment variables be > set. At the moment, libpam-systemd only sets DBUS_SESSION_BUS_ADDRESS for kdbus systems. This is actually a good thing, sort of, because it means distro builders can choose one of these three options: * live in the past, stick to login-sessions (until kdbus forces the issue) * live in the future, user-sessions all the way! * leave it up to the sysadmin: if they install dbus-user-session (or non-Debian equivalent) they are in the future, if not, they are in the past without patching their systemd.
Comment on attachment 113368 [details] [review] [1/4] On Unix platforms, try $XDG_RUNTIME_DIR/bus before default address Review of attachment 113368 [details] [review]: ----------------------------------------------------------------- ::: dbus/dbus-sysdeps-unix.c @@ +3877,5 @@ > + /* We have the XDG_RUNTIME_DIR, but no bus scoped to it. > + * Not an error. */ > + *supported = FALSE; > + ret = TRUE; > + goto out; Might want some _dbus_verbose() logging in this case and the one below.
Comment on attachment 113369 [details] [review] [2/4] Add a regression test for connecting to XDG_RUNTIME_DIR/bus by default Review of attachment 113369 [details] [review]: ----------------------------------------------------------------- ::: test/dbus-daemon.c @@ +665,5 @@ > + > + /* we're relying on being single-threaded for this to be safe */ > + g_setenv ("XDG_RUNTIME_DIR", f->saved_runtime_dir, TRUE); > + g_free (f->saved_runtime_dir); > + g_free (f->tmp_runtime_dir); For consistency with the rest of the teardown() function you might want to clear saved_runtime_dir and tmp_runtime_dir to NULL, though I don’t think it matters because the fixture struct is freed after each test.
Comment on attachment 113370 [details] [review] [3/4] Optionally install systemd user units for a per-user bus Review of attachment 113370 [details] [review]: ----------------------------------------------------------------- ::: configure.ac @@ +1516,5 @@ > + PKG_CHECK_EXISTS([systemd], > + [with_systemduserunitdir=$($PKG_CONFIG --variable=systemduserunitdir systemd)], > + [with_systemduserunitdir='${libdir}/systemd/user']) > + ]) > +AS_IF([test "x$have_systemd" = xno], [with_systemduserunitdir=no]) If systemd is not installed, with_systemduserunitdir=no, so the files will be installed in the wrong place, I think? Why is this AS_IF here?
Comment on attachment 113370 [details] [review] [3/4] Optionally install systemd user units for a per-user bus Review of attachment 113370 [details] [review]: ----------------------------------------------------------------- Missing corresponding CMake build system changes, if we care. (Why does D-Bus have two build systems?)
Comment on attachment 113371 [details] [review] [4/4] Add dbus-update-activation-environment tool Review of attachment 113371 [details] [review]: ----------------------------------------------------------------- ::: doc/Makefile.am @@ +8,4 @@ > dbus-run-session.1 \ > dbus-send.1 \ > dbus-test-tool.1 \ > + dbus-update-activation-environment.1 \ Missing corresponding CMake changes. ::: doc/dbus-update-activation-environment.1.xml.in @@ +62,5 @@ > + <term><option>--all</option></term> > + <listitem> > + <para>Set all environment variables present in > + the environment used by > + <command>dbus-update-activation-environment</command>. Need to specify that any additional variables given on the command line should be in the form VAR=VAL, not just VAR. ::: tools/dbus-update-activation-environment.c @@ +56,5 @@ > +#endif > + > +#ifndef EX_OSERR > +# define EX_OSERR 71 > +#endif Perhaps these should be predicated on #ifndef HAVE_SYSEXITS_H instead? Or does the support for different codes within sysexits.h vary? @@ +102,5 @@ > + * because we know we're on Linux. */ > + if (asprintf (&path, "%s/systemd", xdg_runtime_dir) < 0) > + oom (); > + > + if (stat (path, &buf) < 0) Leaks path in this case. @@ +151,5 @@ > + { > +#ifdef __linux__ > + systemd = TRUE; > +#else > + /* ignore, systemd is Linux-specific */ Is it worth logging something in this case? The user has explicitly requested --systemd. @@ +166,5 @@ > + " session services\n" > + "\n" > + "%1$s [options] VAR[=VAL] [VAR2[=VAL2] ...]\n" > + " Add specified variables to D-Bus activation environment.\n" > + " If omitted, values are taken from current environment;\n" ‘taken from _the_ current environment’. @@ +193,5 @@ > + if (conn == NULL) > + { > + fprintf (stderr, > + "%s: error: unable to connect to D-Bus: %s\n", PROGNAME, > + error.message); Leaks error in this case. @@ +215,5 @@ > + { > + if (!systemd_user_running ()) > + { > + /* This is only best-effort. */ > + systemd = FALSE; Maybe worth logging in this case? @@ +359,5 @@ > + if (reply == NULL) > + { > + fprintf (stderr, > + "%s: error sending to dbus-daemon: %s: %s\n", > + PROGNAME, error.name, error.message); Leaks conn, msg, error on this path. @@ +367,5 @@ > + if (!dbus_message_get_args (msg, &error, DBUS_TYPE_INVALID)) > + { > + fprintf (stderr, > + "%s: error from dbus-daemon: %s: %s\n", > + PROGNAME, error.name, error.message); Leaks conn, msg, reply, error on this path. @@ +392,5 @@ > + { > + fprintf (stderr, > + "%s: warning: error from systemd: %s: %s\n", > + PROGNAME, error.name, error.message); > + } Leaks sd_msg, error. @@ +394,5 @@ > + "%s: warning: error from systemd: %s: %s\n", > + PROGNAME, error.name, error.message); > + } > + } > +#endif Leaks conn, msg.
(In reply to Philip Withnall from comment #31) > Might want some _dbus_verbose() logging in this case and the one below. Fair point. (In reply to Philip Withnall from comment #32) > For consistency with the rest of the teardown() function you might want to > clear saved_runtime_dir and tmp_runtime_dir to NULL, though I don’t think it > matters because the fixture struct is freed after each test. Indeed, it doesn't actually matter. I'm inclined to leave this as-is. (In reply to Philip Withnall from comment #33) > If systemd is not installed, with_systemduserunitdir=no, so the files will > be installed in the wrong place, I think? Why is this AS_IF here? It's a leftover from an earlier version in which --with[out]-systemduserunitdir was the discriminator for whether to install the user units. I'll get rid of it. (In reply to Philip Withnall from comment #34) > Missing corresponding CMake build system changes, if we care. (Why does > D-Bus have two build systems?) Because Windows people prefer CMake (it can produce MSVC "solutions" as well as msys/mingw/gcc makefiles) but Unix OS integrators prefer Autotools. The CMake build system can also build Linux binaries, but only so that we can confirm we haven't horribly broken it for Windows users - it's OK if those Linux binaries are only as capable as the Windows ones, and it's OK if it has to be Linux and not *BSD or whatever. systemd is Linux-specific (and uses Autotools itself), and anyone who wants this systemd hookup is the sort of OS integrator who should definitely be using the Autotools build. (In reply to Philip Withnall from comment #35) > > + dbus-update-activation-environment.1 \ > > Missing corresponding CMake changes. Valid point. > ::: doc/dbus-update-activation-environment.1.xml.in > @@ +62,5 @@ > > + <term><option>--all</option></term> > > + <listitem> > > + <para>Set all environment variables present in > > + the environment used by > > + <command>dbus-update-activation-environment</command>. > > Need to specify that any additional variables given on the command line > should be in the form VAR=VAL, not just VAR. I don't think that actually works as-implemented. We should probably just forbid combining --all with non-option arguments: if anyone really wants "upload my environment, but also set FOO=bar" they can run d-u-a-e twice. > > +#ifndef EX_OSERR > > +# define EX_OSERR 71 > > +#endif > > Perhaps these should be predicated on #ifndef HAVE_SYSEXITS_H instead? Or > does the support for different codes within sysexits.h vary? sysexits.h is Unix tradition rather than anything standardized. It seems as good a set of exit codes as any, hence using it if available rather than hard-coding, but I'd rather not rely too heavily on miscellaneous Unixes being 100% consistent. > Leaks path in this case. Good catch - although none of the memory leaks actually matter, since d-u-a-e exits quickly, and uses a fixed amount of memory for any given argv/environ. > > + /* ignore, systemd is Linux-specific */ > > Is it worth logging something in this case? The user has explicitly > requested --systemd. The main use-case for this tool is OS integration scripts, which in practice are going to be the same on Debian with systemd and Debian with sysvinit, or on Debian GNU/Linux and Debian GNU/kFreeBSD. I could use say() to say something about it if --verbose, though. > > + " If omitted, values are taken from current environment;\n" > > ‘taken from _the_ current environment’. I'm deliberately being somewhat terse in the --help text, for maximum information density ("newspaper headline" style). I could sprinkle "the" through it, but it isn't clear to me that it would add any additional clarity. > Leaks error in this case. ... and immediately exits :-P > > + /* This is only best-effort. */ > > + systemd = FALSE; > > Maybe worth logging in this case? Not by default, but say() would be reasonable. > Leaks conn, msg, error on this path. ... and then exits, making the leak somewhat irrelevant. > Leaks conn, msg, reply, error on this path. Same. > Leaks sd_msg, error. Same (although the leaked error would matter if we did anything else with it afterwards, because "piling up" errors is not allowed, so this one is somewhat higher-priority to fix). > Leaks conn, msg. And immediately returns from main(). In practice @conn is "leaked" regardless, because it's the global shared connection.
Created attachment 113403 [details] [review] [1/4 v3] On Unix platforms, try $XDG_RUNTIME_DIR/bus before default address --- See http://cgit.freedesktop.org/~smcv/dbus/log/?h=fixup/unix-runtime-61301 for individual changes that have been squashed into this.
Created attachment 113404 [details] [review] [2/4 v3] Add a regression test for connecting to XDG_RUNTIME_DIR/bus by default
Created attachment 113405 [details] [review] [3/4 v3] Optionally install systemd user units for a per-user bus
Created attachment 113406 [details] [review] [4/4 v3] Add dbus-update-activation-environment tool
Things Philip brought up that I have not changed: > (In reply to Philip Withnall from comment #32) > > For consistency with the rest of the teardown() function you might want to > > clear saved_runtime_dir and tmp_runtime_dir to NULL, though I don’t think it > > matters because the fixture struct is freed after each test. > > Indeed, it doesn't actually matter. I'm inclined to leave this as-is. > (In reply to Philip Withnall from comment #34) > > Missing corresponding CMake build system changes, if we care. (Why does > > D-Bus have two build systems?) > > systemd is Linux-specific (and uses Autotools itself), and anyone who wants > this systemd hookup is the sort of OS integrator who should definitely be > using the Autotools build. > > > +#ifndef EX_OSERR > > > +# define EX_OSERR 71 > > > +#endif > > > > Perhaps these should be predicated on #ifndef HAVE_SYSEXITS_H instead? Or > > does the support for different codes within sysexits.h vary? > > sysexits.h is Unix tradition rather than anything standardized. > > > + " If omitted, values are taken from current environment;\n" > > > > ‘taken from _the_ current environment’. > > I'm deliberately being somewhat terse in the --help text > > Leaks error in this case. > > ... and immediately exits :-P I didn't stop it leaking, but I did make it exit() instead of returning, to make it more obvious that the leak really doesn't matter.
Created attachment 113416 [details] [review] cope with not having an XDG_RUNTIME_DIR to restore This can happen if running integration tests under sudo. --- Could be squashed into Attachment #113404 [details] if desired.
(In reply to Simon McVittie from comment #30) > (In reply to Thiago Macieira from comment #24) > > Remember that the autolaunch mechanism is there for compatibility with > > systems that didn't start D-Bus in the first place. We can expect systemd to > > do it, so we should simply require that the proper environment variables be > > set. > > At the moment, libpam-systemd only sets DBUS_SESSION_BUS_ADDRESS for kdbus > systems. So it stands to reason it could set in other situations too. > This is actually a good thing, sort of, because it means distro builders can > choose one of these three options: > > * live in the past, stick to login-sessions (until kdbus forces the issue) > * live in the future, user-sessions all the way! > * leave it up to the sysadmin: if they install dbus-user-session (or > non-Debian equivalent) they are in the future, if not, they are in the past > > without patching their systemd. I don't understand why you're suggesting that patching systemd is necessary. We've all agreed that environments without DBUS_SESSION_BUS_ADDRESS set are legacy, therefore I assume every modern system has a way to set the variable properly. Given that, why does the library need an update for the client in the first place? If we assume that the environment variable is set correctly, the job is done.
Comment on attachment 113403 [details] [review] [1/4 v3] On Unix platforms, try $XDG_RUNTIME_DIR/bus before default address Review of attachment 113403 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 113405 [details] [review] [3/4 v3] Optionally install systemd user units for a per-user bus Review of attachment 113405 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 113406 [details] [review] [4/4 v3] Add dbus-update-activation-environment tool Review of attachment 113406 [details] [review]: ----------------------------------------------------------------- ::: tools/dbus-update-activation-environment.c @@ +190,5 @@ > + > + if (all && first_non_option < argc) > + { > + fprintf (stderr, "%s: error: --all cannot be used with VAR or " > + "VAR=VAL arguments\n", PROGNAME); I thought the possibility of combining --all with VAR=VAL was rather neat, but it’s your call.
Comment on attachment 113416 [details] [review] cope with not having an XDG_RUNTIME_DIR to restore Review of attachment 113416 [details] [review]: ----------------------------------------------------------------- ++
(In reply to Simon McVittie from comment #41) > Things Philip brought up that I have not changed: ++ to all of them. > > > Leaks error in this case. > > > > ... and immediately exits :-P > > I didn't stop it leaking, but I did make it exit() instead of returning, to > make it more obvious that the leak really doesn't matter. I was going for making the whole thing valgrind-clean, but I’m not massively fussed.
(In reply to Philip Withnall from comment #46) > I thought the possibility of combining --all with VAR=VAL was rather neat, > but it’s your call. RESOLVED LATER, tbh. I don't think anyone is actually ever going to do that, and the implementation never supported it (some refactoring would be necessary).
(In reply to Thiago Macieira from comment #43) > I don't understand why you're suggesting that patching systemd is necessary. Well, I suppose I really meant "patching libpam-systemd, or adding a snippet to /etc/profile.d, or adding a new libpam-dbus", none of which I plan to do in practice. > We've all agreed that environments without DBUS_SESSION_BUS_ADDRESS set are > legacy *GUI* environments without DBUS_SESSION_BUS_ADDRESS set are certainly not something that exists in a modern distribution: a well-integrated distro will set it up in xinitrc.d or Xsession.d or equivalent. However, non-GUI environments (getty, ssh, cron) do not typically set D_S_B_A. A PAM module would be the obvious place where this could happen if desired. My patch 1/4 here makes libdbus fall back to XDG_RUNTIME_DIR/bus if D_S_B_A is not set. sd-bus (systemd's D-Bus library) already does that, so it does not need an equivalent change. Other reimplementations (GDBus, dbus-sharp, dbus-java) should ideally be changed to have the same fallback.
(In reply to Simon McVittie from comment #50) > My patch 1/4 here makes libdbus fall back to XDG_RUNTIME_DIR/bus if D_S_B_A > is not set. sd-bus (systemd's D-Bus library) already does that, so it does > not need an equivalent change. Other reimplementations (GDBus, dbus-sharp, > dbus-java) should ideally be changed to have the same fallback. My ulterior motive here is that in a few years' time, I would like distributions that follow the user-session model to be able to stop setting D_S_B_A altogether, and assume that all D-Bus implementations will use XDG_RUNTIME_DIR/bus (or the kdbus equivalent if supported). Environment variables that must always be set to the same value are rather an API wart.
Created attachment 113457 [details] [review] [2/4 v4] Add a regression test for connecting to XDG_RUNTIME_DIR/bus by default This test requires the unix:runtime=yes sub-transport from Bug #61303. --- This is just Attachment #113404 [details] + Attachment #113416 [details], to simplify the reviewed patch series.
(In reply to Simon McVittie from comment #51) > (In reply to Simon McVittie from comment #50) > > My patch 1/4 here makes libdbus fall back to XDG_RUNTIME_DIR/bus if D_S_B_A > > is not set. sd-bus (systemd's D-Bus library) already does that, so it does > > not need an equivalent change. Other reimplementations (GDBus, dbus-sharp, > > dbus-java) should ideally be changed to have the same fallback. > > My ulterior motive here is that in a few years' time, I would like > distributions that follow the user-session model to be able to stop setting > D_S_B_A altogether, and assume that all D-Bus implementations will use > XDG_RUNTIME_DIR/bus (or the kdbus equivalent if supported). Environment > variables that must always be set to the same value are rather an API wart. I don't think we should have that, as it's a behaviour change. So I recommend checking the XDG_RUNTIME_DIR socket *after* autolaunch. On a virtual terminal, DISPLAY won't be set so autolaunch will fail and you'll get the behaviour you're describing. But on existing, legacy systems where autolaunch is used, the behaviour is kept.
(In reply to Thiago Macieira from comment #53) > > > My patch 1/4 here makes libdbus fall back to XDG_RUNTIME_DIR/bus > > I don't think we should have that, as it's a behaviour change. So I > recommend checking the XDG_RUNTIME_DIR socket *after* autolaunch. I understand your point of view, but I disagree. It is only a behaviour change if you have deliberately done the setup to make something listen on XDG_RUNTIME_DIR/bus, i.e. you've done something that already changes the system's behaviour. If you haven't, we fall back again, this time to autolaunch, and everything proceeds as before. On systems where "per session" is interpreted as "per-login-session", we should not be offering a per-user-session bus daemon at all - that leads to the possibility of having both run at the same time, which does not agree with either of the two proposed designs (i.e. two proposed interpretations of "what is a session?") and would break interop between client libraries with different priority orders. So I think we do need an on/off switch for whether to listen on XDG_RUNTIME_DIR/bus, and that's system infrastructure rather than part of the client library - it is not libdbus' decision whether anything listens there or not. If that switch is on (i.e. you *have* opted-in to running a per-user dbus-daemon by enabling the systemd units, or set up some sort of out-of-tree infrastructure to make that happen by another means), then I really do think it would be more confusing and error-prone, and not actually any more useful, if you had to apply a second configuration option / shell script snippet to teach the client library to connect to it. The default behaviour is still that nothing will create XDG_RUNTIME_DIR/bus. If you or your OS integrator does not choose to create it (i.e. you built from source without --enable-user-session, or you do not have systemd-logind, or you did not install dbus-user-session in Debian) then the fallback to autolaunch gives you a per-(graphical-)login-session bus. The correct answer to "what is a session?" is mainly a question for OS integrators, not individual sysadmins, although some OS integrators (e.g. Debian) will want to pass the decision on to sysadmins by making the systemd units an optional sub-package.
(In reply to Simon McVittie from comment #54) > (In reply to Thiago Macieira from comment #53) > > > > My patch 1/4 here makes libdbus fall back to XDG_RUNTIME_DIR/bus > > > > I don't think we should have that, as it's a behaviour change. So I > > recommend checking the XDG_RUNTIME_DIR socket *after* autolaunch. > > I understand your point of view, but I disagree. It is only a behaviour > change if you have deliberately done the setup to make something listen on > XDG_RUNTIME_DIR/bus, i.e. you've done something that already changes the > system's behaviour. If you haven't, we fall back again, this time to > autolaunch, and everything proceeds as before. True, that is the case, but that is not taking into account applications using libraries that have not got the update to search XDG_RUNTIME_DIR/bus. That would mean some applications connect to one bus daemon and some others connect to another daemon. With the impeding arrival of kdbus, dbus-daemon-based user-global buses are not going to be common, so I really insist on avoiding incompatibility of any kind. My recommendation is to do nothing: DBUS_SESSION_BUS_ADDRESS needs to be set correctly, period. Failing that, if you really insist on something for console sessions, then add it after the autolaunch search -- or, shall we say, as part of.
(In reply to Thiago Macieira from comment #55) > True, that is the case, but that is not taking into account applications > using libraries that have not got the update to search XDG_RUNTIME_DIR/bus. Altering libdbus covers dbus-python and QtDBus. I have a patch for GDBus, although it still needs testing (I'll get to that next week). sd-bus' maintainers refuse to have anything to do with autolaunch, and it already defaults to X_R_D/bus. I think that leaves dbus-sharp and dbus-java? > With the impeding arrival of kdbus, dbus-daemon-based user-global buses are > not going to be common, so I really insist on avoiding incompatibility of > any kind. I do not consider it to be an incompatible change to take advantage of a new piece of OS-wide infrastructure that is absent from the current systems with which we want to remain compatible. Meanwhile, the kdbus user-space components in systemd use X_R_D/bus for the "bus proxy" used to provide backwards compat for existing D-Bus implementations, which would currently be used by everything except sd-bus; so if we want existing D-Bus client libraries to work properly on kdbus systems in the near future, trying X_R_D/bus before X11 autolaunching is a better default. (There's a kdbus branch for GDBus, but it's unmerged; I've heard rumours of Tizen adding kdbus support to libdbus, but no patches have ever been proposed upstream.) (In reply to Thiago Macieira from comment #55) > My recommendation is to do nothing: DBUS_SESSION_BUS_ADDRESS needs to be set > correctly, period. Failing that, if you really insist on something for > console sessions, then add it after the autolaunch search -- or, shall we > say, as part of. I don't really want to have to do another round of patching and testing to accommodate this, but if you insist on vetoing the current version, it would be technically possible to alter 1/4 to use this search order. However, I don't see any situation in which this search order would be an improvement. Is there one? If there is, please describe it. I am aware that in the short/medium term, OS integrators that use the per-user-session bus will need to set DBUS_SESSION_BUS_ADDRESS in their graphical sessions for the benefit of (versions of) D-Bus client libraries that do not know about X_R_D/bus; indeed, my dbus-user-session package for Debian does exactly that. However, I would like that to only have to remain the case for as long as they support those (versions of) client libraries, rather than persisting forever as a trap for the unwary. Trying autolaunch before or after X_R_D/bus does not make any difference on systems where the concept of "per-session" has been interpreted as per-login-session, because on such systems, there is no X_R_D/bus anyway. On systems where "per-session" has been interpreted as per-user-session, X_R_D/bus is the correct thing to use, and trying autolaunch before X_R_D/bus can potentially result in the wrong thing - a per-login-session bus even though there is already a per-user-session bus. So I continue to believe that trying X_R_D/bus first is the better version, because it is no worse on any system, and is actively better for a minority.
(In reply to Simon McVittie from comment #56) > (In reply to Thiago Macieira from comment #55) > > True, that is the case, but that is not taking into account applications > > using libraries that have not got the update to search XDG_RUNTIME_DIR/bus. > > Altering libdbus covers dbus-python and QtDBus. I have a patch for GDBus, > although it still needs testing (I'll get to that next week). sd-bus' > maintainers refuse to have anything to do with autolaunch, and it already > defaults to X_R_D/bus. > > I think that leaves dbus-sharp and dbus-java? You're assuming that the libraries get updated together. It's possible that one gets updated and the other(s) don't (think of static/bundled libraries). Hence, a source of incompatibility. > Meanwhile, the kdbus user-space components in systemd use X_R_D/bus for the > "bus proxy" used to provide backwards compat for existing D-Bus > implementations, which would currently be used by everything except sd-bus; > so if we want existing D-Bus client libraries to work properly on kdbus > systems in the near future, trying X_R_D/bus before X11 autolaunching is a > better default. (There's a kdbus branch for GDBus, but it's unmerged; I've > heard rumours of Tizen adding kdbus support to libdbus, but no patches have > ever been proposed upstream.) I disagree because I see no gain in the above. Any kdbus-based systems will need to set the environment variable anyway to let clients know that a kdbus bus is available, so the fallback will be present in the environment variable too. The search code you're adding would not be used. > (In reply to Thiago Macieira from comment #55) > > My recommendation is to do nothing: DBUS_SESSION_BUS_ADDRESS needs to be set > > correctly, period. Failing that, if you really insist on something for > > console sessions, then add it after the autolaunch search -- or, shall we > > say, as part of. > > I don't really want to have to do another round of patching and testing to > accommodate this, but if you insist on vetoing the current version, it would > be technically possible to alter 1/4 to use this search order. However, I > don't see any situation in which this search order would be an improvement. > Is there one? If there is, please describe it. I don't have the power of veto, I'm just exposing my arguments so we can come to a conclusion. If I'm overruled, I am overruled. I would just prefer not to add any source of incompatibility, however small it may be. > I am aware that in the short/medium term, OS integrators that use the > per-user-session bus will need to set DBUS_SESSION_BUS_ADDRESS in their > graphical sessions for the benefit of (versions of) D-Bus client libraries > that do not know about X_R_D/bus; indeed, my dbus-user-session package for > Debian does exactly that. However, I would like that to only have to remain > the case for as long as they support those (versions of) client libraries, > rather than persisting forever as a trap for the unwary. Please expect this to be at least 5 years. Like I said, client libraries will need to be told that there is a kdbus bus available, so the environment variable will need to be set for some time. > Trying autolaunch before or after X_R_D/bus does not make any difference on > systems where the concept of "per-session" has been interpreted as > per-login-session, because on such systems, there is no X_R_D/bus anyway. On > systems where "per-session" has been interpreted as per-user-session, > X_R_D/bus is the correct thing to use, and trying autolaunch before > X_R_D/bus can potentially result in the wrong thing - a per-login-session > bus even though there is already a per-user-session bus. So I continue to > believe that trying X_R_D/bus first is the better version, because it is no > worse on any system, and is actively better for a minority. I agree it would be the wrong thing for applications to do, which is why I am arguing that it should remain as-is. All the applications should get this "wrong" behaviour so that the integrator realises this problem and corrects it. Otherwise, it's possible the integrator only tested a handful of applications using the most common libraries, leaving the uncommon applications untested and thus creating problems.
(In reply to Thiago Macieira from comment #57) > > I am aware that in the short/medium term, OS integrators that use the > > per-user-session bus will need to set DBUS_SESSION_BUS_ADDRESS > > Please expect this to be at least 5 years. Like I said, client libraries > will need to be told that there is a kdbus bus available, so the environment > variable will need to be set for some time. That's fine, 5 years is a finite time. A per-user dbus-daemon has been discussed for literally a decade, and this particular bug has been open for 2 years. If it's a few more years or even a decade before general-purpose Linux distributions can safely stop setting DBUS_SESSION_BUS_ADDRESS, at least we're heading in the direction of "the right thing happens by default". This does let the OS integrator make the change at their own pace: closer-to-embedded platforms with more control over their exact set of libraries, like the ones I often work on, will be able to drop DBUS_SESSION_BUS_ADDRESS rather sooner than that. > I agree it would be the wrong thing for applications to do, which is why I > am arguing that it should remain as-is. All the applications should get this > "wrong" behaviour so that the integrator realises this problem and corrects > it. I see your point, but I don't think creating a "split-brain" situation for the session bus is an appropriate diagnostic. OS integrators who are "doing D-Bus right" will not get into this situation anyway, and OS integrators with insufficient understanding will probably fail to debug why their apps aren't communicating with each other and just blame us. I did consider making dbus-launch yield XDG_RUNTIME_DIR/bus if it exists, which would mean that autolaunching did the right thing in real desktop sessions; but that would break a lot of regression tests[1] that use "xvfb-run dbus-launch ..." and expect that to give them a new, unique per-pseudo-login-session bus. Those tests should really be using dbus-run-session, but that's only a year old, so developer folklore doesn't know about it yet. Possibly making "dbus-launch --autolaunch" yield XDG_RUNTIME_DIR/bus if it exists (and only autolaunch, not plain dbus-launch --exit-with-session) would work? [1] http://codesearch.debian.net/results/dbus-launch/page_0
(In reply to Simon McVittie from comment #58) > Possibly making "dbus-launch --autolaunch" yield XDG_RUNTIME_DIR/bus if it > exists (and only autolaunch, not plain dbus-launch --exit-with-session) > would work? Yes, that sounds like a fine compromise.
Created attachment 113575 [details] [review] [5/6] dbus-launch: use libdbus to read the UUID As a side benefit, this means that dbus-launch now understands /etc/machine-id and not just /var/lib/dbus/machine-id. I'm using the "internal" (static) version of libdbus rather than the shared version, because my next commit is going to need that anyway.
Created attachment 113576 [details] [review] [6/6] dbus-launch: if autolaunching, use XDG_RUNTIME_DIR/bus if available This provides backwards-compatible autolaunching behaviour, as long as dbus-launch inherits the XDG_RUNTIME_DIR (which it presumably did if it's going to work at all, since it must also have inherited the DISPLAY). In particular, we go through the motions of starting the dbus-daemon, so that we can start the "babysitter" process that will maintain the X11 window to store the bus address. --- dbus-launch already copes with the bus's apparent process ID being 0, and will not attempt to kill it. That's the right thing here: the user bus is meant to outlive the X11 session anyway, if there are concurrent non-X11 sessions for the same uid, and its lifetime is under systemd's control.
Could someone with an interest in this feature also review my three patches on Bug #61303, please? Attachment #113457 [details] (the test for this feature) depends on those, or at least the first one (the non-test change).
Comment on attachment 113575 [details] [review] [5/6] dbus-launch: use libdbus to read the UUID Review of attachment 113575 [details] [review]: ----------------------------------------------------------------- ::: tools/dbus-launch.c @@ +123,3 @@ > > + /* one is allocated with malloc and the other with dbus_malloc so copy it */ > + machine_uuid = xstrdup (uuid); Could you not instead change the semantics of machine_uuid to use the D-Bus allocator instead of the libc one? As I read the code, machine_uuid is never freed anyway.
Comment on attachment 113576 [details] [review] [6/6] dbus-launch: if autolaunching, use XDG_RUNTIME_DIR/bus if available Review of attachment 113576 [details] [review]: ----------------------------------------------------------------- As always, this is just a review of the code, not the thinking behind it. I’m still not qualified enough with D-Bus internals for that. All looks good to me in that respect.
Created attachment 113619 [details] [review] [5/6 v2] dbus-launch: use libdbus to read the UUID As a side benefit, this means that dbus-launch now understands /etc/machine-id and not just /var/lib/dbus/machine-id. I'm using the "internal" (static) version of libdbus rather than the shared version, because my next commit is going to need that anyway; with that in mind, it makes sense to switch machine_uuid to be allocated with dbus_malloc, rather than having to copy it from malloc-allocated to dbus_malloc-allocated storage. However, I'm deliberately not changing the allocation model of any other strings in dbus-launch right now. --- (In reply to Philip Withnall from comment #63) > Could you not instead change the semantics of machine_uuid to use the D-Bus > allocator instead of the libc one? As I read the code, machine_uuid is never > freed anyway. I didn't want to get into the level of yak shaving required to fix all the memory allocations in dbus-launch{,-x11}.c to be dbus_malloc rather than malloc now that it links libdbus anyway; but you're right, it's a net simplification to do it for just this one.
Created attachment 113701 [details] [review] [5/6 v3] dbus-launch: use libdbus to read the UUID As a side benefit, this means that dbus-launch now understands /etc/machine-id and not just /var/lib/dbus/machine-id. Since machine_uuid comes out of libdbus allocated with dbus_malloc, to avoid having to copy it from malloc-allocated to dbus_malloc-allocated storage, it makes sense to change it to be consistently dbus_malloc-allocated (particularly now that Bug #83115 has made use of internal symbols relatively painless). However, I'm deliberately not changing the allocation model of any other strings in dbus-launch right now; that's a larger yak-shaving exercise. --- Rebased now that I've landed Bug #83115
Created attachment 113703 [details] [review] [6/6 v2] dbus-launch: if autolaunching, use XDG_RUNTIME_DIR/bus if available This provides backwards-compatible autolaunching behaviour, as long as dbus-launch inherits the XDG_RUNTIME_DIR (which it presumably did if it's going to work at all, since it must also have inherited the DISPLAY). In particular, we go through the motions of starting the dbus-daemon, so that we can start the "babysitter" process that will maintain the X11 window to store the bus address. Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk> [smcv: decorate _dbus_lookup_user_bus with DBUS_PRIVATE_EXPORT so we can still call it after fixing fd.o#83115] --- Trivial change since Philip reviewed it, doesn't really need re-review from anyone who already looked at v1.
All applied for 1.9.14. (In reply to Simon McVittie from comment #66) > [5/6 v3] dbus-launch: use libdbus to read the UUID I applied this unreviewed; it's a simple fix for Philip's review comments on the earlier version. http://cgit.freedesktop.org/dbus/dbus/commit/?id=71c11a9e487ec5c4b533371098201efb5d966961 if anyone wants to do post-commit review.
Comment on attachment 113701 [details] [review] [5/6 v3] dbus-launch: use libdbus to read the UUID Review of attachment 113701 [details] [review]: ----------------------------------------------------------------- Looks good.
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.