Merge lp:~xnox/upstart/no-multibuild into lp:upstart

Proposed by Dimitri John Ledkov on 2013-11-16
Status: Merged
Merged at revision: 1575
Proposed branch: lp:~xnox/upstart/no-multibuild
Merge into: lp:upstart
Diff against target: 98 lines (+25/-22)
5 files modified
ChangeLog (+8/-0)
init/conf.c (+17/-0)
init/xdg.c (+0/-20)
test/Makefile.am (+0/-1)
util/Makefile.am (+0/-1)
To merge this branch: bzr merge lp:~xnox/upstart/no-multibuild
Reviewer Review Type Date Requested Status
James Hunt 2013-11-16 Approve on 2013-11-18
Review via email: mp+195471@code.launchpad.net

Description of the change

At the moment xdg.c is compiled twice:

init/Makefile.am compiles init/xdg.c -> init/xdg.o
 - used by init/init and some init/test_* binaries

util/Makefile.am compiles init/xdg.c -> util/xdg.o
 - used by util/initctl and some util/test_* binaries

When subdir-objects option to automake is enabled (which will be the default in automake 2.0),
both init/Makefile.am and util/Makefile.am will compile init/xdg.c into init/xdg.o.

That's going to be a problem, because at the moment there is conditional compilation in xdg.c, and the two xdg.o are different. With subdir-objects, it will then fail to compile either one of init/* or util/*

So, I propose to remove the conditional compilation from xdg.c, and move the definition of the two variables (user_mode and session_file) from xdg.c into conf.c. Those two variables are declared as extern in other places in init/* and have a conflicting definition in util/*.

I'm not sure what's the best practice to managing defitions of extern variables. I guess we can split them into init/init_externs.c where all externs definitions are stored, instead of being scattered around various init/*.c files. In practice, xdg.c is the only one that is common across multiple subdirs so far.

To post a comment you must log in.
James Hunt (jamesodhunt) wrote :

LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-11-14 14:28:26 +0000
3+++ ChangeLog 2013-11-16 13:03:00 +0000
4@@ -1,3 +1,11 @@
5+2013-11-16 Dmitrijs Ledkovs <xnox@ubuntu.com>
6+
7+ * init/xdg.c, util/Makefile.am, test/Makefile.am, init/conf.c:
8+ - juggle user_mode & session_file from xdg.c to conf.c
9+ - remove INITCTL_BUILD conditional compilation
10+ - make it possible to use the same xdg.o in both init & initctl
11+ - this is in preparation to handle subdir-objects automake option
12+
13 2013-11-14 James Hunt <james.hunt@ubuntu.com>
14
15 * NEWS: Release 1.11
16
17=== modified file 'init/conf.c'
18--- init/conf.c 2013-07-16 15:47:12 +0000
19+++ init/conf.c 2013-11-16 13:03:00 +0000
20@@ -96,6 +96,23 @@
21 __attribute__ ((warn_unused_result));
22
23 /**
24+ * user_mode:
25+ *
26+ * If TRUE, upstart runs in user session mode.
27+ **/
28+int user_mode = FALSE;
29+
30+/**
31+ * session_file:
32+ *
33+ * Full path to file containing UPSTART_SESSION details (only set when
34+ * user_mode in operation).
35+ *
36+ * File is created on startup and removed on clean shutdown.
37+ **/
38+const char *session_file = NULL;
39+
40+/**
41 * conf_sources:
42 *
43 * This list holds the list of known sources of configuration; each item
44
45=== modified file 'init/xdg.c'
46--- init/xdg.c 2013-05-30 13:14:49 +0000
47+++ init/xdg.c 2013-11-16 13:03:00 +0000
48@@ -34,26 +34,6 @@
49 #include "paths.h"
50 #include "xdg.h"
51
52-#ifndef INITCTL_BUILD
53-
54-/**
55- * user_mode:
56- *
57- * If TRUE, upstart runs in user session mode.
58- **/
59-int user_mode = FALSE;
60-
61-/**
62- * session_file:
63- *
64- * Full path to file containing UPSTART_SESSION details (only set when
65- * user_mode in operation).
66- *
67- * File is created on startup and removed on clean shutdown.
68- **/
69-const char *session_file = NULL;
70-
71-#endif /* INITCTL_BUILD */
72
73 /**
74 * get_subdir:
75
76=== modified file 'test/Makefile.am'
77--- test/Makefile.am 2013-11-11 16:15:52 +0000
78+++ test/Makefile.am 2013-11-16 13:03:00 +0000
79@@ -10,7 +10,6 @@
80
81 AM_CPPFLAGS = \
82 -DLOCALEDIR="\"$(localedir)\"" \
83- -DINITCTL_BUILD \
84 -DSBINDIR="\"$(sbindir)\"" \
85 -DUPSTART_BINARY="\"$(UPSTART_BINARY)\"" \
86 -DINITCTL_BINARY="\"$(INITCTL_BINARY)\"" \
87
88=== modified file 'util/Makefile.am'
89--- util/Makefile.am 2013-11-12 14:00:36 +0000
90+++ util/Makefile.am 2013-11-16 13:03:00 +0000
91@@ -7,7 +7,6 @@
92
93 AM_CPPFLAGS = \
94 -DLOCALEDIR="\"$(localedir)\"" \
95- -DINITCTL_BUILD \
96 -DSBINDIR="\"$(sbindir)\"" \
97 -I$(top_builddir) -I$(top_srcdir) -iquote$(builddir) -iquote$(srcdir) \
98 -I$(top_srcdir)/intl \

Subscribers

People subscribed via source and target branches