Merge lp:~jamesodhunt/upstart/use-session-init-for-init-checkconf into lp:upstart

Proposed by James Hunt on 2013-11-18
Status: Merged
Merged at revision: 1600
Proposed branch: lp:~jamesodhunt/upstart/use-session-init-for-init-checkconf
Merge into: lp:upstart
Diff against target: 331 lines (+106/-93)
3 files modified
ChangeLog (+7/-0)
scripts/init-checkconf.sh (+95/-87)
scripts/man/init-checkconf.8 (+4/-6)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/use-session-init-for-init-checkconf
Reviewer Review Type Date Requested Status
Upstart Reviewers 2013-11-18 Pending
Review via email: mp+195667@code.launchpad.net

Description of the change

Making the Session Init connect to the D-Bus session bus on request ('initctl notify-dbus-address') breaks init-checkconf.

This branch simplifies init-checkconf to not rely on D-bus, but instead spawn a Session Init.

I'll raise a follow-on MP that will add tests for the Upstart scripts to ensure they are not inadvertently broken in the future.

To post a comment you must log in.

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-18 10:31:21 +0000
3+++ ChangeLog 2013-11-18 20:18:10 +0000
4@@ -1,3 +1,10 @@
5+2013-11-18 James Hunt <james.hunt@ubuntu.com>
6+
7+ * scripts/init-checkconf.sh:
8+ - Don't rely on D-Bus any more: spawn a Session Init instead since
9+ this is simpler and allows the two major limitations to be dropped.
10+ * scripts/man/init-checkconf.8: Updated date and limitations section.
11+
12 2013-11-16 Dmitrijs Ledkovs <xnox@ubuntu.com>
13
14 * init/xdg.c, util/Makefile.am, test/Makefile.am, init/conf.c:
15
16=== modified file 'scripts/init-checkconf.sh'
17--- scripts/init-checkconf.sh 2013-01-23 14:28:07 +0000
18+++ scripts/init-checkconf.sh 2013-11-18 20:18:10 +0000
19@@ -8,7 +8,7 @@
20 #
21 #---------------------------------------------------------------------
22 #
23-# Copyright (C) 2011 Canonical Ltd.
24+# Copyright (C) 2011-2013 Canonical Ltd.
25 #
26 # Author: James Hunt <james.hunt@canonical.com>
27 #
28@@ -28,34 +28,38 @@
29
30 script_name=${0##*/}
31 confdir=$(mktemp -d /tmp/${script_name}.XXXXXXXXXX)
32+xdg_runtime_dir=$(mktemp -d /tmp/${script_name}.XXXXXXXXXX)
33 upstart_path=/sbin/init
34 initctl_path=/sbin/initctl
35 debug_enabled=n
36 file_valid=n
37 running=n
38+set_session=n
39 check_scripts=y
40-dbus_cmd=dbus-launch
41-started_dbus=n
42
43 cleanup()
44 {
45- if [ ! -z "$upstart_pid" ]
46- then
47- debug "stopping secondary Upstart (running with PID $upstart_pid)"
48- kill -0 "$upstart_pid" >/dev/null 2>&1 && \
49- kill -9 "$upstart_pid" >/dev/null 2>&1
50- fi
51-
52- if [ "$started_dbus" = y ] && [ -n "$DBUS_SESSION_BUS_PID" ]
53- then
54- debug "stopping dbus-daemon (running with PID $DBUS_SESSION_BUS_PID)"
55- kill -0 "$DBUS_SESSION_BUS_PID" >/dev/null 2>&1 && \
56- kill -9 "$DBUS_SESSION_BUS_PID" >/dev/null 2>&1
57- fi
58-
59- [ -d "$confdir" ] && rm -rf "$confdir"
60- [ $file_valid = y ] && exit 0
61- exit 1
62+ # Restore
63+ [ -n "$saved_xdg_runtime_dir" ] && \
64+ debug "Restoring XDG_RUNTIME_DIR to '$saved_xdg_runtime_dir'"
65+ export XDG_RUNTIME_DIR="$saved_xdg_runtime_dir"
66+
67+ [ -n "$saved_upstart_session" ] && \
68+ debug "Restoring UPSTART_SESSION to '$saved_upstart_session'"
69+ export UPSTART_SESSION="$saved_upstart_session"
70+
71+ if [ ! -z "$upstart_pid" ]
72+ then
73+ debug "Stopping secondary Upstart (running with PID $upstart_pid)"
74+ kill -9 "$upstart_pid" >/dev/null 2>&1
75+ fi
76+
77+ [ -d "$confdir" ] && rm -rf "$confdir"
78+ dir="$xdg_runtime_dir/upstart/sessions"
79+ [ -d "$dir" ] && rm -rf "$xdg_runtime_dir"
80+ [ "$file_valid" = y ] && exit 0
81+
82+ exit 1
83 }
84
85 usage()
86@@ -84,39 +88,36 @@
87
88 debug()
89 {
90- msg="$*"
91- [ $debug_enabled = y ] && echo "DEBUG: $msg"
92+ msg="$*"
93+ [ $debug_enabled = y ] && echo "DEBUG: $msg"
94 }
95
96 error()
97 {
98- msg="$*"
99- printf "ERROR: %s\n" "$msg" >&2
100+ msg="$*"
101+ printf "ERROR: %s\n" "$msg" >&2
102 }
103
104 die()
105 {
106- error "$*"
107- exit 1
108+ error "$*"
109+ exit 1
110 }
111
112-# Return 0 if Upstart is running on the D-Bus session bus, else 1.
113+# Return 0 if Upstart is running, else 1
114 upstart_running()
115 {
116- dbus-send --session --print-reply \
117- --dest='com.ubuntu.Upstart' /com/ubuntu/Upstart \
118- org.freedesktop.DBus.Properties.GetAll \
119- string:'com.ubuntu.Upstart0_6' >/dev/null 2>&1
120+ initctl --user version >/dev/null 2>&1
121 }
122
123 trap cleanup EXIT INT TERM
124
125 args=$(getopt \
126- -n "$script_name" \
127- -a \
128- --options="df:hi:sx:" \
129- --longoptions="debug file: help initctl-path: noscript upstart-path:" \
130- -- "$@")
131+ -n "$script_name" \
132+ -a \
133+ --options="df:hi:sx:" \
134+ --longoptions="debug file: help initctl-path: noscript upstart-path:" \
135+ -- "$@")
136
137 eval set -- "$args"
138 [ $? -ne 0 ] && { usage; exit 1; }
139@@ -162,23 +163,28 @@
140 done
141
142 [ -z "$file" ] && file="$1"
143-
144-# safety first
145-[ "$(id -u)" -eq 0 ] && die "cannot run as root"
146-
147-[ -z "$file" ] && die "must specify configuration file"
148-[ ! -f "$file" ] && die "file $file does not exist"
149+[ -z "$file" ] && die "Must specify configuration file"
150+[ ! -f "$file" ] && die "File $file does not exist"
151
152 debug "upstart_path=$upstart_path"
153 debug "initctl_path=$initctl_path"
154
155 for cmd in "$upstart_path" "$initctl_path"
156 do
157- [ -f "$cmd" ] || die "Path $cmd does not exist"
158- [ -x "$cmd" ] || die "File $cmd not executable"
159- "$cmd" --help | grep -q -- --session || die "version of $cmd too old"
160+ [ -f "$cmd" ] || die "Path $cmd does not exist"
161+ [ -x "$cmd" ] || die "File $cmd not executable"
162+ "$cmd" --help | grep -q -- --user || die "version of $cmd too old"
163 done
164
165+export saved_xdg_runtime_dir="$XDG_RUNTIME_DIR"
166+debug "Setting XDG_RUNTIME_DIR='$xdg_runtime_dir'"
167+export XDG_RUNTIME_DIR="$xdg_runtime_dir"
168+
169+export saved_upstart_session="$UPSTART_SESSION"
170+[ -n "$UPSTART_SESSION" ] \
171+ && debug "Unsetting UPSTART_SESSION ($UPSTART_SESSION)" \
172+ && unset UPSTART_SESSION
173+
174 # this is the only safe way to run another instance of Upstart
175 "$upstart_path" --help|grep -q -- --no-startup-event || die "$upstart_path too old"
176
177@@ -187,79 +193,81 @@
178
179 filename=$(basename $file)
180
181-echo "$filename" | egrep -q '\.conf$' || die "file must end in .conf"
182+echo "$filename" | egrep -q '\.conf$' || die "File must end in .conf"
183
184 job="${filename%.conf}"
185
186-cp "$file" "$confdir" || die "failed to copy file $file to $confdir"
187+cp "$file" "$confdir" || die "Failed to copy file $file to $confdir"
188 debug "job=$job"
189
190-upstart_running
191-[ $? -eq 0 ] && die "Another instance of this program is already running"
192-debug "ok - no other running instances detected"
193-
194 upstart_out="$(mktemp --tmpdir "${script_name}-upstart-output.XXXXXXXXXX")"
195 debug "upstart_out=$upstart_out"
196
197-# auto-start dbus if it isn't already running (required in non-desktop
198-# environments).
199-if [ -z "$DBUS_SESSION_BUS_ADDRESS" ]; then
200- [ -z "$(which $dbus_cmd)" ] && die "cannot find $dbus_cmd"
201- eval $($dbus_cmd --auto-syntax)
202- started_dbus=y
203- debug "started $dbus_cmd"
204-fi
205-
206 upstart_cmd=$(printf \
207- "%s --session --no-sessions --no-startup-event --verbose --confdir %s" \
208- "$upstart_path" \
209- "$confdir")
210+ "%s --user --no-dbus --no-sessions --no-startup-event --verbose --confdir %s" \
211+ "$upstart_path" \
212+ "$confdir")
213 debug "upstart_cmd=$upstart_cmd"
214
215 nohup $upstart_cmd >"$upstart_out" 2>&1 &
216 upstart_pid=$!
217+debug "Upstart pid=$upstart_pid"
218
219 # Stop the shell outputting a message when Upstart is killed.
220 # We handle this ourselves in cleanup().
221 disown
222
223-# wait for Upstart to initialize
224+# wait for Upstart to initialise
225 for i in $(seq 1 5)
226 do
227- debug "Waiting for Upstart to reply over D-Bus (attempt $i)"
228- upstart_running
229- if [ $? -eq 0 ]
230- then
231- running=y
232- break
233- fi
234- sleep 1
235+ sessions=$("$initctl_path" list-sessions)
236+
237+ if [ "$set_session" = n ] && [ -n "$sessions" ]
238+ then
239+ count=$(echo "$sessions"|wc -l)
240+ [ "$count" -gt 1 ] && die "Got unexpected session count: $count"
241+ session=$(echo "$sessions"|awk '{print $2}')
242+ debug "Joining Upstart session '$session'"
243+ export UPSTART_SESSION="$session"
244+ set_session=y
245+ fi
246+
247+ debug "Waiting for Upstart to initialise (attempt $i)"
248+
249+ upstart_running
250+ if [ $? -eq 0 ]
251+ then
252+ running=y
253+ break
254+ fi
255+
256+ sleep 1
257 done
258
259-[ $running = n ] && die "failed to ask Upstart to check conf file"
260+[ $running = n ] && die "Failed to ask Upstart to check conf file"
261
262 debug "Secondary Upstart ($upstart_cmd) running with PID $upstart_pid"
263
264 if [ "$check_scripts" = y ]
265 then
266- for section in pre-start post-start script pre-stop post-stop
267- do
268- if egrep -q "\<${section}\>" "$file"
269- then
270- cmd='sed -n "/^ *${section}/,/^ *end script/p" $file | /bin/sh -n 2>&1'
271- errors=$(eval "$cmd")
272- [ $? -ne 0 ] && \
273- die "$(printf "File $file: shell syntax invalid in $section section:\n${errors}")"
274- fi
275- done
276+ for section in pre-start post-start script pre-stop post-stop
277+ do
278+ if egrep -q "\<${section}\>" "$file"
279+ then
280+ cmd='sed -n "/^ *${section}/,/^ *end script/p" $file | /bin/sh -n 2>&1'
281+ errors=$(eval "$cmd")
282+ [ $? -ne 0 ] && \
283+ die "$(printf "File $file: shell syntax invalid in $section section:\n${errors}")"
284+ fi
285+ done
286 fi
287
288-"$initctl_path" --session list|grep -q "^${job}"
289+"$initctl_path" --user list|grep -q "^${job}"
290 if [ $? -eq 0 ]
291 then
292- file_valid=y
293- echo "File $file: syntax ok"
294- exit 0
295+ file_valid=y
296+ echo "File $file: syntax ok"
297+ exit 0
298 fi
299
300 errors=$(grep "$job" "$upstart_out"|sed "s,${confdir}/,,g")
301
302=== modified file 'scripts/man/init-checkconf.8'
303--- scripts/man/init-checkconf.8 2011-06-01 14:57:41 +0000
304+++ scripts/man/init-checkconf.8 2013-11-18 20:18:10 +0000
305@@ -1,4 +1,4 @@
306-.TH init\-checkconf 8 2011-04-06 "Upstart"
307+.TH init\-checkconf 8 2013-11-19 "Upstart"
308 .\"
309 .SH NAME
310 init\-checkconf \- manual page for init-checkconf
311@@ -52,17 +52,15 @@
312 .\"
313 .SH LIMITATIONS
314 .IP \(bu 4
315-This program will not run as the root user.
316-.IP \(bu 4
317-It is not possible for a user to run multiple simultaneous
318-instances of this program.
319+.B exec
320+stanzas containing shell meta-characters will not be checked as scripts.
321 .\"
322 .SH REPORTING BUGS
323 Report bugs at
324 .RB < https://launchpad.net/upstart/+bugs >
325 .\"
326 .SH COPYRIGHT
327-Copyright \(co 2011 Canonical Ltd.
328+Copyright \(co 2011-2013 Canonical Ltd.
329 .br
330 This is free software; see the source for copying conditions. There is NO
331 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Subscribers

People subscribed via source and target branches