PKGBUILD review request: kerneloops

tsh · 2016-05-20 11:28:36

I recently came up with this PKGBUILD for kerneloops, a client for parsing and submitting kernel OOPS diagnostic information to http://oops.kernel.org/.

Since this is the first time I am doing this, I'd like to hear your opinion on the build:

# Maintainer: Sree Harsha Totakura <sreeharsha at lastname dot in>

pkgname=kerneloops-git
_pkgname=kerneloops
pkgver=latest
pkgrel=1
pkgdesc="kerneloops client tool for reporting OOPS to http://oops.kernel.org/"
arch=('i686' 'x86_64')
url="http://oops.kernel.org/"
license=(GPL2)
groups=()
depends=('dbus-glib' 'libnotify' 'gtk2' 'curl')
makedepends=('git')
provides=()
conflicts=()
replaces=()
backup=('etc/kerneloops.conf')
options=()
install=
source=("git+https://github.com/oops-kernel-org/kerneloops")
noextract=()
sha256sums=('SKIP')

pkgver() {
  cd "$srcdir/$_pkgname"
  ( set -o pipefail
    git describe --long --tags 2>/dev/null | sed 's/\([^-]*-g\)/r\1/;s/-/./g' ||
    printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
  )
}

build() {
  cd "$_pkgname"
  sed -i 's@^SBINDIR=.*$@SBINDIR=/usr/bin@' Makefile
  make
}

package() {
  cd "$_pkgname"
  make DESTDIR="$pkgdir/" install
}

Scimmia · 2016-05-20 11:56:25

Overall, it looks very good. Most of my comments are picky things.

Get rid of all of the empty variables.
pkgdesc should not repeat the pkgname. See https://wiki.archlinux.org/index.php/Ar … _etiquette
Be consistent with your quoting (entries in the license array)
in pkgver(), you cd to "$srcdir/$_pkgname", but in the others, you just use "$_pkgname". Why the difference?
I personally don't like that pkgver function. It's much easier to just look at which command to use and use it.
The latest release has a "v" prefix on the tag. You should strip that.
"sed -i 's@^SBINDIR=.*$@SBINDIR=/usr/bin@' Makefile" should generally be done in the prepare function, BUT you should really just get rid of that, see the next point.
You're better off overriding variables by just passing them to make, the same way DESTDIR is done in the package function.

Last edited by Scimmia (2016-05-20 11:57:11)

eschwartz · 2016-05-20 14:20:40

"${pkgname%-git}" can be used instead of a separate _pkgname variable.

(This allows you to reuse some boilerplate between similar packages.)

tsh · 2016-05-20 14:43:42

Thank you Scimma and Eschwartz for your feedback. I incorporated your suggestions.

# Maintainer: Sree Harsha Totakura <sreeharsha at lastname dot in>

pkgname=kerneloops-git
pkgver=0.12.r28.g64e6bd9
pkgrel=3
pkgdesc="kerneloops client tool for reporting OOPS to http://oops.kernel.org/"
arch=('i686' 'x86_64')
url="http://oops.kernel.org/"
license=('GPL2')
depends=('dbus-glib' 'libnotify' 'gtk2' 'curl')
makedepends=('git')
backup=('etc/kerneloops.conf')
source=("git+https://github.com/oops-kernel-org/kerneloops")
sha256sums=('SKIP')

pkgver() {
  cd "${pkgname%-git}"
  git describe --long --tags | sed 's/^v//;s/\([^-]*-g\)/r\1/;s/-/./g'
}

build() {
  cd "${pkgname%-git}"
  make DESTDIR="$pkgdir" SBINDIR=/usr/bin
}

package() {
  cd "${pkgname%-git}"
  make DESTDIR="$pkgdir" SBINDIR=/usr/bin install
}

tsh · 2016-05-20 14:49:37

Ah, I forgot to remove the pkgname from the description. Now it stays corrected.

mis · 2016-05-20 15:31:49

In the build() function DESTDIR and SBINDIR isn't needed, simply make is enough.

build() {
  cd "${pkgname%-git}"
  make
}

tsh · 2016-05-21 16:14:41

mis wrote:

In the build() function DESTDIR and SBINDIR isn't needed, simply make is enough.

Right, thanks; fixed now.

I however have this question: which version should I put in .SRCINFO since this is a package which pulls latest updates from upstream's Git repository.

mis · 2016-05-21 16:29:36

The package version is updated by the pkgver() function each time one builds the package.
So the pkgver of the resulting package will always match the hash of the latest upstream commit.

To create the .SRCINFO file you just use mksrcinfo from community/pkgbuild-introspection.

Last edited by mis (2016-05-21 16:30:00)

Scimmia · 2016-05-21 16:34:39

mksrcinfo is obsolete. makepkg --printsrcinfo does the job.

It really doesn't matter what pkgver you put in.

mis · 2016-05-21 16:51:52

Scimmia wrote:

mksrcinfo is obsolete. makepkg --printsrcinfo does the job.

Ah, didn't know that.

progandy · 2016-05-21 20:15:31

tsh wrote:

I however have this question: which version should I put in .SRCINFO since this is a package which pulls latest updates from upstream's Git repository.

I think it should contain the version number you used to test the PKGBUILD. If you run the build before committing, then makepkg should set it for you.

Last edited by progandy (2016-05-21 20:16:40)

tsh · 2016-05-22 10:23:24

OK, thanks. I used `makepkg --printsrcinfo` to populate it. Should I do this every time I bump pkgrel?

Last edited by tsh (2016-05-22 10:23:38)

mis · 2016-05-22 12:18:04

Yes, otherwise the AUR Website and other tools like cower, yaourt, etc. will not recognize the pkgrel bump.

edit: If you just always do it as the last step of your changes you don't have to worry about it.

Last edited by mis (2016-05-22 12:49:18)

eschwartz · 2016-05-22 14:07:04

I advise using a git pre-commit hook to ensure the .SRCINFO always matches the PKGBUILD you are committing.

See e.g. https://github.com/eli-schwartz/pkgbuilds

tsh · 2016-05-22 18:09:08

Thanks; the pre-commit hooks is invaluable here.

Arch Linux

#1 2016-05-20 11:28:36

PKGBUILD review request: kerneloops

#2 2016-05-20 11:56:25

Re: PKGBUILD review request: kerneloops

#3 2016-05-20 14:20:40

Re: PKGBUILD review request: kerneloops

#4 2016-05-20 14:43:42

Re: PKGBUILD review request: kerneloops

#5 2016-05-20 14:49:37

Re: PKGBUILD review request: kerneloops

#6 2016-05-20 15:31:49

Re: PKGBUILD review request: kerneloops

#7 2016-05-21 16:14:41

Re: PKGBUILD review request: kerneloops

#8 2016-05-21 16:29:36

Re: PKGBUILD review request: kerneloops

#9 2016-05-21 16:34:39

Re: PKGBUILD review request: kerneloops

#10 2016-05-21 16:51:52

Re: PKGBUILD review request: kerneloops

#11 2016-05-21 20:15:31

Re: PKGBUILD review request: kerneloops

#12 2016-05-22 10:23:24

Re: PKGBUILD review request: kerneloops

#13 2016-05-22 12:18:04

Re: PKGBUILD review request: kerneloops

#14 2016-05-22 14:07:04

Re: PKGBUILD review request: kerneloops

#15 2016-05-22 18:09:08

Re: PKGBUILD review request: kerneloops

Board footer