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 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Ancell | Approve | ||
Review via email: mp+71939@code.launchpad.net |
Commit message
Description of the change
This uses the org.freedesktop
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_
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.
- 1057. By Michael Terry
-
add back dmrc support as a fallback for accounts service
- 1058. By Michael Terry
-
move Accounts class into User class
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.)
- 1059. By Michael Terry
-
liblightdm-gobject: look at Accounts object when checking per-user session and language values
Michael Terry (mterry) wrote : | # |
Sorry, I was wrong about never loading per-user values from them. I missed the glue in liblightdm-
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.
Robert Ancell (robert-ancell) wrote : | # |
Pushed with some minor changes:
- Missing 'return TRUE' at the end of load_accounts_
- Replace '//' comments with '/*' ones
- Add NEWS entry
Preview Diff
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_ */ |
I think we need to continue to support .dmrc for systems that don't have accounts service: lists.freedeskt op.org/ archives/ lightdm/ 2011-August/ 000051. html
http://
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.