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 <[email protected]>
---
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
+                       warning "Skipping $_pkgname-$fullver-$_arch: failed to 
locate package file"
                        skip_arches+=($_arch)
                        continue 2
                fi
-- 
1.7.8.1

Reply via email to