[aur-general] Review request, jedit-pkgbuild-edit-mode

[Eli Schwartz]

On 07/13/2017 06:54 PM, Vitaliy Berdinskikh via aur-general wrote:
> Hi all,
> 
> this package adds to jedit new edit mode for PKGBUILD file. It needs to
> highlight syntax.
> 
> PKGBUILD
> ++++++
> # Maintainer: Vitaliy Berdinskikh <ur6lad at gmail dot com>
> pkgname=jedit-pkgbuild-edit-mode
> pkgver=1
> pkgrel=1
> pkgdesc='jEdit PKGBUILD edit mode'
> arch=('any')
> url='https://bitbucket.org/ur6lad/jedit-pkgbuild'
> license=('GPL')
> depends=('jedit')
> makedepends=('xmlstarlet')
> install=jedit-pkgbuild-edit-mode.install
> source=('catalog.dtd', 'pkgbuild.xml')

You should download a versioned release tarball from bitbucket, since it
is forbidden to bundle a package's source code in the AUR.

> md5sums=()

checksums are not optional, the PKGBUILD you posted will not even run.
Also consider using something a little less ancient, like sha256sums

> package() {
> mkdir -p $pkgdir/usr/share/$pkgname
> mkdir -p $pkgdir/usr/share/java/jedit/modes
> 
> install -m 644 $srcdir/catalog.dtd $pkgdir/usr/share/jedit/modes/catalog.dtd
> install -m 644 $srcdir/pkgbuild.xml
> $pkgdir/usr/share/jedit/modes/pkgbuild.xml
> }

"$srcdir" and "$pkgdir" can contain spaces, and must therefore be quoted
in order to prevent shell word splitting.

> ++++++
> 
> and the install script
> ++++++
> post_install() {
> ln -s /usr/share/jedit-pkgbuild-edit-mode/catalot.dtd catalog.dtd

You actually misspelled that file, and later seem to be operating
xmlstarlet with no input file. Although again, this needs to be done in
build() or in the upstream files...

> ln -s /usr/share/java/jedit/modes/catalog catalog.xml
> 
> xmlstarlet sel -Q -t -c //MODE[@FILE_NAME_GLOB=\"PKGBUILD\"] catalog.xml &&
> xmlstarlet ed -L -d //MODE[@FILE_NAME_GLOB=\"PKGBUILD\"] catalog.xml
> xmlstarlet ed -s /MODES -t elem -n MODE -i //MODE[last()] -t attr -n NAME
> -v PKGBUILD -i //MODE[last()] -t attr -n FILE -v pkgbuild.xml -i
> //MODE[last()] -t attr -n FILE_NAME_GLOB -v PKGBUILD >
> /usr/share/java/jedit/modes/catalog
> 
> rm catalog.dtd catalog.xml
> }
> 
> pre_remove() {
> ln -s /usr/share/jedit-pkgbuild-edit-mode/catalot.dtd catalog.dtd
> 
> xmlstarlet ed -L -d //MODE[@FILE_NAME_GLOB=\"PKGBUILD\"] catalog.xml
> 
> rm catalog.dtd
> }
> ++++++

That install script is being used to modify the package files, which is
completely not okay -- you should be modifying the files in build()
inside the PKGBUILD. I'd also like to point out that xmlstarlet is
listed as a makedepends when you used it in the post_install -- which
means it might not be installed anymore when the package is being
installed, and those commands would fail to run.

I'm rather curious why you feel the need to modify the file in the first
place, as the original sources should surely be accurate -- and if they
aren't they should be fixed.

-- 
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/20170713/ca197150/attachment.asc>