Light Display Manager

Merge lp:~mterry/lightdm/xdg-dirs into lp:lightdm

  1. xdg-dirs
  2. Merge into trunk
Proposed by Michael Terry
Status: Merged
Merged at revision: 1940
Proposed branch: lp:~mterry/lightdm/xdg-dirs
Merge into: lp:lightdm
Diff against target: 453 lines (+168/-134)
8 files modified
common/Makefile.am (+2/-1)
common/configuration.c (+137/-0)
common/configuration.h (+2/-0)
liblightdm-gobject/session.c (+13/-25)
src/Makefile.am (+0/-3)
src/lightdm.c (+2/-103)
tests/src/libsystem.c (+6/-0)
tests/src/test-runner.c (+6/-2)
To merge this branch: bzr merge lp:~mterry/lightdm/xdg-dirs
Reviewer Review Type Date Requested Status
Robert Ancell Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+211444@code.launchpad.net

Commit message

Read config data from both XDG_DATA_DIRS and XDG_CONFIG_DIRS.

Description of the change

Read config data from both XDG_DATA_DIRS and XDG_CONFIG_DIRS.

This is useful for system builders and admins. It will also be useful in Ubuntu Touch, where customizations can be put in /custom (part of XDG_DATA_DIRS).

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1942
http://jenkins.qa.ubuntu.com/job/lightdm-ci/260/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-trusty-amd64-ci/54
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-trusty-armhf-ci/54

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

review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Odd indentation:

108 + gchar *full_dir = g_build_filename (dirs[i], "lightdm", "lightdm.conf.d", NULL);
109 + if (messages)
110 + *messages = g_list_append (*messages, g_strdup_printf ("Loading configuration dirs from %s", full_dir));
111 + load_config_directory (full_dir, messages);

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

You need to continue to support loading configuration from /etc. XDG_CONFIG_DIRS only contains /etc/xdg by default.

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

Otherwise looks good.

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

> You need to continue to support loading configuration from /etc.
> XDG_CONFIG_DIRS only contains /etc/xdg by default.

Oh, I think you are doing this correctly...

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

Works for me, committed with a few minor cosmetic changes.

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

Thanks! I screwed up that indentation, yeah. And right, this still loads from /etc, but I switched the test to use /etc/xdg because I thought that was more important to test (easier to screw up loading from a dynamic XDG var, rather than a hard-coded path).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'common/Makefile.am'
2--- common/Makefile.am 2014-02-04 22:55:32 +0000
3+++ common/Makefile.am 2014-03-18 03:18:39 +0000
4@@ -14,7 +14,8 @@
5
6 libcommon_la_CFLAGS = \
7 $(WARN_CFLAGS) \
8- $(GLIB_CFLAGS)
9+ $(GLIB_CFLAGS) \
10+ -DCONFIG_DIR=\"$(sysconfdir)/lightdm\"
11
12 libcommon_la_LIBADD = \
13 $(GLIB_LDFLAGS)
14
15=== modified file 'common/configuration.c'
16--- common/configuration.c 2014-03-06 01:45:51 +0000
17+++ common/configuration.c 2014-03-18 03:18:39 +0000
18@@ -9,6 +9,8 @@
19 * license.
20 */
21
22+#include <string.h>
23+
24 #include "configuration.h"
25
26 struct ConfigurationPrivate
27@@ -70,6 +72,141 @@
28 return TRUE;
29 }
30
31+static gchar *
32+path_make_absolute (gchar *path)
33+{
34+ gchar *cwd, *abs_path;
35+
36+ if (!path)
37+ return NULL;
38+
39+ if (g_path_is_absolute (path))
40+ return path;
41+
42+ cwd = g_get_current_dir ();
43+ abs_path = g_build_filename (cwd, path, NULL);
44+ g_free (path);
45+
46+ return abs_path;
47+}
48+
49+static int
50+compare_strings (gconstpointer a, gconstpointer b)
51+{
52+ return strcmp (a, b);
53+}
54+
55+static void
56+load_config_directory (const gchar *path, GList **messages)
57+{
58+ GDir *dir;
59+ GList *files = NULL, *link;
60+ GError *error = NULL;
61+
62+ /* Find configuration files */
63+ dir = g_dir_open (path, 0, &error);
64+ if (error && !g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
65+ g_printerr ("Failed to open configuration directory %s: %s\n", path, error->message);
66+ g_clear_error (&error);
67+ if (dir)
68+ {
69+ const gchar *name;
70+ while ((name = g_dir_read_name (dir)))
71+ files = g_list_append (files, g_strdup (name));
72+ g_dir_close (dir);
73+ }
74+
75+ /* Sort alphabetically and load onto existing configuration */
76+ files = g_list_sort (files, compare_strings);
77+ for (link = files; link; link = link->next)
78+ {
79+ gchar *filename = link->data;
80+ gchar *conf_path;
81+
82+ conf_path = g_build_filename (path, filename, NULL);
83+ if (g_str_has_suffix (filename, ".conf"))
84+ {
85+ if (messages)
86+ *messages = g_list_append (*messages, g_strdup_printf ("Loading configuration from %s", conf_path));
87+ config_load_from_file (config_get_instance (), conf_path, &error);
88+ if (error && !g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
89+ g_printerr ("Failed to load configuration from %s: %s\n", filename, error->message);
90+ g_clear_error (&error);
91+ }
92+ else
93+ g_debug ("Ignoring configuration file %s, it does not have .conf suffix", conf_path);
94+ g_free (conf_path);
95+ }
96+ g_list_free_full (files, g_free);
97+}
98+
99+static void
100+load_config_directories (const gchar * const *dirs, GList **messages)
101+{
102+ gint i;
103+
104+ // Load in reverse order, because XDG_* fields are preference-ordered and
105+ // the directories in front should override directories in back.
106+ for (i = g_strv_length ((gchar **)dirs) - 1; i >= 0; i--)
107+ {
108+ gchar *full_dir = g_build_filename (dirs[i], "lightdm", "lightdm.conf.d", NULL);
109+ if (messages)
110+ *messages = g_list_append (*messages, g_strdup_printf ("Loading configuration dirs from %s", full_dir));
111+ load_config_directory (full_dir, messages);
112+ g_free (full_dir);
113+ }
114+}
115+
116+gboolean
117+config_load_from_standard_locations (Configuration *config, const gchar *config_path, GList **messages)
118+{
119+ gchar *config_dir, *config_d_dir = NULL;
120+ gboolean explicit_config = FALSE;
121+ gboolean success = TRUE;
122+ GError *error = NULL;
123+
124+ load_config_directories (g_get_system_data_dirs (), messages);
125+ load_config_directories (g_get_system_config_dirs (), messages);
126+
127+ if (config_path)
128+ {
129+ config_dir = g_path_get_basename (config_path);
130+ config_dir = path_make_absolute (config_dir);
131+ explicit_config = TRUE;
132+ }
133+ else
134+ {
135+ config_dir = g_strdup (CONFIG_DIR);
136+ config_d_dir = g_build_filename (config_dir, "lightdm.conf.d", NULL);
137+ config_path = g_build_filename (config_dir, "lightdm.conf", NULL);
138+ }
139+ config_set_string (config, "LightDM", "config-directory", config_dir);
140+ g_free (config_dir);
141+
142+ if (config_d_dir)
143+ load_config_directory (config_d_dir, messages);
144+ g_free (config_d_dir);
145+
146+ if (messages)
147+ *messages = g_list_append (*messages, g_strdup_printf ("Loading configuration from %s", config_path));
148+ if (!config_load_from_file (config, config_path, &error))
149+ {
150+ gboolean is_empty;
151+
152+ is_empty = error && g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT);
153+
154+ if (explicit_config || !is_empty)
155+ {
156+ if (error)
157+ g_printerr ("Failed to load configuration from %s: %s\n", config_path, error->message);
158+ success = FALSE;
159+ }
160+ }
161+ g_clear_error (&error);
162+
163+ return success;
164+}
165+
166 gchar **
167 config_get_groups (Configuration *config)
168 {
169
170=== modified file 'common/configuration.h'
171--- common/configuration.h 2014-02-04 22:55:32 +0000
172+++ common/configuration.h 2014-03-18 03:18:39 +0000
173@@ -38,6 +38,8 @@
174
175 gboolean config_load_from_file (Configuration *config, const gchar *path, GError **error);
176
177+gboolean config_load_from_standard_locations (Configuration *config, const gchar *config_path, GList **messages);
178+
179 gchar **config_get_groups (Configuration *config);
180
181 gchar **config_get_keys (Configuration *config, const gchar *group_name);
182
183=== modified file 'liblightdm-gobject/session.c'
184--- liblightdm-gobject/session.c 2014-02-04 22:55:32 +0000
185+++ liblightdm-gobject/session.c 2014-03-18 03:18:39 +0000
186@@ -192,11 +192,10 @@
187 static void
188 update_sessions (void)
189 {
190- gchar *config_path = NULL;
191 gchar *sessions_dir;
192 gchar *remote_sessions_dir;
193 gboolean result;
194- GError *error = NULL;
195+ gchar *value;
196
197 if (have_sessions)
198 return;
199@@ -205,32 +204,21 @@
200 remote_sessions_dir = g_strdup (REMOTE_SESSIONS_DIR);
201
202 /* Use session directory from configuration */
203- /* FIXME: This should be sent in the greeter connection */
204- config_path = g_build_filename (CONFIG_DIR, "lightdm.conf", NULL);
205- /* FIXME: This should load from lightdm.conf.d as well */
206- result = config_load_from_file (config_get_instance (), config_path, &error);
207- if (error && !g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
208- g_warning ("Failed to open configuration file: %s", error->message);
209- g_clear_error (&error);
210- if (result)
211- {
212- gchar *value;
213+ config_load_from_standard_locations (config_get_instance (), NULL, NULL);
214
215- value = config_get_string (config_get_instance (), "LightDM", "sessions-directory");
216- if (value)
217- {
218- g_free (sessions_dir);
219- sessions_dir = value;
220- }
221+ value = config_get_string (config_get_instance (), "LightDM", "sessions-directory");
222+ if (value)
223+ {
224+ g_free (sessions_dir);
225+ sessions_dir = value;
226+ }
227
228- value = config_get_string (config_get_instance (), "LightDM", "remote-sessions-directory");
229- if (value)
230- {
231- g_free (remote_sessions_dir);
232- remote_sessions_dir = value;
233- }
234+ value = config_get_string (config_get_instance (), "LightDM", "remote-sessions-directory");
235+ if (value)
236+ {
237+ g_free (remote_sessions_dir);
238+ remote_sessions_dir = value;
239 }
240- g_free (config_path);
241
242 local_sessions = load_sessions (sessions_dir);
243 remote_sessions = load_sessions (remote_sessions_dir);
244
245=== modified file 'src/Makefile.am'
246--- src/Makefile.am 2014-02-08 23:36:27 +0000
247+++ src/Makefile.am 2014-03-18 03:18:39 +0000
248@@ -78,12 +78,10 @@
249 $(LIGHTDM_CFLAGS) \
250 -I"$(top_srcdir)/common" \
251 -DSBIN_DIR=\"$(sbindir)\" \
252- -DCONFIG_DIR=\"$(sysconfdir)/lightdm\" \
253 -DUSERS_DIR=\"$(localstatedir)/lib/lightdm-data\" \
254 -DLOG_DIR=\"$(localstatedir)/log/lightdm\" \
255 -DRUN_DIR=\"$(localstatedir)/run/lightdm\" \
256 -DCACHE_DIR=\"$(localstatedir)/cache/lightdm\" \
257- -DSYSTEM_CONFIG_DIR=\"$(pkgdatadir)/lightdm.conf.d\" \
258 -DSESSIONS_DIR=\"$(pkgdatadir)/sessions:$(datadir)/xsessions\" \
259 -DREMOTE_SESSIONS_DIR=\"$(pkgdatadir)/remote-sessions\" \
260 -DGREETERS_DIR=\"$(pkgdatadir)/greeters:$(datadir)/xgreeters\"
261@@ -100,7 +98,6 @@
262 dm_tool_CFLAGS = \
263 $(WARN_CFLAGS) \
264 $(LIGHTDM_CFLAGS) \
265- -DCONFIG_DIR=\"$(sysconfdir)/lightdm\" \
266 -DLOCALE_DIR=\"$(datadir)/locale\"
267
268 dm_tool_LDADD = \
269
270=== modified file 'src/lightdm.c'
271--- src/lightdm.c 2014-02-18 19:49:01 +0000
272+++ src/lightdm.c 2014-03-18 03:18:39 +0000
273@@ -903,73 +903,6 @@
274 exit (EXIT_FAILURE);
275 }
276
277-static gchar *
278-path_make_absolute (gchar *path)
279-{
280- gchar *cwd, *abs_path;
281-
282- if (!path)
283- return NULL;
284-
285- if (g_path_is_absolute (path))
286- return path;
287-
288- cwd = g_get_current_dir ();
289- abs_path = g_build_filename (cwd, path, NULL);
290- g_free (path);
291-
292- return abs_path;
293-}
294-
295-static int
296-compare_strings (gconstpointer a, gconstpointer b)
297-{
298- return strcmp (a, b);
299-}
300-
301-static void
302-load_config_directory (const gchar *path, GList **messages)
303-{
304- GDir *dir;
305- GList *files = NULL, *link;
306- GError *error = NULL;
307-
308- /* Find configuration files */
309- dir = g_dir_open (path, 0, &error);
310- if (error && !g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
311- g_printerr ("Failed to open configuration directory %s: %s\n", path, error->message);
312- g_clear_error (&error);
313- if (dir)
314- {
315- const gchar *name;
316- while ((name = g_dir_read_name (dir)))
317- files = g_list_append (files, g_strdup (name));
318- g_dir_close (dir);
319- }
320-
321- /* Sort alphabetically and load onto existing configuration */
322- files = g_list_sort (files, compare_strings);
323- for (link = files; link; link = link->next)
324- {
325- gchar *filename = link->data;
326- gchar *conf_path;
327-
328- conf_path = g_build_filename (path, filename, NULL);
329- if (g_str_has_suffix (filename, ".conf"))
330- {
331- *messages = g_list_append (*messages, g_strdup_printf ("Loading configuration from %s", conf_path));
332- config_load_from_file (config_get_instance (), conf_path, &error);
333- if (error && !g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
334- g_printerr ("Failed to load configuration from %s: %s\n", filename, error->message);
335- g_clear_error (&error);
336- }
337- else
338- g_debug ("Ignoring configuration file %s, it does not have .conf suffix", conf_path);
339- g_free (conf_path);
340- }
341- g_list_free_full (files, g_free);
342-}
343-
344 int
345 main (int argc, char **argv)
346 {
347@@ -978,10 +911,8 @@
348 gboolean result;
349 gchar **groups, **i, *dir;
350 gint n_seats = 0;
351- gboolean explicit_config = FALSE;
352 gboolean test_mode = FALSE;
353 gchar *pid_path = "/var/run/lightdm.pid";
354- gchar *config_dir, *config_d_dir = NULL;
355 gchar *log_dir = NULL;
356 gchar *run_dir = NULL;
357 gchar *cache_dir = NULL;
358@@ -1056,21 +987,6 @@
359 return EXIT_SUCCESS;
360 }
361
362- if (config_path)
363- {
364- config_dir = g_path_get_basename (config_path);
365- config_dir = path_make_absolute (config_dir);
366- explicit_config = TRUE;
367- }
368- else
369- {
370- config_dir = g_strdup (CONFIG_DIR);
371- config_d_dir = g_build_filename (config_dir, "lightdm.conf.d", NULL);
372- config_path = g_build_filename (config_dir, "lightdm.conf", NULL);
373- }
374- config_set_string (config_get_instance (), "LightDM", "config-directory", config_dir);
375- g_free (config_dir);
376-
377 if (!test_mode && getuid () != 0)
378 {
379 g_printerr ("Only root can run Light Display Manager. To run as a regular user for testing run with the --test-mode flag.\n");
380@@ -1125,25 +1041,8 @@
381 }
382
383 /* Load config file(s) */
384- load_config_directory (SYSTEM_CONFIG_DIR, &messages);
385- if (config_d_dir)
386- load_config_directory (config_d_dir, &messages);
387- g_free (config_d_dir);
388- messages = g_list_append (messages, g_strdup_printf ("Loading configuration from %s", config_path));
389- if (!config_load_from_file (config_get_instance (), config_path, &error))
390- {
391- gboolean is_empty;
392-
393- is_empty = error && g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT);
394-
395- if (explicit_config || !is_empty)
396- {
397- if (error)
398- g_printerr ("Failed to load configuration from %s: %s\n", config_path, error->message);
399- exit (EXIT_FAILURE);
400- }
401- }
402- g_clear_error (&error);
403+ if (!config_load_from_standard_locations (config_get_instance (), config_path, &messages))
404+ exit (EXIT_FAILURE);
405 g_free (config_path);
406
407 /* Set default values */
408
409=== modified file 'tests/src/libsystem.c'
410--- tests/src/libsystem.c 2014-03-10 23:44:49 +0000
411+++ tests/src/libsystem.c 2014-03-18 03:18:39 +0000
412@@ -195,6 +195,12 @@
413 if (g_str_has_prefix (path, "/tmp"))
414 return g_build_filename (g_getenv ("LIGHTDM_TEST_ROOT"), "tmp", path + strlen ("/tmp"), NULL);
415
416+ if (g_str_has_prefix (path, "/etc/xdg"))
417+ return g_build_filename (g_getenv ("LIGHTDM_TEST_ROOT"), "etc", "xdg", path + strlen ("/etc/xdg"), NULL);
418+
419+ if (g_str_has_prefix (path, "/usr/share/lightdm"))
420+ return g_build_filename (g_getenv ("LIGHTDM_TEST_ROOT"), "usr", "share", "lightdm", path + strlen ("/usr/share/lightdm"), NULL);
421+
422 return g_strdup (path);
423 }
424
425
426=== modified file 'tests/src/test-runner.c'
427--- tests/src/test-runner.c 2014-03-17 18:33:02 +0000
428+++ tests/src/test-runner.c 2014-03-18 03:18:39 +0000
429@@ -1995,6 +1995,10 @@
430 /* Don't contact our X server */
431 g_unsetenv ("DISPLAY");
432
433+ /* Don't let XDG vars from system affect tests */
434+ g_unsetenv ("XDG_CONFIG_DIRS");
435+ g_unsetenv ("XDG_DATA_DIRS");
436+
437 /* Override system calls */
438 ld_preload = g_build_filename (BUILDDIR, "tests", "src", ".libs", "libsystem.so", NULL);
439 g_setenv ("LD_PRELOAD", ld_preload, TRUE);
440@@ -2108,11 +2112,11 @@
441 {
442 gchar **files;
443
444- g_mkdir_with_parents (g_strdup_printf ("%s/etc/lightdm/lightdm.conf.d", temp_dir), 0755);
445+ g_mkdir_with_parents (g_strdup_printf ("%s/etc/xdg/lightdm/lightdm.conf.d", temp_dir), 0755);
446
447 files = g_strsplit (additional_config, " ", -1);
448 for (i = 0; files[i]; i++)
449- if (system (g_strdup_printf ("cp %s/tests/scripts/%s %s/etc/lightdm/lightdm.conf.d", SRCDIR, files[i], temp_dir)))
450+ if (system (g_strdup_printf ("cp %s/tests/scripts/%s %s/etc/xdg/lightdm/lightdm.conf.d", SRCDIR, files[i], temp_dir)))
451 perror ("Failed to copy configuration");
452 g_strfreev (files);
453 }

Subscribers

People subscribed via source and target branches