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

[Allan McRae]

On 13/11/15 11:33, Allan McRae wrote:
> 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.
> 

Try the new patches I have just submitted (or use my patchqueue branch).
 They should be very robust to false positives, and only make false
negatives in silly cases...

Allan