[pacman-dev] [PATCH] libmakepkg: check if PKGBUILD variables are arrays or not as appropriate

[Allan McRae]

On 07/08/15 16:23, Dave Reisner wrote:
> On Fri, Aug 7, 2015 at 6:50 AM, Allan McRae <allan at archlinux.org> wrote:
> 
>> On 22/07/15 01:33, Dave Reisner wrote:
>>> On Mon, Jul 20, 2015 at 03:52:43PM +1000, Allan McRae wrote:
>>>> When extracting variables from PKGBUILD (e.g. for .SRCINFO creation) we
>> make
>>>> assumptions about whether variables are arrays or not.  This adds a
>> check to
>>>> the PKGBUILD linter to ensure variables are arrays or not as
>> appropriate.
>>>>
>>>> Signed-off-by: Allan McRae <allan at archlinux.org>
>>>> ---
>>>>  scripts/Makefile.am                          |  1 +
>>>>  scripts/libmakepkg/lint_pkgbuild/array.sh.in | 62
>> ++++++++++++++++++++++++++++
>>>
>>> Pedantic, but I'm not sure I like the name of the check. It isn't
>>> strictly about arrays. Maybe decls.sh ? structure.sh ?
>>>
>>
>> It is checking if variables are arrays or not arrays.  Anyway,
>> variables.sh?
>>
>>
> Sure, sounds fine. My concern is that maybe in the future this extends to
> something more than just being about arrays, and the name "array" make it
> sounds more like functions which manipulate/build arrays more than linting
> the structure of variables.
> 
> 
>> <snip>
>>
>>>
>>>> +            if grep -q "$i=[^(]" $BUILDSCRIPT; then
>>>
>>> If you have some sort of logic which appends to an array, you'll get a
>>> false positive here. We allow this elsewhere.
>>>
>>
>> I am confused by this comment.   Can you give an example of what could
>> cause an issue?
>>
>>
> ...
> depends=('bar')
> ...
> 
> package_foo() {
>   depends+=('bar')
> }
> 
> package_bar() {
>   ...
> }
> 

But that will not be matched by "$i=(" or "$i=[^(]", so there is no
issue there, apart from those not being checked.

(aside: we should play a game of guess the dependencies of foo() with
that example!)

Allan