[pacman-dev] [PATCH v2 2/3] pkgdelta: implement requirments for delta generation

[Allan McRae]

On 07/04/12 08:07, Florian Pritz wrote:
> Big deltas or deltas for very small packages are not needed so we should
> check that and not generate any.
> 
> Signed-off-by: Florian Pritz <bluewind at xinu.at>
> ---
> This now checks for numeric values as per Dave suggestion.
> 
>  scripts/pkgdelta.sh.in |   51 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in
> index 0986986..da5d7c9 100644
> --- a/scripts/pkgdelta.sh.in
> +++ b/scripts/pkgdelta.sh.in
> @@ -30,6 +30,13 @@ declare -r myver='@PACKAGE_VERSION@'
>  
>  QUIET=0
>  
> +# minimal of package before deltas are generated (bytes)
> +min_pkg_size=$((1024*1024))
> +
> +# percent of new package above which the delta will be discarded
> +max_delta_size=80

Weird number to pick given pacman by default downloads deltas if it is
less than 70% of the full package download.

Also, this is inconsistent with how this value is specified in
pacman.conf with the UseDelta option.

> +
>  # ensure we have a sane umask set
>  umask 0022
>  
> @@ -46,6 +53,8 @@ This delta file can then be added to a database using repo-add.\n\n")"
>  	echo
>  	printf -- "$(gettext "Options:\n")"
>  	printf -- "  -q                ""$(gettext "quiet\n")"
> +	printf -- "  --min-pkg-size    ""$(gettext "minimal of package before deltas are generated (bytes)\n")"
> +	printf -- "  --max-delta-size  ""$(gettext "percent of new package above which the delta will be discarded\n")"

There is a man page that needs updated when you add new options...

>  }
>  
>  version() {
> @@ -56,6 +65,10 @@ This is free software; see the source for copying conditions.\n\
>  There is NO WARRANTY, to the extent permitted by law.\n")"
>  }
>  
> +isnumeric() {
> +	[[ $1 != *[!0-9]* ]]
> +}
> +
>  read_pkginfo()
>  {
>  	pkgname= pkgver= arch=
> @@ -96,6 +109,13 @@ create_xdelta()
>  	newver="$pkgver"
>  	newarch="$arch"
>  
> +	pkgsize="$(@SIZECMD@ -L "$newfile")"
> +
> +	if ((pkgsize < min_pkg_size)); then
> +		msg "$(gettext "Ignoring small package: %s")" "$pkgsize"

This message is unclear...   I'm not even sure $pkgsize is what you want
to print there.  How about:

"Skipping delta creation for small package: %s - size %s)  $newname $pkgsise

> +		return 0
> +	fi
> +
>  	if [[ $oldname != "$newname" ]]; then
>  		error "$(gettext "The package names don't match : '%s' and '%s'")" "$oldname" "$newname"
>  		return 1
> @@ -119,10 +139,19 @@ create_xdelta()
>  	if (( ret )); then
>  		error "$(gettext "Delta could not be created.")"
>  		return 1
> -	else
> -		msg "$(gettext "Generated delta : '%s'")" "$deltafile"
> -		(( QUIET )) && echo "$deltafile"
>  	fi
> +
> +	deltasize="$(@SIZECMD@ -L "$deltafile")"
> +
> +	if ((max_delta_size * pkgsize / 100 < deltasize)); then
> +		msg "$(gettext "Ignoring big delta: %s vs %s")" "$deltasize" "$pkgsize"

Again, a very unclear message.  Something like "Delta package larger
than maximum size. Removing."?

> +		rm -f "$deltafile"
> +		return 0
> +	fi
> +
> +	msg "$(gettext "Generated delta : '%s'")" "$deltafile"
> +	(( QUIET )) && echo "$deltafile"
> +
>  	return 0
>  }
>  
> @@ -134,6 +163,22 @@ while (( $# )); do
>  		-h|--help)    usage; exit 0 ;;
>  		-V|--version) version; exit 0 ;;
>  		-q|--quiet) QUIET=1;;
> +		--min-pkg-size)
> +			if ! isnumeric "$2"; then
> +				echo "invalid argument '$2' for option -- '$1'"
> +				exit 1
> +			fi
> +			min_pkg_size=$2
> +			shift
> +			;;
> +		--max-delta-size)
> +			if ! isnumeric "$2"; then

Apply the same restrictions as in UseDelta in pacman.conf

> +				echo "invalid argument '$2' for option -- '$1'"
> +				exit 1
> +			fi
> +			max_delta_size=$2
> +			shift
> +			;;
>  		--) shift; args+=("$@"); break 2 ;;
>  		-*) echo "invalid option -- '$1'"; usage; exit 1 ;;
>  		*) args+=("$1");;