Break apart each of the blocks into their own separate functions. And,
instead of the hand crafted eval statements, reuse the logic from
pkgbuild-introspection[0] to abstract away the complexities of parsing
bash.

This commit fixes at least 3 bugs in check_sanity:

1) The wrong variable is shown for the error which would be thrown
when, e.g.  pkgname=('foopkg' 'bar^pkg')
2) The "arch" variable is not sanity checked when the PKGBUILD has
an arch override, but only one output package.
3) https://bugs.archlinux.org/task/40361

Lastly, there's some string changes here which should help to clarify
a few errors emitted in the linting process.

[0] https://github.com/falconindy/pkgbuild-introspection
---
 scripts/makepkg.sh.in | 406 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 299 insertions(+), 107 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 9ae1e95..e8cf60c 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -2190,18 +2190,103 @@ have_function() {
        declare -f "$1" >/dev/null
 }
 
-check_sanity() {
-       # check for no-no's in the build script
-       local i
-       local ret=0
-       for i in 'pkgname' 'pkgrel'; do
-               if [[ -z ${!i} ]]; then
-                       error "$(gettext "%s is not allowed to be empty.")" "$i"
-                       ret=1
-               fi
+array_build() {
+       local dest=$1 src=$2 i keys values
+
+       # it's an error to try to copy a value which doesn't exist.
+       declare -p "$2" &>/dev/null || return 1
+
+       # Build an array of the indicies of the source array.
+       eval "keys=(\"\${!$2[@]}\")"
+
+       # Read values indirectly via their index. This approach gives us support
+       # for associative arrays, sparse arrays, and empty strings as elements.
+       for i in "${keys[@]}"; do
+               values+=("printf -v '$dest[$i]' %s \"\${$src[$i]}\";")
        done
 
+       eval "${values[*]}"
+}
+
+funcgrep() {
+       { declare -f "$1" || declare -f package; } 2>/dev/null | grep -E "$2"
+}
+
+extract_global_var() {
+       # $1: variable name
+       # $2: multivalued
+       # $3: name of output var
+
+       local attr=$1 isarray=$2 outputvar=$3
+
+       if (( isarray )); then
+               array_build ref "$attr"
+               [[ ${ref[@]} ]] && array_build "$outputvar" "$attr"
+       else
+               [[ -v $attr ]] && printf -v "$outputvar" %s "${!attr}"
+       fi
+}
+
+extract_function_var() {
+       # $1: function name
+       # $2: variable name
+       # $3: multivalued
+       # $4: name of output var
+
+       local funcname=$1 attr=$2 isarray=$3 outputvar=$4 attr_regex= decl= r=1
+
+       if (( isarray )); then
+               printf -v attr_regex '^[[:space:]]*(declare( -[[:alpha:]])*)? 
%q\+?=\(' "$2"
+       else
+               printf -v attr_regex '^[[:space:]]*(declare( -[[:alpha:]])*)? 
%q\+?=[^(]' "$2"
+       fi
+
+       while read -r; do
+               # strip leading whitespace and any usage of declare
+               decl=${REPLY##*([[:space:]])?(declare +(-+([[:alpha:]]) ))}
+               eval "${decl/#$attr/$outputvar}"
+
+               # entering this loop at all means we found a match, so notify 
the caller.
+               r=0
+       done < <(funcgrep "$funcname" "$attr_regex")
+
+       return $r
+}
+
+pkgbuild_get_attribute() {
+       # $1: package name
+       # $2: attribute name
+       # $3: name of output var
+       # $4: multivalued
+
+       local pkgname=$1 attrname=$2 outputvar=$3 isarray=$4
+
+       printf -v "$outputvar" %s ''
+
+       if [[ $pkgname ]]; then
+               extract_global_var "$attrname" "$isarray" "$outputvar"
+               extract_function_var "package_$pkgname" "$attrname" "$isarray" 
"$outputvar"
+       else
+               extract_global_var "$attrname" "$isarray" "$outputvar"
+       fi
+}
+
+lint_pkgbase() {
+       if [[ ${pkgbase:0:1} = "-" ]]; then
+               error "$(gettext "%s is not allowed to start with a hyphen.")" 
"pkgname"
+               return 1
+       fi
+}
+
+lint_pkgname() {
+       local ret=0 i
+
        for i in "${pkgname[@]}"; do
+               if [[ -z $i ]]; then
+                       error "$(gettext "%s is not allowed to be empty.")" 
"pkgname"
+                       ret=1
+                       continue
+               fi
                if [[ ${i:0:1} = "-" ]]; then
                        error "$(gettext "%s is not allowed to start with a 
hyphen.")" "pkgname"
                        ret=1
@@ -2212,155 +2297,262 @@ check_sanity() {
                fi
                if [[ $i = *[^[:alnum:]+_.@-]* ]]; then
                        error "$(gettext "%s contains invalid characters: 
'%s'")" \
-                                       'pkgname' "${pkgname//[[:alnum:]+_.@-]}"
+                                       'pkgname' "${i//[[:alnum:]+_.@-]}"
                        ret=1
                fi
        done
 
-       if [[ ${pkgbase:0:1} = "-" ]]; then
-               error "$(gettext "%s is not allowed to start with a hyphen.")" 
"pkgbase"
-               ret=1
+       return $ret
+}
+
+lint_pkgrel() {
+       if [[ -z $pkgrel ]]; then
+               error "$(gettext "%s is not allowed to be empty.")" "pkgrel"
+               return 1
        fi
 
-       if (( ! PKGVERFUNC )) ; then
-               check_pkgver || ret=1
+       if [[ $pkgrel != +([0-9])?(.+([0-9])) ]]; then
+               error "$(gettext "%s must be a decimal, not %s.")" "pkgrel" 
"$pkgrel"
+               return 1
        fi
+}
 
-       awk -F'=' '$1 ~ /^[[:space:]]*pkgrel$/' "$BUILDFILE" | sed 
"s/[[:space:]]*#.*//" |
-       while IFS='=' read -r _ i; do
-               eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<< 
"${i%%+([[:space:]])}")\"
-               if [[ $i != +([0-9])?(.+([0-9])) ]]; then
-                       error "$(gettext "%s must be a decimal.")" "pkgrel"
-                       return 1
-               fi
-       done || ret=1
+lint_pkgver() {
+       if (( PKGVERFUNC )); then
+               # defer check to after getting version from pkgver function
+               return 0
+       fi
 
-       awk -F'=' '$1 ~ /^[[:space:]]*epoch$/' "$BUILDFILE" |
-       while IFS='=' read -r _ i; do
-               eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<< 
"${i%%+([[:space:]])}")\"
-               if [[ $i != *([[:digit:]]) ]]; then
-                       error "$(gettext "%s must be an integer.")" "epoch"
-                       return 1
-               fi
-       done || ret=1
+       check_pkgver
+}
+
+lint_epoch() {
+       if [[ $epoch != *([[:digit:]]) ]]; then
+               error "$(gettext "%s must be an integer, not %s.")" "epoch" 
"$epoch"
+               return 1
+       fi
+}
 
-       if [[ $arch != 'any' ]]; then
-               if ! in_array $CARCH "${arch[@]}"; then
+lint_arch() {
+       local name list=("${@:-"${arch[@]}"}")
+
+       if [[ $list == 'any' ]]; then
+               return 0
+       fi
+
+       if ! in_array "$CARCH" "${list[@]}" && (( ! IGNOREARCH )); then
+               error "$(gettext "%s is not available for the '%s' 
architecture.")" "$pkgbase" "$CARCH"
+               plain "$(gettext "Note that many packages may need a line added 
to their %s")" "$BUILDSCRIPT"
+               plain "$(gettext "such as %s.")" "arch=('$CARCH')"
+               return 1
+       fi
+
+       for name in "${pkgname[@]}"; do
+               pkgbuild_get_attribute "$name" 'arch' list 1
+               if [[ $list && $list != 'any' ]] && ! in_array $CARCH 
"${list[@]}"; then
                        if (( ! IGNOREARCH )); then
-                               error "$(gettext "%s is not available for the 
'%s' architecture.")" "$pkgbase" "$CARCH"
-                               plain "$(gettext "Note that many packages may 
need a line added to their %s")" "$BUILDSCRIPT"
-                               plain "$(gettext "such as %s.")" 
"arch=('$CARCH')"
+                               error "$(gettext "%s is not available for the 
'%s' architecture.")" "$name" "$CARCH"
                                ret=1
                        fi
                fi
-       fi
+       done
+}
 
-       if (( ${#pkgname[@]} > 1 )); then
-               for i in ${pkgname[@]}; do
-                       local arch_list=""
-                       eval $(declare -f package_${i} | sed -n 
's/\(^[[:space:]]*arch=\)/arch_list=/p')
-                       if [[ ${arch_list[@]} && ${arch_list} != 'any' ]]; then
-                               if ! in_array $CARCH "${arch_list[@]}"; then
-                                       if (( ! IGNOREARCH )); then
-                                               error "$(gettext "%s is not 
available for the '%s' architecture.")" "$i" "$CARCH"
-                                               ret=1
-                                       fi
-                               fi
-                       fi
-               done
-       fi
+lint_provides() {
+       local list name provides_list ret=0
+
+       provides_list=("${provides[@]}")
+       for name in "${pkgname[@]}"; do
+               if extract_function_var "package_$name" provides 1 list; then
+                       provides_list+=("${list[@]}")
+               fi
+       done
 
-       local provides_list=()
-       eval $(awk '/^[[:space:]]*provides=/,/\)/' "$BUILDFILE" | \
-               sed -e "s/provides=/provides_list+=/" -e "s/#.*//" -e 's/\\$//')
-       for i in ${provides_list[@]}; do
-               if [[ $i == *['<>']* ]]; then
+       for provide in "${provides_list[@]}"; do
+               if [[ $provide == *['<>']* ]]; then
                        error "$(gettext "%s array cannot contain comparison (< 
or >) operators.")" "provides"
                        ret=1
                fi
        done
 
-       local backup_list=()
-       eval $(awk '/^[[:space:]]*backup=/,/\)/' "$BUILDFILE" | \
-               sed -e "s/backup=/backup_list+=/" -e "s/#.*//" -e 's/\\$//')
-       for i in "${backup_list[@]}"; do
-               if [[ ${i:0:1} = "/" ]]; then
-                       error "$(gettext "%s entry should not contain leading 
slash : %s")" "backup" "$i"
+       return $ret
+}
+
+lint_backup() {
+       local list name backup_list ret=0
+
+       backup_list=("${backup[@]}")
+       for name in "${pkgname[@]}"; do
+               if extract_function_var "package_$name" backup 1 list; then
+                       backup_list+=("${list[@]}")
+               fi
+       done
+
+       for name in "${backup_list[@]}"; do
+               if [[ ${name:0:1} = "/" ]]; then
+                       error "$(gettext "%s entry should not contain leading 
slash : %s")" "backup" "$name"
                        ret=1
                fi
        done
 
-       local optdepends_list=()
-       eval $(awk '/^[[:space:]]*optdepends=\(/,/\)[[:space:]]*(#.*)?$/' 
"$BUILDFILE" | \
-               sed -e "s/optdepends=/optdepends_list+=/" -e "s/#.*//" -e 
's/\\$//')
-       for i in "${optdepends_list[@]}"; do
-               local pkg=${i%%:[[:space:]]*}
+       return $ret
+}
+
+lint_optdepends() {
+       local list name optdepends_list ret=0
+
+       optdepends_list=("${optdepends[@]}")
+       for name in "${pkgname[@]}"; do
+               if extract_function_var "package_$name" optdepends 1 list; then
+                       optdepends_list+=("${list[@]}")
+               fi
+       done
+
+       for name in "${optdepends_list[@]}"; do
+               local pkg=${name%%:[[:space:]]*}
                # the '-' character _must_ be first or last in the character 
range
                if [[ $pkg != +([-[:alnum:]><=.+_:]) ]]; then
-                       error "$(gettext "Invalid syntax for %s : '%s'")" 
"optdepend" "$i"
+                       error "$(gettext "Invalid syntax for %s: '%s'")" 
"optdepend" "$name"
                        ret=1
                fi
        done
 
-       for i in 'changelog' 'install'; do
-               local file
-               while read -r file; do
-                       # evaluate any bash variables used
-                       eval file=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<< 
"$file")\"
-                       if [[ $file && ! -f $file ]]; then
-                               error "$(gettext "%s file (%s) does not 
exist.")" "$i" "$file"
-                               ret=1
-                       fi
-               done < <(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE")
+       return $ret
+}
+
+check_files_exist() {
+       local kind=$1 files=("${@:2}") file ret
+
+       for file in "${files[@]}"; do
+               if [[ $file && ! -f $file ]]; then
+                       error "$(gettext "%s file (%s) does not exist or is not 
a regular file.")" \
+                               "$kind" "$file"
+                       ret=1
+               fi
+       done
+
+       return $ret
+}
+
+lint_install() {
+       local list file name install_list ret=0
+
+       install_list=("${install[@]}")
+       for name in "${pkgname[@]}"; do
+               extract_function_var "package_$name" install 0 file
+               install_list+=("$file")
+       done
+
+       check_files_exist 'install' "${install_list[@]}"
+}
+
+lint_changelog() {
+       local list name file changelog_list ret=0
+
+       changelog_list=("${changelog[@]}")
+       for name in "${pkgname[@]}"; do
+               if extract_function_var "package_$name" changelog 0 file; then
+                       changelog_list+=("$file")
+               fi
+       done
+
+       check_files_exist 'changelog' "${changelog_list[@]}"
+}
+
+lint_options() {
+       local ret=0 list name kopt options_list
+
+       options_list=("${options[@]}")
+       for name in "${pkgname[@]}"; do
+               if extract_function_var "package_$name" options 1 list; then
+                       options_list+=("${list[@]}")
+               fi
        done
 
-       local valid_options=1
-       local known kopt options_list
-       eval $(awk '/^[[:space:]]*options=/,/\)/' "$BUILDFILE" | \
-               sed -e "s/options=/options_list+=/" -e "s/#.*//" -e 's/\\$//')
-       for i in ${options_list[@]}; do
-               known=0
+       for i in "${options_list[@]}"; do
                # check if option matches a known option or its inverse
-               for kopt in ${packaging_options[@]} ${other_options[@]}; do
-                       if [[ ${i} = "${kopt}" || ${i} = "!${kopt}" ]]; then
-                               known=1
+               for kopt in "${packaging_options[@]}" "${other_options[@]}"; do
+                       if [[ $i = "$kopt" || $i = "!$kopt" ]]; then
+                               # continue to the next $i
+                               continue 2
                        fi
                done
-               if (( ! known )); then
-                       error "$(gettext "%s array contains unknown option 
'%s'")" "options" "$i"
-                       valid_options=0
-               fi
-       done
-       if (( ! valid_options )); then
+
+               error "$(gettext "%s array contains unknown option '%s'")" 
"options" "$i"
                ret=1
+       done
+
+       return $ret
+}
+
+lint_source() {
+       local idx=("${!source[@]}")
+
+       if (( ${#source[*]} > 0 && (idx[-1] + 1) != ${#source[*]} )); then
+               error "$(gettext "Sparse arrays are not allowed for source")"
+               return 1
        fi
+}
+
+lint_pkglist() {
+       local i ret=0
+
+       for i in "${PKGLIST[@]}"; do
+               if ! in_array "$i" "${pkgname[@]}"; then
+                       error "$(gettext "Requested package %s is not provided 
in %s")" "$i" "$BUILDFILE"
+                       ret=1
+               fi
+       done
+
+       return $ret
+}
+
+lint_packagefn() {
+       local i ret=0
 
        if (( ${#pkgname[@]} == 1 )); then
-               if have_function build && ! ( have_function package || 
have_function package_${pkgname}); then
+               if have_function 'build' && ! { have_function 'package' || 
have_function "package_$pkgname"; }; then
                        error "$(gettext "Missing %s function in %s")" 
"package()" "$BUILDFILE"
                        ret=1
                fi
        else
-               for i in ${pkgname[@]}; do
-                       if ! have_function package_${i}; then
+               for i in "${pkgname[@]}"; do
+                       if ! have_function "package_$i"; then
                                error "$(gettext "Missing %s function for split 
package '%s'")" "package_$i()" "$i"
                                ret=1
                        fi
                done
        fi
 
-       for i in ${PKGLIST[@]}; do
-               if ! in_array $i ${pkgname[@]}; then
-                       error "$(gettext "Requested package %s is not provided 
in %s")" "$i" "$BUILDFILE"
-                       ret=1
-               fi
-       done
+       return $ret
+}
 
-       local idx=("${!source[@]}")
-       if (( ${#source[*]} > 0 && (idx[-1] + 1) != ${#source[*]} )); then
-               error "$(gettext "Sparse arrays are not allowed for source")"
-               ret=1
-       fi
+check_sanity() {
+       # check for no-no's in the build script
+       local ret=0
+       local lintfn lint_functions
+
+       lint_functions=(
+               lint_pkgbase
+               lint_pkgname
+               lint_pkgver
+               lint_pkgrel
+               lint_epoch
+               lint_arch
+               lint_provides
+               lint_backup
+               lint_optdepends
+               lint_changelog
+               lint_install
+               lint_options
+               lint_packagefn
+               lint_pkglist
+               lint_source
+       )
+
+       for lintfn in "${lint_functions[@]}"; do
+               "$lintfn" || ret=1
+       done
 
        return $ret
 }
-- 
2.0.4

Reply via email to