Code review comment for lp:~ubuntu-core-dev/ubuntu/utopic/sysvinit/unreviewed

Revision history for this message
Martin Pitt (pitti) wrote :

Steve Langasek [2014-05-20 1:12 -0000]:
> 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".

Please note that this isn't about actually starting services in a
chroot. Dimitri's intention was to replace the [ -e /etc/init/$JOB.conf ]
with "ask upstart if it knows about $JOB", and the first iteration was
"initctl status $JOB doesn't fail". That doesn't work due to what I
described in https://launchpad.net/ubuntu/+source/sysvinit/2.88dsf-41ubuntu12
but during our live builds we do have policy-rc.d, so we don't
actually start services.

So we either need a fallback like that (which isn't really correct
though), or maintain a list of possible job locations in invoke-rc.d
(like /lib/init/*.conf, /etc/upstart/overrides/*.conf, or whereever)

> 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?

I don't have a strong opinion about either way, but my gut feeling is
that it's more predictable to just not start anything at all if the
host upstart doesn't support chroot mode.

> > +# 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.

+1

Thanks,
Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

« Back to merge proposal