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

[David Grayson]

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
> +
> +
> +
>  }
>