On Mon, Aug 04, 2014 at 03:48:38PM +1000, Allan McRae wrote:
On 03/08/14 23:43, Allan McRae wrote:
On 03/08/14 22:54, Dave Reisner wrote:
On Sun, Aug 03, 2014 at 06:39:54PM +1000, Allan McRae wrote:
On 03/08/14 01:54, Dave Reisner wrote:
Break apart each of the blocks into their own separate functions. And, instead of the hand crafted eval statements, reuse the logic from pkgbuild-introspection[0] to abstract away the complexities of parsing bash.
This commit fixes at least 3 bugs in check_sanity, and one other:
Can you clarify these bugs for me?
1) The wrong variable is shown for the error which would be thrown when, e.g. pkgname=('foopkg' 'bar@pkg')
What error? This pkgname actually works here...
Sorry, I found this error my static analysis, and then randomly chose a name which turns out to be valid (I recall we made an exception for @). If you have something like pkgname=('foopkg' 'bar^pkg') you'll get the error:
==> ERROR: pkgname contains invalid characters: ''
Whereas, if you use pkgname=('bar^pkg' 'foopkg'), or apply this patch, you'll see:
==> ERROR: pkgname contains invalid characters: '^'
2) The "arch" variable is not sanity checked when the PKGBUILD has package_$pkgname() instead of package().
Do you mean with an override in package_$pkgname()?
Actually, it seems to be the case that arch is never sanity checked at all for a singular package -- has nothing to do with how the package function is named. e.g.:
pkgname=foo arch=('i686' 'x86_64') ...
package_foo() { # or name this package() arch=('invalid')
... }
This should explode, but doesn't, and happily makes a package for x86_64 (since my host is x86_64). After patch:
==> ERROR: foo is not available for the 'x86_64' architecture.
Ah... I see that check_sanity tested if the number of packages > 1 because at that stage we have not tested whether package_foo() or package() is used. And now also see we can override stuff in package(), so the #packages > 1 check was useless.
I'll take a look at the patch itself tomorrow.
Patch looks good. Apart from the removing pkver/pkgrel/epoch override checks. I'd suggest just removing this first (it is a trivial patch, remove it from the override array, adjust get_full_version, and check_sanity), and then adjusting this patch.
Will do.
I also assume this fixes FS#40361. Can you add a note of that in the commit message.
Yes it should. I'll update the commit message and rebase my branch.