[pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array

[Allan McRae]

On 21/03/15 10:19, joyfulgirl at archlinux.us wrote:
> From: Ivy Foster <joyfulgirl at archlinux.us>
> 
> Signed-off-by: Ivy Foster <joyfulgirl at archlinux.us>
> ---
> v3: Separate change to get_pkg_arch into separate commit
>  scripts/makepkg.sh.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>  mode change 100644 => 100755 scripts/makepkg.sh.in
> 

This took me a whole lot of reviewing for a single character change! I
only knew that this could potentially break something on the basis of
skimming IRC chat a while ago.  So I tested...

pkgname=f
arch=('any')

package_f() {
	arch='x86_64'           #1
	arch=('x86_64')         #2
	arch=('i686 'x86_64')   #3

}

for the --packagelist patch, currently #1 works and #2 and #3 fail.
After the patch, #2 and #3 succeed and #1 fails.

All our documentation says that arch should be an array, so the patch is
correct.  In fact, the .SRCINFO generating code uses the array version
of pkgbuild_get_attribute, and so "fails" when the arch variable is not
an array.

This issue was never exposed because we don't use get_pkg_arch with a
parameter very often, and I guess no-one has looked at a .SRCINFO file
from a bad PKGBUILD before.

TODO: We should probably add a PKGBUILD lint function that confirms the
fields we expect to be arrays are in fact arrays. And that those that
are not arrays are not arrays.

tl;dr - Ack.  I'll expand the commit message.


> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> old mode 100644
> new mode 100755
> index 168f334..5b3bffd
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -879,7 +879,7 @@ get_pkg_arch() {
>  		fi
>  	else
>  		local arch_override
> -		pkgbuild_get_attribute "$1" arch 0 arch_override
> +		pkgbuild_get_attribute "$1" arch 1 arch_override
>  		(( ${#arch_override[@]} == 0 )) && arch_override=("${arch[@]}")
>  		if [[ $arch_override = "any" ]]; then
>  			printf "%s\n" "any"
>