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

On 21/03/15 10:19, joyfulgirl@archlinux.us wrote:

From: Ivy Foster

Signed-off-by: Ivy Foster --- 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"