diff mbox

paccache: add --age-atime and --age-mtime options

Message ID 20180914225826.4895-1-wisp3rwind@posteo.eu
State Superseded
Headers show

Commit Message

wisp3rwind Sept. 14, 2018, 10:58 p.m. UTC
---
 I would feel a lot more confident about using the paccache systemd
 service if it kept packages based on age instead of just the few most
 recent.

 This patch adds the functionality to skip candidates that are not older
 (in terms of atime or mtime) than some specified age. It seems to work,
 but I'm not exactly a bash expert, so please review with care. I'd
 appreciate if this could be merged!

 doc/paccache.8.txt |  5 ++++
 src/paccache.sh.in | 58 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

Comments

Johannes Löthberg Sept. 16, 2018, 9:22 p.m. UTC | #1
Hey,

Excerpts from wisp3rwind's message of September 15, 2018 0:58:
> ---
>  I would feel a lot more confident about using the paccache systemd
>  service if it kept packages based on age instead of just the few most
>  recent.
> 
>  This patch adds the functionality to skip candidates that are not older
>  (in terms of atime or mtime) than some specified age. It seems to work,
>  but I'm not exactly a bash expert, so please review with care. I'd
>  appreciate if this could be merged!
> 

So I would overall be okay with adding something like this, but there 
are some changes I would want to have made first.  First of all, is 
there any specific case where you would need both supported?  Because 
only having mtime support sounds like it should be good enough?

>  doc/paccache.8.txt |  5 ++++
>  src/paccache.sh.in | 58 +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/paccache.8.txt b/doc/paccache.8.txt
> index db81283..c9e3807 100644
> --- a/doc/paccache.8.txt
> +++ b/doc/paccache.8.txt
> @@ -38,6 +38,11 @@ Options
>  	Scan for packages for a specific architecture. Default is to scan for
>  	all architectures.
>  
> +*\--age-atime <age>*::
> +*\--age-mtime <age>*::
> +	Only consider packages for removal with atime respectively mtime older than
> +	specified. The age can be given as '10d', '1m', '1y', '1y1m' etc.
> +

I would much rather these be called min-.time, "age-mtime" feels rather 
non-obvious.

>  *-c, \--cachedir <dir>*::
>  	Specify a different cache directory. This option can be used more than once.
>  	Default is to use the cache directory configured in 'pacman.conf'.
> diff --git a/src/paccache.sh.in b/src/paccache.sh.in
> index 012ba9f..8ee5792 100644
> --- a/src/paccache.sh.in
> +++ b/src/paccache.sh.in
> @@ -27,6 +27,7 @@ declare -r myver='@PACKAGE_VERSION@'
>  
>  declare -a cachedirs=() candidates=() cmdopts=() whitelist=() blacklist=()
>  declare -i delete=0 dryrun=0 filecount=0 move=0 needsroot=0 totalsaved=0 verbose=0
> +declare -i age_atime=0 age_mtime=0
>  declare    delim=$'\n' keep=3 movedir= scanarch=
>  
>  QUIET=0
> @@ -40,6 +41,29 @@ die() {
>  	exit 1
>  }
>  
> +# Parses the age --age-atime and --age-mtime arguments
> +parse_age() {
> +	declare -i age=0
> +	if [[ $2 =~ ^[[:space:]]*([0-9]+[dmy][[:space:]]*)+$ ]]; then
> +		# Add spaces to facilitate splitting
> +		temp=${2//d/d }
> +		temp=${temp//m/m }
> +		temp=${temp//y/y }
> +		read -a temp <<< "${temp[*]}"
> +		for a in ${temp[@]}; do
> +			num=${a:0: -1}
> +			case ${a: -1} in
> +				d) age=$(( age + num )) ;;
> +				m) age=$(( age + num * 30 )) ;;
> +				y) age=$(( age + num * 365 )) ;;
> +			esac
> +		done
> +	else
> +		die "argument '%s' to option '%s' must be of the form '([0-9]+[dmy])+'" "$2" "$1"
> +	fi
> +	echo $(( age * 24 * 60 * 60 ))
> +}
> +
>  # reads a list of files on stdin and prints out deletion candidates
>  pkgfilter() {
>  	# there's whitelist and blacklist parameters passed to this
> @@ -174,6 +198,10 @@ Usage: ${myname} <operation> [options] [targets...]
>      -r, --remove          remove candidate packages.
>  
>    Options:
> +    --age-atime <age>
> +    --age-mtime <age>     keep packages with an atime/mtime that is not at least
> +                          <age> ago, where <age> is given as '10d', '1m', '1y',
> +                          '1y1m' etc.
>      -a, --arch <arch>     scan for "arch" (default: all architectures).
>      -c, --cachedir <dir>  scan "dir" for packages. can be used more than once.
>                            (default: read from @sysconfdir@/pacman.conf).
> @@ -200,7 +228,8 @@ version() {
>  
>  OPT_SHORT=':a:c:dfhi:k:m:qrsuVvz'
>  OPT_LONG=('arch:' 'cachedir:' 'dryrun' 'force' 'help' 'ignore:' 'keep:' 'move'
> -          'nocolor' 'quiet' 'remove' 'uninstalled' 'version' 'verbose' 'null')
> +          'nocolor' 'quiet' 'remove' 'uninstalled' 'version' 'verbose' 'null'
> +          'age-atime:' 'age-mtime:')
>  
>  if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
>  	exit 1
> @@ -210,6 +239,18 @@ unset OPT_SHORT OPT_LONG OPTRET
>  
>  while :; do
>  	case $1 in
> +		--age-atime)
> +			age_atime=$(parse_age "$1" "$2")
> +			if (( $? )); then
> +			  exit 1
> +			fi
> +			shift ;;
> +		--age-mtime)
> +			age_mtime=$(parse_age "$1" "$2")
> +			if (( $? )); then
> +			  exit 1
> +			fi
> +			shift ;;
>  		-a|--arch)
>  			scanarch=$2
>  			shift ;;
> @@ -319,6 +360,21 @@ for cachedir in "${cachedirs[@]}"; do
>  	popd &>/dev/null
>  done
>  
> +# remove any candidates that are not old enough yet
> +if (( $age_atime || $age_mtime )); then

Variables do not need to be dollar prefixed in arithmetic expansion.

> +  currtime=$(date +%s)
> +  for cand in "${candidates[@]}"; do
> +    IFS=';' read -d '' -a temp <<< $(stat --format '%X;%Y' "$cand")

There's no reason to have this be read into an array, something like 
this would be simpler:

  IFS=';' read atime mtime < <(stat --format '%X;%Y' "$cand")

> +    if (( ( $(( $currtime - ${temp[0]} )) > $age_atime ) && \
> +          ( $(( $currtime - ${temp[1]} )) > $age_mtime ) \

I think this should work?:

if (( currtime - atime > min_atime )) && \
   (( currtime - mtime > min_mtime ))

> +	  )); then
> +      candtemp+=("$cand")
> +    fi
> +  done
> +  candidates=("${candtemp[@]}")
> +  unset candtemp
> +fi
> +
>  if (( ! ${#candidates[*]} )); then
>  	msg 'no candidate packages found for pruning'
>  	exit 0
> -- 
> 2.19.0
>
Eli Schwartz Sept. 16, 2018, 10:25 p.m. UTC | #2
On 9/14/18 6:58 PM, wisp3rwind wrote:
> ---
>  I would feel a lot more confident about using the paccache systemd
>  service if it kept packages based on age instead of just the few most
>  recent.
> 
>  This patch adds the functionality to skip candidates that are not older
>  (in terms of atime or mtime) than some specified age. It seems to work,
>  but I'm not exactly a bash expert, so please review with care. I'd
>  appreciate if this could be merged!
> 
>  doc/paccache.8.txt |  5 ++++
>  src/paccache.sh.in | 58 +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/paccache.8.txt b/doc/paccache.8.txt
> index db81283..c9e3807 100644
> --- a/doc/paccache.8.txt
> +++ b/doc/paccache.8.txt
> @@ -38,6 +38,11 @@ Options
>  	Scan for packages for a specific architecture. Default is to scan for
>  	all architectures.
>  
> +*\--age-atime <age>*::
> +*\--age-mtime <age>*::
> +	Only consider packages for removal with atime respectively mtime older than
> +	specified. The age can be given as '10d', '1m', '1y', '1y1m' etc.
> +
>  *-c, \--cachedir <dir>*::
>  	Specify a different cache directory. This option can be used more than once.
>  	Default is to use the cache directory configured in 'pacman.conf'.
> diff --git a/src/paccache.sh.in b/src/paccache.sh.in
> index 012ba9f..8ee5792 100644
> --- a/src/paccache.sh.in
> +++ b/src/paccache.sh.in
> @@ -27,6 +27,7 @@ declare -r myver='@PACKAGE_VERSION@'
>  
>  declare -a cachedirs=() candidates=() cmdopts=() whitelist=() blacklist=()
>  declare -i delete=0 dryrun=0 filecount=0 move=0 needsroot=0 totalsaved=0 verbose=0
> +declare -i age_atime=0 age_mtime=0
>  declare    delim=$'\n' keep=3 movedir= scanarch=
>  
>  QUIET=0
> @@ -40,6 +41,29 @@ die() {
>  	exit 1
>  }
>  
> +# Parses the age --age-atime and --age-mtime arguments
> +parse_age() {
> +	declare -i age=0
> +	if [[ $2 =~ ^[[:space:]]*([0-9]+[dmy][[:space:]]*)+$ ]]; then
> +		# Add spaces to facilitate splitting
> +		temp=${2//d/d }
> +		temp=${temp//m/m }
> +		temp=${temp//y/y }
> +		read -a temp <<< "${temp[*]}"
> +		for a in ${temp[@]}; do
> +			num=${a:0: -1}
> +			case ${a: -1} in
> +				d) age=$(( age + num )) ;;
> +				m) age=$(( age + num * 30 )) ;;
> +				y) age=$(( age + num * 365 )) ;;
> +			esac
> +		done
> +	else
> +		die "argument '%s' to option '%s' must be of the form '([0-9]+[dmy])+'" "$2" "$1"
> +	fi
> +	echo $(( age * 24 * 60 * 60 ))
> +}

This seems extremely complex, and I can think of two alternatives that
would be a lot simpler. First, find -mtime 2, with a suitable glob
pattern, by replacing

printf '%s\n' "$PWD"/*.pkg.tar!(*.sig)

when calculating the "candidates" array.

Second:

compare_file="$(mktemp -t paccache_timestamp.XXXXXX)"
touch -d '2 days ago' "$compare_file"

and then instead of this:

> +# remove any candidates that are not old enough yet
> +if (( $age_atime || $age_mtime )); then
> +  currtime=$(date +%s)
> +  for cand in "${candidates[@]}"; do
> +    IFS=';' read -d '' -a temp <<< $(stat --format '%X;%Y' "$cand")
> +    if (( ( $(( $currtime - ${temp[0]} )) > $age_atime ) && \
> +          ( $(( $currtime - ${temp[1]} )) > $age_mtime ) \
> +	  )); then
> +      candtemp+=("$cand")
> +    fi
> +  done
> +  candidates=("${candtemp[@]}")
> +  unset candtemp
> +fi

for cand in "${candidates[@]}"; do
    if [[ $cand -nt $compare_file ]]; then
        candtemp+=("$cand")
    fi
done

The find command accepts a number of days, the touch command accepts a
fairly decent "natural language" description.
diff mbox

Patch

diff --git a/doc/paccache.8.txt b/doc/paccache.8.txt
index db81283..c9e3807 100644
--- a/doc/paccache.8.txt
+++ b/doc/paccache.8.txt
@@ -38,6 +38,11 @@  Options
 	Scan for packages for a specific architecture. Default is to scan for
 	all architectures.
 
+*\--age-atime <age>*::
+*\--age-mtime <age>*::
+	Only consider packages for removal with atime respectively mtime older than
+	specified. The age can be given as '10d', '1m', '1y', '1y1m' etc.
+
 *-c, \--cachedir <dir>*::
 	Specify a different cache directory. This option can be used more than once.
 	Default is to use the cache directory configured in 'pacman.conf'.
diff --git a/src/paccache.sh.in b/src/paccache.sh.in
index 012ba9f..8ee5792 100644
--- a/src/paccache.sh.in
+++ b/src/paccache.sh.in
@@ -27,6 +27,7 @@  declare -r myver='@PACKAGE_VERSION@'
 
 declare -a cachedirs=() candidates=() cmdopts=() whitelist=() blacklist=()
 declare -i delete=0 dryrun=0 filecount=0 move=0 needsroot=0 totalsaved=0 verbose=0
+declare -i age_atime=0 age_mtime=0
 declare    delim=$'\n' keep=3 movedir= scanarch=
 
 QUIET=0
@@ -40,6 +41,29 @@  die() {
 	exit 1
 }
 
+# Parses the age --age-atime and --age-mtime arguments
+parse_age() {
+	declare -i age=0
+	if [[ $2 =~ ^[[:space:]]*([0-9]+[dmy][[:space:]]*)+$ ]]; then
+		# Add spaces to facilitate splitting
+		temp=${2//d/d }
+		temp=${temp//m/m }
+		temp=${temp//y/y }
+		read -a temp <<< "${temp[*]}"
+		for a in ${temp[@]}; do
+			num=${a:0: -1}
+			case ${a: -1} in
+				d) age=$(( age + num )) ;;
+				m) age=$(( age + num * 30 )) ;;
+				y) age=$(( age + num * 365 )) ;;
+			esac
+		done
+	else
+		die "argument '%s' to option '%s' must be of the form '([0-9]+[dmy])+'" "$2" "$1"
+	fi
+	echo $(( age * 24 * 60 * 60 ))
+}
+
 # reads a list of files on stdin and prints out deletion candidates
 pkgfilter() {
 	# there's whitelist and blacklist parameters passed to this
@@ -174,6 +198,10 @@  Usage: ${myname} <operation> [options] [targets...]
     -r, --remove          remove candidate packages.
 
   Options:
+    --age-atime <age>
+    --age-mtime <age>     keep packages with an atime/mtime that is not at least
+                          <age> ago, where <age> is given as '10d', '1m', '1y',
+                          '1y1m' etc.
     -a, --arch <arch>     scan for "arch" (default: all architectures).
     -c, --cachedir <dir>  scan "dir" for packages. can be used more than once.
                           (default: read from @sysconfdir@/pacman.conf).
@@ -200,7 +228,8 @@  version() {
 
 OPT_SHORT=':a:c:dfhi:k:m:qrsuVvz'
 OPT_LONG=('arch:' 'cachedir:' 'dryrun' 'force' 'help' 'ignore:' 'keep:' 'move'
-          'nocolor' 'quiet' 'remove' 'uninstalled' 'version' 'verbose' 'null')
+          'nocolor' 'quiet' 'remove' 'uninstalled' 'version' 'verbose' 'null'
+          'age-atime:' 'age-mtime:')
 
 if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
 	exit 1
@@ -210,6 +239,18 @@  unset OPT_SHORT OPT_LONG OPTRET
 
 while :; do
 	case $1 in
+		--age-atime)
+			age_atime=$(parse_age "$1" "$2")
+			if (( $? )); then
+			  exit 1
+			fi
+			shift ;;
+		--age-mtime)
+			age_mtime=$(parse_age "$1" "$2")
+			if (( $? )); then
+			  exit 1
+			fi
+			shift ;;
 		-a|--arch)
 			scanarch=$2
 			shift ;;
@@ -319,6 +360,21 @@  for cachedir in "${cachedirs[@]}"; do
 	popd &>/dev/null
 done
 
+# remove any candidates that are not old enough yet
+if (( $age_atime || $age_mtime )); then
+  currtime=$(date +%s)
+  for cand in "${candidates[@]}"; do
+    IFS=';' read -d '' -a temp <<< $(stat --format '%X;%Y' "$cand")
+    if (( ( $(( $currtime - ${temp[0]} )) > $age_atime ) && \
+          ( $(( $currtime - ${temp[1]} )) > $age_mtime ) \
+	  )); then
+      candtemp+=("$cand")
+    fi
+  done
+  candidates=("${candtemp[@]}")
+  unset candtemp
+fi
+
 if (( ! ${#candidates[*]} )); then
 	msg 'no candidate packages found for pruning'
 	exit 0