2016-06-22 22:45 GMT+02:00 Julian Andres Klode <j...@debian.org>:
> 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 <niko.le...@gmail.com>
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 <niko.le...@gmail.com>
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-java5-jdk (>> 0), openjdk-6-jdk (<< 6b24-1.11-0ubuntu1~), bash-completion (<< 1:2.1-4.2+fakesync1), apt-utils (<< 1.3~exp2~)
Breaks: manpages-pl (<< 20060617-3~), manpages-it (<< 2.80-4~), sun-java6-jdk (>> 0), sun-java5-jdk (>> 0), openjdk-6-jdk (<< 6b24-1.11-0ubuntu1~), apt-utils (<< 1.3~exp2~)
Recommends: gnupg | gnupg2
-Suggests: aptitude | synaptic | wajig, dpkg-dev (>= 1.17.2), apt-doc, python-apt
+Suggests: aptitude | synaptic | wajig, dpkg-dev (>= 1.17.2), apt-doc, python-apt, powermgmt-base
Description: commandline package manager
This package provides commandline tools for searching and
managing as well as querying information about packages
--
2.8.1
