Bug#827930: apt: Make apt-daily leverage the ConditionACPower feature of systemd

2016-06-22 Thread Nicolas LE CAM 
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

2016-06-22 Thread Julian Andres Klode 
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

2016-06-22 Thread Julian Andres Klode 
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 Thread Nicolas Le Cam 
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

2016-06-22 Thread Nicolas Le Cam 
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-23 Thread Nicolas Le Cam 
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