On 13/11/15 03:12, David Grayson wrote:
> On Thu, Nov 12, 2015 at 1:28 AM, Allan McRae <[email protected]> wrote:
>>> This does not catch bad variables in the package() arrays.
>
> Thank you for the feedback, Allan! I forgot that variables can be redefined
> in the package() function and could be defined to bad types at that time.
> How about we use my original patch but add a call to lint_variable right
> after package (or similar) is called? (Also I should fix it to use tabs as
> indentation.) The downside is that someone might build a giant package and
> then later find out that it fails linting at the very last step, but I think
> they could use "pacman --repackage" if they want to save time, and I think
> they would also forgive you for not catching errors in code that has not
> executed yet.
>
> If that sounds good to you, I will work on a new patch that does that.
>
Nope - erroring out during package() is a bad idea. These errors need
to be detected at the start.
Note there is code for extracting features from the PKGBUILD used to
make .SRCINFO files already there. It has issues when arrays are not
arrays etc, which is why I added this check. Anyway, maybe we can
utilize that instead.
>
>> + if grep -q -e "^[[:space:]]*$i=[^(]" -e
>> "^[[:space:]]*$i+=[^(]" "$BUILDSCRIPT"; then
>
> Your patch violate's pacman's 80-character line length convention, and it got
> wrapped by your mail client. I tested your new patch, and it does fix the
> bugs I was complaining about in my original message, but there are still
> false positives and false negatives.
>
> False positives:
>
> make \
> arch=${_arch}
>
> False negative:
>
> eval lic""ense=bad
>
> Of course, these are contrived cases that I made up.
I'm sure that last one is genuine! I'm not too worried about the false
negative, but any false positive is an issue.
A