Review request, jedit-pkgbuild-edit-mode

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Review request, jedit-pkgbuild-edit-mode

tur-users mailing list
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')
md5sums=()

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
}

++++++

and the install script
++++++
post_install() {
ln -s /usr/share/jedit-pkgbuild-edit-mode/catalot.dtd catalog.dtd
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
}
++++++

--
Regards,
Vitaliy Berdikskikh AKA UR6LAD <http://ur6lad.co.ua/>
73!
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request, jedit-pkgbuild-edit-mode

tur-users mailing list
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


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request, jedit-pkgbuild-edit-mode

Rafael Fontenelle-2
In reply to this post by tur-users mailing list
2017-07-13 19:54 GMT-03:00 Vitaliy Berdinskikh via aur-general <
[hidden email]>:

> 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
>

Regarding this "edit-mode" in the pkgname, is there a mode other than
"edit" for the rest of source files in your bitbucket project? If the
answer is yes, consider do a split package that contains everything. If the
answer is no, consider just naming the package as "jedit-pkgbbuild" to be
the same as the project name.

By the way, when possible, fix the broken link for the AUR package in
https://bitbucket.org/ur6lad/jedit-pkgbuild/wiki/

Best regards,
Rafael Fontenelle
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request, jedit-pkgbuild-edit-mode

tur-users mailing list
In reply to this post by tur-users mailing list
Hi all,

I've updated the package:

* replace sources with downloaded tarball
* fix md5sums, add sha256sums
* add quotes to $pkgdir and $srcdir
* fix package name: jedit-pkgbuild-edit-mode -> jedit-pkgbuild
* update the install script with new package name
* rewrite the install script: now it doesn't use temporary softlinks to
modify files

Note why I modify catalog(catalog.xml) with xmlstarlet.

The file /usr/share/java/jedit/modes/catalog is part of the package [jedit](
https://www.archlinux.org/packages/community/any/jedit/).
I need to add new record for the PKGBUILD edit mode: to point my edit mode
file pkgbuild.xml. So after install I add this record and remove it before
uninstall.

PKGBUILD
~~~~~~~~
# Maintainer: Vitaliy Berdinskikh <ur6lad at gmail dot com>
pkgname=jedit-pkgbuild
pkgver=4.0.0
pkgrel=1
pkgdesc='jEdit PKGBUILD edit mode'
arch=('any')
url='https://bitbucket.org/ur6lad/jedit-pkgbuild'
license=('GPL')
depends=('jedit' 'xmlstarlet')
install=jedit-pkgbuild.install
source=(https://bitbucket.org/ur6lad/$pkgname/get/$pkgver.tar.bz2)
noextract=($pkgver.tar.bz2)
md5sums=('f9ab018b0a281d18e1ead326c4654757')
sha256sums=('c0b6f9360a8f7e4f6dc20ebdb0af286c5cd137c4fb73c1ed0b4eff245f52f0e8')

prepare() {
cd "$srcdir"

tar -xf $pkgver.tar.bz2 --strip-components=1
}

package() {
#  catalog,dtd is required to edit the edit mode catalog (XML)
mkdir -p "$pkgdir"/usr/share/$pkgname
install -m 644 "$srcdir"/catalog.dtd $pkgdir/usr/share/$pkgname/catalog.dtd

# edit mode file
mkdir -p "$pkgdir"/usr/share/java/jedit/modes
install -m 644 "$srcdir"/pkgbuild.xml
$pkgdir/usr/share/java/jedit/modes/pkgbuild.xml
}
~~~~~~~~

jedit-pkgbuild.install
~~~~~~~~
post_install() {
ln -s /usr/share/java/jedit/modes/catalog
/usr/share/jedit-pkgbuild/catalog.xml

xmlstarlet sel -Q -t -c //MODE[@FILE_NAME_GLOB=\"PKGBUILD\"]
/usr/share/jedit-pkgbuild/catalog.xml && xmlstarlet ed -L -d
//MODE[@FILE_NAME_GLOB=\"PKGBUILD\"] /usr/share/jedit-pkgbuild/catalog.xml
xmlstarlet ed -L -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/jedit-pkgbuild/catalog.xml
}

pre_remove() {
xmlstarlet ed -L -d //MODE[@FILE_NAME_GLOB=\"PKGBUILD\"]
/usr/share/jedit-pkgbuild/catalog.xml

rm /usr/share/jedit-pkgbuild/catalog.xml
}
~~~~~~~~

If it's OK I will push the package to AUR.

--
Regards,
Vitaliy Berdikskikh AKA UR6LAD <http://ur6lad.co.ua/>
73!
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request, jedit-pkgbuild-edit-mode

Rafael Fontenelle-2
2017-07-15 7:15 GMT-03:00 Vitaliy Berdinskikh via aur-general <
[hidden email]>:

>
> PKGBUILD
> ~~~~~~~~
> # Maintainer: Vitaliy Berdinskikh <ur6lad at gmail dot com>
> pkgname=jedit-pkgbuild
> pkgver=4.0.0
> pkgrel=1
> pkgdesc='jEdit PKGBUILD edit mode'
> arch=('any')
> url='https://bitbucket.org/ur6lad/jedit-pkgbuild'
> license=('GPL')
> depends=('jedit' 'xmlstarlet')
> install=jedit-pkgbuild.install
> source=(https://bitbucket.org/ur6lad/$pkgname/get/$pkgver.tar.bz2)
>

I suggest setting your source to

source=($pkgname-$pkgver.tar.bz2::
https://bitbucket.org/ur6lad/$pkgname/get/$pkgver.tar.bz2)

so the downloaded tarball will be named jedit-pkgbuild-4.0.0.tar.bz2,
instead of just 4.0.0.tar.bz2 (more info about this solution
<https://wiki.archlinux.org/index.php/PKGBUILD#source>). This is
particularly useful to ease identification of the tarball when makepkg.conf
has $SRCDEST enabled, which stores all sources data in one directory
<https://wiki.archlinux.org/index.php/Makepkg#Configuration>.



> noextract=($pkgver.tar.bz2)
> md5sums=('f9ab018b0a281d18e1ead326c4654757')
> sha256sums=('c0b6f9360a8f7e4f6dc20ebdb0af286c5cd137c4fb73c1ed0b4eff245f52
> f0e8')
>
> prepare() {
> cd "$srcdir"
>
> tar -xf $pkgver.tar.bz2 --strip-components=1
> }
>
> package() {
> #  catalog,dtd is required to edit the edit mode catalog (XML)
> mkdir -p "$pkgdir"/usr/share/$pkgname
> install -m 644 "$srcdir"/catalog.dtd $pkgdir/usr/share/$pkgname/
> catalog.dtd
>
> # edit mode file
> mkdir -p "$pkgdir"/usr/share/java/jedit/modes
> install -m 644 "$srcdir"/pkgbuild.xml
> $pkgdir/usr/share/java/jedit/modes/pkgbuild.xml
>

Missing quotes for $pkgdir.

Also, the install location for pkgbuild.xml seems to be in a different line
of the 'install' command. It could be just the email adding a wrapped line,
but just to make sure it is correct.

}
> ~~~~~~~~
>
> jedit-pkgbuild.install
> ~~~~~~~~
> post_install() {
> ln -s /usr/share/java/jedit/modes/catalog
> /usr/share/jedit-pkgbuild/catalog.xml
Loading...