Merge lp:~mterry/lightdm/no-dmrc into lp:lightdm

Proposed by Michael Terry
Status: Merged
Merged at revision: 1065
Proposed branch: lp:~mterry/lightdm/no-dmrc
Merge into: lp:lightdm
Diff against target: 443 lines (+292/-15)
4 files modified
liblightdm-gobject/user.c (+74/-3)
src/display.c (+4/-10)
src/user.c (+208/-1)
src/user.h (+6/-1)
To merge this branch: bzr merge lp:~mterry/lightdm/no-dmrc
Reviewer Review Type Date Requested Status
Robert Ancell Approve
Review via email: mp+71939@code.launchpad.net

Description of the change

This uses the org.freedesktop.Accounts service instead of dmrc for saving default xsession information.

I could not find where LightDM was loading per-user default xsession information? Seems like it isn't.

If true, we should add that to the liblightdm libraries as a call like get_default_session_hint_for_user and have the lightdm daemon ask accountsservice.

To post a comment you must log in.
Revision history for this message
Robert Ancell (robert-ancell) wrote :

I think we need to continue to support .dmrc for systems that don't have accounts service:
http://lists.freedesktop.org/archives/lightdm/2011-August/000051.html

What we should do is merge user.c into the new accounts.c so that internally LightDM uses the accounts service interface but the backend can fallback to passwd and .dmrc.

Also since it doesn't make sense to have multiple accounts instances inside LightDM I'd make it a singleton and just access through functions for simplicity.

In terms of Unity/GNOME, I definitely think we should be using accounts service.

review: Needs Fixing
Revision history for this message
Michael Terry (mterry) wrote :

Each Accounts object is per-user, not global. So a singleton doesn't make as much sense. Unless you want a global Accounts object that hides the per-user stuff and keeps an internal cache of proxies. But that seems like forcing it.

lp:~mterry/lightdm/no-dmrc updated
1057. By Michael Terry

add back dmrc support as a fallback for accounts service

1058. By Michael Terry

move Accounts class into User class

Revision history for this message
Michael Terry (mterry) wrote :

OK, I've updated the branch. Now we write to .dmrc as well as the Accounts service. And we fallback to .dmrc when trying to read from the Accounts service.

Additionally, I've moved the code into the User class.

Could you review again, please?

(Again, this branch writes correctly to Accounts/dmrc, but still doesn't solve the problem that we never load per-user values from them.)

lp:~mterry/lightdm/no-dmrc updated
1059. By Michael Terry

liblightdm-gobject: look at Accounts object when checking per-user session and language values

Revision history for this message
Michael Terry (mterry) wrote :

Sorry, I was wrong about never loading per-user values from them. I missed the glue in liblightdm-gobject/user.c that did that.

So I've now updated this branch to modify that file to also look at the Accounts service for session info. This fixes bug 818201.

Revision history for this message
Robert Ancell (robert-ancell) wrote :

Pushed with some minor changes:
- Missing 'return TRUE' at the end of load_accounts_service ()
- Replace '//' comments with '/*' ones
- Add NEWS entry

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'liblightdm-gobject/user.c'
2--- liblightdm-gobject/user.c 2011-08-17 00:26:17 +0000
3+++ liblightdm-gobject/user.c 2011-08-22 20:39:30 +0000
4@@ -1095,11 +1095,82 @@
5
6 // FIXME: Watch for changes
7
8+ if (priv->language)
9+ g_free (priv->language);
10+ if (priv->layout)
11+ g_free (priv->layout);
12+ if (priv->session)
13+ g_free (priv->session);
14+
15 priv->language = g_key_file_get_string (priv->dmrc_file, "Desktop", "Language", NULL);
16 priv->layout = g_key_file_get_string (priv->dmrc_file, "Desktop", "Layout", NULL);
17 priv->session = g_key_file_get_string (priv->dmrc_file, "Desktop", "Session", NULL);
18 }
19
20+static gchar *
21+get_string_property (GDBusProxy *proxy, const gchar *property)
22+{
23+ GVariant *answer;
24+ gchar *rv;
25+
26+ if (!proxy)
27+ return NULL;
28+
29+ answer = g_dbus_proxy_get_cached_property (proxy, property);
30+
31+ if (!answer) {
32+ g_warning ("Could not get accounts property %s", property);
33+ return NULL;
34+ }
35+
36+ if (!g_variant_is_of_type (answer, G_VARIANT_TYPE ("s"))) {
37+ g_warning ("Unexpected accounts property type for %s: %s",
38+ property, g_variant_get_type_string (answer));
39+ g_variant_unref (answer);
40+ return NULL;
41+ }
42+
43+ g_variant_get (answer, "s", &rv);
44+
45+ g_variant_unref (answer);
46+ return rv;
47+}
48+
49+static gboolean
50+load_accounts_service (LightDMUser *user)
51+{
52+ // First, find AccountObject proxy
53+ LightDMUserPrivate *priv = GET_USER_PRIVATE (user);
54+ LightDMUserListPrivate *list_priv = GET_LIST_PRIVATE (priv->user_list);
55+
56+ UserAccountObject *account = NULL;
57+ GList *iter;
58+ for (iter = list_priv->user_account_objects; iter; iter = iter->next) {
59+ if (((UserAccountObject *)iter->data)->user == user) {
60+ account = (UserAccountObject *)iter->data;
61+ break;
62+ }
63+ }
64+ if (!account)
65+ return FALSE;
66+
67+ // We have proxy, let's grab some properties
68+ if (priv->language)
69+ g_free (priv->language);
70+ if (priv->session)
71+ g_free (priv->session);
72+ priv->language = get_string_property (account->proxy, "Language");
73+ priv->session = get_string_property (account->proxy, "XSession");
74+}
75+
76+/* Loads language/layout/session info for user */
77+static void
78+load_user_values (LightDMUser *user)
79+{
80+ load_dmrc (user);
81+ load_accounts_service (user); // overrides dmrc values
82+}
83+
84 /**
85 * lightdm_user_get_language
86 * @user: A #LightDMUser
87@@ -1112,7 +1183,7 @@
88 lightdm_user_get_language (LightDMUser *user)
89 {
90 g_return_val_if_fail (LIGHTDM_IS_USER (user), NULL);
91- load_dmrc (user);
92+ load_user_values (user);
93 return GET_USER_PRIVATE (user)->language;
94 }
95
96@@ -1128,7 +1199,7 @@
97 lightdm_user_get_layout (LightDMUser *user)
98 {
99 g_return_val_if_fail (LIGHTDM_IS_USER (user), NULL);
100- load_dmrc (user);
101+ load_user_values (user);
102 return GET_USER_PRIVATE (user)->layout;
103 }
104
105@@ -1144,7 +1215,7 @@
106 lightdm_user_get_session (LightDMUser *user)
107 {
108 g_return_val_if_fail (LIGHTDM_IS_USER (user), NULL);
109- load_dmrc (user);
110+ load_user_values (user);
111 return GET_USER_PRIVATE (user)->session;
112 }
113
114
115=== modified file 'src/display.c'
116--- src/display.c 2011-08-18 00:11:39 +0000
117+++ src/display.c 2011-08-22 20:39:30 +0000
118@@ -1,4 +1,5 @@
119-/*
120+/* -*- Mode: C; indent-tabs-mode: nil; tab-width: 4 -*-
121+ *
122 * Copyright (C) 2010-2011 Robert Ancell.
123 * Author: Robert Ancell <robert.ancell@canonical.com>
124 *
125@@ -18,7 +19,6 @@
126 #include "configuration.h"
127 #include "user.h"
128 #include "pam-session.h"
129-#include "dmrc.h"
130 #include "ldm-marshal.h"
131 #include "greeter.h"
132 #include "xserver-local.h" // FIXME: Shouldn't know if it's an xserver
133@@ -771,7 +771,6 @@
134 start_user_session (Display *display, PAMSession *authentication)
135 {
136 User *user;
137- GKeyFile *dmrc_file;
138 gchar *log_filename;
139 gboolean result = FALSE;
140
141@@ -779,13 +778,8 @@
142
143 user = pam_session_get_user (authentication);
144
145- /* Load the users login settings (~/.dmrc) */
146- dmrc_file = dmrc_load (user_get_name (user));
147-
148- /* Update the .dmrc with changed settings */
149- g_key_file_set_string (dmrc_file, "Desktop", "Session", display->priv->user_session);
150- dmrc_save (dmrc_file, user_get_name (user));
151- g_key_file_free (dmrc_file);
152+ /* Update user's xsession setting */
153+ user_set_session (user, display->priv->user_session);
154
155 // FIXME: Copy old error file
156 log_filename = g_build_filename (user_get_home_directory (user), ".xsession-errors", NULL);
157
158=== modified file 'src/user.c'
159--- src/user.c 2011-08-06 15:00:26 +0000
160+++ src/user.c 2011-08-22 20:39:30 +0000
161@@ -1,4 +1,5 @@
162-/*
163+/* -*- Mode: C; indent-tabs-mode: nil; tab-width: 4 -*-
164+ *
165 * Copyright (C) 2010-2011 Robert Ancell.
166 * Author: Robert Ancell <robert.ancell@canonical.com>
167 *
168@@ -15,12 +16,16 @@
169 #include <string.h>
170
171 #include "user.h"
172+#include "dmrc.h"
173
174 struct UserPrivate
175 {
176 /* Name of user */
177 gchar *name;
178
179+ /* Accounts interface proxy */
180+ GDBusProxy *proxy;
181+
182 /* User ID */
183 uid_t uid;
184
185@@ -41,6 +46,157 @@
186
187 static gchar *passwd_file = NULL;
188
189+static gboolean
190+call_method (GDBusProxy *proxy, const gchar *method, GVariant *args,
191+ const gchar *expected, GVariant **result)
192+{
193+ GVariant *answer;
194+ GError *error = NULL;
195+
196+ if (!proxy)
197+ return FALSE;
198+
199+ answer = g_dbus_proxy_call_sync (proxy,
200+ method,
201+ args,
202+ G_DBUS_CALL_FLAGS_NONE,
203+ -1,
204+ NULL,
205+ &error);
206+
207+ if (!answer) {
208+ g_warning ("Could not call %s: %s", method, error->message);
209+ g_error_free (error);
210+ return FALSE;
211+ }
212+
213+ if (!g_variant_is_of_type (answer, G_VARIANT_TYPE (expected))) {
214+ g_warning ("Unexpected response from %s: %s",
215+ method, g_variant_get_type_string (answer));
216+ g_variant_unref (answer);
217+ return FALSE;
218+ }
219+
220+ if (result)
221+ *result = answer;
222+ else
223+ g_variant_unref (answer);
224+ return TRUE;
225+}
226+
227+static gboolean
228+get_property (GDBusProxy *proxy, const gchar *property,
229+ const gchar *expected, GVariant **result)
230+{
231+ GVariant *answer;
232+
233+ if (!proxy)
234+ return FALSE;
235+
236+ answer = g_dbus_proxy_get_cached_property (proxy, property);
237+
238+ if (!answer) {
239+ g_warning ("Could not get accounts property %s", property);
240+ return FALSE;
241+ }
242+
243+ if (!g_variant_is_of_type (answer, G_VARIANT_TYPE (expected))) {
244+ g_warning ("Unexpected accounts property type for %s: %s",
245+ property, g_variant_get_type_string (answer));
246+ g_variant_unref (answer);
247+ return FALSE;
248+ }
249+
250+ if (result)
251+ *result = answer;
252+ else
253+ g_variant_unref (answer);
254+ return TRUE;
255+}
256+
257+static void
258+save_string_to_dmrc (const gchar *username, const gchar *group,
259+ const gchar *key, const gchar *value)
260+{
261+ GKeyFile *dmrc;
262+
263+ dmrc = dmrc_load (username);
264+ g_key_file_set_string (dmrc, group, key, value);
265+ dmrc_save (dmrc, username);
266+
267+ g_key_file_free (dmrc);
268+}
269+
270+static gchar *
271+get_string_from_dmrc (const gchar *username, const gchar *group,
272+ const gchar *key)
273+{
274+ GKeyFile *dmrc;
275+ gchar *value;
276+
277+ dmrc = dmrc_load (username);
278+ value = g_key_file_get_string (dmrc, group, key, NULL);
279+
280+ g_key_file_free (dmrc);
281+ return value;
282+}
283+
284+static GDBusProxy *
285+get_accounts_proxy_for_user (const gchar *user)
286+{
287+ g_return_val_if_fail (user != NULL, NULL);
288+
289+ GDBusProxy *proxy;
290+ GError *error = NULL;
291+ GVariant *result;
292+ gboolean success;
293+ gchar *user_path = NULL;
294+
295+ proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
296+ G_DBUS_PROXY_FLAGS_NONE,
297+ NULL,
298+ "org.freedesktop.Accounts",
299+ "/org/freedesktop/Accounts",
300+ "org.freedesktop.Accounts",
301+ NULL, &error);
302+
303+ if (!proxy) {
304+ g_warning ("Could not get accounts proxy: %s", error->message);
305+ g_error_free (error);
306+ return NULL;
307+ }
308+
309+ success = call_method (proxy, "FindUserByName", g_variant_new ("(s)", user),
310+ "(o)", &result);
311+ g_object_unref (proxy);
312+
313+ if (!success)
314+ return NULL;
315+
316+ g_variant_get (result, "(o)", &user_path);
317+ g_variant_unref (result);
318+
319+ if (!user_path)
320+ return NULL;
321+
322+ proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
323+ G_DBUS_PROXY_FLAGS_NONE,
324+ NULL,
325+ "org.freedesktop.Accounts",
326+ user_path,
327+ "org.freedesktop.Accounts.User",
328+ NULL, &error);
329+ g_free (user_path);
330+
331+ if (!proxy) {
332+ g_warning ("Could not get accounts user proxy: %s", error->message);
333+ g_error_free (error);
334+ return NULL;
335+ }
336+
337+ return proxy;
338+}
339+
340 static User *
341 user_from_passwd (struct passwd *user_info)
342 {
343@@ -53,6 +209,7 @@
344 user->priv->gecos = g_strdup (user_info->pw_gecos);
345 user->priv->home_directory = g_strdup (user_info->pw_dir);
346 user->priv->shell = g_strdup (user_info->pw_shell);
347+ user->priv->proxy = get_accounts_proxy_for_user (user->priv->name);
348
349 return user;
350 }
351@@ -236,6 +393,40 @@
352 return user->priv->shell;
353 }
354
355+void
356+user_set_session (User *user, const gchar *session)
357+{
358+ g_return_if_fail (user != NULL);
359+
360+ call_method (user->priv->proxy, "SetXSession",
361+ g_variant_new ("(s)", session), "()", NULL);
362+
363+ save_string_to_dmrc (user->priv->name, "Desktop", "Session", session);
364+}
365+
366+gchar *
367+user_get_session (User *user)
368+{
369+ g_return_val_if_fail (user != NULL, NULL);
370+
371+ GVariant *result;
372+ gchar *session;
373+
374+ if (!get_property (user->priv->proxy, "XSession",
375+ "s", &result))
376+ return get_string_from_dmrc (user->priv->name, "Desktop", "Session");
377+
378+ g_variant_get (result, "s", &session);
379+ g_variant_unref (result);
380+
381+ if (g_strcmp0 (session, "") == 0) {
382+ g_free (session);
383+ return NULL;
384+ }
385+
386+ return session;
387+}
388+
389 static void
390 user_init (User *user)
391 {
392@@ -243,6 +434,21 @@
393 }
394
395 static void
396+user_dispose (GObject *object)
397+{
398+ User *self;
399+
400+ self = USER (object);
401+
402+ if (self->priv->proxy) {
403+ g_object_unref (self->priv->proxy);
404+ self->priv->proxy = NULL;
405+ }
406+
407+ G_OBJECT_CLASS (user_parent_class)->dispose (object);
408+}
409+
410+static void
411 user_finalize (GObject *object)
412 {
413 User *self;
414@@ -262,6 +468,7 @@
415 {
416 GObjectClass *object_class = G_OBJECT_CLASS (klass);
417
418+ object_class->dispose = user_dispose;
419 object_class->finalize = user_finalize;
420
421 g_type_class_add_private (klass, sizeof (UserPrivate));
422
423=== modified file 'src/user.h'
424--- src/user.h 2011-06-27 11:17:39 +0000
425+++ src/user.h 2011-08-22 20:39:30 +0000
426@@ -1,4 +1,5 @@
427-/*
428+/* -*- Mode: C; indent-tabs-mode: nil; tab-width: 4 -*-
429+ *
430 * Copyright (C) 2010-2011 Robert Ancell.
431 * Author: Robert Ancell <robert.ancell@canonical.com>
432 *
433@@ -57,6 +58,10 @@
434
435 const gchar *user_get_shell (User *user);
436
437+gchar *user_get_session (User *user);
438+
439+void user_set_session (User *user, const gchar *session);
440+
441 G_END_DECLS
442
443 #endif /* _USER_H_ */

Subscribers

People subscribed via source and target branches