[pacman-dev] [PATCH] Use pipe to create compressed package instead of an intermediate tar file

[Dan McGee]

On Fri, Aug 27, 2010 at 8:50 AM, Juergen Hoetzel <juergen at 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 at 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
>
>
>