[Pkg-sysvinit-devel] Bug#768450: Correct second version of the patch

[Martin Pitt]

Didier Roche [2014-11-07 16:38 +0100]:
> +  * invoke-rc.d: don't start disabled systemd services when invoke-rc.d
> +    is directly invoked. The job is then started unconditionally on
> +    invoke-rc.d [restart|start]. (Closes: #768450)
> +    Use a similar logic than for upstart:
> +    - invoke-rc.d start <unit> don't do anything on systemd if the service is
> +      disabled.
> +    - invoke-rc.d restart <unit> only restart a disabled service if the daemon
> +      was already running (forced by the admin).

Thanks for that! The patch looks logically correct to me.

> +                        systemctl --quiet is-enabled "${UNIT}"
> +                        ENABLED=$?
> +                        systemctl --quiet is-active "${UNIT}"
> +                        RUNNING=$?
> +
> +                        if [ $ENABLED != 0 ] && [ "$saction" = "start" ]; then
> +                            exit 0
> +                        fi
> +                        # If the job is disabled and is not currently
> +                        # running, the job is not restarted. However, if
> +                        # the job is disabled but has been forced into
> +                        # the running state, we *do* stop and restart it
> +                        # since this is expected behaviour
> +                        # for the admin who forced the start.
> +                        if [ $ENABLED != 0 ] && [ $RUNNING != 0 ] && [ "$saction" = "restart" ]; then
> +                            exit 0
> +                        fi

Maybe that's just my limited imagination, but I find this hard to read
("[ $ENABLED != 0 ]). I find this structure easier to read:

  # We never start disabled jobs; we only restart them if they are
  # already running (got started manually)
  if ! systemctl --quiet is-enabled "${UNIT}"; then
      if [ "$saction" = "start" ]; then
          exit 0
      elif [ "$saction" = "restart" ] && ! systemctl --quiet is-active "${UNIT}"; then
          exit 0
      fi
  fi

Anyway, I guess which one to take is for the sysvinit maintainers to
decide. :-)

Thanks!

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