Created attachment 334151 [details] [review]
gdm-{x,wayland}-session: drop dependency on DBUS_SESSION_BUS_ADDRESS

Attachment 334151 [details] pushed as 957e75e - gdm-{x,wayland}-session: drop dependency on DBUS_SESSION_BUS_ADDRESS

(In reply to Ray Strode [halfline] from comment #0)
> DBUS_SESSION_BUS_ADDRESS is not the preferred way to find the session
> bus these days.  Instead it's expected to be found at $XDG_RUNTIME_DIR/bus

FYI, this is not true. DBUS_SESSION_BUS_ADDRESS is still the preferred way to find a session bus, and takes precedence over everything else. (If in doubt, the reference implementation libdbus is always right.)

What has changed is that the fallback path used to be:

$DBUS_SESSION_BUS_ADDRESS > autolaunch:

and it is now:

$DBUS_SESSION_BUS_ADDRESS > unix:path=$XDG_RUNTIME_DIR/bus > autolaunch:

> This commit changes the session launcher code, to just try to get a
> connection to the bus explicitly, instead relying on
> the presence of DBUS_SESSION_BUS_ADDRESS to know whether to start a
> fallback bus.

However, this logic is probably fine, assuming nobody has broken GDBus recently.

So before the code relied on DBUS_SESSION_BUS_ADDRESS being set. Now it works if just unix:path=$XDG_RUNTIME_DIR/bus is set, so the new code is more technically correct.  I probably could have worded the commit message better.

To me, anyway, unix:path=$XDG_RUNTIME_DIR/bus is preferred, though, even if DBUS_SESSION_BUS_ADDRESS takes precendence, because I don't like environment variables, and my eventual goal is to stop relying on them as much as possible.

See also this comment in the systemd source (src/login/pam_systemd.c):

        /* FIXME: We *really* should move the access() check into the•
         * daemons that spawn dbus-daemon, instead of forcing•
         * DBUS_SESSION_BUS_ADDRESS= here. */•

Anyway, one less getenv() call makes the world a better place.

(In reply to Ray Strode [halfline] from comment #4)
> So before the code relied on DBUS_SESSION_BUS_ADDRESS being set. Now it
> works if just unix:path=$XDG_RUNTIME_DIR/bus is set, so the new code is more
> technically correct.

Right. I was correcting what I saw as an incorrect statement in the commit message, not the implementation.

(In reply to Ray Strode [halfline] from comment #4)
> To me, anyway, unix:path=$XDG_RUNTIME_DIR/bus is preferred, though, even if
> DBUS_SESSION_BUS_ADDRESS takes precendence, because I don't like environment
> variables, and my eventual goal is to stop relying on them as much as
> possible.

To be clear: I would like unix:path=$XDG_RUNTIME_DIR/bus to be the normal thing to use (and I'm 90% of the way through filing hundreds of Debian bugs to ensure that dbus-launch/dbus-x11 can go away too), but it is important for interoperability that DBUS_SESSION_BUS_ADDRESS must always take precedence over that.

I anticipate that for the next 5 years or so, distributions will have to set DBUS_SESSION_BUS_ADDRESS anyway, to avoid breaking code that second-guesses this; but maybe one day it can become something you only set when you have weird requirements (often involving automated tests), like LD_LIBRARY_PATH or PYTHONPATH.

Right. I'm just glad GDM isn't a second guesser now.