[pacman-dev] [PATCH] makepkg: unify tarball creation
Introduce ext_to_tar_opt() to unify package and source package tarball creation. This requires bsdtar to support the compression options -z, -j, -J and -Z. Note also the 'compress' command is not available in Arch (FS#25908), so 'bsdtar -Z' seems to be the only way to support .tar.Z archive creation. Signed-off-by: lolilolicon <lolilolicon@gmail.com> --- scripts/makepkg.sh.in | 107 ++++++++++++++++++------------------------------ 1 files changed, 40 insertions(+), 67 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 86e74a6..75c3730 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1212,6 +1212,22 @@ check_package() { } +# get bsdtar compression flag from extension +ext_to_tar_opt() { + local extension=$1 TAR_OPT= + case "$extension" in + *.tar.gz) TAR_OPT="z" ;; + *.tar.bz2) TAR_OPT="j" ;; + *.tar.xz) TAR_OPT="J" ;; + *.tar.Z) TAR_OPT="Z" ;; + *.tar) ;; + *) warning "$(gettext "'%s' is not a valid archive extension.")" \ + "$extension" ;; + esac + + printf '%s' "$TAR_OPT" +} + create_package() { if [[ ! -d $pkgdir ]]; then error "$(gettext "Missing %s directory.")" "pkg/" @@ -1253,71 +1269,43 @@ create_package() { fi done - # tar it up - msg2 "$(gettext "Compressing package...")" - - local EXT - case "$PKGEXT" in - *tar.gz) EXT=${PKGEXT%.gz} ;; - *tar.bz2) EXT=${PKGEXT%.bz2} ;; - *tar.xz) EXT=${PKGEXT%.xz} ;; - *tar.Z) EXT=${PKGEXT%.Z} ;; - *tar) EXT=${PKGEXT} ;; - *) warning "$(gettext "'%s' is not a valid archive extension.")" \ - "$PKGEXT" ; EXT=$PKGEXT ;; - esac - local fullver=$(get_full_version) local pkg_file="$PKGDEST/${nameofpkg}-${fullver}-${PKGARCH}${PKGEXT}" - local ret=0 [[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig" + # tar it up + msg2 "$(gettext "Compressing package...")" + local TAR_OPT=$(ext_to_tar_opt "$PKGEXT") # when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s 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.Z) compress -c -f ;; - *tar) cat ;; - esac > "${pkg_file}" || ret=$? - - shopt -u nullglob - shopt -u -o pipefail - - if (( ret )); then + if ! bsdtar -c${TAR_OPT}f "$pkg_file" $comp_files *; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code - fi - - create_signature "$pkg_file" - - if (( ! ret )) && [[ ! "$PKGDEST" -ef "${startdir}" ]]; then - rm -f "${pkg_file/$PKGDEST/$startdir}" - ln -s "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" - ret=$? - if [[ -f $pkg_file.sig ]]; then - rm -f "${pkg_file/$PKGDEST/$startdir}.sig" - ln -s "$pkg_file.sig" "${pkg_file/$PKGDEST/$startdir}.sig" + else + create_signature "$pkg_file" + if [[ ! "$PKGDEST" -ef "${startdir}" ]]; then + rm -f "${pkg_file/$PKGDEST/$startdir}" + if ! ln -s "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" + warning "$(gettext "Failed to create symlink to package file.")" + fi + if [[ -f $pkg_file.sig ]]; then + rm -f "${pkg_file/$PKGDEST/$startdir}.sig" + if ! ln -s "$pkg_file.sig" "${pkg_file/$PKGDEST/$startdir}.sig" + warning "$(gettext "Failed to create symlink to signature file.")" + fi + fi fi fi - - if (( ret )); then - warning "$(gettext "Failed to create symlink to package file.")" - fi + shopt -u nullglob } create_signature() { if [[ $SIGNPKG != 'y' ]]; then return fi - local ret=0 local filename="$1" msg "$(gettext "Signing package...")" @@ -1326,10 +1314,7 @@ create_signature() { SIGNWITHKEY="-u ${GPGKEY}" fi # The signature will be generated directly in ascii-friendly format - gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null || ret=$? - - - if (( ! ret )); then + if gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null; then msg2 "$(gettext "Created signature file %s.")" "$filename.sig" else warning "$(gettext "Failed to sign package file.")" @@ -1369,35 +1354,23 @@ create_srcpackage() { done < <(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE") done - local TAR_OPT - case "$SRCEXT" in - *tar.gz) TAR_OPT="z" ;; - *tar.bz2) TAR_OPT="j" ;; - *tar.xz) TAR_OPT="J" ;; - *tar) TAR_OPT="" ;; - *) warning "$(gettext "'%s' is not a valid archive extension.")" \ - "$SRCEXT" ;; - esac - local fullver=$(get_full_version) local pkg_file="$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}" # tar it up msg2 "$(gettext "Compressing source package...")" cd "${srclinks}" + local TAR_OPT=$(ext_to_tar_opt "$SRCEXT") if ! bsdtar -c${TAR_OPT}Lf "$pkg_file" ${pkgbase}; then error "$(gettext "Failed to create source package file.")" exit 1 # TODO: error code fi - if (( ! ret )) && [[ ! "$SRCPKGDEST" -ef "${startdir}" ]]; then + elif [[ ! "$SRCPKGDEST" -ef "${startdir}" ]]; then rm -f "${pkg_file/$SRCPKGDEST/$startdir}" - ln -s "${pkg_file}" "${pkg_file/$SRCPKGDEST/$startdir}" - ret=$? - fi - - if (( ret )); then - warning "$(gettext "Failed to create symlink to source package file.")" + if ! ln -s "${pkg_file}" "${pkg_file/$SRCPKGDEST/$startdir}"; then + warning "$(gettext "Failed to create symlink to source package file.")" + fi fi cd "${startdir}" -- 1.7.6.4
On Fri, Sep 30, 2011 at 01:23:37AM +0800, lolilolicon wrote:
Introduce ext_to_tar_opt() to unify package and source package tarball creation. This requires bsdtar to support the compression options -z, -j, -J and -Z. Note also the 'compress' command is not available in Arch (FS#25908), so 'bsdtar -Z' seems to be the only way to support .tar.Z archive creation.
Signed-off-by: lolilolicon <lolilolicon@gmail.com> --- scripts/makepkg.sh.in | 107 ++++++++++++++++++------------------------------ 1 files changed, 40 insertions(+), 67 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 86e74a6..75c3730 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1212,6 +1212,22 @@ check_package() {
}
+# get bsdtar compression flag from extension +ext_to_tar_opt() { + local extension=$1 TAR_OPT= + case "$extension" in + *.tar.gz) TAR_OPT="z" ;; + *.tar.bz2) TAR_OPT="j" ;; + *.tar.xz) TAR_OPT="J" ;; + *.tar.Z) TAR_OPT="Z" ;; + *.tar) ;; + *) warning "$(gettext "'%s' is not a valid archive extension.")" \ + "$extension" ;; + esac + + printf '%s' "$TAR_OPT" +} +
You've failed to account for an unsupported/invalid extension being passed. We throw a warning, print nothing, and probably end up with a tar archive called $pkgname-$pkgver-$pkgrel-$arch.pkg.tar.LOLLERCAUST.
create_package() { if [[ ! -d $pkgdir ]]; then error "$(gettext "Missing %s directory.")" "pkg/" @@ -1253,71 +1269,43 @@ create_package() { fi done
- # tar it up - msg2 "$(gettext "Compressing package...")" - - local EXT - case "$PKGEXT" in - *tar.gz) EXT=${PKGEXT%.gz} ;; - *tar.bz2) EXT=${PKGEXT%.bz2} ;; - *tar.xz) EXT=${PKGEXT%.xz} ;; - *tar.Z) EXT=${PKGEXT%.Z} ;; - *tar) EXT=${PKGEXT} ;; - *) warning "$(gettext "'%s' is not a valid archive extension.")" \ - "$PKGEXT" ; EXT=$PKGEXT ;; - esac - local fullver=$(get_full_version) local pkg_file="$PKGDEST/${nameofpkg}-${fullver}-${PKGARCH}${PKGEXT}" - local ret=0
[[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
+ # tar it up + msg2 "$(gettext "Compressing package...")" + local TAR_OPT=$(ext_to_tar_opt "$PKGEXT") # when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s 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.Z) compress -c -f ;; - *tar) cat ;; - esac > "${pkg_file}" || ret=$? - - shopt -u nullglob - shopt -u -o pipefail - - if (( ret )); then + if ! bsdtar -c${TAR_OPT}f "$pkg_file" $comp_files *; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code - fi - - create_signature "$pkg_file" - - if (( ! ret )) && [[ ! "$PKGDEST" -ef "${startdir}" ]]; then - rm -f "${pkg_file/$PKGDEST/$startdir}" - ln -s "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" - ret=$? - if [[ -f $pkg_file.sig ]]; then - rm -f "${pkg_file/$PKGDEST/$startdir}.sig" - ln -s "$pkg_file.sig" "${pkg_file/$PKGDEST/$startdir}.sig" + else + create_signature "$pkg_file" + if [[ ! "$PKGDEST" -ef "${startdir}" ]]; then + rm -f "${pkg_file/$PKGDEST/$startdir}" + if ! ln -s "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" + warning "$(gettext "Failed to create symlink to package file.")" + fi + if [[ -f $pkg_file.sig ]]; then + rm -f "${pkg_file/$PKGDEST/$startdir}.sig" + if ! ln -s "$pkg_file.sig" "${pkg_file/$PKGDEST/$startdir}.sig" + warning "$(gettext "Failed to create symlink to signature file.")" + fi + fi fi fi
Unrelated changes here... as far as I can see, we gain nothing from disturbing this block of code but introducing duplication.
- - if (( ret )); then - warning "$(gettext "Failed to create symlink to package file.")" - fi + shopt -u nullglob }
create_signature() { if [[ $SIGNPKG != 'y' ]]; then return fi - local ret=0 local filename="$1" msg "$(gettext "Signing package...")"
@@ -1326,10 +1314,7 @@ create_signature() { SIGNWITHKEY="-u ${GPGKEY}" fi # The signature will be generated directly in ascii-friendly format - gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null || ret=$? - - - if (( ! ret )); then + if gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null; then msg2 "$(gettext "Created signature file %s.")" "$filename.sig" else warning "$(gettext "Failed to sign package file.")" @@ -1369,35 +1354,23 @@ create_srcpackage() { done < <(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE") done
- local TAR_OPT - case "$SRCEXT" in - *tar.gz) TAR_OPT="z" ;; - *tar.bz2) TAR_OPT="j" ;; - *tar.xz) TAR_OPT="J" ;; - *tar) TAR_OPT="" ;; - *) warning "$(gettext "'%s' is not a valid archive extension.")" \ - "$SRCEXT" ;; - esac - local fullver=$(get_full_version) local pkg_file="$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}"
# tar it up msg2 "$(gettext "Compressing source package...")" cd "${srclinks}" + local TAR_OPT=$(ext_to_tar_opt "$SRCEXT") if ! bsdtar -c${TAR_OPT}Lf "$pkg_file" ${pkgbase}; then error "$(gettext "Failed to create source package file.")" exit 1 # TODO: error code fi
- if (( ! ret )) && [[ ! "$SRCPKGDEST" -ef "${startdir}" ]]; then + elif [[ ! "$SRCPKGDEST" -ef "${startdir}" ]]; then rm -f "${pkg_file/$SRCPKGDEST/$startdir}" - ln -s "${pkg_file}" "${pkg_file/$SRCPKGDEST/$startdir}" - ret=$? - fi - - if (( ret )); then - warning "$(gettext "Failed to create symlink to source package file.")" + if ! ln -s "${pkg_file}" "${pkg_file/$SRCPKGDEST/$startdir}"; then + warning "$(gettext "Failed to create symlink to source package file.")" + fi fi
More churn, here...
cd "${startdir}" -- 1.7.6.4
On Fri, Sep 30, 2011 at 1:40 AM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 30, 2011 at 01:23:37AM +0800, lolilolicon wrote:
Introduce ext_to_tar_opt() to unify package and source package tarball creation. This requires bsdtar to support the compression options -z, -j, -J and -Z. Note also the 'compress' command is not available in Arch (FS#25908), so 'bsdtar -Z' seems to be the only way to support .tar.Z archive creation.
Signed-off-by: lolilolicon <lolilolicon@gmail.com> --- scripts/makepkg.sh.in | 107 ++++++++++++++++++------------------------------ 1 files changed, 40 insertions(+), 67 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 86e74a6..75c3730 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1212,6 +1212,22 @@ check_package() {
}
+# get bsdtar compression flag from extension +ext_to_tar_opt() { + local extension=$1 TAR_OPT= + case "$extension" in + *.tar.gz) TAR_OPT="z" ;; + *.tar.bz2) TAR_OPT="j" ;; + *.tar.xz) TAR_OPT="J" ;; + *.tar.Z) TAR_OPT="Z" ;; + *.tar) ;; + *) warning "$(gettext "'%s' is not a valid archive extension.")" \ + "$extension" ;; + esac + + printf '%s' "$TAR_OPT" +} +
You've failed to account for an unsupported/invalid extension being passed. We throw a warning, print nothing, and probably end up with a tar archive called $pkgname-$pkgver-$pkgrel-$arch.pkg.tar.LOLLERCAUST.
I don't see any problem if that's what the user wants- a funny name maybe, a tar nontheless, no?
create_package() { if [[ ! -d $pkgdir ]]; then error "$(gettext "Missing %s directory.")" "pkg/" @@ -1253,71 +1269,43 @@ create_package() { fi done
- # tar it up - msg2 "$(gettext "Compressing package...")" - - local EXT - case "$PKGEXT" in - *tar.gz) EXT=${PKGEXT%.gz} ;; - *tar.bz2) EXT=${PKGEXT%.bz2} ;; - *tar.xz) EXT=${PKGEXT%.xz} ;; - *tar.Z) EXT=${PKGEXT%.Z} ;; - *tar) EXT=${PKGEXT} ;; - *) warning "$(gettext "'%s' is not a valid archive extension.")" \ - "$PKGEXT" ; EXT=$PKGEXT ;; - esac - local fullver=$(get_full_version) local pkg_file="$PKGDEST/${nameofpkg}-${fullver}-${PKGARCH}${PKGEXT}" - local ret=0
[[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
+ # tar it up + msg2 "$(gettext "Compressing package...")" + local TAR_OPT=$(ext_to_tar_opt "$PKGEXT") # when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s 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.Z) compress -c -f ;; - *tar) cat ;; - esac > "${pkg_file}" || ret=$? - - shopt -u nullglob - shopt -u -o pipefail - - if (( ret )); then + if ! bsdtar -c${TAR_OPT}f "$pkg_file" $comp_files *; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code - fi - - create_signature "$pkg_file" - - if (( ! ret )) && [[ ! "$PKGDEST" -ef "${startdir}" ]]; then - rm -f "${pkg_file/$PKGDEST/$startdir}" - ln -s "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" - ret=$? - if [[ -f $pkg_file.sig ]]; then - rm -f "${pkg_file/$PKGDEST/$startdir}.sig" - ln -s "$pkg_file.sig" "${pkg_file/$PKGDEST/$startdir}.sig" + else + create_signature "$pkg_file" + if [[ ! "$PKGDEST" -ef "${startdir}" ]]; then + rm -f "${pkg_file/$PKGDEST/$startdir}" + if ! ln -s "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" + warning "$(gettext "Failed to create symlink to package file.")" + fi + if [[ -f $pkg_file.sig ]]; then + rm -f "${pkg_file/$PKGDEST/$startdir}.sig" + if ! ln -s "$pkg_file.sig" "${pkg_file/$PKGDEST/$startdir}.sig" + warning "$(gettext "Failed to create symlink to signature file.")" + fi + fi fi fi
Unrelated changes here... as far as I can see, we gain nothing from disturbing this block of code but introducing duplication.
What do you mean by duplication? This just eliminates the use of $ret... Did this because I noticed ret was used uninitialized, but yes it's unrelated...
- - if (( ret )); then - warning "$(gettext "Failed to create symlink to package file.")" - fi + shopt -u nullglob }
create_signature() { if [[ $SIGNPKG != 'y' ]]; then return fi - local ret=0 local filename="$1" msg "$(gettext "Signing package...")"
@@ -1326,10 +1314,7 @@ create_signature() { SIGNWITHKEY="-u ${GPGKEY}" fi # The signature will be generated directly in ascii-friendly format - gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null || ret=$? - - - if (( ! ret )); then + if gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null; then msg2 "$(gettext "Created signature file %s.")" "$filename.sig" else warning "$(gettext "Failed to sign package file.")" @@ -1369,35 +1354,23 @@ create_srcpackage() { done < <(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE") done
- local TAR_OPT - case "$SRCEXT" in - *tar.gz) TAR_OPT="z" ;; - *tar.bz2) TAR_OPT="j" ;; - *tar.xz) TAR_OPT="J" ;; - *tar) TAR_OPT="" ;; - *) warning "$(gettext "'%s' is not a valid archive extension.")" \ - "$SRCEXT" ;; - esac - local fullver=$(get_full_version) local pkg_file="$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}"
# tar it up msg2 "$(gettext "Compressing source package...")" cd "${srclinks}" + local TAR_OPT=$(ext_to_tar_opt "$SRCEXT") if ! bsdtar -c${TAR_OPT}Lf "$pkg_file" ${pkgbase}; then error "$(gettext "Failed to create source package file.")" exit 1 # TODO: error code fi
- if (( ! ret )) && [[ ! "$SRCPKGDEST" -ef "${startdir}" ]]; then + elif [[ ! "$SRCPKGDEST" -ef "${startdir}" ]]; then rm -f "${pkg_file/$SRCPKGDEST/$startdir}" - ln -s "${pkg_file}" "${pkg_file/$SRCPKGDEST/$startdir}" - ret=$? - fi - - if (( ret )); then - warning "$(gettext "Failed to create symlink to source package file.")" + if ! ln -s "${pkg_file}" "${pkg_file/$SRCPKGDEST/$startdir}"; then + warning "$(gettext "Failed to create symlink to source package file.")" + fi fi
More churn, here...
cd "${startdir}" -- 1.7.6.4
On Thu, Sep 29, 2011 at 12:23 PM, lolilolicon <lolilolicon@gmail.com> wrote:
Introduce ext_to_tar_opt() to unify package and source package tarball creation. This requires bsdtar to support the compression options -z, -j, -J and -Z. Note also the 'compress' command is not available in Arch (FS#25908), so 'bsdtar -Z' seems to be the only way to support .tar.Z archive creation.
When you write a patch and the "why do they do it this way, it seems so silly!" thought comes into your head, I encourage you to review the extensive git history surrounding this code, notably: http://projects.archlinux.org/pacman.git/commit/?id=c8beffa7904abe7e0ad01fed... Until you teach bsdtar to not store a new gzip timestamp each time it creates a package (and thus makes packaging actually produce the same package every time), this patch totally breaks lessons we learned 2 and a half years ago. -Dan
On Fri, Sep 30, 2011 at 1:52 AM, Dan McGee <dpmcgee@gmail.com> wrote:
Until you teach bsdtar to not store a new gzip timestamp each time it creates a package (and thus makes packaging actually produce the same package every time), this patch totally breaks lessons we learned 2 and a half years ago.
-Dan
Wow, that's pretty crazzzy. We could use a comment at the silly spot. But still the usage of $EXT should be gone now that we just use a pipe. I also think we can still unify tarball creation- just always do it the silly way. We should probably lose .tar.Z though, since we don't have 'compress'.
On Fri, Sep 30, 2011 at 1:52 AM, Dan McGee <dpmcgee@gmail.com> wrote:
Until you teach bsdtar to not store a new gzip timestamp each time it creates a package (and thus makes packaging actually produce the same package every time), this patch totally breaks lessons we learned 2 and a half years ago.
-Dan
Wow, that's pretty crazzzy. We could use a comment at the silly spot. Agreed. And you can fix up the $EXT bit if we have dead code, but we have to use a pipe for now. But still the usage of $EXT should be gone now that we just use a pipe. I also think we can still unify tarball creation- just always do it the silly way. We should probably lose .tar.Z though, since we don't have 'compress'. You're assuming Arch, and a default install at that. We don't assume
On Thu, Sep 29, 2011 at 1:37 PM, lolilolicon <lolilolicon@gmail.com> wrote: that- if you want to use compress, go for it, just make sure you have it installed. -Dan
participants (3)
-
Dan McGee
-
Dave Reisner
-
lolilolicon