On Sat, Jun 07, 2014 at 11:25:53PM +0400, Yanus Poluektovich wrote:
> If pv is installed, use it to provide progress indication for package
> compression stage.
So, I like this idea, but there's some issues I'd like to raise with
your approach, mostly stylistic:
> ---
> scripts/makepkg.sh.in | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 96e5349..0bb619f 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -2006,10 +2006,21 @@ create_package() {
> msg2 "$(gettext "Compressing package...")"
> # TODO: Maybe this can be set globally for robustness
> shopt -s -o pipefail
> +
> + comp_files+=( $(ls) )
You should be using globbing here -- exactly what you took away later on
from the bsdtar call. But, I'd go one step further and just add this to
the original declaration of comp_files.
> + local pv_uncompressed="cat"
> + local pv_compressed="cat"
> + if [[ -f "/usr/bin/pv" ]]; then
You don't need to assume pv is in /usr/bin. To check existence, use:
if type -P pv >/dev/null; then
> + local uncompressed_size=$( du -sb "${comp_files[@]}" | awk
> 'BEGIN {s=0} {s += $1} END {print s}' )
Other usage of du in this script are necessarily done via a preprocessor
replacement, i.e. @DUPATH@ @DUFLAGS@
You'll need to follow this. There's also no need for your BEGIN block --
variables are implicitly defined in awk.
> + pv_uncompressed="/usr/bin/pv -c -N uncompressed -perab -s
> $uncompressed_size"
> + pv_compressed="/usr/bin/pv -c -N compressed -trab"
Commands should always be declared in an array, not as a simple string.
I'd opt for slightly different variable names, too:
if type -P pv >/dev/null; then
uncompressed_pipe=(pv -c -N uncompressed -perab -s "$uncompressed_size")
compressed_pipe=(pv -c -N compressed -trab)
else
uncompressed_pipe=(cat)
compressed_pipe=(cat)
fi
> + fi
> +
> # bsdtar's gzip compression always saves the time stamp, making one
> # archive created using the same command line distinct from another.
> # Disable bsdtar compression and use gzip -n for now.
> - bsdtar -cf - "${comp_files[@]}" * |
> + bsdtar -cf - "${comp_files[@]}" |
>
> + $pv_uncompressed |
This would be "${uncompressed_pipe[@]}"
> case "$PKGEXT" in
> *tar.gz) ${COMPRESSGZ[@]:-gzip -c -f -n} ;;
> *tar.bz2) ${COMPRESSBZ2[@]:-bzip2 -c -f} ;;
> @@ -2020,7 +2031,7 @@ create_package() {
> *tar) cat ;;
> *) warning "$(gettext "'%s' is not a valid archive
> extension.")" \
> "$PKGEXT"; cat ;;
> - esac > "${pkg_file}" || ret=$?
> + esac | $pv_compressed > "${pkg_file}" || ret=$?
This would be "${compressed_pipe[@]}"
>
> shopt -u nullglob
> shopt -u -o pipefail
> --
> 2.0.0
>
>
