On Sun, Oct 19, 2014 at 09:28:12PM +1000, Allan McRae wrote:
Having been forced to watch this by my daughter, I still do not understand the appeal of ponies...
I don't even care about the show anymore, I just like using ponies for test data (*especially* at work).
Having not even looked at your patch at this stage... Do we detect when this fails? Could we detect it at the .PKGINFO construction stage and print a warning?
Not sure if this is a typo. There won't be a .PKGINFO built at the same time as a .SRCINFO so we can't diff them in any way. We could possibly detect it at .SRCINFO construction time for some subset of the failures. My take on parsing problems: There's a bunch of unit tests which highlight known features and deficiencies (see the expectedFailure decorated methods): https://github.com/falconindy/pkgbuild-introspection/blob/master/test/testca... There's probably others. I think only HandlesShellVarInPackageAttr is worth putting effort into fixing/detecting. We can detect *some* cases of it by calling out empty attributes. As for the rest -- we can largely "fix" these by shaming people and asking for patches and/or more tests. Speaking of tests, I have to remind you about this patch (or at least the idea of this patch): https://lists.archlinux.org/pipermail/pacman-dev/2014-August/019261.html Merging something like that would allow me to at least move the existing tests that I've written against this code into the pacman repo. We'd then be in a much better position to accept patches against the SRCINFO generation code and be less afraid of regressions.
This was something that had been flagged as potential for inclusion anyway. And I want this in for what I have planned post release...
My usual test PKGBUILD:
PKGBUILD: pkgname='foobar' pkgver=1 pkgrel=1 arch=('i686' 'x86_64') source=('tmp.txt') source_x86_64+=('foo.bar') md5sums=('d41d8cd98f00b204e9800998ecf8427e') md5sums_x86_64=('d41d8cd98f00b204e9800998ecf8427e')
.SRCINFO # Generated by makepkg 4.1.2-468-gf74e # Sun Oct 19 11:22:00 UTC 2014 pkgbase = foobar pkgver = 1 pkgrel = 1 epoch = 0 arch = i686 arch = x86_64 checkdepends = makedepends = depends = optdepends = provides = conflicts = replaces = source = tmp.txt md5sums = d41d8cd98f00b204e9800998ecf8427e source_x86_64 = foo.bar
pkgname = foobar
So there are some bugs: - epoch = 0 (should be not printed)
I didn't see this in mkaurball because it operates in a much cleaner environment. makepkg explicitly does this: # set defaults if they weren't specified in buildfile pkgbase=${pkgbase:-${pkgname[0]}} epoch=${epoch:-0} basever=$(get_full_version) ...which is where an epoch of 0 comes from. I'm sure I can fix this, but I'm punting it to another patch.
- Lots of empty values
Porting error -- just needed to localize a var in extract_global_var.
- No md5sum_x86_64
Missing from the arch-specific list. An oversight that exists in mkaurball, too!
Otherwise, all good. Small query:
+ +srcinfo_write_attr() { + # $1: attr name + # $2: attr values + + local attrname=$1 attrvalues=("${@:2}") + + # normalize whitespace, strip leading and trailing + attrvalues=("${attrvalues[@]//+([[:space:]])/ }") + attrvalues=("${attrvalues[@]#[[:space:]]}") + attrvalues=("${attrvalues[@]%[[:space:]]}")
Make all whitespace a single space then strip whitespace from start and finish? What is the first step for?
We do something similar for .PKGINFO (but without steps 2 and 3 which we should probably do there, too). Consider some busted ass shit like this: pkgdesc=" ponies of the world unite!!! " Step 1 normalizes the whitespace, yielding this: pkgdesc=" ponies of the world unite!!! " Steps 2/3 get rid of the remaining leading/trailing. I've thought about making this a lint error, but I think it's a bit too esoteric. Let's just clean it up for the user. d