[debhelper-devel] maintainer scripts and `exit 0`

[Niels Thykier]

Lukas Schwaighofer:
> Hi,
> 
> I looked at a postinst script earlier today and realized that an
> `exit 0` in maintainer scripts before the commands expanded from
> #DEBHELPER# is (almost always) wrong.  That is because the debhelper
> inserted code may still want to act, even though the maintainer's own
> code is done.
> 

Hi Lukas,

Thanks for bring this up. :)

CC'ing Ralf: As I recall, he is working on some analysis tool of
maintainer scripts and this seems like a good thing to catch.

> 
> A cursory search revealed a few examples where the resulting code is
> wrong, although the resulting problem is very minor.  The most common
> case was that the postinst aborts (succeeds) if not called with
> configure, e.g.
> 
>     [ "$1" = "configure" ] || exit 0
> 
> and later debhelper inserts something like
> 
>     if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ]; then
>     # (...)
> 

Indeed, this looks wrong.

> but the "abort-upgrade" part can never be reached.  Affected scripts I
> noticed are:
>  * cron.postinst
>  * ipsec-tools.postinst
>  * isc-dhcp-server.postinst
>  * man-db.postinst
> 
> There are other cases, e.g. where scripts succeed with `exit 0` if
> their user/group already exists (and thereby also skip the debhelper
> code).
> 

I have not verified them yet, but we should probably file bugs for them.
 But it is starting to smell like a "Mass Bug-Filing" so ideally we
should have a tool to find thing kind of bug.

> 
> I suspect (without much packaging experience), that any kind of
> successful termination (`exit 0`) inserted before #DEBHELPER# is a
> potential bug and should be avoided.
> 

Generally, yes.  There may be exceptions - like "triggered" you
mentioned yourself later) or rare cases where the maintainer
deliberately want to overrule debhelper's code for one case.

But the "triggered" case aside,  we can probably safely assume that an
exit before the #DEBHELPER# token is undesirable.

> This also applies to the "called with unknown argument" error which is
> still handled using `exit 0` by quite a few maintainer scripts.  (The
> default in the example script/template provided by the dh-make package
> has been `exit 1` since version 0.27 from 2001.)
> 

Indeed.  I am inclined to believe we should also remove the "exit 1"s
and scripts should ignore unknown parameters.  But perhaps we should
take that as a separate issue.

> I suspect the only exception where the `exit 0` may be justified is
> the "triggered" parameter of postinst.  But if we were to allow that,
> writing a lintian tag that can reliably detect this problem would
> become very hard.
> 

Indeed.

> 
> One way to avoid this problem altogether would be to have #DEBHELPER#
> at the start of the script by convention.  I do not know if this has
> unintended side-effects or if there are other reasons why that code is
> most often at the end of the script.
> 
> 
> Regards,
> Lukas
> 
> [...]

It solves that problem, but it is not generally applicable.  E.g. it
cannot be done if the postinst creates a user for a service (as the
#DEBHELPER# token will generally start the service and that would fail
because it cannot start as the user it is supposed to do).

Thanks,
~Niels