[pacman-dev] [PATCH v3] paccache: adding the possibility for multiple cachedirs

[Dave Reisner]

On Tue, Aug 26, 2014 at 10:59:59AM +0200, Maxim Andersson wrote:
> Either by specifing several -c parameters or only
> one -c parameter with a comma separated value.
> One could for example be for the official arch packages
> and one for the aur.
> 
> Signed-off-by: Maxim Andersson <thesilentboatman at gmail.com>
> ---
>  contrib/paccache.sh.in | 90 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 53 insertions(+), 37 deletions(-)
> 
> diff --git a/contrib/paccache.sh.in b/contrib/paccache.sh.in
> index 039ac8a..6d0a21b 100644
> --- a/contrib/paccache.sh.in
> +++ b/contrib/paccache.sh.in
> @@ -23,9 +23,9 @@ shopt -s extglob
>  declare -r myname='paccache'
>  declare -r myver='@PACKAGE_VERSION@'
>  
> -declare -a candidates=() cmdopts=() whitelist=() blacklist=()
> +declare -a cachedirs=() candidates=() cmdopts=() whitelist=() blacklist=()
>  declare -i delete=0 dryrun=0 filecount=0 move=0 needsroot=0 totalsaved=0 verbose=0
> -declare    cachedir=@localstatedir@/cache/pacman/pkg delim=$'\n' keep=3 movedir= scanarch=
> +declare    delim=$'\n' keep=3 movedir= scanarch=
>  
>  USE_COLOR='y'
>  
> @@ -133,13 +133,13 @@ summarize() {
>  					[[ $pkg =~ $pkg_re ]] && name=${BASH_REMATCH[1]} arch=${BASH_REMATCH[2]}
>  					if [[ -z $seen || $seenarch != "$arch" || $seen != "$name" ]]; then
>  						seen=$name seenarch=$arch
> -						printf '%s (%s):\n' "$name" "$arch"
> +						printf '%s (%s):\n' "$(basename $name)" "$arch"

Please don't use basename(1) when bash can do this for us. Your quoting
is wrong, too (you'd need "$(basename "$name")"). What you meant to
write was:

  printf '%s (%s):\n' "${name##*/}" "$arch"

>  					fi
> -					printf '  %s\n' "$pkg"
> +					printf '  %s\n' "$(basename $pkg)"

Same nag as above:

  printf '  %s\n' "${pkg##*/}"


>  				elif (( verbose >= 2 )); then
> -					printf "$PWD/%s$delim" "$pkg"
> -				else
>  					printf "%s$delim" "$pkg"
> +				else
> +					printf "%s$delim" "$(basename $pkg)"

Same nag as above:

  printf "%s$delim" "${pkg##*/}"

>  				fi
>  			done < <(printf '%s\n' "$@" | pacsort --files)
>  		fi
> @@ -159,25 +159,25 @@ A flexible pacman cache cleaning utility.
>  Usage: ${myname} <operation> [options] [targets...]
>  
>    Operations:
> -    -d, --dryrun          perform a dry run, only finding candidate packages.
> -    -m, --move <dir>      move candidate packages to "dir".
> -    -r, --remove          remove candidate packages.
> +    -d, --dryrun           perform a dry run, only finding candidate packages.
> +    -m, --move <dir>       move candidate packages to "dir".
> +    -r, --remove           remove candidate packages.
>  
>    Options:
> -    -a, --arch <arch>     scan for "arch" (default: all architectures).
> -    -c, --cachedir <dir>  scan "dir" for packages.
> -                          (default: @localstatedir@/cache/pacman/pkg).
> -    -f, --force           apply force to mv(1) and rm(1) operations.
> -    -h, --help            display this help message and exit.
> -    -i, --ignore <pkgs>   ignore "pkgs", comma-separated. Alternatively, specify
> -                          "-" to read package names from stdin, newline-
> -                          delimited.
> -    -k, --keep <num>      keep "num" of each package in the cache (default: 3).
> -    --nocolor             remove color from output.
> -    -u, --uninstalled     target uninstalled packages.
> -    -v, --verbose         increase verbosity. specify up to 3 times.
> -    -z, --null            use null delimiters for candidate names (only with -v
> -                          and -vv).
> +    -a, --arch <arch>      scan for "arch" (default: all architectures).
> +    -c, --cachedir <dirs>  scan "dirs" for packages, comma-separated.
> +                           (default: @localstatedir@/cache/pacman/pkg).
> +    -f, --force            apply force to mv(1) and rm(1) operations.
> +    -h, --help             display this help message and exit.
> +    -i, --ignore <pkgs>    ignore "pkgs", comma-separated. Alternatively,
> +                           pecify "-" to read package names from stdin, newline-
> +                           delimited.
> +    -k, --keep <num>       keep "num" of each package in the cache (default: 3).
> +    --nocolor              remove color from output.
> +    -u, --uninstalled      target uninstalled packages.
> +    -v, --verbose          increase verbosity. specify up to 3 times.
> +    -z, --null             use null delimiters for candidate names (only with -v
> +                           and -vv).
>  
>  EOF
>  }
> @@ -203,7 +203,9 @@ while :; do
>  			scanarch=$2
>  			shift ;;
>  		-c|--cachedir)
> -			cachedir=$2
> +			IFS=',' read -r -a dirs <<< "$2"
> +			cachedirs+=("${dirs[@]}")
> +			unset dirs
>  			shift ;;
>  		-d|--dryrun)
>  			dryrun=1 ;;
> @@ -256,6 +258,9 @@ done
>  
>  m4_include(../scripts/library/term_colors.sh)
>  
> +# setting default cachedir
> +cachedirs=("${cachedirs[@]:- at localstatedir@/cache/pacman/pkg}")
> +
>  # remaining args are a whitelist
>  whitelist=("$@")
>  
> @@ -265,28 +270,39 @@ case $(( dryrun+delete+move )) in
>  	[^1]) die "only one operation may be used at a time" ;;
>  esac
>  
> -[[ -d $cachedir ]] ||
> -	die "cachedir '%s' does not exist or is not a directory" "$cachedir"
> -
>  [[ $movedir && ! -d $movedir ]] &&
>  	die "destination directory '%s' does not exist or is not a directory" "$movedir"
>  
>  if (( move || delete )); then
>  	# make it an absolute path since we're about to chdir
>  	[[ ${movedir:0:1} != '/' ]] && movedir=$PWD/$movedir
> -	[[ ! -w $cachedir || ( $movedir && ! -w $movedir ) ]] && needsroot=1
> +	[[ $movedir && ! -w $movedir ]] && needsroot=1
>  fi
>  
> -# unlikely that this will fail, but better make sure
> -cd "$cachedir" >/dev/null || die "failed to chdir to '%s'" "$cachedir"
> +for cachedir in "${cachedirs[@]}"; do
> +	[[ -d $cachedir ]] ||
> +		die "cachedir '%s' does not exist or is not a directory" "$cachedir"
> +
> +	if (( move || delete )); then
> +		[[ ! -w $cachedir  ]] && needsroot=1

There's an extra space here by the closing ]].

> +	fi
> +
> +	# unlikely that this will fail, but better make sure
> +	pushd "$cachedir" &>/dev/null || die "failed to chdir to '%s'" "$cachedir"
> +
> +	# note that these results are returned in an arbitrary order from awk, but
> +	# they'll be resorted (in summarize) iff we have a verbosity level set.
> +	IFS=$'\n' read -r -d '' -a cand < \
> +		<(printf $PWD'/%s\n' *.pkg.tar?(.+([^.])) | pacsort --files |

Again with the lack of quoting...

  <(printf "$PWD/%s\n" *.pkg.tar?(.+([^.])) | pacsort --files |

I'm somewhat concerned about this because we're injecting a path into
the format string (yes, we've done this elsewhere and it's equally
concerning). Since it's pre-existing, we can probably ignore this bug
for now. Could you leave a TODO to address it, though? It's probably
sufficient to just escape % and \ chars in $PWD, but it needs testing.

> +			pkgfilter "$keep" "$scanarch" \
> +				"${#whitelist[*]}" "${whitelist[@]}" \
> +				"${#blacklist[*]}" "${blacklist[@]}")
>  
> -# note that these results are returned in an arbitrary order from awk, but
> -# they'll be resorted (in summarize) iff we have a verbosity level set.
> -IFS=$'\n' read -r -d '' -a candidates < \
> -	<(printf '%s\n' *.pkg.tar?(.+([^.])) | pacsort --files |
> -		pkgfilter "$keep" "$scanarch" \
> -			"${#whitelist[*]}" "${whitelist[@]}" \
> -			"${#blacklist[*]}" "${blacklist[@]}")
> +	candidates+=("${cand[@]}")
> +	unset cand
> +
> +	popd &>/dev/null
> +done
>  
>  if (( ! ${#candidates[*]} )); then
>  	msg 'no candidate packages found for pruning'
> -- 
> 2.0.3
>