[Merge] lp:~ubuntu-core-dev/ubuntu/utopic/sysvinit/unreviewed into lp:ubuntu/sysvinit

[Steve Langasek]

On Mon, May 19, 2014 at 09:06:40AM -0000, Dimitri John Ledkov wrote:
> @@ -1,3 +1,20 @@
> +sysvinit (2.88dsf-41ubuntu13) UNRELEASED; urgency=medium
> +
> +  * Revert previous upload, but keep UPSTARTDIR defined which was still
> +    used in the fallback codepath to specifically prevent configuration
> +    issues in chroots.

What is the reason for this revert, which seems to me to be unrelated to the
rest of the changes?  This seems like a lot of fuss for handling of upstart
jobs in a chroot, for which the right answer is almost always "install a
policy-rc.d that blocks all services from starting".  Do we have a consensus
on what the right behavior is here if the host upstart does not support
chroot sessions, and no policy-rc.d is in place?  Should invoke-rc.d error
out, or should it fall back to starting via the init script, which seem to
be the two options we're flipping between here?

> === modified file 'debian/control'
> --- debian/control	2013-05-17 06:03:10 +0000
> +++ debian/control	2014-05-19 09:06:30 +0000
> @@ -43,7 +43,8 @@
>  Depends:
>   ${misc:Depends},
>   sysvinit-utils (>= 2.86.ds1-62),
> - insserv (>> 1.12.0-10)
> + insserv (>> 1.12.0-10),
> + initscripts (>= ${binary:Version})
>  Breaks: initscripts (<< 2.86.ds1-63)
>  Multi-Arch: foreign
>  Description: System-V-like runlevel change mechanism

I don't understand this dependency addition.  Maybe this is meant to be
initscripts (>= 2.88dsf-41ubuntu13), rather than initscripts (>=
${binary:Version}) ?

I notice when diffing against initscripts 2.88dsf-41 that there are some
init scripts that have not been re-added:

-/etc/init.d/bootlogs
-/etc/init.d/motd
-/etc/init.d/mtab.sh
-/etc/init.d/rmnologin

What's different about these?

> === modified file 'debian/initscripts.postinst'
> --- debian/initscripts.postinst	2013-05-17 06:03:10 +0000
> +++ debian/initscripts.postinst	2014-05-19 09:06:30 +0000
> @@ -140,6 +140,39 @@
>  #
>  # Links in runlevel S
>  #
> +if [ -x /etc/init.d/mountkernfs.sh ]; then
> +update-rc.d mountkernfs.sh         start 02 S . >/dev/null || exit $?
> +fi

<snip>

Per
<https://bugs.launchpad.net/ubuntu/+source/ifupdown/+bug/1312836/comments/7>,
we don't want to reintroduce the calls to update-rc.d until /after/ insserv
has been re-enabled and fully vetted (i.e., step 4 - drop the debhelper
change that causes update-rc.d to be skipped... this also applies to
sysvinit, even though sysvinit doesn't use the debhelper implementation.)

Also, please note that this postinst still contains a delta to call
update-rc.d *remove* immediately above your restored calls, which ought to
be dropped at the same time as restoring the code to add the links.

And the code in debian/initscripts.preinst which deletes these restored
initscripts definitely wants to be removed.

> === added file 'debian/src/initscripts/etc/init.d/bootmisc.sh'
> --- debian/src/initscripts/etc/init.d/bootmisc.sh	1970-01-01 00:00:00 +0000
> +++ debian/src/initscripts/etc/init.d/bootmisc.sh	2014-05-19 09:06:30 +0000
<snip>
> +# NB! on ubuntu this file is usually not used at all, as it's
> +# redundant under upstart or systemd. This file is purely introduced
> +# to enable insserv / systemd init.d scripts activation
> +. /lib/lsb/init-functions

IMHO the benefit of this expository comment is outweighed by the detriment
of having the extra delta in these scripts; but that's just my opinion.

> === modified file 'debian/sysv-rc.postinst'
> --- debian/sysv-rc.postinst	2013-05-17 06:03:10 +0000
> +++ debian/sysv-rc.postinst	2014-05-19 09:06:30 +0000
> @@ -7,7 +7,7 @@
>  logfile="$logdir/run-$now.log"
>  
>  # Make sure insserv is in path
> -PATH=/sbin:$PATH
> +PATH=/sbin:/usr/lib/insserv/:$PATH

I don't understand why this is needed - is this a bug in Debian that's been
overlooked because the insserv activation has already been done there, so
this postinst code is no longer relevant in Debian?

>  # Based on code from dash postinst
>  check_divert() {
> @@ -41,7 +41,7 @@
>  }
>  
>  legacy_bootordering() {
> -    for f in /etc/rc0.d/S* ; do
> +    for f in /etc/rc0.d/S* /etc/rc6.d/S* ; do
>  	if [ -f $f ] ; then
>  	    return 0
>  	fi

This corresponds to the commit / changelog entry:

  adjust legacy_boot check to examine both rc0.d and rc6.d and the
  stamp file

But why is this needed?  What is your case where /etc/rc0.d/S* wouldn't
match any files, but /etc/rc6.d/S* would or /etc/init.d/.legacy-bootordering
would be present?

If there's not a specific reason for this construction, I think it would be
better to use the Debian implementation and further reduce our delta.

-- 
https://code.launchpad.net/~ubuntu-core-dev/ubuntu/utopic/sysvinit/unreviewed/+merge/219999
Your team Ubuntu Core Development Team is subscribed to branch lp:~ubuntu-core-dev/ubuntu/utopic/sysvinit/unreviewed.