[pacman-dev] [PATCH 2/2] lint_pkgbuild: increase robustness

[David Grayson]

It looks like this hasn't been committed yet.  Would it help if my
made a version of the patch with the improvements I mentioned?

--David

On Thu, Nov 12, 2015 at 9:33 PM, David Grayson <davidegrayson at gmail.com> wrote:
> Allan: Thank you for your work on this issue.  I tried these two patches and now pacman doesn't seem to suffer from the "--with-arch=${_arch}" false positive that originally made me look into this.   This is definitely an improvement.
>
> It looks like you are both checking the variable's current status (like my patch does) and you are also using grep to check variables defined inside the package functions.
>
> Your code for checking whether a variable is defined as an array seems pretty complicated and I'm not sure why it was written that way.  The following line at the top of a PKGBUILD will trigger a false positive because of the way grep is used:
>
>   pkgdesc="declare -a"
>
> The solution I used in my patch was to require that "declare -a" appears the beginning of the output from declare.  You could do that with grep, but bash has a built-in feature for that too:
>
>>  if [[ "$(declare -p $i)" == "declare -a "* ]]; then
>
> Also, just a minor comment: the variables "a", "v", and "pkg" should probably local.
>
> --David
>
>
> On 11/12/2015 7:57 PM, Allan McRae wrote:
>> Approach the detection of variables of the wrong type using an approach
>> similar to that used for construction of .SRCINFO files. While doing silly
>> things in bash could still result in false negatives, this approach should
>> be very robust to generatinf false positives results.
>>
>> Signed-off-by: Allan McRae <allan at archlinux.org>
>> ---
>>  scripts/libmakepkg/lint_pkgbuild/variable.sh.in | 67 ++++++++++++++++++++-----
>>  1 file changed, 55 insertions(+), 12 deletions(-)
>>
>> diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
>> index 1daac26..80ef895 100644
>> --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
>> +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
>> @@ -24,7 +24,7 @@ LIBMAKEPKG_LINT_PKGBUILD_VARIABLE_SH=1
>>  LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
>>
>>  source "$LIBRARY/util/message.sh"
>> -
>> +source "$LIBRARY/util/pkgbuild.sh"
>>
>>  lint_pkgbuild_functions+=('lint_variable')
>>
>> @@ -37,11 +37,16 @@ lint_variable() {
>>                         replaces sha1sums sha256sums sha384sums sha512sums source)
>>       local string=(changelog epoch install pkgdesc pkgrel pkgver url)
>>
>> -     local i
>> -     for i in ${array[@]}; do
>> -             if grep -w -q -e "$i=[^(]" -e "$i+=[^(]" "$BUILDSCRIPT"; then
>> -                     error "$(gettext "%s should be an array")" "$i"
>> -                     ret=1
>> +     local i keys out bad
>> +
>> +     # global variables
>> +     for i in ${array[@]} ${arch_array[@]}; do
>> +             eval "keys=(\"\${!$i[@]}\")"
>> +             if (( ${#keys[*]} > 0 )); then
>> +                             if ! declare -p $i | grep -q "declare -a"; then
>> +                             error "$(gettext "%s should be an array")" "$i"
>> +                             ret=1
>> +                     fi
>>               fi
>>       done
>>
>> @@ -49,17 +54,55 @@ lint_variable() {
>>               [[ $a == "any" ]] && continue
>>
>>               for i in ${arch_array[@]}; do
>> -                     if grep -w -q -e "$i_$a=[^(]" -e "$i_$a+=[^(]" "$BUILDSCRIPT"; then
>> -                             error "$(gettext "%s_%s should be an array")" "$i" "$a"
>> -                             ret=1
>> +                     v="${i}_${a}"
>> +                     eval "keys=(\"\${!${v}[@]}\")"
>> +                     if (( ${#keys[*]} > 0 )); then
>> +                             if ! declare -p ${v} | grep -q "declare -a"; then
>> +                                     error "$(gettext "%s_%s should be an array")" "$i" "$a"
>> +                                     ret=1
>> +                             fi
>>                       fi
>>               done
>>       done
>>
>>       for i in ${string[@]}; do
>> -             if grep -w -q -e "$i=(" -e "$i+=(" "$BUILDSCRIPT"; then
>> -                     error "$(gettext "%s should not be an array")" "$i"
>> -                     ret=1
>> +             eval "keys=(\"\${!$i[@]}\")"
>> +             if (( ${#keys[*]} > 0 )); then
>> +                     if declare -p $i | grep -q "declare -a"; then
>> +                             error "$(gettext "%s should not be an array")" "$i"
>> +                             ret=1
>> +                     fi
>>               fi
>>       done
>> +
>> +     # package function variables
>> +     for pkg in ${pkgname[@]}; do
>> +             for i in ${array[@]} ${arch_array[@]}; do
>> +                     if extract_function_variable "package_$pkg" $i 0 out; then
>> +                             error "$(gettext "%s should be an array")" "$i"
>> +                             ret=1
>> +                     fi
>> +             done
>> +
>> +             for a in ${arch[@]}; do
>> +                     [[ $a == "any" ]] && continue
>> +
>> +                     for i in ${arch_array[@]}; do
>> +                             if extract_function_variable "package_$pkg" "${i}_${a}" 0 out; then
>> +                                     error "$(gettext "%s_%s should be an array")" "$i" "$a"
>> +                                     ret=1
>> +                             fi
>> +                     done
>> +             done
>> +
>> +             for i in ${string[@]}; do
>> +                     if extract_function_variable "package_$pkg" $i 1 out; then
>> +                             error "$(gettext "%s should not be an array")" "$i"
>> +                             ret=1
>> +                     fi
>> +             done
>> +     done
>> +
>> +
>> +
>>  }
>>
>