On 25/03/15 14:39, Allan McRae wrote:
> On 21/03/15 10:19, joyfulg...@archlinux.us wrote:
>> From: Ivy Foster <joyfulg...@archlinux.us>
>>
>> Signed-off-by: Ivy Foster <joyfulg...@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

Also - BAD!  file permission change.

>>
> 
> 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"
>>
> 
> 

Reply via email to