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

[Dave Reisner]

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() {
  ...
}


> > $BUILDSCRIPT must be quoted.
> >
> > You also want word boundaries on the regex such that we don't throw
> > errors like "depends should be an array" given the fragment:
> >
> >   optdepends="foo"
> >   depends=("bar" "baz")
>
> Good point (I ran into this...)
>
>
> A
>