Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd
Package: apt Version: 1.2.13 Severity: normal Tags: patch Dear Maintainer, The rationnal for this is to be able to disable this "feature" without modifying package's script. My laptop has a pretty descent battery and I prefer to keep automatic updates rather than saving a hundreth percent of power. Actually this feature is hardcoded deep down in the script without any configuration possibility, by using systemd feature, I'll be able to disable this behaviour with a simple /etc/systemd/system/apt-daily.service.d/some.conf file. I've made the compatibility cron job functionally equivalent (without any configuration possibility unfortunately). The first patch is a nit-pick I saw while hacking apt scripts. Actually the apt.systemd.daily script (or apt.apt-compat.cron.daily with my patch) is using the on_ac_power utility without recommending powermgmt-base, not sure it's useful. regards, Nicolas -- Package-specific info: -- (no /etc/apt/preferences present) -- -- (no /etc/apt/sources.list present) -- -- System Information: Debian Release: stretch/sid APT prefers testing APT policy: (900, 'testing'), (50, 'unstable'), (1, 'experimental') Architecture: amd64 (x86_64) Kernel: Linux 4.6.0-1-amd64 (SMP w/4 CPU cores) Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) Versions of packages apt depends on: ii adduser 3.114 ii debian-archive-keyring 2014.3 ii gnupg 1.4.20-6 ii gnupg2 2.1.11-7 ii gpgv1.4.20-6 ii init-system-helpers 1.34 ii libapt-pkg5.0 1.2.13 ii libc6 2.22-11 ii libgcc1 1:6.1.1-4 ii libstdc++6 6.1.1-4 apt recommends no packages. Versions of packages apt suggests: pn apt-doc ii aptitude0.7.5-3 pn dpkg-dev pn python-apt -- no debconf information >From c9ac5bf1d53d7a346b1532a74ebcdbeb476ee1ce Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 22 Jun 2016 21:39:38 +0200 Subject: Leverage the ConditionACPower feature of systemd instead of hardcoding it in the apt.systemd.daily script. Also make the compatibility cron job provide the same functionnality for systems that do not use systemd. --- debian/apt-daily.service | 1 + debian/apt.apt-compat.cron.daily | 24 debian/apt.systemd.daily | 26 -- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/debian/apt-daily.service b/debian/apt-daily.service index 941263d..904ed5d 100644 --- a/debian/apt-daily.service +++ b/debian/apt-daily.service @@ -1,6 +1,7 @@ [Unit] Description=Daily apt activities Documentation=man:apt(8) +ConditionACPower=true [Service] Type=oneshot diff --git a/debian/apt.apt-compat.cron.daily b/debian/apt.apt-compat.cron.daily index 1ea8430..e41fecb 100644 --- a/debian/apt.apt-compat.cron.daily +++ b/debian/apt.apt-compat.cron.daily @@ -11,6 +11,27 @@ if [ -d /run/systemd/system ]; then exit 0 fi +check_power() +{ +# laptop check, on_ac_power returns: +# 0 (true)System is on main power +# 1 (false) System is not on main power +# 255 (false) Power status could not be determined +# Desktop systems always return 255 it seems +if which on_ac_power >/dev/null 2>&1; then +on_ac_power +POWER=$? +if [ $POWER -eq 1 ]; then +debug_echo "exit: system NOT on main power" +return 1 +elif [ $POWER -ne 0 ]; then +debug_echo "power status ($POWER) undetermined, continuing" +fi +debug_echo "system is on main power." +fi +return 0 +} + # sleep for a random interval of time (default 30min) # (some code taken from cron-apt, thanks) random_sleep() @@ -28,6 +49,9 @@ random_sleep() sleep $TIME } +# ensure we don't do this on battery +check_power || exit 0 + # run daily job random_sleep exec /usr/lib/apt/apt.systemd.daily diff --git a/debian/apt.systemd.daily b/debian/apt.systemd.daily index 15024c8..d034d8c 100644 --- a/debian/apt.systemd.daily +++ b/debian/apt.systemd.daily @@ -290,27 +290,6 @@ debug_echo() fi } -check_power() -{ -# laptop check, on_ac_power returns: -# 0 (true)System is on main power -# 1 (false) System is not on main power -# 255 (false) Power status could not be determined -# Desktop systems always return 255 it seems -if which on_ac_power >/dev/null 2>&1; then -on_ac_power -POWER=$? -if [ $POWER -eq 1 ]; then - debug_echo "exit: system NOT on main power" - return 1 -elif [ $POWER -ne 0 ]; then - debug_echo "power status ($POWER) undetermined, continuing" -fi -debug_echo "system is on main power." -fi -return 0 -} - # main if test -r /var/lib
Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd
On Wed, Jun 22, 2016 at 10:27:19PM +0200, Nicolas LE CAM wrote: > Package: apt > Version: 1.2.13 > Severity: normal > Tags: patch > > Dear Maintainer, > > The rationnal for this is to be able to disable this "feature" without > modifying package's script. My laptop has a pretty descent battery and > I prefer to keep automatic updates rather than saving a hundreth > percent of power. Actually this feature is hardcoded deep down in the > script without any configuration possibility, by using systemd > feature, I'll be able to disable this behaviour with a simple > /etc/systemd/system/apt-daily.service.d/some.conf file. > > I've made the compatibility cron job functionally equivalent (without > any configuration possibility unfortunately). The commit short description is a bit too long, can you shorten that to about 70 characters? I'm not sure why the cron job does the check 2 times, I'll have to check that. Apart from that: lgtm. > > The first patch is a nit-pick I saw while hacking apt scripts. > Actually the apt.systemd.daily script (or apt.apt-compat.cron.daily > with my patch) is using the on_ac_power utility without recommending > powermgmt-base, not sure it's useful. Maybe a Suggests instead, especially once we applied 2. (better switch those around then) -- Debian Developer - deb.li/jak | jak-linux.org - free software dev When replying, only quote what is necessary, and write each reply directly below the part(s) it pertains to (`inline'). Thank you.
Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd
On Wed, Jun 22, 2016 at 10:45:52PM +0200, Julian Andres Klode wrote: > On Wed, Jun 22, 2016 at 10:27:19PM +0200, Nicolas LE CAM wrote: > > Package: apt > > Version: 1.2.13 > > Severity: normal > > Tags: patch > > > > Dear Maintainer, > > > > The rationnal for this is to be able to disable this "feature" without > > modifying package's script. My laptop has a pretty descent battery and > > I prefer to keep automatic updates rather than saving a hundreth > > percent of power. Actually this feature is hardcoded deep down in the > > script without any configuration possibility, by using systemd > > feature, I'll be able to disable this behaviour with a simple > > /etc/systemd/system/apt-daily.service.d/some.conf file. > > > > I've made the compatibility cron job functionally equivalent (without > > any configuration possibility unfortunately). > > The commit short description is a bit too long, can you shorten that > to about 70 characters? Same for the second paragraph, that should be wrapped. -- Debian Developer - deb.li/jak | jak-linux.org - free software dev When replying, only quote what is necessary, and write each reply directly below the part(s) it pertains to (`inline'). Thank you.
Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd
2016-06-22 22:45 GMT+02:00 Julian Andres Klode : > The commit short description is a bit too long, can you shorten that > to about 70 characters? Sure done (for the second paragraph too). > I'm not sure why the cron job does the check 2 times, I'll have > to check that. > Historically, first time to fast exit, second time to check power after the random wait (see commit e20d3bcf). The random wait was moved to the timer unit (and the compatibility cron job) but the two checks stayed. > Maybe a Suggests instead, especially once we applied 2. (better > switch those around then) done. Thanks for the review regards, Nicolas From 2c86bf71cd9d8a184df4667409ee688ba020be52 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 22 Jun 2016 21:39:38 +0200 Subject: Use the ConditionACPower feature of systemd in the apt-daily service .. instead of hardcoding the functionnality in the apt.systemd.daily script. Also make the compatibility cron job provide the same functionnality for systems that do not use systemd. --- debian/apt-daily.service | 1 + debian/apt.apt-compat.cron.daily | 24 debian/apt.systemd.daily | 26 -- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/debian/apt-daily.service b/debian/apt-daily.service index 941263d..904ed5d 100644 --- a/debian/apt-daily.service +++ b/debian/apt-daily.service @@ -1,6 +1,7 @@ [Unit] Description=Daily apt activities Documentation=man:apt(8) +ConditionACPower=true [Service] Type=oneshot diff --git a/debian/apt.apt-compat.cron.daily b/debian/apt.apt-compat.cron.daily index 1ea8430..e41fecb 100644 --- a/debian/apt.apt-compat.cron.daily +++ b/debian/apt.apt-compat.cron.daily @@ -11,6 +11,27 @@ if [ -d /run/systemd/system ]; then exit 0 fi +check_power() +{ +# laptop check, on_ac_power returns: +# 0 (true)System is on main power +# 1 (false) System is not on main power +# 255 (false) Power status could not be determined +# Desktop systems always return 255 it seems +if which on_ac_power >/dev/null 2>&1; then +on_ac_power +POWER=$? +if [ $POWER -eq 1 ]; then +debug_echo "exit: system NOT on main power" +return 1 +elif [ $POWER -ne 0 ]; then +debug_echo "power status ($POWER) undetermined, continuing" +fi +debug_echo "system is on main power." +fi +return 0 +} + # sleep for a random interval of time (default 30min) # (some code taken from cron-apt, thanks) random_sleep() @@ -28,6 +49,9 @@ random_sleep() sleep $TIME } +# ensure we don't do this on battery +check_power || exit 0 + # run daily job random_sleep exec /usr/lib/apt/apt.systemd.daily diff --git a/debian/apt.systemd.daily b/debian/apt.systemd.daily index 15024c8..d034d8c 100644 --- a/debian/apt.systemd.daily +++ b/debian/apt.systemd.daily @@ -290,27 +290,6 @@ debug_echo() fi } -check_power() -{ -# laptop check, on_ac_power returns: -# 0 (true)System is on main power -# 1 (false) System is not on main power -# 255 (false) Power status could not be determined -# Desktop systems always return 255 it seems -if which on_ac_power >/dev/null 2>&1; then -on_ac_power -POWER=$? -if [ $POWER -eq 1 ]; then - debug_echo "exit: system NOT on main power" - return 1 -elif [ $POWER -ne 0 ]; then - debug_echo "power status ($POWER) undetermined, continuing" -fi -debug_echo "system is on main power." -fi -return 0 -} - # main if test -r /var/lib/apt/extended_states; then @@ -358,8 +337,6 @@ if [ "$VERBOSE" -ge 3 ]; then set -x fi -check_power || exit 0 - # check if we can lock the cache and if the cache is clean if which apt-get >/dev/null 2>&1 && ! eval apt-get check $XAPTOPT $XSTDERR ; then debug_echo "error encountered in cron job with \"apt-get check\"." @@ -410,9 +387,6 @@ fi # deal with BackupArchiveInterval do_cache_backup $BackupArchiveInterval -# ensure we don't do this on battery -check_power || exit 0 - # include default system language so that "apt-get update" will # fetch the right translated package descriptions if [ -r /etc/default/locale ]; then -- 2.8.1 From 26fe38d01ed782473b974bc654c8ebeb81d7fef6 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 22 Jun 2016 20:55:18 +0200 Subject: Add a apt suggests powermgmt-base debian/apt.apt-compat.cron.daily is using on_ac_power utility --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index f5c5a75..15eb607 100644 --- a/debian/control +++ b/debian/control @@ -22,7 +22,7 @@ Depends: ${shlibs:Depends}, ${misc:Depends}, ${apt:keyring}, gpgv | gpgv2, addus Replaces: manpages-pl (<< 20060617-3~), manpages-it (<< 2.80-4~), sun-java6-jdk (>> 0), sun-j
Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd
Sorry, First patch was flawed, I've moved the check_power() function without seeing it was using debug_echo() internally. Fixed now. I've followed what was done when moving random_sleep() from the script to the cron job, i.e. remove calls to debug_echo(), see 14669d4b. Second patch has not been touched. regards, Nicolas From 2a7e2b1552f15f72a0bb1b8992227bb1b5e44355 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 22 Jun 2016 21:39:38 +0200 Subject: Use the ConditionACPower feature of systemd in the apt-daily service .. instead of hardcoding the functionnality in the apt.systemd.daily script. Also make the compatibility cron job provide the same functionnality for systems that do not use systemd. --- debian/apt-daily.service | 1 + debian/apt.apt-compat.cron.daily | 20 debian/apt.systemd.daily | 26 -- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/debian/apt-daily.service b/debian/apt-daily.service index 941263d..904ed5d 100644 --- a/debian/apt-daily.service +++ b/debian/apt-daily.service @@ -1,6 +1,7 @@ [Unit] Description=Daily apt activities Documentation=man:apt(8) +ConditionACPower=true [Service] Type=oneshot diff --git a/debian/apt.apt-compat.cron.daily b/debian/apt.apt-compat.cron.daily index 1ea8430..e7d76d2 100644 --- a/debian/apt.apt-compat.cron.daily +++ b/debian/apt.apt-compat.cron.daily @@ -11,6 +11,23 @@ if [ -d /run/systemd/system ]; then exit 0 fi +check_power() +{ +# laptop check, on_ac_power returns: +# 0 (true)System is on main power +# 1 (false) System is not on main power +# 255 (false) Power status could not be determined +# Desktop systems always return 255 it seems +if which on_ac_power >/dev/null 2>&1; then +on_ac_power +POWER=$? +if [ $POWER -eq 1 ]; then +return 1 +fi +fi +return 0 +} + # sleep for a random interval of time (default 30min) # (some code taken from cron-apt, thanks) random_sleep() @@ -28,6 +45,9 @@ random_sleep() sleep $TIME } +# ensure we don't do this on battery +check_power || exit 0 + # run daily job random_sleep exec /usr/lib/apt/apt.systemd.daily diff --git a/debian/apt.systemd.daily b/debian/apt.systemd.daily index 15024c8..d034d8c 100644 --- a/debian/apt.systemd.daily +++ b/debian/apt.systemd.daily @@ -290,27 +290,6 @@ debug_echo() fi } -check_power() -{ -# laptop check, on_ac_power returns: -# 0 (true)System is on main power -# 1 (false) System is not on main power -# 255 (false) Power status could not be determined -# Desktop systems always return 255 it seems -if which on_ac_power >/dev/null 2>&1; then -on_ac_power -POWER=$? -if [ $POWER -eq 1 ]; then - debug_echo "exit: system NOT on main power" - return 1 -elif [ $POWER -ne 0 ]; then - debug_echo "power status ($POWER) undetermined, continuing" -fi -debug_echo "system is on main power." -fi -return 0 -} - # main if test -r /var/lib/apt/extended_states; then @@ -358,8 +337,6 @@ if [ "$VERBOSE" -ge 3 ]; then set -x fi -check_power || exit 0 - # check if we can lock the cache and if the cache is clean if which apt-get >/dev/null 2>&1 && ! eval apt-get check $XAPTOPT $XSTDERR ; then debug_echo "error encountered in cron job with \"apt-get check\"." @@ -410,9 +387,6 @@ fi # deal with BackupArchiveInterval do_cache_backup $BackupArchiveInterval -# ensure we don't do this on battery -check_power || exit 0 - # include default system language so that "apt-get update" will # fetch the right translated package descriptions if [ -r /etc/default/locale ]; then -- 2.8.1
Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd
2016-06-22 23:40 GMT+02:00 Nicolas Le Cam : > Historically, first time to fast exit, second time to check power > after the random wait (see commit e20d3bcf). > The random wait was moved to the timer unit (and the compatibility > cron job) but the two checks stayed. It did take some time to figure out I was explaining something I didn't reproduce in the compatibility job. Fixed now. Except I didn't reproduce the fast exit. sleep() should not consume much battery and the user could plug its laptop in the meantime. Second patch didn't change. Sorry for the noise. regards, Nicolas From 4dab1ed3d3a4ad430134e1d49fb3b0ede9b0a8d1 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 22 Jun 2016 21:39:38 +0200 Subject: Use the ConditionACPower feature of systemd in the apt-daily service .. instead of hardcoding the functionnality in the apt.systemd.daily script. Also make the compatibility cron job provide the same functionnality for systems that do not use systemd. --- debian/apt-daily.service | 1 + debian/apt.apt-compat.cron.daily | 24 +++- debian/apt.systemd.daily | 26 -- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/debian/apt-daily.service b/debian/apt-daily.service index 941263d..904ed5d 100644 --- a/debian/apt-daily.service +++ b/debian/apt-daily.service @@ -1,6 +1,7 @@ [Unit] Description=Daily apt activities Documentation=man:apt(8) +ConditionACPower=true [Service] Type=oneshot diff --git a/debian/apt.apt-compat.cron.daily b/debian/apt.apt-compat.cron.daily index 1ea8430..095a44c 100644 --- a/debian/apt.apt-compat.cron.daily +++ b/debian/apt.apt-compat.cron.daily @@ -11,6 +11,23 @@ if [ -d /run/systemd/system ]; then exit 0 fi +check_power() +{ +# laptop check, on_ac_power returns: +# 0 (true)System is on main power +# 1 (false) System is not on main power +# 255 (false) Power status could not be determined +# Desktop systems always return 255 it seems +if which on_ac_power >/dev/null 2>&1; then +on_ac_power +POWER=$? +if [ $POWER -eq 1 ]; then +return 1 +fi +fi +return 0 +} + # sleep for a random interval of time (default 30min) # (some code taken from cron-apt, thanks) random_sleep() @@ -28,6 +45,11 @@ random_sleep() sleep $TIME } -# run daily job +# delay the job execution by a random amount of time random_sleep + +# ensure we don't do this on battery +check_power || exit 0 + +# run daily job exec /usr/lib/apt/apt.systemd.daily diff --git a/debian/apt.systemd.daily b/debian/apt.systemd.daily index 15024c8..d034d8c 100644 --- a/debian/apt.systemd.daily +++ b/debian/apt.systemd.daily @@ -290,27 +290,6 @@ debug_echo() fi } -check_power() -{ -# laptop check, on_ac_power returns: -# 0 (true)System is on main power -# 1 (false) System is not on main power -# 255 (false) Power status could not be determined -# Desktop systems always return 255 it seems -if which on_ac_power >/dev/null 2>&1; then -on_ac_power -POWER=$? -if [ $POWER -eq 1 ]; then - debug_echo "exit: system NOT on main power" - return 1 -elif [ $POWER -ne 0 ]; then - debug_echo "power status ($POWER) undetermined, continuing" -fi -debug_echo "system is on main power." -fi -return 0 -} - # main if test -r /var/lib/apt/extended_states; then @@ -358,8 +337,6 @@ if [ "$VERBOSE" -ge 3 ]; then set -x fi -check_power || exit 0 - # check if we can lock the cache and if the cache is clean if which apt-get >/dev/null 2>&1 && ! eval apt-get check $XAPTOPT $XSTDERR ; then debug_echo "error encountered in cron job with \"apt-get check\"." @@ -410,9 +387,6 @@ fi # deal with BackupArchiveInterval do_cache_backup $BackupArchiveInterval -# ensure we don't do this on battery -check_power || exit 0 - # include default system language so that "apt-get update" will # fetch the right translated package descriptions if [ -r /etc/default/locale ]; then -- 2.8.1