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