On Sun, Sep 10, 2017 at 04:55:16PM +0000, Eli Schwartz wrote:
On 09/10/2017 09:16 AM, Alad Wenter via aur-general wrote:
Hello everyone,
My name is Alad Wenter, and I would like to apply for a position as Trusted User. I’m a student in Germany, majoring in Mathematics with a focus on Algebraic Topology. Many thanks to Johannes Löthberg who is sponsoring my TU application.
Somme comments on your AUR packages:
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.
bash-devel-git: - You do not follow our repo bash package in defining, as per FS#50567, -DNON_INTERACTIVE_LOGIN_SHELLS - Apparently this doesn't work with the system readline or something??? - system.bashrc is out of sync with the core/bash package - Do you even pacman hooks? Get rid of bash.install
Fixed.
cottage: - source tarball violates shared SRCDEST, v$pkgver.tar.gz clashes with other packages that have the same version.
Fixed.
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.
Fixed. I agree about DESTDIR and filed a pull request upstream. https://github.com/HarveyHunt/cottage/pull/9
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.
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.
howm-x11: - source tarball violates shared SRCDEST, v$pkgver.tar.gz clashes with other packages that have the same version. - Makefile has install target. - Those find commands are awkward, `mkdir -p ... && cp -a ...` is not a sin so please use it. - So much whitespace separating variables... - That install script depresses me, shouldn't users be expected to determine for themselves how to use this?
Fixed. The Makefile missed the -D argument to install, so I've filed a pull request upstream. https://github.com/HarveyHunt/howm/pull/67
howm-x11-git: - Basically just the same things mentioned for howm-x11 or cottage-git.
Fixed apart from the Makefile, which actually does not include the correct install target for debug builds... https://github.com/HarveyHunt/howm/issues/68
jshon-git: - pkgver should use tags via `git describe --tags`
Fixed.
kkrieger: - Downloads an unversioned kkrieger-beta.zip -- I understand this is web.archive.org that will never be updated but it is still painful. :D - kkrieger.jpg is a 404 not found, also you never even try to do anything with it...
Fixed.
lightspark-git: - Do you even pacman hooks? Get rid of lightspark-git.install - Depends on desktop-file-utils, not needed because of hooks, and curl, arguably not needed as it's a dependency of pacman among other things.
Fixed.
mc-git: - autogen should really be done in prepare()
Fixed.
nvtv: - Empty variables should be deleted. - md5sums at the bottom of the PKGBUILD are weird, move this up with the other variables, like source=() - You really need quilt for this? - make prefix="$pkgdir/usr" install seems like it should be ./configure --prefix=/usr && make DESTDIR since this is after all autotools and if there is one decent thing about autotools it is the fact that DESTDIR can be essentially guaranteed to exist...
Fixed, though for now still using quilt. Since it's a debian patch package, using quilt seemed the correct tool and didn't require me to use an own ad-hoc method.
repoctl-git: - pkgver() does not strip leading "v" - Why does this depend on xz, which is a dependency of lots of core things e.g. libarchive, and unlikely to be explicitly needed vs. the many other compression formats makepkg/repo-add supports? - Why !strip, does this package actually break if you try stripping it?
There's several upstream files that depend explicitely on xz: https://github.com/cassava/repoctl/blob/1a140a3f928bfc69a2ec2e3a4bc9e033bee7... It's however true that the dependency is already provided by pacman, so I've removed it. Also removed !strip and fixed pkgver.
thunar-git: - autogen should be done in prepare()
Fixed.
vim-bracketed-paste: - The source is a github master.zip, I'm not even sure how to properly express my disappointment. It should be a -git package since there are no releases.
Fixed to use pkgver. I've however requested to delete the package, due to related commits vim upstream and frequent issues with tmux.
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 Some improvements regarding the Makefile should also be made.
I have opened a repository with modifications from the original AUR packages to better suit them as candidates for [community]. [8]
physlock: - source tarball violates shared SRCDEST, v$pkgver.tar.gz clashes with other packages that have the same version.
Fixed.
polkit-explorer: - Since I see you already filed one pull request upstream for PEP 394, why not also contribute a decent setup.py and that desktop file... - uses the ISC ("custom:ISC") license, which is not a common license (`pacman -Ql licenses`)and must therefore be installed separately.
Fixed the license array; the license file itself was already installed to /usr/share/licenses/polkit-explorer. I agree on contributing a setup.py.
python-i3-py: - It is probably not important to point out in the description which programming language it uses, especially when the pkgname already includes that info. - git source at pinned commit should not re-clone itself to a new $pkgname-$pkgver every time you bump the pkgver - Python packages which are intended to install a command-line tool rather than a library should not be prefixed with python- and do not need to be installed for both Python 3 and Python 2.
Fixed the description and the git source. Regarding the command-line tool, the package is meant as a library, but the repository ships some examples which can be used as command-line tools. I've moved them to a separate "-examples" split package.
qpdfview: - Does this really need desktop-file-utils and hicolor-icon-theme or was this a remnant of some pre-hooks install script?q
Fixed.
-- Eli Schwartz
Thank you very much for your elaborate review! Alad