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