Merge lp:~mterry/lightdm/dbus-activate-logind into lp:lightdm

[Michael Terry]

DBus-activate logind

DBus-activate logind.

While debugging a bug introduced by some upstart changes in Touch, I noticed that if LightDM starts before logind happens to be dbus-activated somewhere else, it will think it isn't available.

LightDM currently only checks if logind is ALREADY started. But since logind is started via dbus-activation instead of upstart, we can't ensure that. So when we check if logind is available, we should instead see if it can be started.

[Michael Terry (mterry)]

Oh, right. Let me change test system for this.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1987
http://jenkins.qa.ubuntu.com/job/lightdm-ci/294/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-utopic-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-utopic-armhf-ci/7

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/lightdm-ci/294/rebuild

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

Is that the appropriate way to start a service now? Shouldn't we just try to access it and that will cause it to be started if necessary? Could this change start logind even if the sysadmin wanted it disabled for some reason?

[Michael Terry (mterry)]

StartServiceByName is what gio does under the hood to start services. I was trying to be as direct as possible. But I realized that only works if there is a .service file for login1. Maybe some versions of login1 will be launched differently (by upstart or systemd) and be available already without a .service file. (like our test runner version of login1 in fact)

So I switched the logic to just create a proxy and check if there is a name owner.

As for sysadmins disabling logind activation, it's not just lightdm that will do this. Any app anywhere could start it up by trying to talk to it. The only safe way for an admin to disable it is deleting the .service file or removing the package. Which seems reasonable if you don't want to use logind.

[Robert Ancell (robert-ancell)]

Looks a lot better than that dodgy "check /run" there was before.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1988
http://jenkins.qa.ubuntu.com/job/lightdm-ci/295/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-utopic-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-utopic-armhf-ci/8

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/lightdm-ci/295/rebuild

review: Approve (continuous-integration)

=== modified file 'src/login1.c'
--- src/login1.c	2014-03-17 16:02:32 +0000
+++ src/login1.c	2014-05-14 20:19:19 +0000
@@ -14,10 +14,46 @@
 
 #include "login1.h"
 
+static gboolean
+check_login1 (void)
+{
+    GDBusProxy *proxy;
+    gchar *owner;
+    gboolean success;
+
+    proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
+                                           G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES,
+                                           NULL,
+                                           "org.freedesktop.login1",
+                                           "/org/freedesktop/login1",
+                                           "org.freedesktop.login1.Manager",
+                                           NULL,
+                                           NULL);
+    if (!proxy)
+        return FALSE;
+
+    owner = g_dbus_proxy_get_name_owner (proxy);
+    g_object_unref (proxy);
+
+    success = (owner != NULL);
+    g_free (owner);
+
+    return success;
+}
+
 gboolean
 login1_is_running (void)
 {
-    return access ("/run/systemd/seats/", F_OK) >= 0;
+    static gboolean have_checked = FALSE;
+    static gboolean is_running = FALSE;
+
+    if (!have_checked)
+    {
+        have_checked = TRUE;
+        is_running = check_login1();
+    }
+
+    return is_running;
 }
 
 gchar *

=== modified file 'tests/src/libsystem.c'
--- tests/src/libsystem.c	2014-03-23 22:28:44 +0000
+++ tests/src/libsystem.c	2014-05-14 20:19:19 +0000
@@ -331,10 +331,6 @@
     gchar *new_path = NULL;
     int ret;
 
-    /* Look like systemd is always running */
-    if (strcmp (pathname, "/run/systemd/seats/") == 0)
-        return 1;
-
     _access = (int (*)(const char *pathname, int mode)) dlsym (RTLD_NEXT, "access");
 
     new_path = redirect_path (pathname);