If pv is installed, use it to provide progress indication for package compression stage. --- 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) ) + local pv_uncompressed="cat" + local pv_compressed="cat" + if [[ -f "/usr/bin/pv" ]]; then + local uncompressed_size=$( du -sb "${comp_files[@]}" | awk 'BEGIN {s=0} {s += $1} END {print s}' ) + pv_uncompressed="/usr/bin/pv -c -N uncompressed -perab -s $uncompressed_size" + pv_compressed="/usr/bin/pv -c -N compressed -trab" + 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 | 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=$?
shopt -u nullglob shopt -u -o pipefail
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
On 06/08/2014 12:30 AM, Dave Reisner wrote:
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.
Actually, after some grepping I found that there is only one other usage of DUPATH/DUFLAGS in the entire codebase. It goes like this: first we set DUFLAGS="-sk --apparent-size" in configure.ac, forcing it to report sizes in kilobytes (the -k option), and then we multiply the result by 1024 to get the useful number (lines 1905-1906 of scripts/makepkg.sh.in). Do you think there's a reason behind it, or should I just change "-sk" to "-sb"?
On 06/08/2014 05:12 AM, Yanus Poluektovich wrote:
On 06/08/2014 12:30 AM, Dave Reisner wrote:
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.
Actually, after some grepping I found that there is only one other usage of DUPATH/DUFLAGS in the entire codebase. It goes like this: first we set DUFLAGS="-sk --apparent-size" in configure.ac, forcing it to report sizes in kilobytes (the -k option), and then we multiply the result by 1024 to get the useful number (lines 1905-1906 of scripts/makepkg.sh.in). Do you think there's a reason behind it, or should I just change "-sk" to "-sb"?
Or, rather, it would be "-s -b" for Linux and Cygwin (if http://www.fiveanddime.net/cygwin-man-pages/usr/share/man/man1/du.1.html is to be believed) and "-A -s -B 1" for BSD.
On 06/08/2014 05:21 AM, Yanus Poluektovich wrote:
On 06/08/2014 05:12 AM, Yanus Poluektovich wrote:
On 06/08/2014 12:30 AM, Dave Reisner wrote:
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.
Actually, after some grepping I found that there is only one other usage of DUPATH/DUFLAGS in the entire codebase. It goes like this: first we set DUFLAGS="-sk --apparent-size" in configure.ac, forcing it to report sizes in kilobytes (the -k option), and then we multiply the result by 1024 to get the useful number (lines 1905-1906 of scripts/makepkg.sh.in). Do you think there's a reason behind it, or should I just change "-sk" to "-sb"?
Or, rather, it would be "-s -b" for Linux and Cygwin (if http://www.fiveanddime.net/cygwin-man-pages/usr/share/man/man1/du.1.html is to be believed) and "-A -s -B 1" for BSD.
Found the reason. Apparently it's Darwin that has a problem, not Cygwin. It doesn't have an option to show size in bytes.
So, new plan: leave the old du-related parts as they were and just pull the size from the .PKGINFO file.
If pv is installed, use it to provide progress indication for package compression stage.
Signed-off-by: Yanus Poluektovich ypoluektovich@gmail.com --- scripts/makepkg.sh.in | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 96e5349..a4eab07 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1971,7 +1971,11 @@ create_package() { pkgarch=$(get_pkg_arch) write_pkginfo > .PKGINFO
- local comp_files=('.PKGINFO') + # when fileglobbing, we want * in an empty directory to expand to + # the null string rather than itself + shopt -s nullglob + + local comp_files=( * '.PKGINFO' )
# check for changelog/install files for i in 'changelog/.CHANGELOG' 'install/.INSTALL'; do @@ -1993,23 +1997,30 @@ create_package() { [[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
- # when fileglobbing, we want * in an empty directory to expand to - # the null string rather than itself - shopt -s nullglob - msg2 "$(gettext "Generating .MTREE file...")" bsdtar -czf .MTREE --format=mtree \ --options='!all,use-set,type,uid,gid,mode,time,size,md5,sha256,link' \ - "${comp_files[@]}" * + "${comp_files[@]}" comp_files+=(".MTREE")
msg2 "$(gettext "Compressing package...")" # TODO: Maybe this can be set globally for robustness shopt -s -o pipefail + + local uncompressed_pipe=(cat) + local compressed_pipe=(cat) + if type -P pv >/dev/null; then + local uncompressed_size=$( grep -E '^size =' .PKGINFO ) + uncompressed_size=${uncompressed_size##*[^0-9]} + uncompressed_pipe=(pv -c -N uncompressed -perab -s "$uncompressed_size") + compressed_pipe=(pv -c -N compressed -trab) + 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[@]}" | + "${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 | "${compressed_pipe[@]}" > "${pkg_file}" || ret=$?
shopt -u nullglob shopt -u -o pipefail
On 08/06/14 12:18, Yanus Poluektovich wrote:
If pv is installed, use it to provide progress indication for package compression stage.
Signed-off-by: Yanus Poluektovich ypoluektovich@gmail.com
scripts/makepkg.sh.in | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 96e5349..a4eab07 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1971,7 +1971,11 @@ create_package() { pkgarch=$(get_pkg_arch) write_pkginfo > .PKGINFO
- local comp_files=('.PKGINFO')
- # when fileglobbing, we want * in an empty directory to expand to
- # the null string rather than itself
- shopt -s nullglob
- local comp_files=( * '.PKGINFO' )
No. We want the .PKGINFO file to appear first in the command line (and thus the tarball). Install file should be second. I'd also suggest that the comp_files change is a completely separate patch.
I'm feeling too lazy to look at pv to review the rest of the patch.
# check for changelog/install files for i in 'changelog/.CHANGELOG' 'install/.INSTALL'; do @@ -1993,23 +1997,30 @@ create_package() { [[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
- # when fileglobbing, we want * in an empty directory to expand to
- # the null string rather than itself
- shopt -s nullglob
- msg2 "$(gettext "Generating .MTREE file...")" bsdtar -czf .MTREE --format=mtree \ --options='!all,use-set,type,uid,gid,mode,time,size,md5,sha256,link' \
"${comp_files[@]}" *
"${comp_files[@]}"comp_files+=(".MTREE")
msg2 "$(gettext "Compressing package...")" # TODO: Maybe this can be set globally for robustness shopt -s -o pipefail
local uncompressed_pipe=(cat)
local compressed_pipe=(cat)
if type -P pv >/dev/null; then
local uncompressed_size=$( grep -E '^size =' .PKGINFO )uncompressed_size=${uncompressed_size##*[^0-9]}uncompressed_pipe=(pv -c -N uncompressed -perab -s "$uncompressed_size")compressed_pipe=(pv -c -N compressed -trab)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[@]}" |
- "${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 | "${compressed_pipe[@]}" > "${pkg_file}" || ret=$?
shopt -u nullglob shopt -u -o pipefail
On Mon, Jun 09, 2014 at 08:38:58PM +1000, Allan McRae wrote:
On 08/06/14 12:18, Yanus Poluektovich wrote:
If pv is installed, use it to provide progress indication for package compression stage.
Signed-off-by: Yanus Poluektovich ypoluektovich@gmail.com
scripts/makepkg.sh.in | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 96e5349..a4eab07 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1971,7 +1971,11 @@ create_package() { pkgarch=$(get_pkg_arch) write_pkginfo > .PKGINFO
- local comp_files=('.PKGINFO')
- # when fileglobbing, we want * in an empty directory to expand to
- # the null string rather than itself
- shopt -s nullglob
- local comp_files=( * '.PKGINFO' )
No. We want the .PKGINFO file to appear first in the command line (and thus the tarball). Install file should be second. I'd also suggest that the comp_files change is a completely separate patch.
Good point -- something as important as this should be documented in the code.
I'm feeling too lazy to look at pv to review the rest of the patch.
# check for changelog/install files for i in 'changelog/.CHANGELOG' 'install/.INSTALL'; do @@ -1993,23 +1997,30 @@ create_package() { [[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
- # when fileglobbing, we want * in an empty directory to expand to
- # the null string rather than itself
- shopt -s nullglob
- msg2 "$(gettext "Generating .MTREE file...")" bsdtar -czf .MTREE --format=mtree \ --options='!all,use-set,type,uid,gid,mode,time,size,md5,sha256,link' \
"${comp_files[@]}" *
"${comp_files[@]}"comp_files+=(".MTREE")
msg2 "$(gettext "Compressing package...")" # TODO: Maybe this can be set globally for robustness shopt -s -o pipefail
local uncompressed_pipe=(cat)
local compressed_pipe=(cat)
if type -P pv >/dev/null; then
local uncompressed_size=$( grep -E '^size =' .PKGINFO )uncompressed_size=${uncompressed_size##*[^0-9]}uncompressed_pipe=(pv -c -N uncompressed -perab -s "$uncompressed_size")compressed_pipe=(pv -c -N compressed -trab)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[@]}" |
- "${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 | "${compressed_pipe[@]}" > "${pkg_file}" || ret=$?
shopt -u nullglob shopt -u -o pipefail
If pv is installed, use it to provide progress indication for package compression stage.
Known issues:
1) The 'uncompressed' progress bar freezes a lot when used with multithreaded xz. This may result in overwriting the last line of terminal output (the 'compressing package...' message).
2) When compression is disabled (PKGEXT=.pkg.tar), the 'compressed' progress bar is redundant. I'm not sure if it's acceptable to hard-code a check for this, however.
Signed-off-by: Yanus Poluektovich ypoluektovich@gmail.com --- scripts/makepkg.sh.in | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 96e5349..2f0b480 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 + + local uncompressed_pipe=(cat) + local compressed_pipe=(cat) + if type -P pv >/dev/null; then + local uncompressed_size=$( grep -E '^size =' .PKGINFO ) + uncompressed_size=${uncompressed_size##*[^0-9]} + uncompressed_pipe=(pv -c -N uncompressed -perab -s "$uncompressed_size") + compressed_pipe=(pv -c -N compressed -trab) + 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[@]}" * | + "${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 | "${compressed_pipe[@]}" > "${pkg_file}" || ret=$?
shopt -u nullglob shopt -u -o pipefail
pacman-dev@lists.archlinux.org