On Sun, Jan 01, 2012 at 06:43:57PM -0500, Dave Reisner wrote:
In the case of a .pkg.tar.xz and a .pkg.tar.gz existing in the same directory, all commitpkg would say is:
==> WARNING: Could not find . Skipping x86_64
Upon digging into the logic, we did a few things poorly, mostly in getpkgfile:
- getpkgfile tried to die in a subshell (within the command substituion assignment to 'pkgfile'). This will never work. - We assumed that proper glob expansion happened when we received exactly 1 arg. This isn't necessarily true without nullglob in effect. - We dumped the real error (spewed by getpkgfile) to /dev/null. - We checked for the package twice in both $PWD and $DESTDIR/. - We checked for file existance multiple times.
Address this by:
- not hiding errors. revamp the wording a little bit to make it more obvious why we failed, particularly in the case of a glob expanding to more than 1 file. Logic here is simplified to pointing out the failure cases of 0 and >1. - setting nullglob so the number of arguments passed into getpkgfile is meaningful from a 'did it decisively resolve' point of view. - not trying to exit the entire script from a subshell. Just return a value (and use it). - avoiding the package file existance check afterwards. this is a freebie from getpkgfile when the glob passed fails to expand.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- Apologies to those subscribed to arch-commits for uploading ed umpteen times...
commitpkg.in | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/commitpkg.in b/commitpkg.in index c298256..9f31149 100644 --- a/commitpkg.in +++ b/commitpkg.in @@ -3,13 +3,19 @@ m4_include(lib/common.sh)
getpkgfile() { - if [[ ${#} -ne 1 ]]; then - die 'No canonical package found!' - elif [[ ! -f $1 ]]; then - die "Package ${1} not found!" - fi + case $# in + 0) + error 'No canonical package found!' + return 1 + ;; + [!1]) + error 'Failed to canonicalize package name -- multiple packages found:' + msg2 '%s' "$@" + return 1 + ;; + esac
- echo ${1} + echo "$1" }
# Source makepkg.conf; fail if it is not found @@ -127,15 +133,9 @@ for _arch in ${arch[@]}; do
for _pkgname in ${pkgname[@]}; do fullver=$(get_full_version $_pkgname) - pkgfile=$(getpkgfile "$_pkgname-$fullver-${_arch}".pkg.tar.?z 2>/dev/null) - pkgdestfile=$(getpkgfile "$PKGDEST/$_pkgname-$fullver-${_arch}".pkg.tar.?z 2>/dev/null)
- if [[ -f $pkgfile ]]; then - pkgfile="./$pkgfile" - elif [[ -f $pkgdestfile ]]; then - pkgfile="$pkgdestfile" - else - warning "Could not find ${pkgfile}. Skipping ${_arch}" + if ! pkgfile=$(shopt -s nullglob; getpkgfile "$_pkgname-$fullver-${_arch}".pkg.tar.?z); then
fffff this is wrong.... will resend.
+ warning "Skipping $_pkgname-$fullver-$_arch: failed to locate package file" skip_arches+=($_arch) continue 2 fi -- 1.7.8.1