[pacman-dev] [PATCH 1/2] pkgdelta: rework option/argument parser
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- scripts/pkgdelta.sh.in | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in index 5a2e6a3..806ae23 100644 --- a/scripts/pkgdelta.sh.in +++ b/scripts/pkgdelta.sh.in @@ -38,11 +38,14 @@ m4_include(library/output_format.sh) # print usage instructions usage() { printf "pkgdelta (pacman) %s\n\n" "$myver" - printf -- "$(gettext "Usage: pkgdelta [-q] <package1> <package2>\n")" + printf -- "$(gettext "Usage: pkgdelta [options] <package1> <package2>\n")" printf -- "$(gettext "\ pkgdelta will create a delta file between two packages.\n\ This delta file can then be added to a database using repo-add.\n\n")" printf -- "$(gettext "Example: pkgdelta pacman-3.0.0.pkg.tar.gz pacman-3.0.1.pkg.tar.gz")\n" + echo + printf -- "$(gettext "Options:\n")" + printf -- " -q ""$(gettext "quiet\n")" } version() { @@ -123,32 +126,36 @@ create_xdelta() return 0 } -case "$1" in - -h|--help) usage; exit 0 ;; - -V|--version) version; exit 0 ;; - -q|--quiet) QUIET=1; shift ;; -esac +declare -a args -if (( $# != 2 )); then +# parse arguments +while (( $# )); do + case "$1" in + -h|--help) usage; exit 0 ;; + -V|--version) version; exit 0 ;; + -q|--quiet) QUIET=1;; + *) args+=("$1");; + esac + shift +done + +if (( ${#args[@]} != 2 )); then usage exit 1 fi -if [[ ! -f $1 ]]; then - error "$(gettext "File '%s' does not exist")" "$1" - exit 0 -fi - -if [[ ! -f $2 ]]; then - error "$(gettext "File '%s' does not exist")" "$2" - exit 0 -fi +for i in "${args[@]}"; do + if [[ ! -f $i ]]; then + error "$(gettext "File '%s' does not exist")" "$i" + exit 0 + fi +done if ! type xdelta3 &>/dev/null; then error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")" exit 1 fi -create_xdelta "$1" "$2" +create_xdelta "${args[0]}" "${args[1]}" # vim: set ts=2 sw=2 noet: -- 1.7.9.6
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@xinu.at> --- scripts/pkgdelta.sh.in | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in index 806ae23..ff34d88 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 + + # 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")" } version() { @@ -96,6 +105,13 @@ create_xdelta() newver="$pkgver" newarch="$arch" + pkgsize="$(@SIZECMD@ -L "$newfile")" + + if ((pkgsize < min_pkg_size)); then + msg "$(gettext "Ignoring small package: %s")" "$pkgsize" + return 0 + fi + if [[ $oldname != "$newname" ]]; then error "$(gettext "The package names don't match : '%s' and '%s'")" "$oldname" "$newname" return 1 @@ -119,10 +135,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" + rm -f "$deltafile" + return 0 + fi + + msg "$(gettext "Generated delta : '%s'")" "$deltafile" + (( QUIET )) && echo "$deltafile" + return 0 } @@ -134,6 +159,8 @@ while (( $# )); do -h|--help) usage; exit 0 ;; -V|--version) version; exit 0 ;; -q|--quiet) QUIET=1;; + --min-pkg-size) min_pkg_size=$2; shift;; + --max-delta-size) max_delta_size=$2; shift;; *) args+=("$1");; esac shift -- 1.7.9.6
On Fri, Apr 06, 2012 at 08:56:05PM +0200, 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@xinu.at> --- scripts/pkgdelta.sh.in | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in index 806ae23..ff34d88 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 + + # 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")" }
version() { @@ -96,6 +105,13 @@ create_xdelta() newver="$pkgver" newarch="$arch"
+ pkgsize="$(@SIZECMD@ -L "$newfile")" + + if ((pkgsize < min_pkg_size)); then + msg "$(gettext "Ignoring small package: %s")" "$pkgsize" + return 0 + fi + if [[ $oldname != "$newname" ]]; then error "$(gettext "The package names don't match : '%s' and '%s'")" "$oldname" "$newname" return 1 @@ -119,10 +135,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" + rm -f "$deltafile" + return 0 + fi + + msg "$(gettext "Generated delta : '%s'")" "$deltafile" + (( QUIET )) && echo "$deltafile" + return 0 }
@@ -134,6 +159,8 @@ while (( $# )); do -h|--help) usage; exit 0 ;; -V|--version) version; exit 0 ;; -q|--quiet) QUIET=1;; + --min-pkg-size) min_pkg_size=$2; shift;; + --max-delta-size) max_delta_size=$2; shift;;
You never do any checking for these inputs to ensure they're strictly numeric. You can do this simply with a simple glob test: isnumeric() { [[ $1 != *[!0-9]* ]] } ... --min-pkg-size) if ! isnuermic "$2"; then echo "invalid blah blah blah" exit 1 fi min_pkg_size=$2 shift ;; ...
*) args+=("$1");; esac shift -- 1.7.9.6
On Fri, Apr 06, 2012 at 08:56:04PM +0200, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- scripts/pkgdelta.sh.in | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in index 5a2e6a3..806ae23 100644 --- a/scripts/pkgdelta.sh.in +++ b/scripts/pkgdelta.sh.in @@ -38,11 +38,14 @@ m4_include(library/output_format.sh) # print usage instructions usage() { printf "pkgdelta (pacman) %s\n\n" "$myver" - printf -- "$(gettext "Usage: pkgdelta [-q] <package1> <package2>\n")" + printf -- "$(gettext "Usage: pkgdelta [options] <package1> <package2>\n")" printf -- "$(gettext "\ pkgdelta will create a delta file between two packages.\n\ This delta file can then be added to a database using repo-add.\n\n")" printf -- "$(gettext "Example: pkgdelta pacman-3.0.0.pkg.tar.gz pacman-3.0.1.pkg.tar.gz")\n" + echo + printf -- "$(gettext "Options:\n")" + printf -- " -q ""$(gettext "quiet\n")" }
version() { @@ -123,32 +126,36 @@ create_xdelta() return 0 }
-case "$1" in - -h|--help) usage; exit 0 ;; - -V|--version) version; exit 0 ;; - -q|--quiet) QUIET=1; shift ;; -esac +declare -a args
-if (( $# != 2 )); then +# parse arguments +while (( $# )); do + case "$1" in + -h|--help) usage; exit 0 ;; + -V|--version) version; exit 0 ;; + -q|--quiet) QUIET=1;; + *) args+=("$1");;
Be aware that this goes against POSIX options parsing (which should do a full stop on the first non-option argument). Invalid options will be treated as arguments as well. If you want longopts like this, maybe add in an extra case or two... while (( $# ));do case $1 in -h|--help) usage; exit 0 ;; -V|--version) version; exit 0 ;; -q|--quiet) QUIET=1 ;; --) shift; args+=("$@"); break 2 ;; -*) echo "invalid option -- '$1'"; usage; exit 1 ;; *) args+=("$1") ;; esac shift done
+ esac + shift +done + +if (( ${#args[@]} != 2 )); then usage exit 1 fi
-if [[ ! -f $1 ]]; then - error "$(gettext "File '%s' does not exist")" "$1" - exit 0 -fi - -if [[ ! -f $2 ]]; then - error "$(gettext "File '%s' does not exist")" "$2" - exit 0 -fi +for i in "${args[@]}"; do + if [[ ! -f $i ]]; then + error "$(gettext "File '%s' does not exist")" "$i" + exit 0
exit 0 seems bad to me here since we're erroring out. I realize that you're just copying the code that used to be here, though... Dan, what's the rationale?
+ fi +done
if ! type xdelta3 &>/dev/null; then error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")" exit 1 fi
-create_xdelta "$1" "$2" +create_xdelta "${args[0]}" "${args[1]}"
You can simply use "${args[@]}" since we know that this is exactly 2 args.
# vim: set ts=2 sw=2 noet: -- 1.7.9.6
participants (2)
-
Dave Reisner
-
Florian Pritz