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
=== modified file 'ChangeLog'
--- ChangeLog 2013-11-14 14:28:26 +0000
+++ ChangeLog 2013-11-16 13:03:00 +0000
@@ -1,3 +1,11 @@
12013-11-16 Dmitrijs Ledkovs <xnox@ubuntu.com>
2
3 * init/xdg.c, util/Makefile.am, test/Makefile.am, init/conf.c:
4 - juggle user_mode & session_file from xdg.c to conf.c
5 - remove INITCTL_BUILD conditional compilation
6 - make it possible to use the same xdg.o in both init & initctl
7 - this is in preparation to handle subdir-objects automake option
8
12013-11-14 James Hunt <james.hunt@ubuntu.com>92013-11-14 James Hunt <james.hunt@ubuntu.com>
210
3 * NEWS: Release 1.1111 * NEWS: Release 1.11
412
=== modified file 'init/conf.c'
--- init/conf.c 2013-07-16 15:47:12 +0000
+++ init/conf.c 2013-11-16 13:03:00 +0000
@@ -96,6 +96,23 @@
96 __attribute__ ((warn_unused_result));96 __attribute__ ((warn_unused_result));
9797
98/**98/**
99 * user_mode:
100 *
101 * If TRUE, upstart runs in user session mode.
102 **/
103int user_mode = FALSE;
104
105/**
106 * session_file:
107 *
108 * Full path to file containing UPSTART_SESSION details (only set when
109 * user_mode in operation).
110 *
111 * File is created on startup and removed on clean shutdown.
112 **/
113const char *session_file = NULL;
114
115/**
99 * conf_sources:116 * conf_sources:
100 *117 *
101 * This list holds the list of known sources of configuration; each item118 * This list holds the list of known sources of configuration; each item
102119
=== modified file 'init/xdg.c'
--- init/xdg.c 2013-05-30 13:14:49 +0000
+++ init/xdg.c 2013-11-16 13:03:00 +0000
@@ -34,26 +34,6 @@
34#include "paths.h"34#include "paths.h"
35#include "xdg.h"35#include "xdg.h"
3636
37#ifndef INITCTL_BUILD
38
39/**
40 * user_mode:
41 *
42 * If TRUE, upstart runs in user session mode.
43 **/
44int user_mode = FALSE;
45
46/**
47 * session_file:
48 *
49 * Full path to file containing UPSTART_SESSION details (only set when
50 * user_mode in operation).
51 *
52 * File is created on startup and removed on clean shutdown.
53 **/
54const char *session_file = NULL;
55
56#endif /* INITCTL_BUILD */
5737
58/**38/**
59 * get_subdir:39 * get_subdir:
6040
=== modified file 'test/Makefile.am'
--- test/Makefile.am 2013-11-11 16:15:52 +0000
+++ test/Makefile.am 2013-11-16 13:03:00 +0000
@@ -10,7 +10,6 @@
1010
11AM_CPPFLAGS = \11AM_CPPFLAGS = \
12 -DLOCALEDIR="\"$(localedir)\"" \12 -DLOCALEDIR="\"$(localedir)\"" \
13 -DINITCTL_BUILD \
14 -DSBINDIR="\"$(sbindir)\"" \13 -DSBINDIR="\"$(sbindir)\"" \
15 -DUPSTART_BINARY="\"$(UPSTART_BINARY)\"" \14 -DUPSTART_BINARY="\"$(UPSTART_BINARY)\"" \
16 -DINITCTL_BINARY="\"$(INITCTL_BINARY)\"" \15 -DINITCTL_BINARY="\"$(INITCTL_BINARY)\"" \
1716
=== modified file 'util/Makefile.am'
--- util/Makefile.am 2013-11-12 14:00:36 +0000
+++ util/Makefile.am 2013-11-16 13:03:00 +0000
@@ -7,7 +7,6 @@
77
8AM_CPPFLAGS = \8AM_CPPFLAGS = \
9 -DLOCALEDIR="\"$(localedir)\"" \9 -DLOCALEDIR="\"$(localedir)\"" \
10 -DINITCTL_BUILD \
11 -DSBINDIR="\"$(sbindir)\"" \10 -DSBINDIR="\"$(sbindir)\"" \
12 -I$(top_builddir) -I$(top_srcdir) -iquote$(builddir) -iquote$(srcdir) \11 -I$(top_builddir) -I$(top_srcdir) -iquote$(builddir) -iquote$(srcdir) \
13 -I$(top_srcdir)/intl \12 -I$(top_srcdir)/intl \

Subscribers

People subscribed via source and target branches