[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#934231: anacron: please provide a runscript for runit



[2019-09-04 19:30] Lorenzo Puliti <lorenzo.ru.g@gmail.com>
> Package: anacron
> Version: 2.3-29
> Followup-For: Bug #934231
>
> Hi,

Hi,

> I have updated the patches and the MR:
> First patch adds Gtilab CI test and include a test for the runscript
>
> https://salsa.debian.org/Lorenzo.ru.g-guest/anacron/pipelines/69420
>
> The second one, compared to the previuos version, adds the following:
>  * bump dh-runit version to 2.8.14 in Build-depends
>  * add presubj option for dh-runit
>  * slightly reword the echo messages in run and finish files

I see all test jobs pass, not just `daemons'. Great!

>  create mode 100644 debian/anacron.runscript/finish

There is nothing in this script specific to anacron. I propose
following:

 * add this finish script into bin:runit as /lib/runit/finish-default or
   something like this.

 * document error codes (161, 160, -1, etc) in invoke-run manpage.

 * modify dh_runit to generate finish script with following content:

   #!/bin/sh
   /lib/runit/finish-default

   Not sure should it be default or yet-another option.

This is more work initially, but will make improvements to finish script
transparent (just upgrade bin:runit), should need appear and avoid
copy-paste work for future runscripts.

FWIW, every time I thought "there will be no need to change <foo>", I
was wrong.

> diff --git a/debian/anacron.runscript/run b/debian/anacron.runscript/run
> new file mode 100644
> index 0000000..dcfb16d
> --- /dev/null
> +++ b/debian/anacron.runscript/run
> @@ -0,0 +1,21 @@
> +#!/usr/bin/env /lib/runit/invoke-run
> +set -e
> +
> +NAME=Anacron
> +
> +exec 2>&1
> +
> +# exit status of on_ac_power:
> +# 0= on-ac // 1= on-battery // 255=unknown, likely a desktop witout APM
> +if [ x"$ANACRON_RUN_ON_BATTERY_POWER" != x"yes" -a -x /usr/bin/on_ac_power ]; then

$ man test
advices aganist use of `-a` option:

NOTE:  Binary  -a  and -o are inherently ambiguous.  Use 'test EXPR1 &&
       test EXPR2' or 'test EXPR1 || test EXPR2' instead.

> +    /usr/bin/on_ac_power || retval=$? 

Minor style issue. Trailing whitespace. You may want to enable default
git pre-commit hook, which catches such issues.

> +    if [ x"$retval" = x1 ]; then
> +        echo "deferred while on battery power" && exit 161
> +    fi
> +fi
> +
> +# don't restart anacron when it's done

Why? Will it work correctly on machine that is up for 24/7?

> index 9a38b23..fc5e3ed 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -3,6 +3,7 @@ Section: admin
>  Priority: optional
>  Build-Depends:
>   debhelper-compat (= 12),
> + dh-runit(>=2.8.14),

Minor style issue. Please keep spacing consistent, like

	foo (>= 1.2.3-4~5)

-- 
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.


Reply to: