Bug 19997 - --disable-x11-autolaunch breaks regression tests and dbus-launch --exit-with-session
Summary: --disable-x11-autolaunch breaks regression tests and dbus-launch --exit-with-...
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.4.x
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://git.collabora.co.uk/?p=user/sm...
Whiteboard: NB#219964
Keywords: patch
Depends on:
Blocks: dbus-1.4
  Show dependency treegraph
 
Reported: 2009-02-06 12:24 UTC by Colin Walters
Modified: 2011-05-25 09:39 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH 1/2] Don't attempt Unix X11 autolaunching if DISPLAY isn't set (1.31 KB, patch)
2011-02-24 10:58 UTC, Simon McVittie
Details | Splinter Review
[PATCH 2/2] Allow X11 autolaunch to be disabled even if the headers/libraries are there (5.52 KB, patch)
2011-02-24 10:59 UTC, Simon McVittie
Details | Splinter Review
test-autolaunch: don't expect autolaunching to work if X11 is disabled (1.47 KB, patch)
2011-02-25 04:55 UTC, Simon McVittie
Details | Splinter Review
Check for X even if X11 autolaunching is disabled (4.65 KB, patch)
2011-02-25 04:56 UTC, Simon McVittie
Details | Splinter Review

Description Colin Walters 2009-02-06 12:24:30 UTC
Currently the session bus autolaunching will fail to execute dbus-launch if there's no $DISPLAY.  The known use cases for autolaunching (ssh -Y firefox, run konqueror in legacy DE) all need $DISPLAY too, so it seems reasonable to return a nice error here rather than failing to run dbus-launch.

This came up in the case of the bzr-dbus plugin.
Comment 1 Havoc Pennington 2009-02-24 20:26:21 UTC
Makes sense.
Comment 2 Simon McVittie 2011-01-06 10:27:47 UTC
Relatedly, Maemo has a patch to disable autolaunching altogether, to force developers to set the GUI session's DBUS_SESSION_BUS_ADDRESS when logging into an embedded device over ssh. This avoids getting "split brain" problems when you run a GUI app from the ssh session for debugging.
Comment 3 Simon McVittie 2011-02-24 10:58:08 UTC
Created attachment 43767 [details] [review]
[PATCH 1/2] Don't attempt Unix X11 autolaunching if DISPLAY isn't set

The known use cases for autolaunching (ssh -Y firefox,
run konqueror in legacy DE) all need $DISPLAY too.
Comment 4 Simon McVittie 2011-02-24 10:59:09 UTC
Created attachment 43768 [details] [review]
[PATCH 2/2] Allow X11 autolaunch to be disabled even if the headers/libraries are there

In an embedded system where the D-Bus session is a core part of the
environment, like Maemo, accidentally auto-launching a second session bus
(for instance for a concurrent ssh session) is a bad idea - it can lead
to a "split brain" situation where half the applications in the GUI are
using a different bus. In these controlled environments, it'd be useful
to prevent autolaunch from ever happening.

(As a side benefit, the changes to configure.in also mean that packagers
can explicitly --enable-x11-autolaunch, to make sure that failure to find
X will make compilation fail cleanly.)
Comment 5 Colin Walters 2011-02-24 11:36:44 UTC
Both these patches look fine to me.
Comment 6 Simon McVittie 2011-02-25 04:55:04 UTC
Created attachment 43795 [details] [review]
test-autolaunch: don't expect autolaunching to work if X11 is disabled
Comment 7 Simon McVittie 2011-02-25 04:56:04 UTC
Created attachment 43796 [details] [review]
Check for X even if X11 autolaunching is disabled

DBUS_ENABLE_X11_AUTOLAUNCH obviously requires DBUS_BUILD_X11. However,
the converse is not true.
 
If DBUS_BUILD_X11 is defined, dbus-launch will be able to connect to
the X server to determine when the session ends; most distributors will
want this, but it can be disabled with the standard Autoconf option
--without-x.

If DBUS_ENABLE_X11_AUTOLAUNCH is *also* defined, dbus-launch and libdbus
will be willing to perform autolaunch. Again, most distributors will want
this, but it can be disabled with --disable-x11-autolaunch.
Comment 8 Simon McVittie 2011-02-25 08:57:20 UTC
(In reply to comment #5)
> Both these patches look fine to me.

Thanks, applied for 1.4.8, 1.5.0.

I also noticed that the regression tests are broken if X libraries aren't available (pre-existing bug), and that my initial implementation of --disable-x11-autolaunch would also disable dbus-launch's "exit with X server" feature; however, the extra patches I've attached here fix those issues.
Comment 9 Colin Walters 2011-05-25 09:26:34 UTC
Both these new patches seem fine.
Comment 10 Simon McVittie 2011-05-25 09:39:57 UTC
Thanks, fixed in git for 1.4.10/1.5.2


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.