[aur-general] TU Application: Alad Wenter

[Eli Schwartz]

On 09/10/2017 04:08 PM, Alad Wenter via aur-general wrote:
>> aurutils/aurutils-git:
>> - uses the ISC ("custom:ISC") license, which is not a common license
>>   (`pacman -Ql licenses`)and must therefore be installed separately.
>> - Thanks you kindly for upgrading the security of the *sums you SKIP
>>   from md5 to sha256 :p :p
>>
> Fixed the license array. The license was already installed by the
> Makefile.

o_O such upstreams!

> 
>> bash-devel-git:
>> mc-git:
>> nvtv:
>> repoctl-git:
>> vim-bracketed-paste:

Did you forget to push it?

>> cottage-git:
>> - Has an abomination of a pkgver(), you know how to do this properly -_-
>>   and upstream even has tags!!!
>> - clones #branch=master which is the default
>> - unconditionally defines options=('debug'), which is meant to be a
>>   makepkg.conf choice and uses a separate build target which just
>>   appends to CFLAGS the same way our DEBUG_CFLAGS do.
>> - again consider asking upstream to support DESTDIR.

lol I forgot to mention it the first time.

> Fixed. I agree about DESTDIR and filed a pull request upstream.
> 
> https://github.com/HarveyHunt/cottage/pull/9

FWIW, usually this gets implemented as
install:
    install -Dm755 progname $(DESTDIR)$(BINDIR)/progname

DESTDIR is limited to use in install since sometimes you will want to
compile the location of e.g. $(PREFIX)/share/progname/ inside a binary
and why move only some instances of $(DESTDIR) up top I guess.

>> datamash-git:
>> - pkgver() doesn't strip "v" from the start
>> - source is cloned over git://git.sv.gnu.org which redirects to
>>   git.savannah.gnu.org, also please clone over https:// as this verifies
>>   the server in addition to just encrypting the transport
>> - It seems bizarre that this makedepends on rsync and wget, the latter
>>   especially seems like some part of the build process attempts to
>>   download itself... I think it wants to bootstrap updated translations
>>   but this should still be done via source=().
>> - Do you even pacman hooks? Get rid of datamash.install
>>
> Fixed. The rsync usage for translation files seems hard-coded in the
> bootstrap script, so this is something to mention to upstream.
I'm still not sure it should be accessing the internet at all :p but I
suppose if it is only downloading .po files there isn't a lot of actual
harm it can do.

>> dpkg:
>> - git source at pinned commit should not re-clone itself to a new
>>   $pkgname-$pkgver every time you bump the pkgver
>> - autoreconf should be done in prepare()
>> - explicitly override options=(emptydirs) rather than depending on the
>>   user's choices in makepkg.conf
>>
> Fixed.
Broken, actually. options=(emptydirs) means keep empty dirs, and
options=(!emptydirs) means don't keep them (delete them).
The default is emptydirs, leaving them alone, but if someone modified
their makepkg.conf to specify !emptydirs then the directory you created
would be deleted during tidy_install. And now it will be deleted no
matter what.

>> xss:
>> - So much whitespace in the variables...
>> - Unversioned source xss-master.tar.gz
>> - None of it works because the upstream website is dead, everything
>>   redirects to https://sites.google.com/view/woozle/
>> - Author still exists at https://github.com/nealey, project has moved to
>>   https://github.com/9wm/xss (he is a member of that org)
>>
> Fixed. Regarding the source, I've asked for a relase upstream:
> 
> https://github.com/9wm/xss/issues/1

Regarding the source, nothing says you cannot use "$_commit" in place of
"$pkgver" and "master", at least until you have a release tag to use
instead.

> Thank you very much for your elaborate review!

Happy to help, reviewing PKGBUILDs is always fun whether as part of a TU
application or not.

-- 
Eli Schwartz

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20170910/f7aaa7eb/attachment.asc>