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

[Dave Reisner]

On Fri, Aug 7, 2015 at 11:08 AM, Allan McRae <allan at archlinux.org> wrote:

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

Ah, right. But, maybe this *should* be checked. For example:

  depends=(foo bar)
  depends+=baz

Almost certainly does *not* do what you want it to, resulting in a depends
array of:

  depends=(foobaz bar)


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


The behavior might not be documented in PKGBUILD(5), but it's an explicitly
tested case for .SRCINFO:

https://github.com/falconindy/pkgbuild-introspection/blob/master/test/testcases.py#L65

Seems to work the same for .PKGINFO (though we know that makepkg -s will
not find it).