[pacman-dev] [PATCH] Fix the implementation of lint_variable.

[Allan McRae]

On 13/11/15 03:12, David Grayson wrote:
> On Thu, Nov 12, 2015 at 1:28 AM, Allan McRae <allan at archlinux.org> 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