[pacman-dev] [PATCH] Use pipe to create compressed package instead of an intermediate tar file
A pipe between tar and compression command is used. This improves performance by running tar and the compression command simultaneously. Using a pipe also reduces IO by not writing an intermediate tar file to disk. Signed-off-by: Juergen Hoetzel <juergen@archlinux.org> --- scripts/makepkg.sh.in | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 66e1225..2200e98 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1032,26 +1032,25 @@ create_package() { *) warning "$(gettext "'%s' is not a valid archive extension.")" \ "$PKGEXT" ; EXT=$PKGEXT ;; esac - local tar_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}" - local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" + local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" local ret=0 # when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s nullglob - bsdtar -cf - $comp_files * > "$tar_file" || ret=$? - shopt -u nullglob + # TODO: Maybe this can be set globally for robustness + shopt -s -o pipefail + bsdtar -cf - $comp_files *| + case "$PKGEXT" in + *tar.gz) gzip -c -f -n ;; + *tar.bz2) bzip2 -c -f ;; + *tar.xz) xz -c -z - ;; + *tar) cat ;; + esac >>${pkg_file} || ret=$? - if (( ! ret )); then - case "$PKGEXT" in - *tar.gz) gzip -f -n "$tar_file" ;; - *tar.bz2) bzip2 -f "$tar_file" ;; - *tar.xz) xz -z -f "$tar_file" ;; - *tar) true ;; - esac - ret=$? - fi + shopt -u nullglob + shopt -u -o pipefail if (( ret )); then error "$(gettext "Failed to create package file.")" -- 1.7.2.2
On Fri, Aug 27, 2010 at 8:50 AM, Juergen Hoetzel <juergen@archlinux.org> wrote:
A pipe between tar and compression command is used. This improves performance by running tar and the compression command simultaneously.
Using a pipe also reduces IO by not writing an intermediate tar file to disk.
Signed-off-by: Juergen Hoetzel <juergen@archlinux.org> --- scripts/makepkg.sh.in | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 66e1225..2200e98 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1032,26 +1032,25 @@ create_package() { *) warning "$(gettext "'%s' is not a valid archive extension.")" \ "$PKGEXT" ; EXT=$PKGEXT ;; esac - local tar_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}" - local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}"
+ local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" local ret=0
# when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s nullglob - bsdtar -cf - $comp_files * > "$tar_file" || ret=$? - shopt -u nullglob + # TODO: Maybe this can be set globally for robustness + shopt -s -o pipefail + bsdtar -cf - $comp_files *| Can we get a space between * and |? I spent a few minutes trying to figure out what the *| operator was.
+ case "$PKGEXT" in + *tar.gz) gzip -c -f -n ;; + *tar.bz2) bzip2 -c -f ;; + *tar.xz) xz -c -z - ;; + *tar) cat ;; + esac >>${pkg_file} || ret=$? Append seems no good here- if you used -f, won't you get a nice invalid package file? We should just be able to use >, and please stick a space between the > and the filename to match our style elsewhere. Also a trailing space here.
This patch looks good otherwise, if you address these things we can take it.
- if (( ! ret )); then - case "$PKGEXT" in - *tar.gz) gzip -f -n "$tar_file" ;; - *tar.bz2) bzip2 -f "$tar_file" ;; - *tar.xz) xz -z -f "$tar_file" ;; - *tar) true ;; - esac - ret=$? - fi + shopt -u nullglob + shopt -u -o pipefail
if (( ret )); then error "$(gettext "Failed to create package file.")" -- 1.7.2.2
2010/8/27 Dan McGee <dpmcgee@gmail.com>:
+ # TODO: Maybe this can be set globally for robustness + shopt -s -o pipefail + bsdtar -cf - $comp_files *| Can we get a space between * and |? I spent a few minutes trying to figure out what the *| operator was.
For sure this was bad style formating.
+ case "$PKGEXT" in + *tar.gz) gzip -c -f -n ;; + *tar.bz2) bzip2 -c -f ;; + *tar.xz) xz -c -z - ;; + *tar) cat ;; + esac >>${pkg_file} || ret=$? Append seems no good here- if you used -f, won't you get a nice invalid package file? We should just be able to use >, and please stick a space between the > and the filename to match our style elsewhere. Also a trailing space here.
Ups. Really good catch Dan. This would have introdroduce a subtle bug. Jürgen
participants (3)
-
Dan McGee
-
Juergen Hoetzel
-
Jürgen Hötzel