[pacman-dev] [PATCH] checkupdates: Add package download mode with "-d"

[Allan McRae]

On 17/01/14 15:44, Chris Down wrote:
> This new mode, which must be run as root, allows for the precaching of
> packages which can then subsequently be updated by calling
> "pacman -Syu" as normal. This can then be integrated with a cron script
> (for example, checkupdates-cron[0]) for automated, safe downloading of
> packages alongside getting the list of packages to update, or run
> manually on the command line to download new packages, but avoid
> updating the main package database.
> 
> As a side effect of adding the -d option, argument handling (which was
> previously not present) is now done using "getopts". This seems like a
> reasonable way to do it instead of just hacking in a check for "-d" as
> $1 now, but I'm not particularly averse to either. "getopts" would just
> allow for better future flexibility in this area.
> 
> It is possible to do this as another script, say, downloadupdates, but
> we would encounter pretty heavy code duplication with "checkupdates", so
> it seems more sensible to do it here.
> 
> Due to the use of --noconfirm, this has some limitations when we
> encounter more complex upgrade scenarios, for example when a new package
> conflict occurs, an upgrade path cannot be correctly/automatically
> determined, a package was replaced, etc. This is not a major problem --
> in the worst case, we just don't get the packages that we wanted
> downloaded and an error message is displayed to the user.
> 
> The choice to hide both STDOUT and STDERR of "pacman -Syuw" is
> potentially up for debate. It makes it more difficult to determine where
> the problem was if the command fails -- we still display a message
> saying that that was the case, and set the exit code accordingly, but
> the actual cause needs to be determined by the user elsewhere. For this
> reason, "-p" exists to force the package download to output as normal.
> 
> It seems desirable that "-p" also affects the silence of the database
> update, but this is also up for discussion if people feel it should not
> be within the remit of the (( silent )) checks.
> 
> [0]: https://github.com/cdown/checkupdates-cron

So there are two completely separate changes here...    Adding "-d" and
"-p".   I'd say "-p" should be "-v" or  "--verbose".   These patches
need separately submitted.

I really do not think getopt is justified here.  There is nothing
complex. Just loop through the arguments.

Also, the output of checkupdates is simple for including in a script.  I
think checkupdates should just stay checking updates.

export CHECKUPDATES_DB="/some/db/path"
updates=("$(checkupdates)")
if [[ -n ${updates[@]} ]]; then
  pacman -Sw --dbpath $CHECKUPDATES_DB $updates
done


> ---
>  contrib/checkupdates.sh.in | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/checkupdates.sh.in b/contrib/checkupdates.sh.in
> index 413b78a..02acb67 100644
> --- a/contrib/checkupdates.sh.in
> +++ b/contrib/checkupdates.sh.in
> @@ -21,16 +21,43 @@
>  declare -r myname='checkupdates'
>  declare -r myver='@PACKAGE_VERSION@'
>  
> -if (( $# > 0 )); then
> +download=0
> +silent=1
> +
> +usage() {
>  	echo "${myname} (pacman) v${myver}"
>  	echo
>  	echo "Safely print a list of pending updates"
>  	echo
>  	echo "Usage: ${myname}"
>  	echo
> +	echo "Options:"
> +	echo "  -d  Download upgradeable packages to pacman package cache."
> +	echo "  -p  Print pacman output from the database upgrade and package download."
> +	echo
>  	echo 'Note: Export the "CHECKUPDATES_DB" variable to change the path of the temporary database.'
>  	exit 0
> -fi
> +}
> +
> +run_silent() (
> +	(( silent )) && exec &>/dev/null
> +	"$@"
> +)
> +
> +while getopts ':dp' opt; do
> +	case $opt in
> +		d)
> +			if (( UID )); then
> +				echo "Download mode can only be run as root." >&2
> +				exit 2
> +			fi
> +
> +			download=1
> +		;;
> +		p) silent=0 ;;
> +		\?) usage ;;
> +	esac
> +done
>  
>  if [[ -z $CHECKUPDATES_DB ]]; then
>  	CHECKUPDATES_DB="${TMPDIR:-/tmp}/checkup-db-${USER}/"
> @@ -43,9 +70,17 @@ eval $(awk -F' *= *' '$1 ~ /DBPath/ { print $1 "=" $2 }' @sysconfdir@/pacman.con
>  
>  mkdir -p "$CHECKUPDATES_DB"
>  ln -s "${DBPath}/local" "$CHECKUPDATES_DB" &> /dev/null
> -fakeroot pacman -Sy --dbpath "$CHECKUPDATES_DB" --logfile /dev/null &> /dev/null
> +run_silent fakeroot pacman -Sy --dbpath "$CHECKUPDATES_DB" --logfile /dev/null
>  pacman -Qqu --dbpath "$CHECKUPDATES_DB" 2> /dev/null
>  
> +if (( download )); then
> +	if ! run_silent pacman -Suw --noconfirm --dbpath "$CHECKUPDATES_DB"; then
> +		ret=$?
> +		(( silent )) && echo "Failed to download packages." >&2
> +		exit "$ret"
> +	fi
> +fi
> +
>  exit 0
>  
>  # vim: set ts=2 sw=2 noet:
>